Merge lp:~cjwatson/launchpad-buildd/backend-run-cwd into lp:launchpad-buildd

[Colin Watson]

Make Backend.run(cwd=) work, and refactor BuildLiveFS and BuildSnap to
use it. This fixes translation templates builds, which were assuming
that this worked.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'debian/changelog'
--- debian/changelog	2017-11-09 14:55:22 +0000
+++ debian/changelog	2017-11-10 20:56:18 +0000
@@ -5,6 +5,9 @@
   * Replace shell_escape function with shlex.quote (Python 3) or pipes.quote
     (Python 2).
   * Fix handling of null/empty-domain case in generate_pots.
+  * Make Backend.run(cwd=) work, and refactor BuildLiveFS and BuildSnap to
+    use it.  This fixes translation templates builds, which were assuming
+    that this worked.
 
  -- Colin Watson <[e-mail]>  Thu, 09 Nov 2017 12:08:42 +0000
 

=== modified file 'lpbuildd/target/backend.py'
--- lpbuildd/target/backend.py	2017-11-01 23:08:22 +0000
+++ lpbuildd/target/backend.py	2017-11-10 20:56:18 +0000
@@ -36,11 +36,12 @@
         """
         raise NotImplementedError
 
-    def run(self, args, env=None, input_text=None, get_output=False,
+    def run(self, args, cwd=None, env=None, input_text=None, get_output=False,
             echo=False, **kwargs):
         """Run a command in the target environment.
 
         :param args: the command and arguments to run.
+        :param cwd: run the command in this working directory in the target.
         :param env: additional environment variables to set.
         :param input_text: input text to pass on the command's stdin.
         :param get_output: if True, return the output from the command.

=== modified file 'lpbuildd/target/build_livefs.py'
--- lpbuildd/target/build_livefs.py	2017-09-13 09:32:19 +0000
+++ lpbuildd/target/build_livefs.py	2017-11-10 20:56:18 +0000
@@ -10,7 +10,6 @@
 import os
 
 from lpbuildd.target.operation import Operation
-from lpbuildd.util import shell_escape
 
 
 RETCODE_FAILURE_INSTALL = 200
@@ -62,25 +61,13 @@
             "--debug", default=False, action="store_true",
             help="enable detailed live-build debugging")
 
-    def run_build_command(self, args, env=None, echo=False):
+    def run_build_command(self, args, **kwargs):
         """Run a build command in the chroot.
 
-        This is unpleasant because we need to run it in /build under sudo
-        chroot, and there's no way to do this without either a helper
-        program in the chroot or unpleasant quoting.  We go for the
-        unpleasant quoting.
-
         :param args: the command and arguments to run.
-        :param env: dictionary of additional environment variables to set.
-        :param echo: if True, print the command before executing it.
+        :param kwargs: any other keyword arguments to pass to Backend.run.
         """
-        args = [shell_escape(arg) for arg in args]
-        if env:
-            args = ["env"] + [
-                "%s=%s" % (key, shell_escape(value))
-                for key, value in env.items()] + args
-        command = "cd /build && %s" % " ".join(args)
-        self.backend.run(["/bin/sh", "-c", command], echo=echo)
+        return self.backend.run(args, cwd="/build", **kwargs)
 
     def install(self):
         deps = ["livecd-rootfs"]

=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2017-10-27 07:52:32 +0000
+++ lpbuildd/target/build_snap.py	2017-11-10 20:56:18 +0000
@@ -32,7 +32,6 @@
     from urlparse import urlparse
 
 from lpbuildd.target.operation import Operation
-from lpbuildd.util import shell_escape
 
 
 RETCODE_FAILURE_INSTALL = 200
@@ -73,34 +72,20 @@
         # appropriate certificate for your codehosting system.
         self.ssl_verify = True
 
-    def run_build_command(self, args, path="/build", env=None,
-                          get_output=False, echo=False):
+    def run_build_command(self, args, cwd="/build", env=None, **kwargs):
         """Run a build command in the target.
 
-        This is unpleasant because we need to run it with /build as the
-        working directory, and there's no way to do this without either a
-        helper program in the target or unpleasant quoting.  We go for the
-        unpleasant quoting.
-
         :param args: the command and arguments to run.
-        :param path: the working directory to use in the target.
+        :param cwd: run the command in this working directory in the target.
         :param env: dictionary of additional environment variables to set.
-        :param get_output: if True, return the output from the command.
-        :param echo: if True, print the command before executing it.
+        :param kwargs: any other keyword arguments to pass to Backend.run.
         """
