From a344407bd8f42b2ae7baf92376559df3bde1b783 Mon Sep 17 00:00:00 2001
From: Richard Maw <richard.maw@codethink.co.uk>
Date: Mon, 10 Sep 2018 17:37:35 +0100
Subject: sandbox: deduplicate default environment logic

---
 buildstream/sandbox/_sandboxbwrap.py  | 16 ++--------------
 buildstream/sandbox/_sandboxchroot.py | 20 +++++---------------
 buildstream/sandbox/sandbox.py        | 26 ++++++++++++++++++++++----
 3 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/buildstream/sandbox/_sandboxbwrap.py b/buildstream/sandbox/_sandboxbwrap.py
index 88b697dca..5effadf41 100644
--- a/buildstream/sandbox/_sandboxbwrap.py
+++ b/buildstream/sandbox/_sandboxbwrap.py
@@ -63,20 +63,8 @@ class SandboxBwrap(Sandbox):
         # Fallback to the sandbox default settings for
         # the cwd and env.
         #
-        if cwd is None:
-            cwd = self._get_work_directory()
-
-        if env is None:
-            env = self._get_environment()
-
-        if cwd is None:
-            cwd = '/'
-
-        # Naive getcwd implementations can break when bind-mounts to different
-        # paths on the same filesystem are present. Letting the command know
-        # what directory it is in makes it unnecessary to call the faulty
-        # getcwd.
-        env['PWD'] = cwd
+        cwd = self._get_work_directory(cwd=cwd)
+        env = self._get_environment(cwd=cwd, env=env)
 
         if not self._has_command(command[0], env):
             raise SandboxError("Staged artifacts do not provide command "
diff --git a/buildstream/sandbox/_sandboxchroot.py b/buildstream/sandbox/_sandboxchroot.py
index 1869468ce..64fb3c1bc 100644
--- a/buildstream/sandbox/_sandboxchroot.py
+++ b/buildstream/sandbox/_sandboxchroot.py
@@ -48,21 +48,11 @@ class SandboxChroot(Sandbox):
 
     def run(self, command, flags, *, cwd=None, env=None):
 
-        # Default settings
-        if cwd is None:
-            cwd = self._get_work_directory()
-
-        if cwd is None:
-            cwd = '/'
-
-        if env is None:
-            env = self._get_environment()
-
-        # Naive getcwd implementations can break when bind-mounts to different
-        # paths on the same filesystem are present. Letting the command know
-        # what directory it is in makes it unnecessary to call the faulty
-        # getcwd.
-        env['PWD'] = cwd
+        # Fallback to the sandbox default settings for
+        # the cwd and env.
+        #
+        cwd = self._get_work_directory(cwd=cwd)
+        env = self._get_environment(cwd=cwd, env=env)
 
         if not self._has_command(command[0], env):
             raise SandboxError("Staged artifacts do not provide command "
diff --git a/buildstream/sandbox/sandbox.py b/buildstream/sandbox/sandbox.py
index 9d34f0195..9d0b69b73 100644
--- a/buildstream/sandbox/sandbox.py
+++ b/buildstream/sandbox/sandbox.py
@@ -279,20 +279,38 @@ class Sandbox():
     # Fetches the environment variables for running commands
     # in the sandbox.
     #
+    # Args:
+    #    cwd (str): The working directory the command has been requested to run in, if any.
+    #    env (str): The environment the command has been requested to run in, if any.
+    #
     # Returns:
     #    (str): The sandbox work directory
-    def _get_environment(self):
-        return self.__env
+    def _get_environment(self, *, cwd=None, env=None):
+        cwd = self._get_work_directory(cwd=cwd)
+        if env is None:
+            env = self.__env
+
+        # Naive getcwd implementations can break when bind-mounts to different
+        # paths on the same filesystem are present. Letting the command know
+        # what directory it is in makes it unnecessary to call the faulty
+        # getcwd.
+        env = dict(env)
+        env['PWD'] = cwd
+
+        return env
 
     # _get_work_directory()
     #
     # Fetches the working directory for running commands
     # in the sandbox.
     #
+    # Args:
+    #    cwd (str): The working directory the command has been requested to run in, if any.
+    #
     # Returns:
     #    (str): The sandbox work directory
-    def _get_work_directory(self):
-        return self.__cwd
+    def _get_work_directory(self, *, cwd=None):
+        return cwd or self.__cwd or '/'
 
     # _get_scratch_directory()
     #
-- 
cgit v1.2.1