launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:oci-build-arg-overquoting into launchpad-buildd:master.

Commit message:
Stop overquoting OCI --build-arg options

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1902007 in launchpad-buildd: "OCI recipe ARG variables break quoting "
  https://bugs.launchpad.net/launchpad-buildd/+bug/1902007

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

We shouldn't use shell_escape when the result goes into an argument list rather than being interpreted by a shell.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:oci-build-arg-overquoting into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index d9471b6..33cc832 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,7 @@
 launchpad-buildd (194) UNRELEASED; urgency=medium
 
   * Stop setting $mailto in .sbuildrc, to work around LP #1859010.
+  * Stop overquoting OCI --build-arg options (LP: #1902007).
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 27 Oct 2020 13:22:05 +0000
 
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index 480c5d6..6a404fd 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -21,7 +21,6 @@ from lpbuildd.debian import (
     DebianBuildState,
     )
 from lpbuildd.snap import SnapBuildProxyMixin
-from lpbuildd.util import shell_escape
 
 
 RETCODE_SUCCESS = 0
@@ -74,8 +73,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
             args.extend(["--build-file", self.build_file])
         if self.build_args:
             for k, v in self.build_args.items():
-                args.extend([
-                    "--build-arg=%s=%s" % (shell_escape(k), shell_escape(v))])
+                args.extend(["--build-arg", "%s=%s" % (k, v)])
         if self.build_path is not None:
             args.extend(["--build-path", self.build_path])
         try:
diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
index bdea5f5..23641e4 100644
--- a/lpbuildd/tests/test_oci.py
+++ b/lpbuildd/tests/test_oci.py
@@ -196,14 +196,14 @@ class TestOCIBuildManagerIteration(TestCase):
             "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
             "git_path": "master",
             "build_file": "build-aux/Dockerfile",
-            "build_args": OrderedDict([("VAR1", "xxx"), ("VAR2", "yyy")]),
+            "build_args": OrderedDict([("VAR1", "xxx"), ("VAR2", "yyy zzz")]),
             }
         expected_options = [
             "--git-repository", "https://git.launchpad.dev/~example/+git/snap";,
             "--git-path", "master",
             "--build-file", "build-aux/Dockerfile",
-            "--build-arg=VAR1=xxx",
-            "--build-arg=VAR2=yyy",
+            "--build-arg", "VAR1=xxx",
+            "--build-arg", "VAR2=yyy zzz",
         ]
         yield self.startBuild(args, expected_options)