launchpad-reviewers team mailing list archive

[Colin Watson <mp+424593@xxxxxxxxxxxxxxxxxx>]

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

Commit message:
Allow CI builds to be uploaded multiple times to different targets

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

It's quite confusing that only one `CIBuildUploadJob` can ever be run for a given CI build, even if you want to release it to multiple channels or multiple archives or similar; this is because there's a unique constraint on `BinaryPackageRelease (ci_build, binarypackagename)`.  Lift this restriction by just creating new `BinaryPackagePublishingHistory` rows for the relevant `BinaryPackageRelease` in that case.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ci-build-duplicate-upload into launchpad:master.

diff --git a/lib/lp/code/interfaces/cibuild.py b/lib/lp/code/interfaces/cibuild.py
index fc7a197..2d75ae1 100644
--- a/lib/lp/code/interfaces/cibuild.py
+++ b/lib/lp/code/interfaces/cibuild.py
@@ -23,6 +23,7 @@ from lazr.restful.declarations import (
     operation_for_version,
     )
 from lazr.restful.fields import Reference
+from zope.interface import Attribute
 from zope.schema import (
     Bool,
     Datetime,
@@ -132,6 +133,10 @@ class ICIBuildView(IPackageBuildView, IPrivacy):
             "A mapping from job IDs to result tokens, retrieved from the "
             "builder.")))
 
