Merge ~cjwatson/launchpad-buildd:oci-build-arg-overquoting into launchpad-buildd:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: a038595d54b52c38e43a3ba4ebd92199b348b9d5
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad-buildd:oci-build-arg-overquoting
Merge into: launchpad-buildd:master
Diff against target: 55 lines (+5/-6)
3 files modified
debian/changelog (+1/-0)
lpbuildd/oci.py (+1/-3)
lpbuildd/tests/test_oci.py (+3/-3)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+393036@code.launchpad.net

Commit message

Stop overquoting OCI --build-arg options

Description of the change

We shouldn't use shell_escape when the result goes into an argument list rather than being interpreted by a shell.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index d9471b6..33cc832 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,6 +1,7 @@
6 launchpad-buildd (194) UNRELEASED; urgency=medium
7
8 * Stop setting $mailto in .sbuildrc, to work around LP #1859010.
9+ * Stop overquoting OCI --build-arg options (LP: #1902007).
10
11 -- Colin Watson <cjwatson@ubuntu.com> Tue, 27 Oct 2020 13:22:05 +0000
12
13diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
14index 480c5d6..6a404fd 100644
15--- a/lpbuildd/oci.py
16+++ b/lpbuildd/oci.py
17@@ -21,7 +21,6 @@ from lpbuildd.debian import (
18 DebianBuildState,
19 )
20 from lpbuildd.snap import SnapBuildProxyMixin
21-from lpbuildd.util import shell_escape
22
23
24 RETCODE_SUCCESS = 0
25@@ -74,8 +73,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
26 args.extend(["--build-file", self.build_file])
27 if self.build_args:
28 for k, v in self.build_args.items():
29- args.extend([
30- "--build-arg=%s=%s" % (shell_escape(k), shell_escape(v))])
31+ args.extend(["--build-arg", "%s=%s" % (k, v)])
32 if self.build_path is not None:
33 args.extend(["--build-path", self.build_path])
34 try:
35diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
36index bdea5f5..23641e4 100644
37--- a/lpbuildd/tests/test_oci.py
38+++ b/lpbuildd/tests/test_oci.py
39@@ -196,14 +196,14 @@ class TestOCIBuildManagerIteration(TestCase):
40 "git_repository": "https://git.launchpad.dev/~example/+git/snap",
41 "git_path": "master",
42 "build_file": "build-aux/Dockerfile",
43- "build_args": OrderedDict([("VAR1", "xxx"), ("VAR2", "yyy")]),
44+ "build_args": OrderedDict([("VAR1", "xxx"), ("VAR2", "yyy zzz")]),
45 }
46 expected_options = [
47 "--git-repository", "https://git.launchpad.dev/~example/+git/snap",
48 "--git-path", "master",
49 "--build-file", "build-aux/Dockerfile",
50- "--build-arg=VAR1=xxx",
51- "--build-arg=VAR2=yyy",
52+ "--build-arg", "VAR1=xxx",
53+ "--build-arg", "VAR2=yyy zzz",
54 ]
55 yield self.startBuild(args, expected_options)
56

Subscribers

People subscribed via source and target branches