launchpad-reviewers team mailing list archive

[Colin Watson <mp+429882@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:canonicalize-python-name into launchpad:master.

Commit message:
Canonicalize Python package names for publishing

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1989510 in Launchpad itself: "Launchpad publishes source packages to artifactory under incorrect names"
  https://bugs.launchpad.net/launchpad/+bug/1989510

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/429882

We'll need to deal with republishing the files that have been published to the wrong place, but we can probably deal with that manually.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:canonicalize-python-name into launchpad:master.

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index a2112f5..4bd7f0a 100644
--- a/lib/lp/archivepublisher/artifactory.py
+++ b/lib/lp/archivepublisher/artifactory.py
@@ -18,6 +18,7 @@ from urllib.parse import quote_plus
 import requests
 from artifactory import ArtifactoryPath
 from dohq_artifactory.auth import XJFrogArtApiAuth
+from packaging import utils as packaging_utils
 from zope.security.proxy import isinstance as zope_isinstance
 
 from lp.archivepublisher.diskpool import FileAddActionEnum, poolify
@@ -58,7 +59,11 @@ def _path_for(
         package_name = release.getUserDefinedField("package-name")
         if package_name is None:
             package_name = source_name
-        path = rootpath / package_name / source_version
+        path = (
+            rootpath
+            / packaging_utils.canonicalize_name(package_name)
+            / source_version
+        )
     elif repository_format == ArchiveRepositoryFormat.CONDA:
         subdir = release.getUserDefinedField("subdir")
         if subdir is None:
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index 86a7d8c..9aea55c 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -769,6 +769,81 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             path.properties,
         )
 
+    def test_updateProperties_python_sdist_canonicalizes_name(self):
+        pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
+        dses = [
+            self.factory.makeDistroSeries(
+                distribution=pool.archive.distribution
+            )
+            for _ in range(2)
+        ]
+        das = self.factory.makeDistroArchSeries(distroseries=dses[0])
+        ci_build = self.factory.makeCIBuild(distro_arch_series=das)
+        spr = self.factory.makeSourcePackageRelease(
+            archive=pool.archive,
+            sourcepackagename="python-foo-bar",
+            version="1.0",
+            format=SourcePackageType.CI_BUILD,
+            ci_build=ci_build,
+            user_defined_fields=[("package-name", "foo_bar")],
+        )
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            archive=pool.archive,
+            sourcepackagerelease=spr,
+            distroseries=dses[0],
+            pocket=PackagePublishingPocket.RELEASE,
+            component="main",
+            sourcepackagename="python-foo-bar",
+            version="1.0",
+            channel="edge",
+            format=SourcePackageType.CI_BUILD,
+        )
+        spr = spph.sourcepackagerelease
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=spr,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="foo_bar-1.0.tar.gz"
+            ),
+            filetype=SourcePackageFileType.SDIST,
+        )
+        spphs = [spph]
+        spphs.append(
+            spph.copyTo(dses[1], PackagePublishingPocket.RELEASE, pool.archive)
+        )
+        transaction.commit()
+        pool.addFile(None, spr.name, spr.version, sprf)
+        path = pool.rootpath / "foo-bar" / "1.0" / "foo_bar-1.0.tar.gz"
+        self.assertTrue(path.exists())
+        self.assertFalse(path.is_symlink())
+        self.assertEqual(
+            {
+                "launchpad.release-id": ["source:%d" % spr.id],
+                "launchpad.source-name": ["python-foo-bar"],
+                "launchpad.source-version": ["1.0"],
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()
+                ],
+                "soss.commit_id": [ci_build.commit_sha1],
+            },
+            path.properties,
+        )
+        pool.updateProperties(spr.name, spr.version, [sprf], spphs)
+        self.assertEqual(
+            {
+                "launchpad.release-id": ["source:%d" % spr.id],
+                "launchpad.source-name": ["python-foo-bar"],
+                "launchpad.source-version": ["1.0"],
+                "launchpad.channel": list(
+                    sorted("%s:edge" % ds.name for ds in dses)
+                ),
+                "soss.source_url": [
+                    ci_build.git_repository.getCodebrowseUrl()
+                ],
+                "soss.commit_id": [ci_build.commit_sha1],
+            },
+            path.properties,
+        )
+
     def test_updateProperties_python_wheel(self):
         pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
         dses = [