gtristan commented on a change in pull request #1609:
URL: https://github.com/apache/buildstream/pull/1609#discussion_r827834896



##########
File path: src/buildstream/storage/directory.py
##########
@@ -16,328 +17,499 @@
 #
 #  Authors:
 #        Jim MacArthur <[email protected]>
+#        Tristan van Berkom <[email protected]>
 
 """
-Directory
-=========
-
-This is a virtual Directory class to isolate the rest of BuildStream
-from the backing store implementation.  Sandboxes are allowed to read
-from and write to the underlying storage, but all others must use this
-Directory class to access files and directories in the sandbox.
-
-See also: :ref:`sandboxing`.
-
+Directory - Interfacing with files
+==================================
+The Directory class is given to plugins by way of the :class:`.Sandbox`
+or in some other instances, and allows plugins to interface with files
+and directory hierarchies owned by BuildStream.
+
+
+.. _directory_path:
+
+Paths
+-----
+The path argument to directory functions depict a relative path. Path elements 
are
+separated with the ``/`` character regardless of the platform. Both ``.`` and 
``..`` entries
+are permitted. Absolute paths are not permitted, as such it is illegal to 
specify a path
+with a leading ``/`` character.
+
+Directory objects represent a directory entry within the context of a 
*directory tree*,
+and the directory returned by
+:func:`Sandbox.get_virtual_directory() 
<buildstream.sandbox.Sandbox.get_virtual_directory>`
+is the root of the sandbox's *directory tree*. Attempts to escape the root of 
a *directory tree*
+using ``..`` entries will not result in an error, instead ``..`` entries which 
cross the
+root boundary will be evaluated as the root directory.
 """
 
 
-import os
-import stat
-from typing import Callable, Optional, Union, List
+from contextlib import contextmanager
+from tarfile import TarFile
+from typing import Callable, Optional, Union, List, IO, Iterator
 
 from .._exceptions import BstError
 from ..exceptions import ErrorDomain
-from ..types import FastEnum
 from ..utils import BST_ARBITRARY_TIMESTAMP, FileListResult
+from ..types import FastEnum
 
 
-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):
+    def __init__(self, message: str, reason: str = None):
         super().__init__(message, domain=ErrorDomain.VIRTUAL_FS, reason=reason)
 
 
+class FileType(FastEnum):
+    """Depicts the type of a file"""
+
+    DIRECTORY: int = 1
+    """A directory"""
+
+    REGULAR_FILE: int = 2
+    """A regular file"""
+
+    SYMLINK: int = 3
+    """A symbolic link"""
+
+    def __str__(self):
+        # https://github.com/PyCQA/pylint/issues/2062
+        return self.name.lower().replace("_", " ")  # pylint: disable=no-member
+
+
+class FileStat:
+    """Depicts stats about a file
+
+    .. note::
+
+       This object can be compared with the equality operator, two 
:class:`.FileStat`
+       objects will be considered equivalent if they have the same 
:class:`.FileType`
+       and have equivalent attributes.
+    """
+
+    def __init__(
+        self, file_type: int, *, executable: bool = False, size: int = 0, 
mtime: float = BST_ARBITRARY_TIMESTAMP
+    ) -> None:
+
+        self.file_type: int = file_type
+        """The :class:`.FileType` of this file"""
+
+        self.executable: bool = executable
+        """Whether this file is executable"""
+
+        self.size: int = size
+        """The size of the file in bytes"""
+
+        self.mtime: float = mtime
+        """The modification time of the file"""
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, FileStat):
+            return NotImplemented
+
+        return (
+            self.file_type == other.file_type
+            and self.executable == other.file_type
+            and self.size == other.size
+            and self.mtime == other.mtime
+        )
+
+
 class Directory:
+    """The Directory object represents a directory in a filesystem hierarchy
+
+    .. note::
+
+       Directory objects can be iterated over. Iterating over a directory 
object
+       will yeild strings depicting the entries in the given directory, similar
+       to ``os.listdir()``.
+    """
+
     def __init__(self, external_directory=None):
         raise NotImplementedError()
 
-    def descend(self, *paths: str, create: bool = False, follow_symlinks: bool 
= False):
+    def __iter__(self) -> Iterator[str]:
+        raise NotImplementedError()
+
+    ###################################################################
+    #                           Public API                            #
+    ###################################################################
+
+    def descend(self, path: str, *, create: bool = False, follow_symlinks: 
bool = False) -> "Directory":

Review comment:
       Done, let's call it `open_directory()` instead.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to