launchpad-reviewers team mailing list archive

[Colin Watson <mp+424005@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:fix-run-ci into launchpad-buildd:master.

Commit message:
Fix passing of apt repository and environment variable options

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1977477 in launchpad-buildd: "launchpad fails to report build logs for failed conda builds"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1977477

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

They need to be passed to `run-ci`, not `run-ci-prepare`.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:fix-run-ci into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index 2924a63..463423d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+launchpad-buildd (214) UNRELEASED; urgency=medium
+
+  * Pass apt repository and environment variable options to run-ci, not
+    run-ci-prepare (LP: #1977477).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Mon, 06 Jun 2022 15:27:40 +0100
+
 launchpad-buildd (213) focal; urgency=medium
 
   [ Colin Watson ]
diff --git a/lpbuildd/ci.py b/lpbuildd/ci.py
index 36a720e..7b334d6 100644
--- a/lpbuildd/ci.py
+++ b/lpbuildd/ci.py
@@ -78,13 +78,6 @@ class CIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
             args.extend(["--git-repository", self.git_repository])
         if self.git_path is not None:
             args.extend(["--git-path", self.git_path])
-        if self.apt_repositories is not None:
-            for repository in self.apt_repositories:
-                args.extend(["--apt-repository", repository])
-        if self.environment_variables is not None:
-            for key, value in self.environment_variables.items():
-                args.extend(
-                    ["--environment-variable", "%s=%s" % (key, value)])
         try:
             snap_store_proxy_url = self._builder._config.get(
                 "proxy", "snapstore")
@@ -144,6 +137,13 @@ class CIBuildManager(BuildManagerProxyMixin, DebianBuildManager):
     def runNextJob(self):
         """Run the next CI job."""
         args = list(self.proxy_args)
+        if self.apt_repositories is not None:
+            for repository in self.apt_repositories:
+                args.extend(["--apt-repository", repository])
+        if self.environment_variables is not None:
+            for key, value in self.environment_variables.items():
+                args.extend(
+                    ["--environment-variable", f"{key}={value}"])
         job_name, job_index = self.current_job
         self.current_job_id = _make_job_id(job_name, job_index)
         args.extend([job_name, str(job_index)])
diff --git a/lpbuildd/tests/test_ci.py b/lpbuildd/tests/test_ci.py
index bd49fe4..7aea827 100644
--- a/lpbuildd/tests/test_ci.py
+++ b/lpbuildd/tests/test_ci.py
@@ -121,20 +121,22 @@ class TestCIBuildManagerIteration(TestCase):
             "jobs": [[("build", "0")], [("test", "0")]],
             "apt_repositories": ["repository one", "repository two"],
             "environment_variables": {
-                "INDEX": "http://example.com";, "PATH":"foo"},
+                "INDEX": "http://example.com";, "PATH": "foo"},
             }
-        expected_options = [
+        expected_prepare_options = [
             "--git-repository", "https://git.launchpad.test/~example/+git/ci";,
             "--git-path", "main",
+            ]
+        yield self.startBuild(args, expected_prepare_options)
+
+        # After preparation, start running the first job.
+        expected_job_options = [
             "--apt-repository", "repository one",
             "--apt-repository", "repository two",
             "--environment-variable", "INDEX=http://example.com";,
             "--environment-variable", "PATH=foo",
             ]
-        yield self.startBuild(args, expected_options)
-
-        # After preparation, start running the first job.
-        yield self.expectRunJob("build", "0")
+        yield self.expectRunJob("build", "0", options=expected_job_options)
         self.buildmanager.backend.add_file(
             "/build/output/build:0.log", b"I am a CI build job log.")
         self.buildmanager.backend.add_file(
@@ -142,7 +144,7 @@ class TestCIBuildManagerIteration(TestCase):
             b"I am output from a CI build job.")
 
         # Collect the output of the first job and start running the second.
-        yield self.expectRunJob("test", "0")
+        yield self.expectRunJob("test", "0", options=expected_job_options)
         self.buildmanager.backend.add_file(
             "/build/output/test:0.log", b"I am a CI test job log.")
         self.buildmanager.backend.add_file(
@@ -244,27 +246,31 @@ class TestCIBuildManagerIteration(TestCase):
             "jobs": [[("lint", "0"), ("build", "0")], [("test", "0")]],
             "apt_repositories": ["repository one", "repository two"],
             "environment_variables": {
-                "INDEX": "http://example.com";, "PATH":"foo"},
+                "INDEX": "http://example.com";, "PATH": "foo"},
             }
-        expected_options = [
+        expected_prepare_options = [
             "--git-repository", "https://git.launchpad.test/~example/+git/ci";,
             "--git-path", "main",
+            ]
+        yield self.startBuild(args, expected_prepare_options)
+
+        # After preparation, start running the first job.
+        expected_job_options = [
             "--apt-repository", "repository one",
             "--apt-repository", "repository two",
             "--environment-variable", "INDEX=http://example.com";,
             "--environment-variable", "PATH=foo",
             ]
-        yield self.startBuild(args, expected_options)
-
-        # After preparation, start running the first job.
-        yield self.expectRunJob("lint", "0")
+        yield self.expectRunJob("lint", "0", options=expected_job_options)
         self.buildmanager.backend.add_file(
             "/build/output/lint:0.log", b"I am a failing CI lint job log.")
 
         # Collect the output of the first job and start running the second.
         # (Note that `retcode` is the return code of the *first* job, not the
         # second.)
-        yield self.expectRunJob("build", "0", retcode=RETCODE_FAILURE_BUILD)
+        yield self.expectRunJob(
+            "build", "0", options=expected_job_options,
+            retcode=RETCODE_FAILURE_BUILD)
         self.buildmanager.backend.add_file(
             "/build/output/build:0.log", b"I am a CI build job log.")