This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/virtual-directory-cleanup in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 5555a91447e748974d76ce12b5459fe22e879584 Author: Tristan van Berkom <[email protected]> AuthorDate: Fri Mar 4 16:20:03 2022 +0900 storage: Rename VirtualDirectoryError -> DirectoryError Make this name a bit more consistent as they are raised from the Directory API Also touch up the documentation of the error. Updated sources and tests which handle this error. --- src/buildstream/__init__.py | 2 +- src/buildstream/element.py | 9 ++++---- src/buildstream/storage/__init__.py | 2 +- src/buildstream/storage/_casbaseddirectory.py | 30 +++++++++++++------------- src/buildstream/storage/_filebaseddirectory.py | 16 +++++++------- src/buildstream/storage/directory.py | 22 ++++++++++--------- tests/internals/storage.py | 5 +++-- tests/internals/storage_vdir_import.py | 10 ++++----- 8 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/buildstream/__init__.py b/src/buildstream/__init__.py index b710431..561e12b 100644 --- a/src/buildstream/__init__.py +++ b/src/buildstream/__init__.py @@ -29,7 +29,7 @@ if "_BST_COMPLETION" not in os.environ: from .utils import UtilError, ProgramNotFoundError from .sandbox import Sandbox, SandboxCommandError - from .storage import Directory + from .storage import Directory, DirectoryError from .types import CoreWarnings, OverlapAction from .node import MappingNode, Node, ProvenanceInformation, ScalarNode, SequenceNode from .plugin import Plugin diff --git a/src/buildstream/element.py b/src/buildstream/element.py index 419353c..b769479 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -108,9 +108,8 @@ from ._elementsources import ElementSources from ._loader import Symbol, DependencyType, MetaSource from ._overlapcollector import OverlapCollector -from .storage.directory import Directory +from .storage import Directory, DirectoryError from .storage._filebaseddirectory import FileBasedDirectory -from .storage.directory import VirtualDirectoryError if TYPE_CHECKING: from typing import Tuple @@ -1778,7 +1777,7 @@ class Element(Plugin): *self.get_variable("build-root").lstrip(os.sep).split(os.sep) ) sandbox._fetch_missing_blobs(sandbox_build_dir) - except VirtualDirectoryError: + except DirectoryError: # Directory could not be found. Pre-virtual # directory behaviour was to continue silently # if the directory could not be found. @@ -1790,7 +1789,7 @@ class Element(Plugin): try: collectvdir = sandbox_vroot.descend(*collect.lstrip(os.sep).split(os.sep)) sandbox._fetch_missing_blobs(collectvdir) - except VirtualDirectoryError: + except DirectoryError: pass # We should always have cache keys already set when caching an artifact @@ -2188,7 +2187,7 @@ class Element(Plugin): try: # Stage all element sources into CAS self.__sources.stage_and_cache() - except (SourceCacheError, VirtualDirectoryError) as e: + except (SourceCacheError, DirectoryError) as e: raise ElementError( "Error trying to stage sources for {}: {}".format(self.name, e), reason="stage-sources-fail" ) diff --git a/src/buildstream/storage/__init__.py b/src/buildstream/storage/__init__.py index b39fd62..0528f88 100644 --- a/src/buildstream/storage/__init__.py +++ b/src/buildstream/storage/__init__.py @@ -19,4 +19,4 @@ from ._filebaseddirectory import FileBasedDirectory from ._casbaseddirectory import CasBasedDirectory -from .directory import Directory +from .directory import Directory, DirectoryError diff --git a/src/buildstream/storage/_casbaseddirectory.py b/src/buildstream/storage/_casbaseddirectory.py index d03a51c..298624a 100644 --- a/src/buildstream/storage/_casbaseddirectory.py +++ b/src/buildstream/storage/_casbaseddirectory.py @@ -35,7 +35,7 @@ from google.protobuf import timestamp_pb2 from .. import utils from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2 -from .directory import Directory, VirtualDirectoryError, _FileType +from .directory import Directory, DirectoryError, _FileType from ._filebaseddirectory import FileBasedDirectory from ..utils import FileListResult, BST_ARBITRARY_TIMESTAMP @@ -175,7 +175,7 @@ class CasBasedDirectory(Directory): with open(self.cas_cache.objpath(digest), "rb") as f: pb2_directory.ParseFromString(f.read()) except FileNotFoundError as e: - raise VirtualDirectoryError("Directory not found in local cache: {}".format(e)) from e + raise DirectoryError("Directory not found in local cache: {}".format(e)) from e for prop in pb2_directory.node_properties.properties: if prop.name == "SubtreeReadOnly": @@ -335,7 +335,7 @@ class CasBasedDirectory(Directory): if entry.type == _FileType.DIRECTORY and not recursive: subdir = entry.get_directory(self) if not subdir.is_empty(): - raise VirtualDirectoryError("{} is not empty".format(str(subdir))) + raise DirectoryError("{} is not empty".format(str(subdir))) del self.index[name] self.__invalidate_digest() @@ -392,7 +392,7 @@ class CasBasedDirectory(Directory): current_dir = current_dir.descend(*newpaths, follow_symlinks=True) else: error = "Cannot descend into {}, which is a '{}' in the directory {}" - raise VirtualDirectoryError( + raise DirectoryError( error.format(path, current_dir.index[path].type, current_dir), reason="not-a-directory" ) else: @@ -407,7 +407,7 @@ class CasBasedDirectory(Directory): current_dir = current_dir._add_directory(path) else: error = "'{}' not found in {}" - raise VirtualDirectoryError(error.format(path, str(current_dir)), reason="directory-not-found") + raise DirectoryError(error.format(path, str(current_dir)), reason="directory-not-found") return current_dir @@ -481,9 +481,9 @@ class CasBasedDirectory(Directory): try: dest_subdir = self.descend(name, create=create_subdir) - except VirtualDirectoryError: + except DirectoryError: filetype = self.index[name].type - raise VirtualDirectoryError( + raise DirectoryError( "Destination is a {}, not a directory: /{}".format(filetype, relative_pathname) ) @@ -609,7 +609,7 @@ class CasBasedDirectory(Directory): f = StringIO(entry.target) tarfile.addfile(tarinfo, f) else: - raise VirtualDirectoryError("can not export file type {} to tar".format(entry.type)) + raise DirectoryError("can not export file type {} to tar".format(entry.type)) def _mark_changed(self): """ It should not be possible to externally modify a CAS-based @@ -779,7 +779,7 @@ class CasBasedDirectory(Directory): entry = subdir.index.get(path[-1]) if entry and entry.type != _FileType.REGULAR_FILE: - raise VirtualDirectoryError("{} in {} is not a file".format(path[-1], str(subdir))) + raise DirectoryError("{} in {} is not a file".format(path[-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)) @@ -814,7 +814,7 @@ class CasBasedDirectory(Directory): def _get_underlying_directory(self): """ There is no underlying directory for a CAS-backed directory, so throw an exception. """ - raise VirtualDirectoryError( + raise DirectoryError( "_get_underlying_directory was called on a CAS-backed directory," + " which has no underlying directory." ) @@ -892,7 +892,7 @@ class CasBasedDirectory(Directory): try: self._entry_from_path(*path, follow_symlinks=follow_symlinks) return True - except (VirtualDirectoryError, FileNotFoundError): + except (DirectoryError, FileNotFoundError): return False def stat(self, *path, follow_symlinks=False): @@ -912,7 +912,7 @@ class CasBasedDirectory(Directory): st_mode |= stat.S_IFLNK st_size = len(entry.target) else: - raise VirtualDirectoryError("Unsupported file type {}".format(entry.type)) + raise DirectoryError("Unsupported file type {}".format(entry.type)) if entry.type == _FileType.DIRECTORY or entry.is_executable: st_mode |= stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH @@ -925,14 +925,14 @@ class CasBasedDirectory(Directory): def file_digest(self, *path): entry = self._entry_from_path(*path) if entry.type != _FileType.REGULAR_FILE: - raise VirtualDirectoryError("Unsupported file type for digest: {}".format(entry.type)) + raise DirectoryError("Unsupported file type for digest: {}".format(entry.type)) return entry.digest.hash def readlink(self, *path): entry = self._entry_from_path(*path) if entry.type != _FileType.SYMLINK: - raise VirtualDirectoryError("Unsupported file type for readlink: {}".format(entry.type)) + raise DirectoryError("Unsupported file type for readlink: {}".format(entry.type)) return entry.target @@ -963,4 +963,4 @@ class CasBasedDirectory(Directory): def __validate_path_component(self, path): if "/" in path: - raise VirtualDirectoryError("Invalid path component: '{}'".format(path)) + raise DirectoryError("Invalid path component: '{}'".format(path)) diff --git a/src/buildstream/storage/_filebaseddirectory.py b/src/buildstream/storage/_filebaseddirectory.py index 9081594..6a06858 100644 --- a/src/buildstream/storage/_filebaseddirectory.py +++ b/src/buildstream/storage/_filebaseddirectory.py @@ -30,7 +30,7 @@ import os import shutil import stat -from .directory import Directory, VirtualDirectoryError, _FileType +from .directory import Directory, DirectoryError, _FileType from .. import utils from ..utils import link_files, copy_files, list_relative_paths, _get_link_mtime, BST_ARBITRARY_TIMESTAMP from ..utils import _set_deterministic_user, _set_deterministic_mtime @@ -77,7 +77,7 @@ class FileBasedDirectory(Directory): else: current_dir = current_dir.descend(*newpaths, follow_symlinks=True) else: - raise VirtualDirectoryError( + raise DirectoryError( "Cannot descend into '{}': '{}' is not a directory".format(path, new_path), reason="not-a-directory", ) @@ -86,7 +86,7 @@ class FileBasedDirectory(Directory): os.mkdir(new_path) current_dir = FileBasedDirectory(new_path, parent=current_dir) else: - raise VirtualDirectoryError("Cannot descend into '{}': '{}' does not exist".format(path, new_path)) + raise DirectoryError("Cannot descend into '{}': '{}' does not exist".format(path, new_path)) return current_dir @@ -251,14 +251,14 @@ class FileBasedDirectory(Directory): try: self.stat(*path, follow_symlinks=follow_symlinks) return True - except (VirtualDirectoryError, FileNotFoundError): + except (DirectoryError, FileNotFoundError): return False def file_digest(self, *path): # Use descend() to avoid following symlinks (potentially escaping the sandbox) subdir = self.descend(*path[:-1]) if subdir.exists(path[-1]) and not subdir.isfile(path[-1]): - raise VirtualDirectoryError("Unsupported file type for digest") + raise DirectoryError("Unsupported file type for digest") newpath = os.path.join(subdir.external_directory, path[-1]) return utils.sha256sum(newpath) @@ -267,7 +267,7 @@ class FileBasedDirectory(Directory): # Use descend() to avoid following symlinks (potentially escaping the sandbox) subdir = self.descend(*path[:-1]) if subdir.exists(path[-1]) and not subdir.islink(path[-1]): - raise VirtualDirectoryError("Unsupported file type for readlink") + raise DirectoryError("Unsupported file type for readlink") newpath = os.path.join(subdir.external_directory, path[-1]) return os.readlink(newpath) @@ -379,9 +379,9 @@ class FileBasedDirectory(Directory): try: create_subdir = not os.path.lexists(dest_path) dest_subdir = self.descend(name, create=create_subdir) - except VirtualDirectoryError: + except DirectoryError: filetype = self._get_filetype(name) - raise VirtualDirectoryError( + raise DirectoryError( "Destination is a {}, not a directory: /{}".format(filetype, relative_pathname) ) diff --git a/src/buildstream/storage/directory.py b/src/buildstream/storage/directory.py index 085442e..9fd106a 100644 --- a/src/buildstream/storage/directory.py +++ b/src/buildstream/storage/directory.py @@ -41,12 +41,14 @@ from ..types import FastEnum from ..utils import BST_ARBITRARY_TIMESTAMP, FileListResult -class VirtualDirectoryError(BstError): - """Raised by Directory functions when system calls fail. - This will be handled internally by the BuildStream core, - if you need to handle this error, then it should be reraised, - or either of the :class:`.ElementError` or :class:`.SourceError` - exceptions should be raised from this error. +class DirectoryError(BstError): + """Raised by Directory functions. + + It is recommended to handle this error and raise a more descriptive + user facing :class:`.ElementError` or :class:`.SourceError` from this error. + + If this is not handled, the BuildStream core will fail the current + task where the error occurs and present the user with the error. """ def __init__(self, message, reason=None): @@ -70,7 +72,7 @@ class Directory: A Directory object representing the found directory. Raises: - VirtualDirectoryError: if any of the components in subdirectory_spec + DirectoryError: if any of the components in subdirectory_spec cannot be found, or are files, or symlinks to files. """ @@ -234,7 +236,7 @@ class Directory: try: st = self.stat(*paths, follow_symlinks=follow_symlinks) return stat.S_ISREG(st.st_mode) - except (VirtualDirectoryError, FileNotFoundError): + except (DirectoryError, FileNotFoundError): return False def isdir(self, *paths: str, follow_symlinks: bool = False) -> bool: @@ -250,7 +252,7 @@ class Directory: try: st = self.stat(*paths, follow_symlinks=follow_symlinks) return stat.S_ISDIR(st.st_mode) - except (VirtualDirectoryError, FileNotFoundError): + except (DirectoryError, FileNotFoundError): return False def islink(self, *paths: str, follow_symlinks: bool = False) -> bool: @@ -266,7 +268,7 @@ class Directory: try: st = self.stat(*paths, follow_symlinks=follow_symlinks) return stat.S_ISLNK(st.st_mode) - except (VirtualDirectoryError, FileNotFoundError): + except (DirectoryError, FileNotFoundError): return False def open_file(self, *paths: str, mode: str = "r"): diff --git a/tests/internals/storage.py b/tests/internals/storage.py index e9932e0..d5a7159 100644 --- a/tests/internals/storage.py +++ b/tests/internals/storage.py @@ -10,10 +10,11 @@ from typing import List, Optional import pytest +from buildstream import DirectoryError from buildstream._cas import CASCache from buildstream.storage._casbaseddirectory import CasBasedDirectory from buildstream.storage._filebaseddirectory import FileBasedDirectory -from buildstream.storage.directory import _FileType, VirtualDirectoryError +from buildstream.storage.directory import _FileType DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "storage") @@ -277,7 +278,7 @@ def test_remove(tmpdir, datafiles, backend): with setup_backend(backend, str(tmpdir)) as c: c.import_files(os.path.join(str(datafiles), "merge-link")) - with pytest.raises((OSError, VirtualDirectoryError)): + with pytest.raises((OSError, DirectoryError)): c.remove("subdirectory") with pytest.raises(FileNotFoundError): diff --git a/tests/internals/storage_vdir_import.py b/tests/internals/storage_vdir_import.py index 26ff638..c1f1b68 100644 --- a/tests/internals/storage_vdir_import.py +++ b/tests/internals/storage_vdir_import.py @@ -18,10 +18,10 @@ import random import pytest +from buildstream import DirectoryError from buildstream.storage._casbaseddirectory import CasBasedDirectory from buildstream.storage._filebaseddirectory import FileBasedDirectory from buildstream._cas import CASCache -from buildstream.storage.directory import VirtualDirectoryError from buildstream.utils import _set_file_mtime, _parse_timestamp from buildstream._testing._utils.site import have_subsecond_mtime @@ -327,15 +327,15 @@ def test_bad_symlinks(tmpdir): d.import_files(test_dir) exp_reason = "not-a-directory" - with pytest.raises(VirtualDirectoryError) as error: + with pytest.raises(DirectoryError) as error: d.descend("a", "l", follow_symlinks=True) assert error.reason == exp_reason - with pytest.raises(VirtualDirectoryError) as error: + with pytest.raises(DirectoryError) as error: d.descend("a", "l") assert error.reason == exp_reason - with pytest.raises(VirtualDirectoryError) as error: + with pytest.raises(DirectoryError) as error: d.descend("a", "f") assert error.reason == exp_reason finally: @@ -416,7 +416,7 @@ def test_bad_sym_escape(tmpdir): generate_import_root(test_dir, filesys_discription) d.import_files(os.path.join(test_dir, "jail")) - with pytest.raises(VirtualDirectoryError) as error: + with pytest.raises(DirectoryError) as error: d.descend("a", "l", follow_symlinks=True) assert error.reason == "directory-not-found" finally:
