diff options
author | Richard Maw <richard.maw@codethink.co.uk> | 2018-09-04 15:09:43 +0100 |
---|---|---|
committer | richardmaw-codethink <richard.maw@codethink.co.uk> | 2018-09-10 12:29:15 +0000 |
commit | 40f25512f3d926f4ef6aa350252c3501f6bdf2b7 (patch) | |
tree | a1b576289c9bb51a0a7892a23c0c081cd8afb6f2 | |
parent | 958ef8ef61eee2221af68f5e1b77c6976dde749f (diff) | |
download | buildstream-40f25512f3d926f4ef6aa350252c3501f6bdf2b7.tar.gz |
subprocesses: Ensure PWD is set in process environment
Naive getcwd implementations (such as in bash 4.4) can break
when bind-mounts to different paths on the same filesystem are present,
since the algorithm needs to know whether it's a mount-point
to know whether it can trust the inode value from the readdir result
or to use stat on the directory.
Less naive implementations (such as in glibc) iterate again using stat
in the case of not finding the directory because the inode in readdir was wrong,
though a Linux-specific implementation could use name_to_handle_at.
Letting the command know what directory it is in makes it unnecessary
for it to call the faulty getcwd in the first place.
-rw-r--r-- | buildstream/sandbox/_sandboxbwrap.py | 12 | ||||
-rw-r--r-- | buildstream/sandbox/_sandboxchroot.py | 6 | ||||
-rw-r--r-- | tests/testutils/repo/git.py | 34 |
3 files changed, 34 insertions, 18 deletions
diff --git a/buildstream/sandbox/_sandboxbwrap.py b/buildstream/sandbox/_sandboxbwrap.py index ea7254c1b..88b697dca 100644 --- a/buildstream/sandbox/_sandboxbwrap.py +++ b/buildstream/sandbox/_sandboxbwrap.py @@ -69,6 +69,15 @@ class SandboxBwrap(Sandbox): 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 + if not self._has_command(command[0], env): raise SandboxError("Staged artifacts do not provide command " "'{}'".format(command[0]), @@ -83,9 +92,6 @@ class SandboxBwrap(Sandbox): mount_map = MountMap(self, flags & SandboxFlags.ROOT_READ_ONLY) root_mount_source = mount_map.get_mount_source('/') - if cwd is None: - cwd = '/' - # Grab the full path of the bwrap binary bwrap_command = [utils.get_host_tool('bwrap')] diff --git a/buildstream/sandbox/_sandboxchroot.py b/buildstream/sandbox/_sandboxchroot.py index a902f22ad..1869468ce 100644 --- a/buildstream/sandbox/_sandboxchroot.py +++ b/buildstream/sandbox/_sandboxchroot.py @@ -58,6 +58,12 @@ class SandboxChroot(Sandbox): 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 + if not self._has_command(command[0], env): raise SandboxError("Staged artifacts do not provide command " "'{}'".format(command[0]), diff --git a/tests/testutils/repo/git.py b/tests/testutils/repo/git.py index eea43d608..8c2a8c177 100644 --- a/tests/testutils/repo/git.py +++ b/tests/testutils/repo/git.py @@ -26,24 +26,30 @@ class Git(Repo): super(Git, self).__init__(directory, subdir) + def _run_git(self, *args, **kwargs): + argv = ['git'] + argv.extend(args) + if 'env' not in kwargs: + kwargs['env'] = dict(GIT_ENV, PWD=self.repo) + kwargs.setdefault('cwd', self.repo) + kwargs.setdefault('check', True) + return subprocess.run(argv, **kwargs) + def create(self, directory): self.copy_directory(directory, self.repo) - subprocess.call(['git', 'init', '.'], env=GIT_ENV, cwd=self.repo) - subprocess.call(['git', 'add', '.'], env=GIT_ENV, cwd=self.repo) - subprocess.call(['git', 'commit', '-m', 'Initial commit'], env=GIT_ENV, cwd=self.repo) + self._run_git('init', '.') + self._run_git('add', '.') + self._run_git('commit', '-m', 'Initial commit') return self.latest_commit() def add_commit(self): - subprocess.call(['git', 'commit', '--allow-empty', '-m', 'Additional commit'], - env=GIT_ENV, cwd=self.repo) + self._run_git('commit', '--allow-empty', '-m', 'Additional commit') return self.latest_commit() def add_file(self, filename): shutil.copy(filename, self.repo) - subprocess.call(['git', 'add', os.path.basename(filename)], env=GIT_ENV, cwd=self.repo) - subprocess.call([ - 'git', 'commit', '-m', 'Added {}'.format(os.path.basename(filename)) - ], env=GIT_ENV, cwd=self.repo) + self._run_git('add', os.path.basename(filename)) + self._run_git('commit', '-m', 'Added {}'.format(os.path.basename(filename))) return self.latest_commit() def add_submodule(self, subdir, url=None, checkout=None): @@ -53,8 +59,8 @@ class Git(Repo): if url is not None: submodule['url'] = url self.submodules[subdir] = submodule - subprocess.call(['git', 'submodule', 'add', url, subdir], env=GIT_ENV, cwd=self.repo) - subprocess.call(['git', 'commit', '-m', 'Added the submodule'], env=GIT_ENV, cwd=self.repo) + self._run_git('submodule', 'add', url, subdir) + self._run_git('commit', '-m', 'Added the submodule') return self.latest_commit() def source_config(self, ref=None, checkout_submodules=None): @@ -74,10 +80,8 @@ class Git(Repo): return config def latest_commit(self): - output = subprocess.check_output([ - 'git', 'rev-parse', 'master' - ], env=GIT_ENV, cwd=self.repo) + output = self._run_git('rev-parse', 'master', stdout=subprocess.PIPE).stdout return output.decode('UTF-8').strip() def branch(self, branch_name): - subprocess.call(['git', 'checkout', '-b', branch_name], env=GIT_ENV, cwd=self.repo) + self._run_git('checkout', '-b', branch_name) |