From e135b78d04dd9688c36bcf97de66419e11c3ed46 Mon Sep 17 00:00:00 2001 From: Tristan van Berkom Date: Fri, 4 Sep 2020 18:15:09 +0900 Subject: ScriptElement: Porting to new API 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 --- src/buildstream/plugins/elements/script.py | 20 ++- src/buildstream/plugins/elements/script.yaml | 28 ++-- src/buildstream/scriptelement.py | 169 +++++++++------------ tests/cachekey/project/elements/script1.expected | 2 +- tests/cachekey/project/target.expected | 2 +- .../project/elements/script/corruption-2.bst | 8 +- .../project/elements/script/corruption.bst | 20 +-- .../project/elements/script/marked-tmpdir.bst | 5 +- .../project/elements/script/script-layout.bst | 17 +-- .../integration/project/elements/script/script.bst | 5 +- .../integration/project/elements/script/tmpdir.bst | 5 +- 11 files changed, 120 insertions(+), 161 deletions(-) diff --git a/src/buildstream/plugins/elements/script.py b/src/buildstream/plugins/elements/script.py index 502212e10..6bcb02c81 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, dependencies): + for dep in dependencies: + + # Determine the location to stage each element, default is "/" + location = "/" + if dep.config: + dep.config.validate_keys(["location"]) + location = dep.config.get_str("location", location) + + # Add each element to the layout + self.layout_add(dep.element, dep.path, 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..0a3a4d3dc 100644 --- a/src/buildstream/scriptelement.py +++ b/src/buildstream/scriptelement.py @@ -36,11 +36,11 @@ 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: - from typing import Dict + from typing import Dict, Tuple class ScriptElement(Element): @@ -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[Tuple[Element, str]]] # The compose element's output is its dependencies, so # we must rebuild if the dependencies change even when @@ -75,8 +75,8 @@ class ScriptElement(Element): called from. Args: - work_dir: The working directory. If called without this argument - set, it'll default to the value of the variable ``cwd``. + work_dir: The working directory. If called without this argument + set, it'll default to the value of the variable ``cwd``. """ if work_dir is None: self.__cwd = self.get_variable("cwd") or "/" @@ -90,8 +90,8 @@ class ScriptElement(Element): once the commands have been run. Args: - install_root: The install root. If called without this argument - set, it'll default to the value of the variable ``install-root``. + install_root: The install root. If called without this argument + set, it'll default to the value of the variable ``install-root``. """ if install_root is None: self.__install_root = self.get_variable("install-root") or "/" @@ -108,47 +108,54 @@ class ScriptElement(Element): If this variable is not set, the default permission is read-write. Args: - root_read_only: Whether to mark the root filesystem as read-only. + root_read_only: Whether to mark the root filesystem as read-only. """ 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, dependency_path: str, 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. + dependency_path (str): The element relative path to the dependency, usually obtained via + :attr:`the dependency configuration ` + 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. + The order of directories in the layout is not significant. - .. note:: + The paths in the layout will be sorted so that elements are staged in parent + directories before subdirectories. - In the case that no element is specified, a read-write directory will - be made available at the specified location. + The elements for each respective staging 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 = {} + + # Get or create the element list + try: + element_list = self.__layout[location] + except KeyError: + element_list = [] + self.__layout[location] = element_list + + element_list.append((element, dependency_path)) def add_commands(self, group_name: str, command_list: List[str]) -> None: """Adds a list of commands under the group-name. @@ -164,8 +171,8 @@ class ScriptElement(Element): :func:`~buildstream.element.Element.node_subst_list`) Args: - group_name (str): The name of the group of commands. - command_list (list): The list of commands to be run. + group_name (str): The name of the group of commands. + command_list (list): The list of commands to be run. """ if not self.__commands: self.__commands = OrderedDict() @@ -174,17 +181,20 @@ 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): + sorted_locations = sorted(self.__layout) + layout_key = { + location: [dependency_path for _, dependency_path in self.__layout[location]] + for location in sorted_locations + } 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 +206,46 @@ 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 + # + sorted_locations = sorted(self.__layout) + + for location in sorted_locations: + element_list = [element for element, _ in self.__layout[location]] + self.stage_dependency_artifacts(sandbox, element_list, path=location) + + # Now integrate any elements staged in the root + # + root_list = self.__layout.get("/", None) + if root_list: + element_list = [element for element, _ in root_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 +266,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..5d9799f1a 100644 --- a/tests/cachekey/project/elements/script1.expected +++ b/tests/cachekey/project/elements/script1.expected @@ -1 +1 @@ -38078af3714d6d5128669dc7b4d7e942a46ea4137c388d44a1f455b4cc1918d0 \ No newline at end of file +1220b2849910f60f864c2c325cda301809dcb9b730281862448508fbeae5fb91 \ No newline at end of file diff --git a/tests/cachekey/project/target.expected b/tests/cachekey/project/target.expected index 38d4abeef..3a718b2ac 100644 --- a/tests/cachekey/project/target.expected +++ b/tests/cachekey/project/target.expected @@ -1 +1 @@ -5bf8ea2d6c0900c226c8fd3dae980f0b92e40f3fc1887b64d31bf04b6210d763 \ No newline at end of file +629e8d188cc2a88e2474bfabd45d4eb6d5e55435445bd1e5596953de564830dd \ 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: -- cgit v1.2.1