launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26755
[Merge] ~cjwatson/launchpad:fix-primary-archive-dependencies-pocket into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:fix-primary-archive-dependencies-pocket into launchpad:master.
Commit message:
Fix various issues around archive dependency handling
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1838789 in Launchpad itself: "snap build request pocket selection doesn't override ppa dependency pocket (i.e. the primary archive)"
https://bugs.launchpad.net/launchpad/+bug/1838789
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400334
A loop variable in expand_dependencies clobbered one of its arguments. This could only be a problem in practice if building against a primary archive that somehow had archive dependencies of its own, but it still made the code unnecessarily confusing.
Make it clear that the snap build pocket doesn't bypass the source archive's configured archive dependencies. This seems overall to be the least confusing option where snap builds from a PPA are concerned.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:fix-primary-archive-dependencies-pocket into launchpad:master.
diff --git a/lib/lp/code/model/tests/test_recipebuilder.py b/lib/lp/code/model/tests/test_recipebuilder.py
index f6e2b70..b353d30 100644
--- a/lib/lp/code/model/tests/test_recipebuilder.py
+++ b/lib/lp/code/model/tests/test_recipebuilder.py
@@ -81,6 +81,8 @@ class TestRecipeBuilderBase(TestCaseWithFactory):
processor = getUtility(IProcessorSet).getByName('386')
distroseries.nominatedarchindep = distroseries.newArch(
'i386', processor, True, self.factory.makePerson())
+ for component_name in ("main", "universe"):
+ self.factory.makeComponentSelection(distroseries, component_name)
sourcepackage = self.factory.makeSourcePackage(spn, distroseries)
if recipe_registrant is None:
recipe_registrant = self.factory.makePerson(
@@ -341,6 +343,31 @@ class TestAsyncRecipeBuilder(TestRecipeBuilderBase):
}, extra_args)
@defer.inlineCallbacks
+ def test_extraBuildArgs_archives(self):
+ # The build uses the release pocket in its target PPA, and the
+ # release, security, and updates pockets in the primary archive.
+ archive = self.factory.makeArchive()
+ job = self.makeJob(archive=archive, with_builder=True)
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive,
+ distroarchseries=job.build.distroseries.architectures[0],
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ primary = job.build.distribution.main_archive
+ expected_archives = [
+ "deb %s %s main" % (
+ archive.archive_url, job.build.distroseries.name),
+ "deb %s %s main universe" % (
+ primary.archive_url, job.build.distroseries.name),
+ "deb %s %s-security main universe" % (
+ primary.archive_url, job.build.distroseries.name),
+ "deb %s %s-updates main universe" % (
+ primary.archive_url, job.build.distroseries.name),
+ ]
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
def test_extraBuildArgs_archive_trusted_keys(self):
# If the archive has a signing key, extraBuildArgs sends it.
yield self.useFixture(InProcessKeyServerFixture()).start()
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index 16012f5..5c4028e 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -514,6 +514,23 @@ class TestAsyncOCIRecipeBuildBehaviour(
}))
@defer.inlineCallbacks
+ def test_extraBuildArgs_archives(self):
+ # The build uses the release, security, and updates pockets in the
+ # primary archive.
+ job = self.makeJob()
+ expected_archives = [
+ "deb %s %s main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ ]
+ with dbuser(config.builddmaster.dbuser):
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
def test_composeBuildRequest_proxy_url_set(self):
[ref] = self.factory.makeGitRefs()
job = self.makeJob(git_ref=ref)
diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
index 701dc19..ac7925c 100644
--- a/lib/lp/snappy/browser/snap.py
+++ b/lib/lp/snappy/browser/snap.py
@@ -106,7 +106,10 @@ from lp.snappy.interfaces.snap import (
SNAP_PRIVATE_FEATURE_FLAG,
SnapPrivateFeatureDisabled,
)
-from lp.snappy.interfaces.snapbuild import ISnapBuildSet
+from lp.snappy.interfaces.snapbuild import (
+ ISnapBuild,
+ ISnapBuildSet,
+ )
from lp.snappy.interfaces.snappyseries import (
ISnappyDistroSeriesSet,
ISnappySeriesSet,
@@ -416,11 +419,8 @@ class SnapRequestBuildsView(LaunchpadFormView):
'If you do not explicitly select any architectures, then the '
'snap package will be built for all architectures allowed by '
'its configuration.'))
- pocket = Choice(
- title='Pocket', vocabulary=PackagePublishingPocket, required=True,
- description=(
- 'The package stream within the source distribution series to '
- 'use when building the snap package.'))
+ pocket = copy_field(
+ ISnapBuild['pocket'], title='Pocket', readonly=False)
channels = Dict(
title='Source snap channels', key_type=TextLine(), required=True,
description=ISnap['auto_build_channels'].description)
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index 9852eb0..486e586 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -2098,8 +2098,10 @@ class TestSnapRequestBuildsView(BaseTestSnapView):
Proposed
Backports
\(\?\)
- The package stream within the source distribution series to use
- when building the snap package.
+ The package stream within the source archive and distribution
+ series to use when building the snap package. If the source
+ archive is a PPA, then the PPA's archive dependencies will be
+ used to select the pocket in the distribution's primary archive.
Source snap channels:
core
core18
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index 35cf6f6..23db204 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -810,8 +810,11 @@ class ISnapEditableAttributes(IHasOwner):
title=_("Pocket for automatic builds"),
vocabulary=PackagePublishingPocket, required=False, readonly=False,
description=_(
- "The package stream within the source distribution series to use "
- "when building the snap package.")))
+ "The package stream within the source archive and distribution "
+ "series to use when building the snap package. If the source "
+ "archive is a PPA, then the PPA's archive dependencies will be "
+ "used to select the pocket in the distribution's primary "
+ "archive.")))
auto_build_channels = exported(Dict(
title=_("Source snap channels for automatic builds"),
diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
index e812f46..ec8b234 100644
--- a/lib/lp/snappy/interfaces/snapbuild.py
+++ b/lib/lp/snappy/interfaces/snapbuild.py
@@ -162,6 +162,12 @@ class ISnapBuildView(IPackageBuild):
pocket = exported(Choice(
title=_("The pocket for which to build."),
+ description=(
+ "The package stream within the source archive and distribution "
+ "series to use when building the snap package. If the source "
+ "archive is a PPA, then the PPA's archive dependencies will be "
+ "used to select the pocket in the distribution's primary "
+ "archive."),
vocabulary=PackagePublishingPocket, required=True, readonly=True))
channels = exported(Dict(
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 0ace53a..4cc91ac 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -700,6 +700,103 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
self.assertNotIn("channels", args)
@defer.inlineCallbacks
+ def test_extraBuildArgs_archives_primary(self):
+ # If the build is configured to use the primary archive as its
+ # source, then by default it uses the release, security, and updates
+ # pockets.
+ job = self.makeJob()
+ expected_archives = [
+ "deb %s %s main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ ]
+ with dbuser(config.builddmaster.dbuser):
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
+ def test_extraBuildArgs_archives_primary_non_default_pocket(self):
+ # If the build is configured to use the primary archive as its
+ # source, and it uses a non-default pocket, then it uses the
+ # corresponding expanded pockets in the primary archive.
+ job = self.makeJob(pocket=PackagePublishingPocket.SECURITY)
+ expected_archives = [
+ "deb %s %s main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ job.archive.archive_url, job.build.distro_series.name),
+ ]
+ with dbuser(config.builddmaster.dbuser):
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
+ def test_extraBuildArgs_archives_ppa(self):
+ # If the build is configured to use a PPA as its source, then by
+ # default it uses the release pocket in its source PPA, and the
+ # release, security, and updates pockets in the primary archive.
+ archive = self.factory.makeArchive()
+ job = self.makeJob(archive=archive)
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive,
+ distroarchseries=job.build.distro_series.architectures[0],
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ primary = job.build.distribution.main_archive
+ expected_archives = [
+ "deb %s %s main" % (
+ archive.archive_url, job.build.distro_series.name),
+ "deb %s %s main universe" % (
+ primary.archive_url, job.build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ primary.archive_url, job.build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ primary.archive_url, job.build.distro_series.name),
+ ]
+ with dbuser(config.builddmaster.dbuser):
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
+ def test_extraBuildArgs_archives_ppa_with_archive_dependencies(self):
+ # If the build is configured to use a PPA as its source, and it has
+ # archive dependencies, then they are honoured.
+ archive = self.factory.makeArchive()
+ lower_archive = self.factory.makeArchive(
+ distribution=archive.distribution)
+ job = self.makeJob(archive=archive)
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=archive,
+ distroarchseries=job.build.distro_series.architectures[0],
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=lower_archive,
+ distroarchseries=job.build.distro_series.architectures[0],
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ primary = job.build.distribution.main_archive
+ archive.addArchiveDependency(
+ lower_archive, PackagePublishingPocket.RELEASE)
+ archive.addArchiveDependency(primary, PackagePublishingPocket.SECURITY)
+ expected_archives = [
+ "deb %s %s main" % (
+ archive.archive_url, job.build.distro_series.name),
+ "deb %s %s main" % (
+ lower_archive.archive_url, job.build.distro_series.name),
+ "deb %s %s main universe" % (
+ primary.archive_url, job.build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ primary.archive_url, job.build.distro_series.name),
+ ]
+ with dbuser(config.builddmaster.dbuser):
+ extra_args = yield job.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
def test_extraBuildArgs_disallow_internet(self):
# If external network access is not allowed for the snap,
# extraBuildArgs does not dispatch a proxy token.
diff --git a/lib/lp/soyuz/adapters/archivedependencies.py b/lib/lp/soyuz/adapters/archivedependencies.py
index 12e5ff2..b80e0fe 100644
--- a/lib/lp/soyuz/adapters/archivedependencies.py
+++ b/lib/lp/soyuz/adapters/archivedependencies.py
@@ -196,10 +196,10 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
components = get_components_for_context(
archive_component, distro_series, archive_dependency.pocket)
# Follow pocket dependencies.
- for pocket in pocket_dependencies[archive_dependency.pocket]:
+ for expanded_pocket in pocket_dependencies[archive_dependency.pocket]:
deps.append(
- (archive_dependency.dependency, distro_arch_series, pocket,
- components))
+ (archive_dependency.dependency, distro_arch_series,
+ expanded_pocket, components))
# Consider build tools archive dependencies.
if tools_source is not None:
@@ -235,8 +235,10 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
components = get_components_for_context(
dsp.component, dep_arch_series.distroseries, dsp.pocket)
# Follow pocket dependencies.
- for pocket in pocket_dependencies[dsp.pocket]:
- deps.append((dep_archive, dep_arch_series, pocket, components))
+ for expanded_pocket in pocket_dependencies[dsp.pocket]:
+ deps.append(
+ (dep_archive, dep_arch_series, expanded_pocket,
+ components))
except NotFoundError:
pass
diff --git a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
index 6377868..35390f3 100644
--- a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
+++ b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
@@ -362,6 +362,87 @@ class TestBinaryBuildPackageBehaviour(StatsMixin, TestCaseWithFactory):
self.assertTrue(extra_args['arch_indep'])
@defer.inlineCallbacks
+ def test_extraBuildArgs_archives_proposed(self):
+ # A build in the primary archive's proposed pocket uses the release,
+ # security, updates, and proposed pockets in the primary archive.
+ builder = self.factory.makeBuilder()
+ build = self.factory.makeBinaryPackageBuild(
+ pocket=PackagePublishingPocket.PROPOSED)
+ for component_name in ("main", "universe"):
+ self.factory.makeComponentSelection(
+ build.distro_series, component_name)
+ behaviour = IBuildFarmJobBehaviour(build)
+ behaviour.setBuilder(builder, None)
+ expected_archives = [
+ "deb %s %s main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-proposed main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ ]
+ extra_args = yield behaviour.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
+ def test_extraBuildArgs_archives_backports(self):
+ # A build in the primary archive's backports pocket uses the
+ # release, security, updates, and backports pockets in the primary
+ # archive.
+ builder = self.factory.makeBuilder()
+ build = self.factory.makeBinaryPackageBuild(
+ pocket=PackagePublishingPocket.BACKPORTS)
+ for component_name in ("main", "universe"):
+ self.factory.makeComponentSelection(
+ build.distro_series, component_name)
+ behaviour = IBuildFarmJobBehaviour(build)
+ behaviour.setBuilder(builder, None)
+ expected_archives = [
+ "deb %s %s main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ "deb %s %s-backports main universe" % (
+ build.archive.archive_url, build.distro_series.name),
+ ]
+ extra_args = yield behaviour.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
+ def test_extraBuildArgs_archives_ppa(self):
+ # A build in a PPA uses the release pocket in the PPA and, by
+ # default, the release, security, and updates pockets in the primary
+ # archive.
+ archive = self.factory.makeArchive()
+ builder = self.factory.makeBuilder()
+ build = self.factory.makeBinaryPackageBuild(archive=archive)
+ for component_name in ("main", "universe"):
+ self.factory.makeComponentSelection(
+ build.distro_series, component_name)
+ self.factory.makeBinaryPackagePublishingHistory(
+ distroarchseries=build.distro_arch_series, pocket=build.pocket,
+ archive=archive, status=PackagePublishingStatus.PUBLISHED)
+ behaviour = IBuildFarmJobBehaviour(build)
+ behaviour.setBuilder(builder, None)
+ primary = build.distribution.main_archive
+ expected_archives = [
+ "deb %s %s main" % (
+ archive.archive_url, build.distro_series.name),
+ "deb %s %s main universe" % (
+ primary.archive_url, build.distro_series.name),
+ "deb %s %s-security main universe" % (
+ primary.archive_url, build.distro_series.name),
+ "deb %s %s-updates main universe" % (
+ primary.archive_url, build.distro_series.name),
+ ]
+ extra_args = yield behaviour.extraBuildArgs()
+ self.assertEqual(expected_archives, extra_args["archives"])
+
+ @defer.inlineCallbacks
def test_extraBuildArgs_archive_trusted_keys(self):
# If the archive has a signing key, extraBuildArgs sends it.
yield self.useFixture(InProcessKeyServerFixture()).start()
Follow ups