+    binarypackages = Attribute(
+        "A list of binary packages that resulted from this build, ordered by "
+        "name.")
+
     def getConfiguration(logger=None):
         """Fetch a CI build's .launchpad.yaml from code hosting, if possible.
 
diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index 4e1ddbd..1982a44 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -8,7 +8,10 @@ __all__ = [
     ]
 
 from datetime import timedelta
-from operator import attrgetter
+from operator import (
+    attrgetter,
+    itemgetter,
+    )
 
 from lazr.lifecycle.event import ObjectCreatedEvent
 import pytz
@@ -85,6 +88,7 @@ from lp.services.macaroons.interfaces import (
     )
 from lp.services.macaroons.model import MacaroonIssuerBase
 from lp.services.propertycache import cachedproperty
+from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
 from lp.soyuz.model.distroarchseries import DistroArchSeries
 
@@ -466,6 +470,17 @@ class CIBuild(PackageBuildMixin, StormBase):
         """See `IPackageBuild`."""
         # We don't currently send any notifications.
 
+    @property
+    def binarypackages(self):
+        """See `ICIBuild`."""
+        releases = IStore(BinaryPackageRelease).find(
+            (BinaryPackageRelease, BinaryPackageName),
+            BinaryPackageRelease.ci_build == self,
+            BinaryPackageRelease.binarypackagename == BinaryPackageName.id)
+        releases = releases.order_by(
+            BinaryPackageName.name, BinaryPackageRelease.id)
+        return DecoratedResultSet(releases, result_decorator=itemgetter(0))
+
     def createBinaryPackageRelease(
             self, binarypackagename, version, summary, description,
             binpackageformat, architecturespecific, installedsize=None,
diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
index b7e0ca5..89a432c 100644
--- a/lib/lp/code/model/tests/test_cibuild.py
+++ b/lib/lp/code/model/tests/test_cibuild.py
@@ -417,6 +417,7 @@ class TestCIBuild(TestCaseWithFactory):
             installedsize=Equals(1024),
             homepage=Equals("https://example.com/";),
             ))
+        self.assertContentEqual([bpr], build.binarypackages)
 
 
 class TestCIBuildSet(TestCaseWithFactory):
diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
index cc9b75b..36eb855 100644
--- a/lib/lp/soyuz/model/archivejob.py
+++ b/lib/lp/soyuz/model/archivejob.py
@@ -324,6 +324,9 @@ class CIBuildUploadJob(ArchiveJobDerived):
         """See `IRunnableJob`."""
         logger = logging.getLogger()
         with tempfile.TemporaryDirectory(prefix="ci-build-copy-job") as tmpdir:
+            releases_by_name = {
+                release.binarypackagename: release
+                for release in self.ci_build.binarypackages}
             binaries = {}
             for artifact in getUtility(
                     IRevisionStatusArtifactSet).findByCIBuild(self.ci_build):
@@ -342,13 +345,20 @@ class CIBuildUploadJob(ArchiveJobDerived):
                         name, self.archive.reference,
                         self.target_distroseries.getSuite(self.target_pocket),
                         self.target_channel))
-                metadata["binarypackagename"] = (
+                metadata["binarypackagename"] = bpn = (
                     getUtility(IBinaryPackageNameSet).ensure(metadata["name"]))
                 del metadata["name"]
                 filetype = self.filetype_by_format[
                     metadata["binpackageformat"]]
-                bpr = self.ci_build.createBinaryPackageRelease(**metadata)
-                bpr.addFile(artifact.library_file, filetype=filetype)
+                bpr = releases_by_name.get(bpn)
+                if bpr is None:
+                    bpr = self.ci_build.createBinaryPackageRelease(**metadata)
+                for bpf in bpr.files:
+                    if (bpf.libraryfile == artifact.library_file and
+                            bpf.filetype == filetype):
+                        break
+                else:
+                    bpr.addFile(artifact.library_file, filetype=filetype)
                 # The publishBinaries interface was designed for .debs,
                 # which need extra per-binary "override" information
                 # (component, etc.).  None of this is relevant here.
diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
index d599c48..294fe89 100644
--- a/lib/lp/soyuz/tests/test_archivejob.py
+++ b/lib/lp/soyuz/tests/test_archivejob.py
@@ -496,6 +496,57 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
                 binarypackageformat=Equals(BinaryPackageFormat.CONDA_V2),
                 distroarchseries=Equals(dases[0]))))
 
+    def test_existing_release(self):
+        # A `CIBuildUploadJob` can be run even if the build in question was
+        # already uploaded somewhere, and in that case may add publications
+        # in other locations for the same package.
+        archive = self.factory.makeArchive()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=archive.distribution)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        build = self.factory.makeCIBuild(distro_arch_series=das)
+        report = build.getOrCreateRevisionStatusReport("build:0")
+        path = "wheel-indep/dist/wheel_indep-0.0.1-py3-none-any.whl"
+        with open(datadir(path), mode="rb") as f:
+            report.attach(name=os.path.basename(path), data=f.read())
+        artifact = IStore(RevisionStatusArtifact).find(
+            RevisionStatusArtifact,
+            report=report,
+            artifact_type=RevisionStatusArtifactType.BINARY).one()
+        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()
+        job = CIBuildUploadJob.create(
+            build, build.git_repository.owner, archive, distroseries,
+            PackagePublishingPocket.RELEASE, target_channel="0.0.1/edge")
+        transaction.commit()
+
+        with dbuser(job.config.dbuser):
+            JobRunner([job]).runAll()
+
+        bpphs = archive.getAllPublishedBinaries()
+        # The publications are for the same binary package release, which
+        # has a single file attached to it.
+        self.assertEqual(1, len({bpph.binarypackagename for bpph in bpphs}))
+        self.assertEqual(1, len({bpph.binarypackagerelease for bpph in bpphs}))
+        self.assertThat(archive.getAllPublishedBinaries(), MatchesSetwise(*(
+            MatchesStructure(
+                binarypackagename=MatchesStructure.byEquality(
+                    name="wheel-indep"),
+                binarypackagerelease=MatchesStructure(
+                    ci_build=Equals(build),
+                    files=MatchesSetwise(
+                        MatchesStructure.byEquality(
+                            libraryfile=artifact.library_file,
+                            filetype=BinaryPackageFileType.WHL))),
+                binarypackageformat=Equals(BinaryPackageFormat.WHL),
+                distroarchseries=Equals(das),
+                channel=Equals(channel))
+            for channel in ("edge", "0.0.1/edge"))))
+
 
 class TestViaCelery(TestCaseWithFactory):