diff options
-rw-r--r-- | src/buildstream/_cas/cascache.py | 21 | ||||
-rw-r--r-- | src/buildstream/utils.py | 37 | ||||
-rw-r--r-- | tests/frontend/pull.py | 14 | ||||
-rw-r--r-- | tests/sources/remote.py | 9 |
4 files changed, 45 insertions, 36 deletions
diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py index 9394ac5b2..6b2f0d1a4 100644 --- a/src/buildstream/_cas/cascache.py +++ b/src/buildstream/_cas/cascache.py @@ -241,21 +241,20 @@ class CASCache: if can_link and mtime is None: utils.safe_link(self.objpath(filenode.digest), fullpath) else: - utils.safe_copy(self.objpath(filenode.digest), fullpath) + utils.safe_copy(self.objpath(filenode.digest), fullpath, copystat=False) if mtime is not None: utils._set_file_mtime(fullpath, mtime) if filenode.is_executable: - os.chmod( - fullpath, - stat.S_IRUSR - | stat.S_IWUSR - | stat.S_IXUSR - | stat.S_IRGRP - | stat.S_IXGRP - | stat.S_IROTH - | stat.S_IXOTH, - ) + st = os.stat(fullpath) + mode = st.st_mode + if mode & stat.S_IRUSR: + mode |= stat.S_IXUSR + if mode & stat.S_IRGRP: + mode |= stat.S_IXGRP + if mode & stat.S_IROTH: + mode |= stat.S_IXOTH + os.chmod(fullpath, mode) for dirnode in directory.directories: fullpath = os.path.join(dest, dirnode.name) diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py index 99c22d169..9c6761ccc 100644 --- a/src/buildstream/utils.py +++ b/src/buildstream/utils.py @@ -363,19 +363,20 @@ def sha256sum(filename: str) -> str: return h.hexdigest() -def safe_copy(src: str, dest: str, *, result: Optional[FileListResult] = None) -> None: - """Copy a file while preserving attributes +def safe_copy(src: str, dest: str, *, copystat: bool = True, result: Optional[FileListResult] = None) -> None: + """Copy a file while optionally preserving attributes Args: src: The source filename dest: The destination filename + copystat: Whether to preserve attributes result: An optional collective result Raises: UtilError: In the case of unexpected system call failures - This is almost the same as shutil.copy2(), except that - we unlink *dest* before overwriting it if it exists, just + This is almost the same as shutil.copy2() when copystat is True, + except that we unlink *dest* before overwriting it if it exists, just incase *dest* is a hardlink to a different file. """ # First unlink the target if it exists @@ -385,22 +386,24 @@ def safe_copy(src: str, dest: str, *, result: Optional[FileListResult] = None) - if e.errno != errno.ENOENT: raise UtilError("Failed to remove destination file '{}': {}".format(dest, e)) from e - shutil.copyfile(src, dest) try: - shutil.copystat(src, dest) - except PermissionError: - # If we failed to copy over some file stats, dont treat - # it as an unrecoverable error, but provide some feedback - # we can use for a warning. - # - # This has a tendency of happening when attempting to copy - # over extended file attributes. - if result: - result.failed_attributes.append(dest) - - except shutil.Error as e: + shutil.copyfile(src, dest) + except (OSError, shutil.Error) as e: raise UtilError("Failed to copy '{} -> {}': {}".format(src, dest, e)) from e + if copystat: + try: + shutil.copystat(src, dest) + except PermissionError: + # If we failed to copy over some file stats, dont treat + # it as an unrecoverable error, but provide some feedback + # we can use for a warning. + # + # This has a tendency of happening when attempting to copy + # over extended file attributes. + if result: + result.failed_attributes.append(dest) + def safe_link(src: str, dest: str, *, result: Optional[FileListResult] = None, _unlink=False) -> None: """Try to create a hardlink, but resort to copying in the case of cross device links. diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py index 6a5a41977..c1fa76ab0 100644 --- a/tests/frontend/pull.py +++ b/tests/frontend/pull.py @@ -442,6 +442,8 @@ def test_pull_access_rights(cli, tmpdir, datafiles): project = str(datafiles) checkout = os.path.join(str(tmpdir), "checkout") + umask = utils.get_umask() + # Work-around datafiles not preserving mode os.chmod(os.path.join(project, "files/bin-files/usr/bin/hello"), 0o0755) @@ -467,15 +469,15 @@ def test_pull_access_rights(cli, tmpdir, datafiles): st = os.lstat(os.path.join(checkout, "usr/include/pony.h")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0644 + assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask st = os.lstat(os.path.join(checkout, "usr/bin/hello")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0755 + assert stat.S_IMODE(st.st_mode) == 0o0777 & ~umask st = os.lstat(os.path.join(checkout, "usr/share/big-file")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0644 + assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask shutil.rmtree(checkout) @@ -493,15 +495,15 @@ def test_pull_access_rights(cli, tmpdir, datafiles): st = os.lstat(os.path.join(checkout, "usr/include/pony.h")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0644 + assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask st = os.lstat(os.path.join(checkout, "usr/bin/hello")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0755 + assert stat.S_IMODE(st.st_mode) == 0o0777 & ~umask st = os.lstat(os.path.join(checkout, "usr/share/big-file")) assert stat.S_ISREG(st.st_mode) - assert stat.S_IMODE(st.st_mode) == 0o0644 + assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask # Tests `bst artifact pull $artifact_ref` diff --git a/tests/sources/remote.py b/tests/sources/remote.py index d546c6131..416d2d53c 100644 --- a/tests/sources/remote.py +++ b/tests/sources/remote.py @@ -5,6 +5,7 @@ import os import stat import pytest +from buildstream import utils from buildstream.testing import ErrorDomain from buildstream.testing import generate_project from buildstream.testing import cli # pylint: disable=unused-import @@ -71,8 +72,12 @@ def test_simple_file_build(cli, tmpdir, datafiles): mode = os.stat(checkout_file).st_mode # Assert not executable by anyone assert not mode & (stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) - # Assert not writeable by anyone other than me - assert not mode & (stat.S_IWGRP | stat.S_IWOTH) + + # Assert not writeable by anyone other than me (unless umask allows it) + if utils.get_umask() & stat.S_IWGRP: + assert not mode & stat.S_IWGRP + if utils.get_umask() & stat.S_IWOTH: + assert not mode & stat.S_IWOTH @pytest.mark.datafiles(os.path.join(DATA_DIR, "single-file-custom-name")) |