launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/run-non-ascii-arguments into lp:launchpad-buildd.

Commit message:
Encode non-bytes subprocess arguments on Python 2.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1832072 in launchpad-buildd: "Snap build failed with "Failed to gather results: execv() arg 2 must contain only strings""
  https://bugs.launchpad.net/launchpad-buildd/+bug/1832072

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/run-non-ascii-arguments/+merge/368588

This avoids crashing on non-ASCII file names under LC_CTYPE=C.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/run-non-ascii-arguments into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2019-06-05 14:10:20 +0000
+++ debian/changelog	2019-06-09 07:50:37 +0000
@@ -2,6 +2,8 @@
 
   * Allow configuring APT or snap store proxies via a new [proxy]
     configuration file section.
+  * Encode non-bytes subprocess arguments on Python 2 to avoid crashing on
+    non-ASCII file names under LC_CTYPE=C (LP: #1832072).
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Wed, 05 Jun 2019 15:06:54 +0100
 

=== modified file 'lpbuildd/target/chroot.py'
--- lpbuildd/target/chroot.py	2019-01-10 18:14:55 +0000
+++ lpbuildd/target/chroot.py	2019-06-09 07:50:37 +0000
@@ -11,6 +11,8 @@
 import subprocess
 import time
 
+import six
+
 from lpbuildd.target.backend import (
     Backend,
     BackendException,
@@ -77,6 +79,10 @@
         if echo:
             print("Running in chroot: %s" % ' '.join(
                 shell_escape(arg) for arg in args))
+        if six.PY2:
+            # The behaviour of non-bytes subprocess arguments in Python 2
+            # depends on the interpreter's startup locale.
+            args = [arg.encode("UTF-8") for arg in args]
         cmd = ["sudo", "/usr/sbin/chroot", self.chroot_path] + args
         if input_text is None and not get_output:
             subprocess.check_call(cmd, **kwargs)

=== modified file 'lpbuildd/target/lxd.py'
--- lpbuildd/target/lxd.py	2019-01-10 18:14:55 +0000
+++ lpbuildd/target/lxd.py	2019-06-09 07:50:37 +0000
@@ -20,6 +20,7 @@
 import netaddr
 import pylxd
 from pylxd.exceptions import LXDAPIException
+import six
 
 from lpbuildd.target.backend import (
     Backend,
@@ -490,6 +491,10 @@
         if echo:
             print("Running in container: %s" % ' '.join(
                 shell_escape(arg) for arg in args))
+        if six.PY2:
+            # The behaviour of non-bytes subprocess arguments in Python 2
+            # depends on the interpreter's startup locale.
+            args = [arg.encode("UTF-8") for arg in args]
         # pylxd's Container.execute doesn't support sending stdin, and it's
         # tedious to implement ourselves.
         cmd = ["lxc", "exec", self.name] + env_params + ["--"] + args

=== modified file 'lpbuildd/target/tests/test_chroot.py'
--- lpbuildd/target/tests/test_chroot.py	2019-01-10 18:14:55 +0000
+++ lpbuildd/target/tests/test_chroot.py	2019-06-09 07:50:37 +0000
@@ -13,6 +13,7 @@
     EnvironmentVariable,
     TempDir,
     )
+import six
 from systemfixtures import (
     FakeFilesystem,
     FakeProcesses,
@@ -117,6 +118,22 @@
             expected_args,
             [proc._args["args"] for proc in processes_fixture.procs])
 
+    def test_run_non_ascii_arguments(self):
+        self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
+        processes_fixture = self.useFixture(FakeProcesses())
+        processes_fixture.add(lambda _: {}, name="sudo")
+        arg = u"\N{SNOWMAN}"
+        Chroot("1", "xenial", "amd64").run(["echo", arg])
+
+        expected_args = [
+            ["sudo", "/usr/sbin/chroot",
+             "/expected/home/build-1/chroot-autobuild",
+             "linux64", "echo", arg.encode("UTF-8") if six.PY2 else arg],
+            ]
+        self.assertEqual(
+            expected_args,
+            [proc._args["args"] for proc in processes_fixture.procs])
+
     def test_copy_in(self):
         self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
         source_dir = self.useFixture(TempDir()).path

=== modified file 'lpbuildd/target/tests/test_lxd.py'
--- lpbuildd/target/tests/test_lxd.py	2019-01-10 18:14:55 +0000
+++ lpbuildd/target/tests/test_lxd.py	2019-06-09 07:50:37 +0000
@@ -25,6 +25,7 @@
     )
 import pylxd
 from pylxd.exceptions import LXDAPIException
+import six
 from systemfixtures import (
     FakeFilesystem,
     FakeProcesses,
@@ -530,6 +531,20 @@
             expected_args,
             [proc._args["args"] for proc in processes_fixture.procs])
 
+    def test_run_non_ascii_arguments(self):
+        processes_fixture = self.useFixture(FakeProcesses())
+        processes_fixture.add(lambda _: {}, name="lxc")
+        arg = u"\N{SNOWMAN}"
+        LXD("1", "xenial", "amd64").run(["echo", arg])
+
+        expected_args = [
+            ["lxc", "exec", "lp-xenial-amd64", "--",
+             "linux64", "echo", arg.encode("UTF-8") if six.PY2 else arg],
+            ]
+        self.assertEqual(
+            expected_args,
+            [proc._args["args"] for proc in processes_fixture.procs])
+
     def test_copy_in(self):
         source_dir = self.useFixture(TempDir()).path
         self.useFixture(MockPatch("pylxd.Client"))