diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py index e33378206..2b571820b 100644 --- a/src/buildstream/storage/_filebaseddirectory.py +++ b/src/buildstream/storage/_filebaseddirectory.py @@ -61,12 +61,19 @@ def open_directory( return self.__open_directory(paths, create=create, follow_symlinks=follow_symlinks) def import_single_file(self, external_pathspec: str) -> FileListResult: - dstpath = os.path.join(self.__external_directory, os.path.basename(external_pathspec)) + basename = os.path.basename(external_pathspec) + dstpath = os.path.join(self.__external_directory, basename) result = FileListResult() - if os.path.exists(dstpath): - result.ignored.append(dstpath) - else: - shutil.copyfile(external_pathspec, dstpath, follow_symlinks=False) + if os.path.lexists(dstpath): + # `os.path.isdir()` follows symlinks. The following condition is + # equivalent to `if not os.path.isdir(dstpath)` but without following symlinks. + if os.path.islink(dstpath) or not os.path.isdir(dstpath): + result.overwritten.append(basename) + if not utils.safe_remove(dstpath): + result.ignored.append(basename) + return result + shutil.copyfile(external_pathspec, dstpath, follow_symlinks=False) + result.files_written.append(basename) return result # Add a directory entry deterministically to a tar file @@ -137,6 +144,9 @@ def open_file(self, path: str, *, mode: str = "r") -> Iterator[IO]: subdir = self.__open_directory(paths[:-1]) newpath = os.path.join(subdir.__external_directory, paths[-1]) + if subdir.exists(paths[-1]) and not subdir.isfile(paths[-1]): + raise DirectoryError("{} in {} is not a file".format(paths[-1], str(subdir))) + if mode not in ["r", "rb", "w", "wb", "w+", "w+b", "x", "xb", "x+", "x+b"]: raise ValueError("Unsupported mode: `{}`".format(mode)) @@ -152,8 +162,8 @@ def open_file(self, path: str, *, mode: str = "r") -> Iterator[IO]: if "x" in mode: # This check is not atomic, however, we're operating with a # single thread in a private directory tree. - if subdir.exists(path[-1]): - raise DirectoryError("{} already exists in {}".format(path[-1], str(subdir))) + if subdir.exists(paths[-1]): + raise DirectoryError("{} already exists in {}".format(paths[-1], str(subdir))) mode = "w" + mode[1:] with utils.save_file_atomic(newpath, mode=mode, encoding=encoding) as f: @@ -261,7 +271,11 @@ def _import_files( ) if update_mtime: for f in import_result.files_written: - os.utime(os.path.join(self.__external_directory, f), times=(update_mtime, update_mtime)) + os.utime( + os.path.join(self.__external_directory, f), + times=(update_mtime, update_mtime), + follow_symlinks=False, + ) else: # # We're dealing with an abstract Directory object @@ -448,7 +462,10 @@ def __import_files_from_directory( if not is_dir: if os.path.lexists(dest_path): # Collect overlaps - if not os.path.isdir(dest_path): + + # `os.path.isdir()` follows symlinks. The following condition is + # equivalent to `if not os.path.isdir(dest_path)` but without following symlinks. + if os.path.islink(dest_path) or not os.path.isdir(dest_path): result.overwritten.append(relative_pathname) if not utils.safe_remove(dest_path):