launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28553
[Merge] ~cjwatson/launchpad:publish-isolated-binaries into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:publish-isolated-binaries into launchpad:master.
Commit message:
Fix publishing of isolated binaries
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/424199
When we upload a CI build to an archive, the resulting `BinaryPackagePublishingHistory` rows have no associated `SourcePackageRelease`, which causes publishing to fail since it uses the name and version from the SPR to compute paths in the pool.
Use the name and version from the `BinaryPackageRelease` instead in this case. This is quite awkward, and we may need to do something different with CI builds in the longer term (perhaps storing a name and version on `CIBuild` instead?), but it does the job for now.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:publish-isolated-binaries into launchpad:master.
diff --git a/lib/lp/archivepublisher/deathrow.py b/lib/lp/archivepublisher/deathrow.py
index 9071a53..ed45eb3 100644
--- a/lib/lp/archivepublisher/deathrow.py
+++ b/lib/lp/archivepublisher/deathrow.py
@@ -87,10 +87,10 @@ class DeathRow:
removed."""
if dry_run:
# Don't actually remove the files if we are dry running
- def _mockRemoveFile(cn, sn, sv, fn):
+ def _mockRemoveFile(cn, pn, pv, fn):
self.logger.debug("(Not really!) removing %s %s/%s/%s" %
- (cn, sn, sv, fn))
- fullpath = self.diskpool.pathFor(cn, sn, sv, fn)
+ (cn, pn, pv, fn))
+ fullpath = self.diskpool.pathFor(cn, pn, pv, fn)
if not fullpath.exists():
raise NotInPool
return fullpath.lstat().st_size
@@ -228,8 +228,8 @@ class DeathRow:
# Calculating the file path in pool.
pub_file_details = (
pub_record.component_name,
- pub_record.source_package_name,
- pub_record.source_package_version,
+ pub_record.pool_name,
+ pub_record.pool_version,
pub_file.libraryfile.filename,
)
file_path = str(self.diskpool.pathFor(*pub_file_details))
@@ -265,11 +265,11 @@ class DeathRow:
"Removing %s files marked for reaping" % len(condemned_files))
for condemned_file in sorted(condemned_files, reverse=True):
- component_name, source_name, source_version, file_name = (
+ component_name, pool_name, pool_version, file_name = (
details[condemned_file])
try:
bytes += self._removeFile(
- component_name, source_name, source_version, file_name)
+ component_name, pool_name, pool_version, file_name)
except NotInPool as info:
# It's safe for us to let this slide because it means that
# the file is already gone.
diff --git a/lib/lp/archivepublisher/tests/deathrow.txt b/lib/lp/archivepublisher/tests/deathrow.txt
index 5f8ac79..e85eece 100644
--- a/lib/lp/archivepublisher/tests/deathrow.txt
+++ b/lib/lp/archivepublisher/tests/deathrow.txt
@@ -207,8 +207,8 @@ Publish files on disk and build a list of all created file paths
... for pub_file in pub.files:
... file_path = quiet_disk_pool.pathFor(
... pub.component.name,
- ... pub.source_package_name,
- ... pub.source_package_version,
+ ... pub.pool_name,
+ ... pub.pool_version,
... pub_file.libraryfile.filename
... )
... unique_file_paths.add(file_path)
diff --git a/lib/lp/archivepublisher/tests/test_deathrow.py b/lib/lp/archivepublisher/tests/test_deathrow.py
index 9fd62e5..0bfb423 100644
--- a/lib/lp/archivepublisher/tests/test_deathrow.py
+++ b/lib/lp/archivepublisher/tests/test_deathrow.py
@@ -52,8 +52,8 @@ class TestDeathRow(TestCase):
"""Return the absolute path to a published file in the disk pool/."""
return diskpool.pathFor(
pub.component.name,
- pub.source_package_name,
- pub.source_package_version,
+ pub.pool_name,
+ pub.pool_version,
pub_file.libraryfile.filename)
def assertIsFile(self, path: Path) -> None:
diff --git a/lib/lp/soyuz/interfaces/publishing.py b/lib/lp/soyuz/interfaces/publishing.py
index 21a3cac..3ddd772 100644
--- a/lib/lp/soyuz/interfaces/publishing.py
+++ b/lib/lp/soyuz/interfaces/publishing.py
@@ -157,6 +157,11 @@ class IPublishingView(Interface):
title=_("Section Name"),
required=False, readonly=True))
+ pool_name = TextLine(
+ title="Name to use when publishing this record in the pool.")
+ pool_version = TextLine(
+ title="Version to use when publishing this record in the pool.")
+
def publish(diskpool, log):
"""Publish or ensure contents of this publish record
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index b17194e..7ae3765 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -178,16 +178,16 @@ class ArchivePublisherBase:
"""See `IPublishing`"""
try:
for pub_file in self.files:
- source_name = self.source_package_name
- source_version = self.source_package_version
+ pool_name = self.pool_name
+ pool_version = self.pool_version
component = (
None if self.component is None else self.component.name)
filename = pub_file.libraryfile.filename
path = diskpool.pathFor(
- component, source_name, source_version, filename)
+ component, pool_name, pool_version, filename)
action = diskpool.addFile(
- component, source_name, source_version, filename, pub_file)
+ component, pool_name, pool_version, filename, pub_file)
if action == diskpool.results.FILE_ADDED:
log.debug("Added %s from library" % path)
elif action == diskpool.results.SYMLINK_ADDED:
@@ -477,6 +477,16 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
return self.sourcepackagerelease.version
@property
+ def pool_name(self):
+ """See `IPublishingView`."""
+ return self.source_package_name
+
+ @property
+ def pool_version(self):
+ """See `IPublishingView`."""
+ return self.source_package_version
+
+ @property
def displayname(self):
"""See `IPublishing`."""
release = self.sourcepackagerelease
@@ -766,6 +776,36 @@ class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase):
return self.binarypackagerelease.sourcepackageversion
@property
+ def pool_name(self):
+ """See `IPublishingView`."""
+ # XXX cjwatson 2022-06-08: If the publishing record came from
+ # uploading a CI build, then it may be an isolated binary package
+ # without a corresponding source package, in which case we won't
+ # have a source package name. For now, use the binary package name
+ # instead in that case so that the pool has a name to use for
+ # publishing files. This is inelegant to say the least, but it gets
+ # the job done for now.
+ pool_name = self.source_package_name
+ if self.build is None and pool_name is None:
+ pool_name = self.binary_package_name
+ return pool_name
+
+ @property
+ def pool_version(self):
+ """See `IPublishingView`."""
+ # XXX cjwatson 2022-06-08: If the publishing record came from
+ # uploading a CI build, then it may be an isolated binary package
+ # without a corresponding source package, in which case we won't
+ # have a source package version. For now, use the binary package
+ # version instead in that case so that the pool has a version to use
+ # for publishing files. This is inelegant to say the least, but it
+ # gets the job done for now.
+ pool_version = self.source_package_version
+ if self.build is None and pool_version is None:
+ pool_version = self.binary_package_version
+ return pool_version
+
+ @property
def architecture_specific(self):
"""See `IBinaryPackagePublishingHistory`"""
return self.binarypackagerelease.architecturespecific
diff --git a/lib/lp/soyuz/tests/test_publishing.py b/lib/lp/soyuz/tests/test_publishing.py
index 51fa9f7..5000e71 100644
--- a/lib/lp/soyuz/tests/test_publishing.py
+++ b/lib/lp/soyuz/tests/test_publishing.py
@@ -25,6 +25,9 @@ from lp.archivepublisher.indices import (
build_binary_stanza_fields,
build_source_stanza_fields,
)
+from lp.archivepublisher.tests.artifactory_fixture import (
+ FakeArtifactoryFixture,
+ )
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.registry.interfaces.distribution import IDistributionSet
@@ -42,7 +45,9 @@ from lp.services.database.constants import UTC_NOW
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.log.logger import DevNullLogger
from lp.soyuz.enums import (
+ ArchivePublishingMethod,
ArchivePurpose,
+ ArchiveRepositoryFormat,
BinaryPackageFormat,
PackageUploadStatus,
)
@@ -76,6 +81,7 @@ from lp.testing import (
)
from lp.testing.dbuser import (
dbuser,
+ lp_dbuser,
switch_dbuser,
)
from lp.testing.factory import LaunchpadObjectFactory
@@ -719,6 +725,40 @@ class TestNativePublishing(TestNativePublishingBase):
pool_path = "%s/main/f/foo/foo-bin_666_all.deb" % self.pool_dir
self.assertEqual(open(pool_path).read().strip(), 'Hello world')
+ def test_publish_isolated_binaries(self):
+ # Some binary publications have no associated source publication
+ # (e.g. Python wheels in an archive published using Artifactory).
+ # In these cases, the binary package name/version is passed to the
+ # pool.
+ base_url = "https://foo.example.com/artifactory";
+ self.pushConfig("artifactory", base_url=base_url)
+ archive = self.factory.makeArchive(
+ distribution=self.distroseries.distribution,
+ publishing_method=ArchivePublishingMethod.ARTIFACTORY,
+ repository_format=ArchiveRepositoryFormat.PYTHON)
+ config = getPubConfig(archive)
+ disk_pool = config.getDiskPool(self.logger)
+ disk_pool.logger = self.logger
+ self.useFixture(FakeArtifactoryFixture(base_url, archive.name))
+ with lp_dbuser():
+ ci_build = self.factory.makeCIBuild(
+ distro_arch_series=self.distroseries.architectures[0])
+ bpn = self.factory.makeBinaryPackageName(name="foo")
+ bpr = self.factory.makeBinaryPackageRelease(
+ binarypackagename=bpn, version="0.1", ci_build=ci_build,
+ binpackageformat=BinaryPackageFormat.WHL)
+ lfa = self.addMockFile("foo-0.1.whl", filecontent=b"Hello world")
+ bpr.addFile(lfa)
+ bpph = self.factory.makeBinaryPackagePublishingHistory(
+ binarypackagerelease=bpr, archive=archive,
+ distroarchseries=self.distroseries.architectures[0],
+ pocket=PackagePublishingPocket.RELEASE, channel="stable")
+ bpph.publish(disk_pool, self.logger)
+ self.assertEqual(PackagePublishingStatus.PUBLISHED, bpph.status)
+ pool_path = disk_pool.rootpath / "foo" / "0.1" / "foo-0.1.whl"
+ with pool_path.open() as pool_file:
+ self.assertEqual(b"Hello world", pool_file.read())
+
def test_publish_ddeb_when_disabled_is_noop(self):
# Publishing a DDEB publication when
# Archive.publish_debug_symbols is false just sets PUBLISHED,
