Merge branch 'juerg/vdirectory' into 'master'

Require virtual directory support for element plugins

See merge request BuildStream/buildstream!1887

13 files changed, 4 insertions, 83 deletions

diff --git a/src/buildstream/_artifactelement.py b/src/buildstream/_artifactelement.py
index 4b6d6dd50..4066cef06 100644
--- a/src/buildstream/_artifactelement.py
+++ b/src/buildstream/_artifactelement.py
@@ -42,10 +42,6 @@ class ArtifactElement(Element):
     # A hash of ArtifactElement by ref
     __instantiated_artifacts = {}  # type: Dict[str, ArtifactElement]
 
-    # ArtifactElement's require this as the sandbox will use a normal
-    # directory when we checkout
-    BST_VIRTUAL_DIRECTORY = True
-
     def __init__(self, context, ref):
         _, element, key = verify_artifact_ref(ref)
 
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 9efe97c54..862f00962 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -187,11 +187,6 @@ class Element(Plugin):
     """Whether to raise exceptions if an element has sources.
     """
 
-    BST_VIRTUAL_DIRECTORY = False
-    """Whether to raise exceptions if an element uses Sandbox.get_directory
-    instead of Sandbox.get_virtual_directory.
-    """
-
     BST_RUN_COMMANDS = True
     """Whether the element may run commands using Sandbox.run.
     """
@@ -2456,14 +2451,6 @@ class Element(Plugin):
 
         if directory is not None and allow_remote and self.__use_remote_execution():
 
-            if not self.BST_VIRTUAL_DIRECTORY:
-                raise ElementError(
-                    "Element {} is configured to use remote execution but plugin does not support it.".format(
-                        self.name
-                    ),
-                    detail="Plugin '{kind}' does not support virtual directories.".format(kind=self.get_kind()),
-                )
-
             self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory))
 
             output_files_required = context.require_artifact_files or self._artifact_files_required()
@@ -2477,7 +2464,6 @@ class Element(Plugin):
                 stderr=stderr,
                 config=config,
                 specs=self.__remote_execution_specs,
-                allow_real_directory=False,
                 output_files_required=output_files_required,
                 output_node_properties=output_node_properties,
             )
@@ -2495,7 +2481,6 @@ class Element(Plugin):
                 stdout=stdout,
                 stderr=stderr,
                 config=config,
-                allow_real_directory=not self.BST_VIRTUAL_DIRECTORY,
                 output_node_properties=output_node_properties,
             )
             yield sandbox
diff --git a/src/buildstream/plugins/elements/autotools.py b/src/buildstream/plugins/elements/autotools.py
index 089c9bca0..c4396c5cc 100644
--- a/src/buildstream/plugins/elements/autotools.py
+++ b/src/buildstream/plugins/elements/autotools.py
@@ -60,9 +60,6 @@ from buildstream import BuildElement, SandboxFlags
 
 # Element implementation for the 'autotools' kind.
 class AutotoolsElement(BuildElement):
-    # Supports virtual directories (required for remote execution)
-    BST_VIRTUAL_DIRECTORY = True
-
     # Enable command batching across prepare() and assemble()
     def configure_sandbox(self, sandbox):
         super().configure_sandbox(sandbox)
diff --git a/src/buildstream/plugins/elements/compose.py b/src/buildstream/plugins/elements/compose.py
index 3ac5dec8d..6e4aed193 100644
--- a/src/buildstream/plugins/elements/compose.py
+++ b/src/buildstream/plugins/elements/compose.py
@@ -55,9 +55,6 @@ class ComposeElement(Element):
     # added, to reduce the potential for confusion
     BST_FORBID_SOURCES = True
 
-    # This plugin has been modified to avoid the use of Sandbox.get_directory
-    BST_VIRTUAL_DIRECTORY = True
-
     def configure(self, node):
         node.validate_keys(["integrate", "include", "exclude", "include-orphans"])
 
diff --git a/src/buildstream/plugins/elements/filter.py b/src/buildstream/plugins/elements/filter.py
index d5749ad2d..37b205fe2 100644
--- a/src/buildstream/plugins/elements/filter.py
+++ b/src/buildstream/plugins/elements/filter.py
@@ -151,9 +151,6 @@ class FilterElement(Element):
     # added, to reduce the potential for confusion
     BST_FORBID_SOURCES = True
 
-    # This plugin has been modified to avoid the use of Sandbox.get_directory
-    BST_VIRTUAL_DIRECTORY = True
-
     # Filter elements do not run any commands
     BST_RUN_COMMANDS = False
 
diff --git a/src/buildstream/plugins/elements/import.py b/src/buildstream/plugins/elements/import.py
index 2b68197a7..2bbd11354 100644
--- a/src/buildstream/plugins/elements/import.py
+++ b/src/buildstream/plugins/elements/import.py
@@ -38,9 +38,6 @@ from buildstream import Element, ElementError
 class ImportElement(Element):
     # pylint: disable=attribute-defined-outside-init
 
-    # This plugin has been modified to avoid the use of Sandbox.get_directory
-    BST_VIRTUAL_DIRECTORY = True
-
     # Import elements do not run any commands
     BST_RUN_COMMANDS = False
 
diff --git a/src/buildstream/plugins/elements/manual.py b/src/buildstream/plugins/elements/manual.py
index 97da41615..c95b8e5a2 100644
--- a/src/buildstream/plugins/elements/manual.py
+++ b/src/buildstream/plugins/elements/manual.py
@@ -36,9 +36,6 @@ from buildstream import BuildElement, SandboxFlags
 
 # Element implementation for the 'manual' kind.
 class ManualElement(BuildElement):
-    # Supports virtual directories (required for remote execution)
-    BST_VIRTUAL_DIRECTORY = True
-
     # Enable command batching across prepare() and assemble()
     def configure_sandbox(self, sandbox):
         super().configure_sandbox(sandbox)
diff --git a/src/buildstream/plugins/elements/pip.py b/src/buildstream/plugins/elements/pip.py
index 93303748d..06e187b69 100644
--- a/src/buildstream/plugins/elements/pip.py
+++ b/src/buildstream/plugins/elements/pip.py
@@ -36,9 +36,6 @@ from buildstream import BuildElement, SandboxFlags
 
 # Element implementation for the 'pip' kind.
 class PipElement(BuildElement):
-    # Supports virtual directories (required for remote execution)
-    BST_VIRTUAL_DIRECTORY = True
-
     # Enable command batching across prepare() and assemble()
     def configure_sandbox(self, sandbox):
         super().configure_sandbox(sandbox)
diff --git a/src/buildstream/plugins/elements/script.py b/src/buildstream/plugins/elements/script.py
index abfb7b3b0..e355e5f05 100644
--- a/src/buildstream/plugins/elements/script.py
+++ b/src/buildstream/plugins/elements/script.py
@@ -40,11 +40,6 @@ import buildstream
 
 # Element implementation for the 'script' kind.
 class ScriptElement(buildstream.ScriptElement):
-    # pylint: disable=attribute-defined-outside-init
-
-    # This plugin has been modified to avoid the use of Sandbox.get_directory
-    BST_VIRTUAL_DIRECTORY = True
-
     def configure(self, node):
         for n in node.get_sequence("layout", []):
             dst = self.node_subst_vars(n.get_scalar("destination"))
diff --git a/src/buildstream/plugins/elements/stack.py b/src/buildstream/plugins/elements/stack.py
index f569199b6..df11a257f 100644
--- a/src/buildstream/plugins/elements/stack.py
+++ b/src/buildstream/plugins/elements/stack.py
@@ -30,9 +30,6 @@ from buildstream import Element
 # Element implementation for the 'stack' kind.
 class StackElement(Element):
 
-    # This plugin has been modified to avoid the use of Sandbox.get_directory
-    BST_VIRTUAL_DIRECTORY = True
-
     # This plugin does not produce any artifacts when built
     BST_ELEMENT_HAS_ARTIFACT = False
 
diff --git a/src/buildstream/sandbox/_sandboxbuildboxrun.py b/src/buildstream/sandbox/_sandboxbuildboxrun.py
index 246fdd450..c10669fe0 100644
--- a/src/buildstream/sandbox/_sandboxbuildboxrun.py
+++ b/src/buildstream/sandbox/_sandboxbuildboxrun.py
@@ -36,10 +36,6 @@ from ._sandboxreapi import SandboxREAPI
 # BuildBox-based sandbox implementation.
 #
 class SandboxBuildBoxRun(SandboxREAPI):
-    def __init__(self, context, project, directory, **kwargs):
-        kwargs["allow_real_directory"] = False
-        super().__init__(context, project, directory, **kwargs)
-
     @classmethod
     def check_available(cls):
         try:
diff --git a/src/buildstream/sandbox/sandbox.py b/src/buildstream/sandbox/sandbox.py
index 0a09788f4..fc8e4f1d0 100644
--- a/src/buildstream/sandbox/sandbox.py
+++ b/src/buildstream/sandbox/sandbox.py
@@ -125,7 +125,6 @@ class Sandbox:
         self.__cwd = None  # type: Optional[str]
         self.__env = None  # type: Optional[Dict[str, str]]
         self.__mount_sources = {}  # type: Dict[str, str]
-        self.__allow_real_directory = kwargs["allow_real_directory"]
         self.__allow_run = True
 
         # Plugin element full name for logging
@@ -154,47 +153,20 @@ class Sandbox:
         self._vdir = None  # type: Optional[Directory]
         self._usebuildtree = False
 
-        # This is set if anyone requests access to the underlying
-        # directory via get_directory.
-        self._never_cache_vdirs = False
-
         # Pending command batch
         self.__batch = None
 
-    def get_directory(self) -> str:
-        """Fetches the sandbox root directory
-
-        The root directory is where artifacts for the base
-        runtime environment should be staged. Only works if
-        BST_VIRTUAL_DIRECTORY is not set.
-
-        Returns:
-           The sandbox root directory
-
-        """
-        if self.__allow_real_directory:
-            self._never_cache_vdirs = True
-            return self._root
-        else:
-            raise BstError("You can't use get_directory")
-
     def get_virtual_directory(self) -> Directory:
         """Fetches the sandbox root directory as a virtual Directory.
 
         The root directory is where artifacts for the base
         runtime environment should be staged.
 
-        Use caution if you use get_directory and
-        get_virtual_directory.  If you alter the contents of the
-        directory returned by get_directory, all objects returned by
-        get_virtual_directory or derived from them are invalid and you
-        must call get_virtual_directory again to get a new copy.
-
         Returns:
            The sandbox root directory
 
         """
-        if self._vdir is None or self._never_cache_vdirs:
+        if self._vdir is None:
             if self._use_cas_based_directory():
                 cascache = self.__context.get_cascache()
                 self._vdir = CasBasedDirectory(cascache)
@@ -400,9 +372,9 @@ class Sandbox:
     #    (bool): Whether to use CasBasedDirectory
     #
     def _use_cas_based_directory(self):
-        # Use CasBasedDirectory as sandbox root if neither Sandbox.get_directory()
-        # nor Sandbox.run() are required. This allows faster staging.
-        if not self.__allow_real_directory and not self.__allow_run:
+        # Use CasBasedDirectory as sandbox root if Sandbox.run() is not used.
+        # This allows faster staging.
+        if not self.__allow_run:
             return True
 
         return "BST_CAS_DIRECTORIES" in os.environ
diff --git a/tests/frontend/project/plugins/randomelement.py b/tests/frontend/project/plugins/randomelement.py
index b36b75c8a..e9be98fc7 100644
--- a/tests/frontend/project/plugins/randomelement.py
+++ b/tests/frontend/project/plugins/randomelement.py
@@ -4,8 +4,6 @@ from buildstream import Element
 
 
 class RandomElement(Element):
-    BST_VIRTUAL_DIRECTORY = True
-
     def configure(self, node):
         pass