launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:snap-base-dispatch-dependencies into launchpad:master.

Commit message:
Dispatch SnapBase archive dependencies

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

We now dispatch these to snap builds in addition to the dependencies of the source archive, if any.  This will be used for the "core" snap base once 16.04 enters ESM.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-base-dispatch-dependencies into launchpad:master.

diff --git a/lib/lp/snappy/model/snapbuildbehaviour.py b/lib/lp/snappy/model/snapbuildbehaviour.py
index 4dd89f0..f554268 100644
--- a/lib/lp/snappy/model/snapbuildbehaviour.py
+++ b/lib/lp/snappy/model/snapbuildbehaviour.py
@@ -171,9 +171,13 @@ class SnapBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
             args["channels"] = removeSecurityProxy(channels)
             tools_source = None
             tools_fingerprint = None
+        archive_dependencies = list(build.archive.dependencies)
+        if build.snap_base is not None:
+            archive_dependencies.extend(build.snap_base.dependencies)
         args["archives"], args["trusted_keys"] = (
             yield get_sources_list_for_building(
                 build, build.distro_arch_series, None,
+                archive_dependencies=archive_dependencies,
                 tools_source=tools_source, tools_fingerprint=tools_fingerprint,
                 logger=logger))
         if build.snap.branch is not None:
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 4cc91ac..c64749d 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -101,6 +101,7 @@ from lp.soyuz.adapters.archivedependencies import (
     )
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.interfaces.archive import ArchiveDisabled
+from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.tests.soyuz import Base64KeyMatches
 from lp.testing import (
     TestCase,
@@ -797,6 +798,93 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
         self.assertEqual(expected_archives, extra_args["archives"])
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_snap_base_with_archive_dependencies(self):
+        # If the build is using a snap base that has archive dependencies,
+        # extraBuildArgs sends them.
+        snap_base = self.factory.makeSnapBase()
+        job = self.makeJob(snap_base=snap_base)
+        dependency = self.factory.makeArchive(
+            distribution=job.archive.distribution)
+        snap_base.addArchiveDependency(
+            dependency, PackagePublishingPocket.RELEASE,
+            getUtility(IComponentSet)["main"])
+        self.factory.makeBinaryPackagePublishingHistory(
+            archive=dependency, distroarchseries=job.build.distro_arch_series,
+            pocket=PackagePublishingPocket.RELEASE,
+            status=PackagePublishingStatus.PUBLISHED)
+        expected_archives = [
+            "deb %s %s main" % (
+                dependency.archive_url, job.build.distro_series.name),
+            "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):
+            args = yield job.extraBuildArgs()
+        self.assertEqual(expected_archives, args["archives"])
+
+    @defer.inlineCallbacks
+    def test_extraBuildArgs_ppa_and_snap_base_with_archive_dependencies(self):
+        # If the build is using a PPA and a snap base that both have archive
+        # dependencies, extraBuildArgs sends them all.
+        snap_base = self.factory.makeSnapBase()
+        upper_archive = self.factory.makeArchive()
+        lower_archive = self.factory.makeArchive(
+            distribution=upper_archive.distribution)
+        snap_base_archive = self.factory.makeArchive(
+            distribution=upper_archive.distribution)
+        job = self.makeJob(archive=upper_archive, snap_base=snap_base)
+        primary = job.build.distribution.main_archive
+        for archive in (upper_archive, lower_archive, snap_base_archive):
+            self.factory.makeBinaryPackagePublishingHistory(
+                archive=archive, distroarchseries=job.build.distro_arch_series,
+                pocket=PackagePublishingPocket.RELEASE,
+                status=PackagePublishingStatus.PUBLISHED)
+        upper_archive.addArchiveDependency(
+            lower_archive, PackagePublishingPocket.RELEASE)
+        snap_base.addArchiveDependency(
+            snap_base_archive, PackagePublishingPocket.RELEASE,
+            getUtility(IComponentSet)["main"])
+        expected_archives = [
+            "deb %s %s main" % (
+                upper_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" % (
+                snap_base_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):
+            args = yield job.extraBuildArgs()
+        self.assertEqual(expected_archives, args["archives"])
+
+    @defer.inlineCallbacks
+    def test_extraBuildArgs_snap_base_without_archive_dependencies(self):
+        # If the build is using a snap base that does not have archive
+        # dependencies, extraBuildArgs only sends the base archive.
+        snap_base = self.factory.makeSnapBase()
+        job = self.makeJob(snap_base=snap_base)
+        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):
+            args = yield job.extraBuildArgs()
+        self.assertEqual(expected_archives, 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 b80e0fe..c005651 100644
--- a/lib/lp/soyuz/adapters/archivedependencies.py
+++ b/lib/lp/soyuz/adapters/archivedependencies.py
@@ -153,8 +153,9 @@ def get_primary_current_component(archive, distroseries, sourcepackagename):
 
 
 def expand_dependencies(archive, distro_arch_series, pocket, component,
-                        source_package_name, tools_source=None,
-                        tools_fingerprint=None, logger=None):
+                        source_package_name, archive_dependencies,
+                        tools_source=None, tools_fingerprint=None,
+                        logger=None):
     """Return the set of dependency archives, pockets and components.
 
     :param archive: the context `IArchive`.
@@ -162,6 +163,8 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
     :param pocket: the context `PackagePublishingPocket`.
     :param component: the context `IComponent`.
     :param source_package_name: A source package name (as text)
+    :param archive_dependencies: a sequence of `IArchiveDependency` objects
+        to use as additional user-selected archive dependencies.
     :param tools_source: if not None, a sources.list entry to use as an
         additional dependency for build tools, just before the default
         primary archive.
@@ -185,7 +188,7 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
     primary_component = get_primary_current_component(
         archive, distro_series, source_package_name)
     # Consider user-selected archive dependencies.
-    for archive_dependency in archive.dependencies:
+    for archive_dependency in archive_dependencies:
         # When the dependency component is undefined, we should use
         # the component where the source is published in the primary
         # archive.
@@ -216,8 +219,9 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
 
     # Consider primary archive dependency override. Add the default
     # primary archive dependencies if it's not present.
-    if archive.getArchiveDependency(
-        archive.distribution.main_archive) is None:
+    if not any(
+            archive_dependency.dependency == archive.distribution.main_archive
+            for archive_dependency in archive_dependencies):
         primary_dependencies = _get_default_primary_dependencies(
             archive, distro_arch_series, component, pocket)
         deps.extend(primary_dependencies)
@@ -247,6 +251,7 @@ def expand_dependencies(archive, distro_arch_series, pocket, component,
 
 @defer.inlineCallbacks
 def get_sources_list_for_building(build, distroarchseries, sourcepackagename,
+                                  archive_dependencies=None,
                                   tools_source=None, tools_fingerprint=None,
                                   logger=None):
     """Return sources.list entries and keys required to build the given item.
@@ -262,6 +267,9 @@ def get_sources_list_for_building(build, distroarchseries, sourcepackagename,
     :param build: a context `IBuild`.
     :param distroarchseries: A `IDistroArchSeries`
     :param sourcepackagename: A source package name (as text)
+    :param archive_dependencies: a sequence of `IArchiveDependency` objects
+        to use as additional user-selected archive dependencies.  If None,
+        use the dependencies of the build's archive.
     :param tools_source: if not None, a sources.list entry to use as an
         additional dependency for build tools, just before the default
         primary archive.
@@ -272,9 +280,11 @@ def get_sources_list_for_building(build, distroarchseries, sourcepackagename,
         sources.list entries (lines) and a list of base64-encoded public
         keys.
     """
+    if archive_dependencies is None:
+        archive_dependencies = build.archive.dependencies
     deps = expand_dependencies(
         build.archive, distroarchseries, build.pocket,
-        build.current_component, sourcepackagename,
+        build.current_component, sourcepackagename, archive_dependencies,
         tools_source=tools_source, tools_fingerprint=tools_fingerprint,
         logger=logger)
     sources_list_lines, trusted_keys = (
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index c3cbaa1..f6ae7b7 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -1100,7 +1100,8 @@ class Archive(SQLBase):
                           source_package_name, dep_name):
         """See `IArchive`."""
         deps = expand_dependencies(
-            self, distro_arch_series, pocket, component, source_package_name)
+            self, distro_arch_series, pocket, component, source_package_name,
+            self.dependencies)
         archive_clause = Or([And(
             BinaryPackagePublishingHistory.archiveID == archive.id,
             BinaryPackagePublishingHistory.pocket == pocket,