launchpad-reviewers team mailing list archive

[Colin Watson <mp+427977@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:soss-license-from-lpcraft into launchpad:master with ~cjwatson/launchpad:ci-build-upload-propagate-properties as a prerequisite.

Commit message:
Set soss.license for publications with license information

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

The `license` property in this case was originally an output property from an `lpcraft` job, and is now propagated all the way through to Artifactory.  We serialize the structured `license` property to either `spdx:<expression>` or (for compatibility with existing properties) an unadorned relative path.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:soss-license-from-lpcraft into launchpad:master.

diff --git a/lib/lp/archivepublisher/artifactory.py b/lib/lp/archivepublisher/artifactory.py
index 5647772..a2112f5 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 zope.security.proxy import isinstance as zope_isinstance
 
 from lp.archivepublisher.diskpool import FileAddActionEnum, poolify
 from lp.services.config import config
@@ -255,20 +256,34 @@ class ArtifactoryPoolEntry:
         # https://docs.google.com/spreadsheets/d/15Xkdi-CRu2NiQfLoclP5PKW63Zw6syiuao8VJG7zxvw
         # (private).
         if ISourcePackageReleaseFile.providedBy(self.pub_file):
-            ci_build = self.pub_file.sourcepackagerelease.ci_build
+            release = self.pub_file.sourcepackagerelease
         elif IBinaryPackageFile.providedBy(self.pub_file):
-            ci_build = self.pub_file.binarypackagerelease.ci_build
+            release = self.pub_file.binarypackagerelease
         else:
-            ci_build = None
-        if ci_build is not None:
+            # There are no other kinds of `IPackageReleaseFile` at the moment.
+            raise AssertionError("Unsupported file: %r" % self.pub_file)
+        if release.ci_build is not None:
             properties.update(
                 {
                     "soss.source_url": [
-                        ci_build.git_repository.getCodebrowseUrl()
+                        release.ci_build.git_repository.getCodebrowseUrl()
                     ],
-                    "soss.commit_id": [ci_build.commit_sha1],
+                    "soss.commit_id": [release.ci_build.commit_sha1],
                 }
             )
+        if "soss.license" not in properties:
+            license_field = release.getUserDefinedField("license")
+            if zope_isinstance(license_field, dict):
+                if "spdx" in license_field:
+                    properties["soss.license"] = [
+                        "spdx:%s" % license_field["spdx"]
+                    ]
+                elif "path" in license_field:
+                    # Not ideal for parsing, but we have precedent for
+                    # putting unadorned paths here, and it doesn't seem very
+                    # likely that a path will begin with "spdx:" so there
+                    # probably won't be significant confusion in practice.
+                    properties["soss.license"] = [license_field["path"]]
         return properties
 
     def addFile(self):
diff --git a/lib/lp/archivepublisher/tests/test_artifactory.py b/lib/lp/archivepublisher/tests/test_artifactory.py
index b86b619..86a7d8c 100644
--- a/lib/lp/archivepublisher/tests/test_artifactory.py
+++ b/lib/lp/archivepublisher/tests/test_artifactory.py
@@ -1147,3 +1147,101 @@ class TestArtifactoryPoolFromLibrarian(TestCaseWithFactory):
             ["text with special characters: ;=|,\\"],
             path.properties["pypi.summary"],
         )
+
+    def test_updateProperties_sets_soss_license_source_spdx(self):
+        # Set the `soss.license` field appropriately for a source package if
+        # the originating job set the `license` property to an SPDX
+        # expression.
+        pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
+        spr = self.factory.makeSourcePackageRelease(
+            archive=pool.archive,
+            sourcepackagename="foo",
+            version="1.0",
+            format=SourcePackageType.CI_BUILD,
+            user_defined_fields=[
+                ("license", {"spdx": "MIT"}),
+                ("package-name", "foo"),
+            ],
+        )
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=spr,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="foo-1.0.tar.gz"
+            ),
+            filetype=SourcePackageFileType.SDIST,
+        )
+        transaction.commit()
+        pool.addFile(None, "foo", "1.0", sprf)
+        path = pool.rootpath / "foo" / "1.0" / "foo-1.0.tar.gz"
+        self.assertEqual(["spdx:MIT"], path.properties["soss.license"])
+
+    def test_updateProperties_sets_soss_license_binary_spdx(self):
+        # Set the `soss.license` field appropriately for a binary package if
+        # the originating job set the `license` property to an SPDX
+        # expression.
+        pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
+        bpr = self.factory.makeBinaryPackageRelease(
+            binarypackagename="foo",
+            version="1.0",
+            binpackageformat=BinaryPackageFormat.WHL,
+            user_defined_fields=[("license", {"spdx": "MIT"})],
+        )
+        bpf = self.factory.makeBinaryPackageFile(
+            binarypackagerelease=bpr,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="foo-1.0-py3-none-any.whl"
+            ),
+            filetype=BinaryPackageFileType.WHL,
+        )
+        transaction.commit()
+        pool.addFile(None, "foo", "1.0", bpf)
+        path = pool.rootpath / "foo" / "1.0" / "foo-1.0-py3-none-any.whl"
+        self.assertEqual(["spdx:MIT"], path.properties["soss.license"])
+
+    def test_updateProperties_sets_soss_license_source_path(self):
+        # Set the `soss.license` field appropriately for a source package if
+        # the originating job set the `license` property to a path.
+        pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
+        spr = self.factory.makeSourcePackageRelease(
+            archive=pool.archive,
+            sourcepackagename="foo",
+            version="1.0",
+            format=SourcePackageType.CI_BUILD,
+            user_defined_fields=[
+                ("license", {"path": "LICENSE"}),
+                ("package-name", "foo"),
+            ],
+        )
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=spr,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="foo-1.0.tar.gz"
+            ),
+            filetype=SourcePackageFileType.SDIST,
+        )
+        transaction.commit()
+        pool.addFile(None, "foo", "1.0", sprf)
+        path = pool.rootpath / "foo" / "1.0" / "foo-1.0.tar.gz"
+        self.assertEqual(["LICENSE"], path.properties["soss.license"])
+
+    def test_updateProperties_sets_soss_license_binary_path(self):
+        # Set the `soss.license` field appropriately for a binary package if
+        # the originating job set the `license` property to a path.
+        pool = self.makePool(ArchiveRepositoryFormat.PYTHON)
+        bpr = self.factory.makeBinaryPackageRelease(
+            binarypackagename="foo",
+            version="1.0",
+            binpackageformat=BinaryPackageFormat.WHL,
+            user_defined_fields=[("license", {"path": "LICENSE"})],
+        )
+        bpf = self.factory.makeBinaryPackageFile(
+            binarypackagerelease=bpr,
+            library_file=self.factory.makeLibraryFileAlias(
+                filename="foo-1.0-py3-none-any.whl"
+            ),
+            filetype=BinaryPackageFileType.WHL,
+        )
+        transaction.commit()
+        pool.addFile(None, "foo", "1.0", bpf)
+        path = pool.rootpath / "foo" / "1.0" / "foo-1.0-py3-none-any.whl"
+        self.assertEqual(["LICENSE"], path.properties["soss.license"])