-        args = [shell_escape(arg) for arg in args]
-        path = shell_escape(path)
         full_env = OrderedDict()
         full_env["LANG"] = "C.UTF-8"
         full_env["SHELL"] = "/bin/sh"
         if env:
             full_env.update(env)
-        args = ["env"] + [
-            "%s=%s" % (key, shell_escape(value))
-            for key, value in full_env.items()] + args
-        command = "cd %s && %s" % (path, " ".join(args))
-        return self.backend.run(
-            ["/bin/sh", "-c", command], get_output=get_output, echo=echo)
+        return self.backend.run(args, cwd=cwd, env=full_env, **kwargs)
 
     def save_status(self, status):
         """Save a dictionary of status information about this build.
@@ -190,7 +175,7 @@
             env["GIT_PROXY_COMMAND"] = "/usr/local/bin/snap-git-proxy"
         self.run_build_command(
             ["snapcraft", "pull"],
-            path=os.path.join("/build", self.args.name),
+            cwd=os.path.join("/build", self.args.name),
             env=env)
 
     def build(self):
@@ -203,7 +188,7 @@
             env["GIT_PROXY_COMMAND"] = "/usr/local/bin/snap-git-proxy"
         self.run_build_command(
             ["snapcraft"],
-            path=os.path.join("/build", self.args.name),
+            cwd=os.path.join("/build", self.args.name),
             env=env)
 
     def revoke_token(self):

=== modified file 'lpbuildd/target/chroot.py'
--- lpbuildd/target/chroot.py	2017-11-09 12:09:12 +0000
+++ lpbuildd/target/chroot.py	2017-11-10 20:56:18 +0000
@@ -52,7 +52,7 @@
         for path in ("/etc/hosts", "/etc/hostname", "/etc/resolv.conf"):
             self.copy_in(path, path)
 
-    def run(self, args, env=None, input_text=None, get_output=False,
+    def run(self, args, cwd=None, env=None, input_text=None, get_output=False,
             echo=False, **kwargs):
         """See `Backend`."""
         if env:
@@ -61,6 +61,16 @@
                 for key, value in env.items()] + args
         if self.arch is not None:
             args = set_personality(args, self.arch, series=self.series)
+        if cwd is not None:
+            # This requires either a helper program in the chroot or
+            # unpleasant quoting.  For now we go for the unpleasant quoting,
+            # though once we have coreutils >= 8.28 everywhere we'll be able
+            # to use "env --chdir".
+            args = [
+                "/bin/sh", "-c", "cd %s && %s" % (
+                    shell_escape(cwd),
+                    " ".join(shell_escape(arg) for arg in args)),
+                ]
         if echo:
             print("Running in chroot: %s" % ' '.join(
                 shell_escape(arg) for arg in args))

=== modified file 'lpbuildd/target/lxd.py'
--- lpbuildd/target/lxd.py	2017-11-01 11:17:44 +0000
+++ lpbuildd/target/lxd.py	2017-11-10 20:56:18 +0000
@@ -419,7 +419,7 @@
                 no_cdn_file.name,
                 "/etc/systemd/system/snapd.service.d/no-cdn.conf")
 
-    def run(self, args, env=None, input_text=None, get_output=False,
+    def run(self, args, cwd=None, env=None, input_text=None, get_output=False,
             echo=False, **kwargs):
         """See `Backend`."""
         env_params = []
@@ -428,6 +428,16 @@
                 env_params.extend(["--env", "%s=%s" % (key, value)])
         if self.arch is not None:
             args = set_personality(args, self.arch, series=self.series)
+        if cwd is not None:
+            # This requires either a helper program in the chroot or
+            # unpleasant quoting.  For now we go for the unpleasant quoting,
+            # though once we have coreutils >= 8.28 everywhere we'll be able
+            # to use "env --chdir".
+            args = [
+                "/bin/sh", "-c", "cd %s && %s" % (
+                    shell_escape(cwd),
+                    " ".join(shell_escape(arg) for arg in args)),
+                ]
         if echo:
             print("Running in container: %s" % ' '.join(
                 shell_escape(arg) for arg in args))

=== modified file 'lpbuildd/target/tests/test_build_livefs.py'
--- lpbuildd/target/tests/test_build_livefs.py	2017-11-09 12:15:39 +0000
+++ lpbuildd/target/tests/test_build_livefs.py	2017-11-10 20:56:18 +0000
@@ -26,12 +26,15 @@
 
 class RanCommand(MatchesListwise):
 
-    def __init__(self, args, echo=None, **env):
+    def __init__(self, args, echo=None, cwd=None, **env):
         kwargs_matcher = {}
         if echo is not None:
             kwargs_matcher["echo"] = Is(echo)
+        if cwd:
+            kwargs_matcher["cwd"] = Equals(cwd)
         if env:
-            kwargs_matcher["env"] = MatchesDict(env)
+            kwargs_matcher["env"] = MatchesDict(
+                {key: Equals(value) for key, value in env.items()})
         super(RanCommand, self).__init__(
             [Equals((args,)), MatchesDict(kwargs_matcher)])
 
@@ -44,9 +47,8 @@
 
 class RanBuildCommand(RanCommand):
 
-    def __init__(self, command):
-        super(RanBuildCommand, self).__init__(
-            ["/bin/sh", "-c", "cd /build && " + command], echo=False)
+    def __init__(self, args, **kwargs):
+        super(RanBuildCommand, self).__init__(args, cwd="/build", **kwargs)
 
 
 class TestBuildLiveFS(TestCase):
@@ -59,7 +61,7 @@
         build_livefs = parse_args(args=args).operation
         build_livefs.run_build_command(["echo", "hello world"])
         self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
-            RanBuildCommand("echo 'hello world'"),
+            RanBuildCommand(["echo", "hello world"]),
             ]))
 
     def test_run_build_command_env(self):
@@ -71,7 +73,7 @@
         build_livefs.run_build_command(
             ["echo", "hello world"], env={"FOO": "bar baz"})
         self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
-            RanBuildCommand("env FOO='bar baz' echo 'hello world'"),
+            RanBuildCommand(["echo", "hello world"], FOO="bar baz"),
             ]))
 
     def test_install(self):
@@ -120,18 +122,22 @@
         build_livefs = parse_args(args=args).operation
         build_livefs.build()
         self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
-            RanBuildCommand("rm -rf auto local"),
-            RanBuildCommand("mkdir -p auto"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/config auto/"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/build auto/"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/clean auto/"),
-            RanBuildCommand("lb clean --purge"),
-            RanBuildCommand(
-                "env PROJECT=ubuntu ARCH=amd64 SUITE=xenial lb config"),
-            RanBuildCommand("env PROJECT=ubuntu ARCH=amd64 lb build"),
+            RanBuildCommand(["rm", "-rf", "auto", "local"]),
+            RanBuildCommand(["mkdir", "-p", "auto"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/config", "auto/"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/build", "auto/"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/clean", "auto/"]),
+            RanBuildCommand(["lb", "clean", "--purge"]),
+            RanBuildCommand(
+                ["lb", "config"],
+                PROJECT="ubuntu", ARCH="amd64", SUITE="xenial"),
+            RanBuildCommand(["lb", "build"], PROJECT="ubuntu", ARCH="amd64"),
             ]))
 
     def test_build_locale(self):
@@ -144,8 +150,8 @@
         build_livefs.build()
         self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
             RanBuildCommand(
-                "ubuntu-defaults-image --locale zh_CN --arch amd64 "
-                "--release xenial"),
+                ["ubuntu-defaults-image", "--locale", "zh_CN",
+                 "--arch", "amd64", "--release", "xenial"]),
             ]))
 
     def test_build_debug(self):
@@ -157,21 +163,25 @@
         build_livefs = parse_args(args=args).operation
         build_livefs.build()
         self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
-            RanBuildCommand("rm -rf auto local"),
-            RanBuildCommand("mkdir -p auto"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/config auto/"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/build auto/"),
-            RanBuildCommand(
-                "ln -s /usr/share/livecd-rootfs/live-build/auto/clean auto/"),
-            RanBuildCommand("mkdir -p local/functions"),
-            RanBuildCommand(
-                "sh -c 'echo '\"'\"'set -x'\"'\"' >local/functions/debug.sh'"),
-            RanBuildCommand("lb clean --purge"),
-            RanBuildCommand(
-                "env PROJECT=ubuntu ARCH=amd64 SUITE=xenial lb config"),
-            RanBuildCommand("env PROJECT=ubuntu ARCH=amd64 lb build"),
+            RanBuildCommand(["rm", "-rf", "auto", "local"]),
+            RanBuildCommand(["mkdir", "-p", "auto"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/config", "auto/"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/build", "auto/"]),
+            RanBuildCommand(
+                ["ln", "-s",
+                 "/usr/share/livecd-rootfs/live-build/auto/clean", "auto/"]),
+            RanBuildCommand(["mkdir", "-p", "local/functions"]),
+            RanBuildCommand(
+                ["sh", "-c", "echo 'set -x' >local/functions/debug.sh"]),
+            RanBuildCommand(["lb", "clean", "--purge"]),
+            RanBuildCommand(
+                ["lb", "config"],
+                PROJECT="ubuntu", ARCH="amd64", SUITE="xenial"),
+            RanBuildCommand(["lb", "build"], PROJECT="ubuntu", ARCH="amd64"),
             ]))
 
     def test_run_succeeds(self):
@@ -185,7 +195,7 @@
         self.assertThat(build_livefs.backend.run.calls, MatchesAll(
             AnyMatch(RanAptGet("install", "livecd-rootfs")),
             AnyMatch(RanBuildCommand(
-                "env PROJECT=ubuntu ARCH=amd64 lb build"))))
+                ["lb", "build"], PROJECT="ubuntu", ARCH="amd64"))))
 
     def test_run_install_fails(self):
         class FailInstall(FakeMethod):
@@ -208,7 +218,7 @@
         class FailBuild(FakeMethod):
             def __call__(self, run_args, *args, **kwargs):
                 super(FailBuild, self).__call__(run_args, *args, **kwargs)
-                if run_args[0] == "/bin/sh":
+                if run_args[0] == "rm":
                     raise subprocess.CalledProcessError(1, run_args)
 
         self.useFixture(FakeLogger())

=== modified file 'lpbuildd/target/tests/test_build_snap.py'
--- lpbuildd/target/tests/test_build_snap.py	2017-09-13 12:21:44 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2017-11-10 20:56:18 +0000
@@ -33,14 +33,17 @@
 
 class RanCommand(MatchesListwise):
 
-    def __init__(self, args, get_output=None, echo=None, **env):
+    def __init__(self, args, get_output=None, echo=None, cwd=None, **env):
         kwargs_matcher = {}
         if get_output is not None:
             kwargs_matcher["get_output"] = Is(get_output)
         if echo is not None:
             kwargs_matcher["echo"] = Is(echo)
+        if cwd:
+            kwargs_matcher["cwd"] = Equals(cwd)
         if env:
-            kwargs_matcher["env"] = MatchesDict(env)
+            kwargs_matcher["env"] = MatchesDict(
+                {key: Equals(value) for key, value in env.items()})
         super(RanCommand, self).__init__(
             [Equals((args,)), MatchesDict(kwargs_matcher)])
 
@@ -53,10 +56,10 @@
 
 class RanBuildCommand(RanCommand):
 
-    def __init__(self, command, path="/build", get_output=False):
-        super(RanBuildCommand, self).__init__(
-            ["/bin/sh", "-c", "cd %s && %s" % (path, command)],
-            get_output=get_output, echo=False)
+    def __init__(self, args, cwd="/build", **kwargs):
+        kwargs.setdefault("LANG", "C.UTF-8")
+        kwargs.setdefault("SHELL", "/bin/sh")
+        super(RanBuildCommand, self).__init__(args, cwd=cwd, **kwargs)
 
 
 class FakeRevisionID(FakeMethod):
@@ -67,10 +70,9 @@
 
     def __call__(self, run_args, *args, **kwargs):
         super(FakeRevisionID, self).__call__(run_args, *args, **kwargs)
-        if run_args[0] == "/bin/sh":
-            command = run_args[2]
-            if "bzr revno" in command or "rev-parse" in command:
-                return "%s\n" % self.revision_id
+        if (run_args[:2] == ["bzr", "revno"] or
+                (run_args[0] == "git" and "rev-parse" in run_args)):
+            return "%s\n" % self.revision_id
 
 
 class TestBuildSnap(TestCase):
@@ -84,8 +86,7 @@
         build_snap = parse_args(args=args).operation
         build_snap.run_build_command(["echo", "hello world"])
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh echo 'hello world'"),
+            RanBuildCommand(["echo", "hello world"]),
             ]))
 
     def test_run_build_command_env(self):
@@ -98,9 +99,7 @@
         build_snap.run_build_command(
             ["echo", "hello world"], env={"FOO": "bar baz"})
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh FOO='bar baz' "
-                "echo 'hello world'"),
+            RanBuildCommand(["echo", "hello world"], FOO="bar baz"),
             ]))
 
     def test_install_bzr(self):
@@ -160,11 +159,10 @@
         build_snap.backend.build_path = self.useFixture(TempDir()).path
         build_snap.backend.run = FakeRevisionID("42")
         build_snap.repo()
-        env = "env LANG=C.UTF-8 SHELL=/bin/sh "
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "ls /build"),
-            RanBuildCommand(env + "bzr branch lp:foo test-snap"),
-            RanBuildCommand(env + "bzr revno test-snap", get_output=True),
+            RanBuildCommand(["ls", "/build"]),
+            RanBuildCommand(["bzr", "branch", "lp:foo", "test-snap"]),
+            RanBuildCommand(["bzr", "revno", "test-snap"], get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
         with open(status_path) as status:
@@ -180,13 +178,14 @@
         build_snap.backend.build_path = self.useFixture(TempDir()).path
         build_snap.backend.run = FakeRevisionID("0" * 40)
         build_snap.repo()
-        env = "env LANG=C.UTF-8 SHELL=/bin/sh "
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "git clone lp:foo test-snap"),
-            RanBuildCommand(
-                env + "git -C test-snap submodule update --init --recursive"),
-            RanBuildCommand(
-                env + "git -C test-snap rev-parse HEAD", get_output=True),
+            RanBuildCommand(["git", "clone", "lp:foo", "test-snap"]),
+            RanBuildCommand(
+                ["git", "-C", "test-snap",
+                 "submodule", "update", "--init", "--recursive"]),
+            RanBuildCommand(
+                ["git", "-C", "test-snap", "rev-parse", "HEAD"],
+                get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
         with open(status_path) as status:
@@ -202,13 +201,15 @@
         build_snap.backend.build_path = self.useFixture(TempDir()).path
         build_snap.backend.run = FakeRevisionID("0" * 40)
         build_snap.repo()
-        env = "env LANG=C.UTF-8 SHELL=/bin/sh "
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "git clone -b next lp:foo test-snap"),
-            RanBuildCommand(
-                env + "git -C test-snap submodule update --init --recursive"),
-            RanBuildCommand(
-                env + "git -C test-snap rev-parse next", get_output=True),
+            RanBuildCommand(
+                ["git", "clone", "-b", "next", "lp:foo", "test-snap"]),
+            RanBuildCommand(
+                ["git", "-C", "test-snap",
+                 "submodule", "update", "--init", "--recursive"]),
+            RanBuildCommand(
+                ["git", "-C", "test-snap", "rev-parse", "next"],
+                get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
         with open(status_path) as status:
@@ -226,18 +227,18 @@
         build_snap.backend.build_path = self.useFixture(TempDir()).path
         build_snap.backend.run = FakeRevisionID("0" * 40)
         build_snap.repo()
-        env = (
-            "env LANG=C.UTF-8 SHELL=/bin/sh "
-            "http_proxy=http://proxy.example:3128/ "
-            "https_proxy=http://proxy.example:3128/ "
-            "GIT_PROXY_COMMAND=/usr/local/bin/snap-git-proxy ")
+        env = {
+            "http_proxy": "http://proxy.example:3128/",
+            "https_proxy": "http://proxy.example:3128/",
+            "GIT_PROXY_COMMAND": "/usr/local/bin/snap-git-proxy",
+            }
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "git clone lp:foo test-snap"),
-            RanBuildCommand(
-                env + "git -C test-snap submodule update --init --recursive"),
-            RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh "
-                "git -C test-snap rev-parse HEAD",
+            RanBuildCommand(["git", "clone", "lp:foo", "test-snap"], **env),
+            RanBuildCommand(
+                ["git", "-C", "test-snap",
+                 "submodule", "update", "--init", "--recursive"], **env),
+            RanBuildCommand(
+                ["git", "-C", "test-snap", "rev-parse", "HEAD"],
                 get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
@@ -252,11 +253,13 @@
             ]
         build_snap = parse_args(args=args).operation
         build_snap.pull()
-        env = (
-            "env LANG=C.UTF-8 SHELL=/bin/sh "
-            "SNAPCRAFT_LOCAL_SOURCES=1 SNAPCRAFT_SETUP_CORE=1 ")
+        env = {
+            "SNAPCRAFT_LOCAL_SOURCES": "1",
+            "SNAPCRAFT_SETUP_CORE": "1",
+            }
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "snapcraft pull", path="/build/test-snap"),
+            RanBuildCommand(
+                ["snapcraft", "pull"], cwd="/build/test-snap", **env),
             ]))
 
     def test_pull_proxy(self):
@@ -268,14 +271,16 @@
             ]
         build_snap = parse_args(args=args).operation
         build_snap.pull()
-        env = (
-            "env LANG=C.UTF-8 SHELL=/bin/sh "
-            "SNAPCRAFT_LOCAL_SOURCES=1 SNAPCRAFT_SETUP_CORE=1 "
-            "http_proxy=http://proxy.example:3128/ "
-            "https_proxy=http://proxy.example:3128/ "
-            "GIT_PROXY_COMMAND=/usr/local/bin/snap-git-proxy ")
+        env = {
+            "SNAPCRAFT_LOCAL_SOURCES": "1",
+            "SNAPCRAFT_SETUP_CORE": "1",
+            "http_proxy": "http://proxy.example:3128/",
+            "https_proxy": "http://proxy.example:3128/",
+            "GIT_PROXY_COMMAND": "/usr/local/bin/snap-git-proxy",
+            }
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "snapcraft pull", path="/build/test-snap"),
+            RanBuildCommand(
+                ["snapcraft", "pull"], cwd="/build/test-snap", **env),
             ]))
 
     def test_build(self):
@@ -286,9 +291,8 @@
             ]
         build_snap = parse_args(args=args).operation
         build_snap.build()
-        env = "env LANG=C.UTF-8 SHELL=/bin/sh "
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "snapcraft", path="/build/test-snap"),
+            RanBuildCommand(["snapcraft"], cwd="/build/test-snap"),
             ]))
 
     def test_build_proxy(self):
@@ -300,13 +304,13 @@
             ]
         build_snap = parse_args(args=args).operation
         build_snap.build()
-        env = (
-            "env LANG=C.UTF-8 SHELL=/bin/sh "
-            "http_proxy=http://proxy.example:3128/ "
-            "https_proxy=http://proxy.example:3128/ "
-            "GIT_PROXY_COMMAND=/usr/local/bin/snap-git-proxy ")
+        env = {
+            "http_proxy": "http://proxy.example:3128/",
+            "https_proxy": "http://proxy.example:3128/",
+            "GIT_PROXY_COMMAND": "/usr/local/bin/snap-git-proxy",
+            }
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
-            RanBuildCommand(env + "snapcraft", path="/build/test-snap"),
+            RanBuildCommand(["snapcraft"], cwd="/build/test-snap", **env),
             ]))
 
     # XXX cjwatson 2017-08-07: Test revoke_token.  It may be easiest to
@@ -325,14 +329,11 @@
         self.assertThat(build_snap.backend.run.calls, MatchesAll(
             AnyMatch(RanAptGet("install", "snapcraft", "bzr")),
             AnyMatch(RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh bzr branch lp:foo test-snap")),
-            AnyMatch(RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh "
-                "SNAPCRAFT_LOCAL_SOURCES=1 SNAPCRAFT_SETUP_CORE=1 "
-                "snapcraft pull", path="/build/test-snap")),
-            AnyMatch(RanBuildCommand(
-                "env LANG=C.UTF-8 SHELL=/bin/sh snapcraft",
-                path="/build/test-snap")),
+                ["bzr", "branch", "lp:foo", "test-snap"])),
+            AnyMatch(RanBuildCommand(
+                ["snapcraft", "pull"], cwd="/build/test-snap",
+                SNAPCRAFT_LOCAL_SOURCES="1", SNAPCRAFT_SETUP_CORE="1")),
+            AnyMatch(RanBuildCommand(["snapcraft"], cwd="/build/test-snap")),
             ))
 
     def test_run_install_fails(self):
@@ -356,10 +357,8 @@
         class FailRepo(FakeMethod):
             def __call__(self, run_args, *args, **kwargs):
                 super(FailRepo, self).__call__(run_args, *args, **kwargs)
-                if run_args[0] == "/bin/sh":
-                    command = run_args[2]
-                    if "bzr branch" in command:
-                        raise subprocess.CalledProcessError(1, run_args)
+                if run_args[:2] == ["bzr", "branch"]:
+                    raise subprocess.CalledProcessError(1, run_args)
 
         self.useFixture(FakeLogger())
         args = [
@@ -375,12 +374,10 @@
         class FailPull(FakeMethod):
             def __call__(self, run_args, *args, **kwargs):
                 super(FailPull, self).__call__(run_args, *args, **kwargs)
-                if run_args[0] == "/bin/sh":
-                    command = run_args[2]
-                    if "bzr revno" in command:
-                        return "42\n"
-                    elif "snapcraft pull" in command:
-                        raise subprocess.CalledProcessError(1, run_args)
+                if run_args[:2] == ["bzr", "revno"]:
+                    return "42\n"
+                elif run_args[:2] == ["snapcraft", "pull"]:
+                    raise subprocess.CalledProcessError(1, run_args)
 
         self.useFixture(FakeLogger())
         args = [
@@ -397,12 +394,10 @@
         class FailBuild(FakeMethod):
             def __call__(self, run_args, *args, **kwargs):
                 super(FailBuild, self).__call__(run_args, *args, **kwargs)
-                if run_args[0] == "/bin/sh":
-                    command = run_args[2]
-                    if "bzr revno" in command:
-                        return "42\n"
-                    elif command.endswith(" snapcraft"):
-                        raise subprocess.CalledProcessError(1, run_args)
+                if run_args[:2] == ["bzr", "revno"]:
+                    return "42\n"
+                elif run_args == ["snapcraft"]:
+                    raise subprocess.CalledProcessError(1, run_args)
 
         self.useFixture(FakeLogger())
         args = [

=== modified file 'lpbuildd/tests/fakeslave.py'
--- lpbuildd/tests/fakeslave.py	2017-09-08 15:57:18 +0000
+++ lpbuildd/tests/fakeslave.py	2017-11-10 20:56:18 +0000
@@ -202,7 +202,7 @@
 class UncontainedBackend(Backend):
     """A partial backend implementation with no containment."""
 
-    def run(self, args, env=None, input_text=None, get_output=False,
+    def run(self, args, cwd=None, env=None, input_text=None, get_output=False,
             echo=False, **kwargs):
         """See `Backend`."""
         if env:
@@ -212,11 +212,12 @@
         if self.arch is not None:
             args = set_personality(args, self.arch, series=self.series)
         if input_text is None and not get_output:
-            subprocess.check_call(args, **kwargs)
+            subprocess.check_call(args, cwd=cwd, **kwargs)
         else:
             if get_output:
                 kwargs["stdout"] = subprocess.PIPE
-            proc = subprocess.Popen(args, stdin=subprocess.PIPE, **kwargs)
+            proc = subprocess.Popen(
+                args, stdin=subprocess.PIPE, cwd=cwd, **kwargs)
             output, _ = proc.communicate(input_text)
             if proc.returncode:
                 raise subprocess.CalledProcessError(proc.returncode, args)