ScriptElement: Porting to new APItristan/port-script-element

Instead of relying on Element.search(), use Element.configure_dependencies() to
configure the layout.

Summary of changes:

  * scriptelement.py:

    Change ScriptElement.layout_add() API to take an Element instead of an element name,
    this is now not optional (one cannot specify a `None` element).

    This is an API breaking change

  * plugins/elements/script.py:

    Implement Element.configure_dependencies() in order to call ScriptElement.layout_add().

    This is a breaking YAML format change.

  * tests/integration: Script integration tests updated to use the new YAML format

  * tests/cachekey: Updated for `script` element changes

11 files changed, 103 insertions, 155 deletions

diff --git a/src/buildstream/plugins/elements/script.py b/src/buildstream/plugins/elements/script.py
index 502212e10..8c313000d 100644
--- a/src/buildstream/plugins/elements/script.py
+++ b/src/buildstream/plugins/elements/script.py
@@ -45,19 +45,25 @@ class ScriptElement(buildstream.ScriptElement):
     BST_MIN_VERSION = "2.0"
 
     def configure(self, node):
-        for n in node.get_sequence("layout", []):
-            dst = n.get_str("destination")
-            elm = n.get_str("element", None)
-            self.layout_add(elm, dst)
-
-        node.validate_keys(["commands", "root-read-only", "layout"])
+        node.validate_keys(["commands", "root-read-only"])
 
         self.add_commands("commands", node.get_str_list("commands"))
-
         self.set_work_dir()
         self.set_install_root()
         self.set_root_read_only(node.get_bool("root-read-only", default=False))
 
+    def configure_dependencies(self, config_list):
+        for element, config in config_list:
+
+            # Determine the location to stage each element, default is "/"
+            location = "/"
+            if config:
+                config.validate_keys(["location"])
+                location = config.get_str("location")
+
+            # Add each element to the layout
+            self.layout_add(element, location)
+
 
 # Plugin entry point
 def setup():
diff --git a/src/buildstream/plugins/elements/script.yaml b/src/buildstream/plugins/elements/script.yaml
index b388378da..390f60355 100644
--- a/src/buildstream/plugins/elements/script.yaml
+++ b/src/buildstream/plugins/elements/script.yaml
@@ -1,3 +1,21 @@
+# The script element allows staging elements into specific locations
+# via it's "location" dependency configuration
+#
+# For example, if you want to stage "foo-tools.bst" into the "/" of
+# the sandbox at buildtime, and the "foo-system.bst" element into
+# the %{build-root}, you can do so as follows:
+#
+# build-depends:
+# - foo-tools.bst
+# - filename: foo-system.bst
+#   config:
+#     location: "%{build-root}"
+#
+# Note: the default of the "location" parameter is "/", so it is not
+#       necessary to specify the location if you want to stage the
+#       element in "/"
+#
+
 # Common script element variables
 variables:
   # Defines the directory commands will be run from.
@@ -10,16 +28,6 @@ config:
   # It is recommended to set root as read-only wherever possible.
   root-read-only: False
 
-  # Defines where to stage elements which are direct or indirect dependencies.
-  # By default, all direct dependencies are staged to '/'.
-  # This is also commonly used to take one element as an environment
-  # containing the tools used to operate on the other element.
-  # layout:
-  # - element: foo-tools.bst
-  #   destination: /
-  # - element: foo-system.bst
-  #   destination: %{build-root}
-
   # List of commands to run in the sandbox.
   commands: []
 
diff --git a/src/buildstream/scriptelement.py b/src/buildstream/scriptelement.py
index 5ecae998c..3e84c598b 100644
--- a/src/buildstream/scriptelement.py
+++ b/src/buildstream/scriptelement.py
@@ -36,7 +36,7 @@ import os
 from collections import OrderedDict
 from typing import List, Optional, TYPE_CHECKING
 
-from .element import Element, ElementError
+from .element import Element
 from .sandbox import SandboxFlags
 
 if TYPE_CHECKING:
@@ -48,7 +48,7 @@ class ScriptElement(Element):
     __cwd = "/"
     __root_read_only = False
     __commands = None  # type: OrderedDict[str, List[str]]
