launchpad-reviewers team mailing list archive

[Colin Watson <mp+436250@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:tolerate-constraints-None into launchpad-buildd:master.

Commit message:
Tolerate receiving "builder_constraints": None

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

While I've changed buildd-manager to pass the empty list rather than None as the quickest way to avoid a crash when `BuildManager.runTargetSubProcess` tries to iterate over `self.constraints`, it seems like a good idea to be cautious at both ends of the protocol.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:tolerate-constraints-None into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 8329ab4..060624f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+launchpad-buildd (227) UNRELEASED; urgency=medium
+
+  * Tolerate receiving "builder_constraints": None.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 24 Jan 2023 13:13:27 +0000
+
 launchpad-buildd (226) focal; urgency=medium
 
   * Remove unused "distribution" argument from the binarypackage manager.
diff --git a/lpbuildd/builder.py b/lpbuildd/builder.py
index a94f53a..c7a0f50 100644
--- a/lpbuildd/builder.py
+++ b/lpbuildd/builder.py
@@ -258,7 +258,7 @@ class BuildManager:
         self.series = extra_args['series']
         self.arch_tag = extra_args.get('arch_tag', self._builder.getArch())
         self.fast_cleanup = extra_args.get('fast_cleanup', False)
-        self.constraints = extra_args.get('builder_constraints', [])
+        self.constraints = extra_args.get('builder_constraints') or []
 
         # Check whether this is a build in a private archive and
         # whether the URLs in the buildlog file should be sanitized
diff --git a/lpbuildd/tests/test_debian.py b/lpbuildd/tests/test_debian.py
index 993260e..f57b7d3 100644
--- a/lpbuildd/tests/test_debian.py
+++ b/lpbuildd/tests/test_debian.py
@@ -469,3 +469,72 @@ class TestDebianBuildManagerIteration(TestCase):
             self.buildmanager.commands[-1])
         self.assertEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+    def test_iterate_no_constraints(self):
+        # If no `builder_constraints` argument is passed, the build manager
+        # passes no --constraint options to backend processes.
+        extra_args = {
+            'arch_tag': 'amd64',
+            'series': 'xenial',
+            }
+        self.startBuild(extra_args)
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.UNPACK, self.getState())
+        self.assertEqual(
+            (['sharepath/bin/in-target', 'in-target', 'unpack-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid,
+              '--image-type', 'chroot',
+              os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+    def test_iterate_constraints_None(self):
+        # If a `builder_constraints` argument of None is passed, the build
+        # manager passes no --constraint options to backend processes.
+        extra_args = {
+            'arch_tag': 'amd64',
+            'builder_constraints': None,
+            'series': 'xenial',
+            }
+        self.startBuild(extra_args)
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.UNPACK, self.getState())
+        self.assertEqual(
+            (['sharepath/bin/in-target', 'in-target', 'unpack-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              self.buildid,
+              '--image-type', 'chroot',
+              os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+    def test_iterate_constraints(self):
+        # If a `builder_constraints` argument is passed, the build manager
+        # passes corresponding --constraint options to backend processes.
+        extra_args = {
+            'arch_tag': 'amd64',
+            'builder_constraints': ['gpu', 'large'],
+            'series': 'xenial',
+            }
+        self.startBuild(extra_args)
+
+        self.buildmanager.iterate(0)
+        self.assertEqual(DebianBuildState.UNPACK, self.getState())
+        self.assertEqual(
+            (['sharepath/bin/in-target', 'in-target', 'unpack-chroot',
+              '--backend=chroot', '--series=xenial', '--arch=amd64',
+              '--constraint=gpu', '--constraint=large',
+              self.buildid,
+              '--image-type', 'chroot',
+              os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+             None),
+            self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])