This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/light-cache-keys-without-sandbox-run in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 86cd1496a7b24e47b307416cc48f785b8e334c25 Author: Tristan van Berkom <[email protected]> AuthorDate: Thu Aug 4 15:20:26 2022 +0900 sandbox.py: Clarifications to the utility of Sandbox._disable_run() In the past, BuildStream had internal sandbox implementations which would require staging files on the local filesystem in order to run, but now we use buildbox-run exclusively and always use CasBasedDirectory. The _use_cas_based_directory() method was removed in the commit b487a11f6ee529c9cba505106d054c92c16e864c, and this was actually the only reason why we had the Element.BST_RUN_COMMANDS flag at the time. We still want BST_RUN_COMMANDS for other reasons, and it is useful to keep the Sandbox._disable_run() function, but it's only purpose now is to ensure the invariant that plugins who set BST_RUN_COMMANDS to False do not actually run commands. --- src/buildstream/element.py | 9 +++++---- src/buildstream/sandbox/sandbox.py | 17 +++++++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/buildstream/element.py b/src/buildstream/element.py index 8f47a5b13..9a2b1bc2f 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -1677,11 +1677,12 @@ class Element(Plugin): rootdir, output_file, output_file, self.__sandbox_config ) as sandbox: # noqa + # Ensure that the plugin does not run commands if it said that it wouldn't + # + # We only disable commands here in _assemble() instead of __sandbox() because + # the user might still run a shell on an element which itself does not run commands. + # if not self.BST_RUN_COMMANDS: - # Element doesn't need to run any commands in the sandbox. - # - # Disable Sandbox.run() to allow CasBasedDirectory for all - # sandboxes. sandbox._disable_run() # By default, the dynamic public data is the same as the static public data. diff --git a/src/buildstream/sandbox/sandbox.py b/src/buildstream/sandbox/sandbox.py index e30414bc1..a7cf9c777 100644 --- a/src/buildstream/sandbox/sandbox.py +++ b/src/buildstream/sandbox/sandbox.py @@ -64,6 +64,13 @@ class SandboxCommandError(SandboxError): self.collect = collect +# An internal exception which can be used to explicitly trigger a bug / exception +# which will be reported with a stack trace instead of reporting a user facing error +# +class _SandboxBug(Exception): + pass + + class Sandbox: """Sandbox() @@ -345,7 +352,7 @@ class Sandbox: label: str = None ) -> Optional[int]: if not self.__allow_run: - raise SandboxError("Sandbox.run() has been disabled") + raise _SandboxBug("Element specified BST_RUN_COMMANDS as False but called Sandbox.run()") # Fallback to the sandbox default settings for # the cwd and env. @@ -553,9 +560,11 @@ class Sandbox: # _disable_run() # - # Raise exception if `Sandbox.run()` is called. This enables use of - # CasBasedDirectory for faster staging when command execution is not - # required. + # Raise exception if `Sandbox.run()` is called. + # + # This enforces an invariant by raising an exception if an element + # plugin ever set BST_RUN_COMMANDS to False but then proceeded to + # attempt to run the sandbox at assemble time. # def _disable_run(self): self.__allow_run = False