-    __layout = []  # type: List[Dict[str, Optional[str]]]
+    __layout = {}  # type: Dict[str, List[Element]]
 
     # The compose element's output is its dependencies, so
     # we must rebuild if the dependencies change even when
@@ -112,43 +112,44 @@ class ScriptElement(Element):
         """
         self.__root_read_only = root_read_only
 
-    def layout_add(self, element: Optional[str], destination: str) -> None:
-        """Adds an element-destination pair to the layout.
+    def layout_add(self, element: Element, location: str) -> None:
+        """Adds an element to the layout.
 
         Layout is a way of defining how dependencies should be added to the
         staging area for running commands.
 
         Args:
-          element: The name of the element to stage, or None. This may be any
-                   element found in the dependencies, whether it is a direct
-                   or indirect dependency.
-          destination: The path inside the staging area for where to
-                       stage this element. If it is not "/", then integration
-                       commands will not be run.
+          element (Element): The element to stage.
+          location (str): The path inside the staging area for where to
+                          stage this element. If it is not "/", then integration
+                          commands will not be run.
 
         If this function is never called, then the default behavior is to just
         stage the build dependencies of the element in question at the
-        sandbox root. Otherwise, the runtime dependencies of each specified
-        element will be staged in their specified destination directories.
+        sandbox root. Otherwise, the specified elements including their
+        runtime dependencies will be staged in their respective locations.
 
         .. note::
 
-           The order of directories in the layout is significant as they
-           will be mounted into the sandbox. It is an error to specify a parent
-           directory which will shadow a directory already present in the layout.
-
-        .. note::
-
-           In the case that no element is specified, a read-write directory will
-           be made available at the specified location.
+           The order of directories in the layout is not significant. The
+           elements for each respective directory in the layout will be staged
+           in the predetermined deterministic staging order.
         """
         #
-        # Even if this is an empty list by default, make sure that its
+        # Even if this is an empty dict by default, make sure that it is
         # instance data instead of appending stuff directly onto class data.
         #
         if not self.__layout:
