launchpad-reviewers team mailing list archive

[Colin Watson <mp+427972@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:ci-build-upload-propagate-properties into launchpad:master.

Commit message:
Make CIBuildUploadJob store report properties in user_defined_fields

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This makes these properties (originally output properties from an `lpcraft` job) accessible to the publisher when publishing files to Artifactory.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ci-build-upload-propagate-properties into launchpad:master.

diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
index fca71f7..2bf29f5 100644
--- a/lib/lp/soyuz/model/archivejob.py
+++ b/lib/lp/soyuz/model/archivejob.py
@@ -19,6 +19,7 @@ from storm.locals import JSON, Int, Reference
 from wheel_filename import parse_wheel_filename
 from zope.component import getUtility
 from zope.interface import implementer, provider
+from zope.security.proxy import removeSecurityProxy
 
 from lp.code.enums import RevisionStatusArtifactType
 from lp.code.interfaces.cibuild import ICIBuildSet
@@ -263,6 +264,24 @@ class ScannedArtifact:
         self.artifact = artifact
         self.metadata = metadata
 
+    @property
+    def extra_fields(self) -> Optional[List[Tuple[str, Any]]]:
+        """Extra fields to be stored in user_defined_fields.
+
+        Combine fields from the report's properties (specified by the job)
+        and the scanned metadata (determined by the scanner).  In cases of
+        conflict, the scanner wins.
+        """
+        fields = {}
+        if self.artifact.report.properties is not None:
+            fields.update(removeSecurityProxy(self.artifact.report.properties))
+        if self.metadata.user_defined_fields is not None:
+            fields.update(self.metadata.user_defined_fields)
+        if fields:
+            return sorted(fields.items())
+        else:
+            return None
+
 
 @implementer(ICIBuildUploadJob)
 @provider(ICIBuildUploadJobSource)
@@ -661,19 +680,19 @@ class CIBuildUploadJob(ArchiveJobDerived):
             for scanned_artifact in scanned:
                 metadata = scanned_artifact.metadata
                 if isinstance(metadata, SourceArtifactMetadata):
-                    first_metadata = metadata
+                    first_scanned_artifact = scanned_artifact
                     break
             else:
-                first_metadata = scanned[0].metadata
+                first_scanned_artifact = scanned[0]
             spr = self.ci_build.createSourcePackageRelease(
                 distroseries=distroseries,
                 sourcepackagename=build_target.sourcepackagename,
                 # We don't have a good concept of source version here, but
                 # the data model demands one.
-                version=first_metadata.version,
+                version=first_scanned_artifact.metadata.version,
                 creator=self.requester,
                 archive=self.archive,
-                user_defined_fields=first_metadata.user_defined_fields,
+                user_defined_fields=first_scanned_artifact.extra_fields,
             )
         for scanned_artifact in scanned:
             metadata = scanned_artifact.metadata
@@ -737,7 +756,7 @@ class CIBuildUploadJob(ArchiveJobDerived):
                     binpackageformat=metadata.format,
                     architecturespecific=metadata.architecturespecific,
                     homepage=metadata.homepage,
-                    user_defined_fields=metadata.user_defined_fields,
+                    user_defined_fields=scanned_artifact.extra_fields,
                 )
             for bpf in bpr.files:
                 if (
diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
index 28c7e80..cc3cff0 100644
--- a/lib/lp/soyuz/tests/test_archivejob.py
+++ b/lib/lp/soyuz/tests/test_archivejob.py
@@ -1045,6 +1045,89 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
         )
         self.assertContentEqual([], archive.getAllPublishedBinaries())
 
+    def test_run_attaches_properties(self):
+        # The upload process attaches properties from the report as
+        # `SourcePackageRelease.user_defined_fields` or
+        # `BinaryPackageRelease.user_defined_fields` as appropriate.
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON
+        )
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution
+        )
+        dases = [
+            self.factory.makeDistroArchSeries(distroseries=distroseries)
+            for _ in range(2)
+        ]
+        build = self.makeCIBuild(
+            archive.distribution, distro_arch_series=dases[0]
+        )
+        report = build.getOrCreateRevisionStatusReport("build:0")
+        report.setLog(b"log data")
+        report.update(
+            properties={
+                "license": {"spdx": "MIT"},
+                # The sdist scanner sets this key.  Ensure that the scanner
+                # wins, so that it can't be confused by oddly-written jobs.
+                "package-name": "nonsense",
+            }
+        )
+        sdist_path = "wheel-indep/dist/wheel-indep-0.0.1.tar.gz"
+        with open(datadir(sdist_path), mode="rb") as f:
+            report.attach(name=os.path.basename(sdist_path), data=f.read())
+        wheel_path = "wheel-indep/dist/wheel_indep-0.0.1-py3-none-any.whl"
+        with open(datadir(wheel_path), mode="rb") as f:
+            report.attach(name=os.path.basename(wheel_path), data=f.read())
+        job = CIBuildUploadJob.create(
+            build,
+            build.git_repository.owner,
+            archive,
+            distroseries,
+            PackagePublishingPocket.RELEASE,
+            target_channel="edge",
+        )
+        transaction.commit()
+
+        with dbuser(job.config.dbuser):
+            JobRunner([job]).runAll()
+
+        self.assertThat(
+            archive.getPublishedSources(),
+            MatchesSetwise(
+                MatchesStructure(
+                    sourcepackagerelease=MatchesStructure(
+                        user_defined_fields=Equals(
+                            [
+                                ["license", {"spdx": "MIT"}],
+                                # The sdist scanner sets this key, and wins.
+                                ["package-name", "wheel-indep"],
+                            ]
+                        ),
+                    ),
+                )
+            ),
+        )
+        self.assertThat(
+            archive.getAllPublishedBinaries(),
+            MatchesSetwise(
+                *(
+                    MatchesStructure(
+                        binarypackagerelease=MatchesStructure(
+                            user_defined_fields=Equals(
+                                [
+                                    ["license", {"spdx": "MIT"}],
+                                    # The wheel scanner doesn't set this
+                                    # key, so the job is allowed to do so.
+                                    ["package-name", "nonsense"],
+                                ]
+                            ),
+                        ),
+                    )
+                    for das in dases
+                )
+            ),
+        )
+
     def test_existing_source_and_binary_releases(self):
         # A `CIBuildUploadJob` can be run even if the build in question was
         # already uploaded somewhere, and in that case may add publications