launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-no-tools-source-if-snapcraft-snap into lp:launchpad.

Commit message:
Only honour snappy.tools_source configuration when snapcraft is being installed from apt.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-no-tools-source-if-snapcraft-snap/+merge/367858

This configuration item is just so that we have a way to use a different version of snapcraft in an emergency to fix regressions, and if we're installing it as a snap then we have other mechanisms for that.  Avoiding it in the snap case means that we don't have to put up with keyserver unreliability at build dispatch time when it doesn't really buy us anything.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-no-tools-source-if-snapcraft-snap into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py	2019-05-22 16:54:09 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py	2019-05-23 18:38:58 +0000
@@ -122,23 +122,27 @@
                     endpoint=config.snappy.builder_proxy_auth_api_endpoint,
                     token=token['username']))
         args["name"] = build.snap.store_name or build.snap.name
-        # XXX cjwatson 2015-08-03: Allow tools_source to be overridden at
-        # some more fine-grained level.
-        args["archives"], args["trusted_keys"] = (
-            yield get_sources_list_for_building(
-                build, build.distro_arch_series, None,
-                tools_source=config.snappy.tools_source,
-                tools_fingerprint=config.snappy.tools_fingerprint,
-                logger=logger))
         channels = build.channels or {}
         if "snapcraft" not in channels:
             channels["snapcraft"] = (
                 getFeatureFlag(SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG) or "apt")
-        if channels.get("snapcraft") != "apt":
+        if channels.get("snapcraft") == "apt":
+            # XXX cjwatson 2015-08-03: Allow tools_source to be overridden
+            # at some more fine-grained level.
+            tools_source = config.snappy.tools_source
+            tools_fingerprint = config.snappy.tools_fingerprint
+        else:
             # We have to remove the security proxy that Zope applies to this
             # dict, since otherwise we'll be unable to serialise it to
             # XML-RPC.
             args["channels"] = removeSecurityProxy(channels)
+            tools_source = None
+            tools_fingerprint = None
+        args["archives"], args["trusted_keys"] = (
+            yield get_sources_list_for_building(
+                build, build.distro_arch_series, None,
+                tools_source=tools_source, tools_fingerprint=tools_fingerprint,
+                logger=logger))
         if build.snap.branch is not None:
             args["branch"] = build.snap.branch.bzr_identity
         elif build.snap.git_ref is not None:

=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py	2019-05-22 16:54:09 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py	2019-05-23 18:38:58 +0000
@@ -106,7 +106,10 @@
     TestCaseWithFactory,
     )
 from lp.testing.dbuser import dbuser
-from lp.testing.gpgkeys import gpgkeysdir
+from lp.testing.gpgkeys import (
+    gpgkeysdir,
+    import_public_key,
+    )
 from lp.testing.keyserver import InProcessKeyServerFixture
 from lp.testing.layers import LaunchpadZopelessLayer
 from lp.xmlrpc.interfaces import IPrivateApplication
@@ -660,6 +663,48 @@
             ]))
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_tools_source_channels_apt(self):
+        # If snapcraft is being installed from apt, extraBuildArgs sends an
+        # extra archive to provide updates.
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        tools_source = (
+            "deb http://ppa.launchpad.net/snappy-dev/snapcraft-daily/ubuntu "
+            "%(series)s main")
+        tools_fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.pushConfig(
+            "snappy",
+            tools_source=tools_source, tools_fingerprint=tools_fingerprint)
+        import_public_key("test@xxxxxxxxxxxxx")
+        job = self.makeJob(channels={"snapcraft": "apt"})
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual(
+            tools_source % {"series": job.build.distro_series.name},
+            args["archives"][0])
+        self.assertThat(args["trusted_keys"], MatchesListwise([
+            Base64KeyMatches("A419AE861E88BC9E04B9C26FBA2B9389DFD20543"),
+            ]))
+
+    @defer.inlineCallbacks
+    def test_extraBuildArgs_tools_source_channels_snap(self):
+        # If snapcraft is being installed from apt, extraBuildArgs ignores
+        # tools_source.
+        yield self.useFixture(InProcessKeyServerFixture()).start()
+        tools_source = (
+            "deb http://ppa.launchpad.net/snappy-dev/snapcraft-daily/ubuntu "
+            "%(series)s main")
+        tools_fingerprint = "A419AE861E88BC9E04B9C26FBA2B9389DFD20543"
+        self.pushConfig(
+            "snappy",
+            tools_source=tools_source, tools_fingerprint=tools_fingerprint)
+        import_public_key("test@xxxxxxxxxxxxx")
+        job = self.makeJob(channels={"snapcraft": "stable"})
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertNotIn(tools_source, args["archives"])
+        self.assertEqual([], args["trusted_keys"])
+
+    @defer.inlineCallbacks
     def test_extraBuildArgs_channels(self):
         # If the build needs particular channels, extraBuildArgs sends them.
         job = self.makeJob(channels={"snapcraft": "edge"})