-            self.__layout = []
-        self.__layout.append({"element": element, "destination": destination})
+            self.__layout = {}
+
+        # Ensure the element list
+        try:
+            element_list = self.__layout[location]
+        except KeyError:
+            element_list = []
+            self.__layout[location] = element_list
+
+        element_list.append(element)
 
     def add_commands(self, group_name: str, command_list: List[str]) -> None:
         """Adds a list of commands under the group-name.
@@ -174,17 +175,19 @@ class ScriptElement(Element):
     #############################################################
     #             Abstract Method Implementations               #
     #############################################################
-
     def preflight(self):
-        # The layout, if set, must make sense.
-        self.__validate_layout()
+        pass
 
     def get_unique_key(self):
+        layout_key = {
+            location: [(element.project_name, element.name) for element in element_list]
+            for location, element_list in self.__layout.items()
+        }
         return {
             "commands": self.__commands,
             "cwd": self.__cwd,
             "install-root": self.__install_root,
-            "layout": self.__layout,
+            "layout": layout_key,
             "root-read-only": self.__root_read_only,
         }
 
@@ -196,67 +199,42 @@ class ScriptElement(Element):
         # Setup environment
         sandbox.set_environment(self.get_environment())
 
-        # Tell the sandbox to mount the install root
-        directories = {self.__install_root: False}
-
-        # set the output directory
-        sandbox.set_output_directory(self.__install_root)
+        # Mark the install root
+        sandbox.mark_directory(self.__install_root, artifact=False)
 
         # Mark the artifact directories in the layout
-        for item in self.__layout:
-            destination = item["destination"]
-            was_artifact = directories.get(destination, False)
-            directories[destination] = item["element"] or was_artifact
-
-        for directory, artifact in directories.items():
-            # Root does not need to be marked as it is always mounted
-            # with artifact (unless explicitly marked non-artifact)
-            if directory != "/":
-                sandbox.mark_directory(directory, artifact=artifact)
+        for location in self.__layout:
+            sandbox.mark_directory(location, artifact=True)
 
     def stage(self, sandbox):
 
-        # Stage the elements, and run integration commands where appropriate.
+        # If self.layout_add() was never called, do the default staging of
+        # everything in "/" and run the integration commands
         if not self.__layout:
 
-            # if no layout set, stage all dependencies into the sandbox root
             with self.timed_activity("Staging dependencies", silent_nested=True):
                 self.stage_dependency_artifacts(sandbox)
 
-            # Run any integration commands provided by the dependencies
-            # once they are all staged and ready
             with sandbox.batch(SandboxFlags.NONE, label="Integrating sandbox"):
                 for dep in self.dependencies():
                     dep.integrate(sandbox)
-        else:
-            # If layout, follow its rules.
-            for item in self.__layout:
-
-                # Skip layout members which dont stage an element
-                if not item["element"]:
-                    continue
-
-                element = self.search(item["element"])
-                with self.timed_activity(
-                    "Staging {} at {}".format(element.name, item["destination"]), silent_nested=True
-                ):
-                    element.stage_dependency_artifacts(sandbox, [element], path=item["destination"])
-
-            with sandbox.batch(SandboxFlags.NONE):
-                for item in self.__layout:
-
-                    # Skip layout members which dont stage an element
-                    if not item["element"]:
-                        continue
-
-                    element = self.search(item["element"])
-
-                    # Integration commands can only be run for elements staged to /
-                    if item["destination"] == "/":
-                        with self.timed_activity("Integrating {}".format(element.name), silent_nested=True):
-                            for dep in element.dependencies():
-                                dep.integrate(sandbox)
 
+        else:
+            # First stage it all
+            #
+            for location, element_list in self.__layout.items():
+                self.stage_dependency_artifacts(sandbox, element_list, path=location)
+
+            # Now integrate any elements staged in the root
+            #
+            element_list = self.__layout.get("/", None)
+            if element_list:
+                with sandbox.batch(SandboxFlags.NONE), self.timed_activity("Integrating sandbox", silent_nested=True):
+                    for dep in self.dependencies(element_list):
+                        dep.integrate(sandbox)
+
+        # Ensure the install root exists
+        #
         install_root_path_components = self.__install_root.lstrip(os.sep).split(os.sep)
         sandbox.get_virtual_directory().descend(*install_root_path_components, create=True)
 
@@ -277,26 +255,6 @@ class ScriptElement(Element):
         # Return where the result can be collected from
         return self.__install_root
 
-    #############################################################
-    #                   Private Local Methods                   #
-    #############################################################
-
-    def __validate_layout(self):
-        if self.__layout:
-            # Cannot proceeed if layout is used, but none are for "/"
-            root_defined = any([(entry["destination"] == "/") for entry in self.__layout])
-            if not root_defined:
-                raise ElementError("{}: Using layout, but none are staged as '/'".format(self))
-
-            # Cannot proceed if layout specifies an element that isn't part
-            # of the dependencies.
-            for item in self.__layout:
-                if item["element"]:
-                    if not self.search(item["element"]):
-                        raise ElementError(
-                            "{}: '{}' in layout not found in dependencies".format(self, item["element"])
-                        )
-
 
 def setup():
     return ScriptElement
diff --git a/tests/cachekey/project/elements/script1.expected b/tests/cachekey/project/elements/script1.expected
index 71be17a37..860d73624 100644
--- a/tests/cachekey/project/elements/script1.expected
+++ b/tests/cachekey/project/elements/script1.expected
@@ -1 +1 @@
-38078af3714d6d5128669dc7b4d7e942a46ea4137c388d44a1f455b4cc1918d0
\ No newline at end of file
+b43bc07179a303eb6346ad331955947945d1488ff58f80e46b33619e7c13b956
\ No newline at end of file
diff --git a/tests/cachekey/project/target.expected b/tests/cachekey/project/target.expected
index 38d4abeef..8b6458b61 100644
--- a/tests/cachekey/project/target.expected
+++ b/tests/cachekey/project/target.expected
@@ -1 +1 @@
-5bf8ea2d6c0900c226c8fd3dae980f0b92e40f3fc1887b64d31bf04b6210d763
\ No newline at end of file
+884a31110fd54c5f63b9daf84124275388b038365f171270b06759e13a2d3c91
\ No newline at end of file
diff --git a/tests/integration/project/elements/script/corruption-2.bst b/tests/integration/project/elements/script/corruption-2.bst
index 39c4f2c23..75c5e92d0 100644
--- a/tests/integration/project/elements/script/corruption-2.bst
+++ b/tests/integration/project/elements/script/corruption-2.bst
@@ -1,10 +1,8 @@
 kind: script
 
-depends:
-- filename: base.bst
-  type: build
-- filename: script/corruption-image.bst
-  type: build
+build-depends:
+- base.bst
+- script/corruption-image.bst
 
 config:
   commands:
diff --git a/tests/integration/project/elements/script/corruption.bst b/tests/integration/project/elements/script/corruption.bst
index 037d4daca..841a2dd69 100644
--- a/tests/integration/project/elements/script/corruption.bst
+++ b/tests/integration/project/elements/script/corruption.bst
@@ -1,21 +1,9 @@
 kind: script
 
-depends:
-- filename: base.bst
-  type: build
-- filename: script/corruption-image.bst
-  type: build
-- filename: script/corruption-integration.bst
-  type: build
+build-depends:
+- base.bst
+- script/corruption-image.bst
+- script/corruption-integration.bst
 
 variables:
   install-root: "/"
-
-config:
-  layout:
-  - element: base.bst
-    destination: "/"
-  - element: script/corruption-image.bst
-    destination: "/"
-  - element: script/corruption-integration.bst
-    destination: "/"
diff --git a/tests/integration/project/elements/script/marked-tmpdir.bst b/tests/integration/project/elements/script/marked-tmpdir.bst
index 506cdd5f4..7abbd3261 100644
--- a/tests/integration/project/elements/script/marked-tmpdir.bst
+++ b/tests/integration/project/elements/script/marked-tmpdir.bst
@@ -1,8 +1,7 @@
 kind: compose
 
-depends:
-- filename: base.bst
-  type: build
+build-depends:
+- base.bst
 
 public:
   bst:
diff --git a/tests/integration/project/elements/script/script-layout.bst b/tests/integration/project/elements/script/script-layout.bst
index 11ca353e3..f19b27e52 100644
--- a/tests/integration/project/elements/script/script-layout.bst
+++ b/tests/integration/project/elements/script/script-layout.bst
@@ -5,19 +5,12 @@ variables:
   install-root: /buildstream/nstall
   build-root: /buildstream/uild
 
-depends:
-  - filename: base.bst
-    type: build
+build-depends:
+  - base.bst
   - filename: script/script.bst
-    type: build
+    config:
+      location: /buildstream/uild
 
 config:
-  layout:
-    - element: base.bst
-      destination: /
-
-    - element: script/script.bst
-      destination: /buildstream/uild
-
   commands:
-    - "cp %{build-root}/test %{install-root}"
+  - "cp %{build-root}/test %{install-root}"
diff --git a/tests/integration/project/elements/script/script.bst b/tests/integration/project/elements/script/script.bst
index ffca23ab7..3f3eb55c7 100644
--- a/tests/integration/project/elements/script/script.bst
+++ b/tests/integration/project/elements/script/script.bst
@@ -1,9 +1,8 @@
 kind: script
 description: Script test
 
-depends:
-  - filename: base.bst
-    type: build
+build-depends:
+- base.bst
 
 config:
   commands:
diff --git a/tests/integration/project/elements/script/tmpdir.bst b/tests/integration/project/elements/script/tmpdir.bst
index 685a694ea..5fbf55dc9 100644
--- a/tests/integration/project/elements/script/tmpdir.bst
+++ b/tests/integration/project/elements/script/tmpdir.bst
@@ -1,8 +1,7 @@
 kind: script
 
-depends:
-- filename: script/no-tmpdir.bst
-  type: build
+build-depends:
+- script/no-tmpdir.bst
 
 config:
   commands: