diff options
author | Tristan Maat <tristan.maat@codethink.co.uk> | 2017-09-28 13:20:33 +0100 |
---|---|---|
committer | Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> | 2017-10-13 15:57:40 +0900 |
commit | a968bab76201856a67c876be25745fee912c136b (patch) | |
tree | 72f3de7bf7da88de4d2542a2ad26c1c19b26c5a0 | |
parent | a580a4a032c2e492c5287a976ced1db178467549 (diff) | |
download | buildstream-a968bab76201856a67c876be25745fee912c136b.tar.gz |
Ensure that artifact file permissions are set in the right order
-rw-r--r-- | buildstream/_artifactcache/ostreecache.py | 5 | ||||
-rw-r--r-- | buildstream/_ostree.py | 6 | ||||
-rw-r--r-- | buildstream/element.py | 6 | ||||
-rw-r--r-- | buildstream/utils.py | 31 |
4 files changed, 39 insertions, 9 deletions
diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py index 0321e8e47..48788f4b9 100644 --- a/buildstream/_artifactcache/ostreecache.py +++ b/buildstream/_artifactcache/ostreecache.py @@ -209,7 +209,10 @@ class OSTreeCache(ArtifactCache): # also store under weak cache key weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK)) - _ostree.commit(self.repo, content, ref, weak_ref) + try: + _ostree.commit(self.repo, content, ref, weak_ref) + except OSTreeError as e: + raise _ArtifactError("Failed to commit artifact: {}".format(e)) from e # pull(): # diff --git a/buildstream/_ostree.py b/buildstream/_ostree.py index 40bfda644..d6e8c9ef5 100644 --- a/buildstream/_ostree.py +++ b/buildstream/_ostree.py @@ -153,6 +153,12 @@ def commit(repo, dir, ref, branch=None): # complete repo transaction repo.commit_transaction(None) + except GLib.IOError as e: + + # Reraise any error as a buildstream error + repo.abort_transaction() + raise OSTreeError(e.message) from e + except: repo.abort_transaction() raise diff --git a/buildstream/element.py b/buildstream/element.py index e3e9a4b34..a956246d9 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -1040,7 +1040,7 @@ class Element(Plugin): # Cleanup the build directory on explicit SIGTERM def cleanup_rootdir(): - shutil.rmtree(rootdir) + utils._force_rmtree(rootdir) with _signals.terminator(cleanup_rootdir), \ self.__sandbox(rootdir, output_file, output_file) as sandbox: # nopep8 @@ -1116,7 +1116,7 @@ class Element(Plugin): self.__artifacts.commit(self, assembledir) # Finally cleanup the build dir - shutil.rmtree(rootdir) + cleanup_rootdir() # _pull(): # @@ -1415,7 +1415,7 @@ class Element(Plugin): yield sandbox # Cleanup the build dir - shutil.rmtree(rootdir) + utils._force_rmtree(rootdir) def __compose_default_splits(self, defaults): project = self.get_project() diff --git a/buildstream/utils.py b/buildstream/utils.py index 273d5dd0b..2a873bf57 100644 --- a/buildstream/utils.py +++ b/buildstream/utils.py @@ -465,14 +465,26 @@ def get_bst_version(): return (int(versions[0]), int(versions[1])) -# Recursively make directories in target area and copy permissions +# Recursively remove directories, ignoring file permissions as much as +# possible. +def _force_rmtree(path, **kwargs): + for root, dirs, _ in os.walk(path): + for d in dirs: + path = os.path.join(root, d.lstrip('/')) + if os.path.exists(path) and not os.path.islink(path): + os.chmod(path, 0o755) + + shutil.rmtree(path, **kwargs) + + +# Recursively make directories in target area def _copy_directories(srcdir, destdir, target): this_dir = os.path.dirname(target) new_dir = os.path.join(destdir, this_dir) if not os.path.lexists(new_dir): if this_dir: - _copy_directories(srcdir, destdir, this_dir) + yield from _copy_directories(srcdir, destdir, this_dir) old_dir = os.path.join(srcdir, this_dir) if os.path.lexists(old_dir): @@ -481,7 +493,7 @@ def _copy_directories(srcdir, destdir, target): if stat.S_ISDIR(mode) or stat.S_ISLNK(mode): os.makedirs(new_dir) - shutil.copystat(old_dir, new_dir) + yield (new_dir, mode) else: raise OSError('Source directory tree has file where ' 'directory expected: %s' % dir) @@ -533,6 +545,10 @@ def _ensure_real_directory(root, destpath): # def _process_list(srcdir, destdir, filelist, actionfunc, result, ignore_missing=False): + # Keep track of directory permissions, since these need to be set + # *after* files have been written. + permissions = [] + # Note we consume the filelist (which is a generator and not a list) # by sorting it, this is necessary to ensure that we processes symbolic # links which lead to directories before processing files inside those @@ -547,7 +563,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, ignore_missing= result.overwritten.append(path) # The destination directory may not have been created separately - _copy_directories(srcdir, destdir, path) + permissions.extend(_copy_directories(srcdir, destdir, path)) # Ensure that broken symlinks to directories have their targets # created before attempting to stage files across broken @@ -557,6 +573,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, ignore_missing= try: file_stat = os.lstat(srcpath) mode = file_stat.st_mode + except FileNotFoundError: # Skip this missing file if ignore_missing: @@ -573,7 +590,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, ignore_missing= if not stat.S_ISDIR(dest_stat.st_mode): raise OSError('Destination not a directory. source has %s' ' destination has %s' % (srcpath, destpath)) - shutil.copystat(srcpath, destpath) + permissions.append((destpath, os.stat(srcpath).st_mode)) elif stat.S_ISLNK(mode): if not safe_remove(destpath): @@ -607,6 +624,10 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result, ignore_missing= # Unsupported type. raise OSError('Cannot extract %s into staging-area. Unsupported type.' % srcpath) + # Write directory permissions now that all files have been written + for d, perms in permissions: + os.chmod(d, perms) + # _relative_symlink_target() # |