Shell API and CLI: Add option for staging additional runtime targets #2147
Draft
nathanwilliams-ct wants to merge 4 commits into
Draft
Shell API and CLI: Add option for staging additional runtime targets #2147nathanwilliams-ct wants to merge 4 commits into
nathanwilliams-ct wants to merge 4 commits into
Conversation
Contributor
Author
|
Thanks, whoever triggered the CI... looks like it needs a little work, despite the tests working locally in the tox environment and I missed the linters. |
This enables users to add additional functionality such as debug tooling in the shell sandbox, without needing to modify the target element. This is achived through introducing a new option to shell. All existing API and UX is maintained, to not break existing scripts. An alternative design was considered, to have the additional elements as positional arguments similar to the existing element, but this would need manual parsing to handle the cases where `--` is present and not present, splitting based on a `.bst` suffix. This UX could be re-visited in future. Example usage: bst shell --with base.bst example.bst -- cat example.txt Where: - example.bst is a simple import element with no dependencies that imports a file called example.txt - base.bst provides a basic alpine sysroot with a standard set of unix tooling (sh, df, cat etc). Changes: - Introduces `test_with_other_targets` integration test to the shell test suite. - Adds `--with` cli option to the shell subcommand and updates it's documentation. - option can be used multiple times by caller, providing a list of targets. - Extends the shell top level calling interface in Buildstream core to accept a list of other_targets - This is where the targets are loaded into elements and checked to make sure they are present - Extends the shell element implementation to accept a list of other targets - This is where the other elements are staged and integrated into the sandbox
Buildstream implements a custom `FastEnum`[1].
mypy only supports `enum.Enum` (and it's official variations) when it comes to doing static type checks on enums [2].
We can't subclass Enum in FastEnum to make mypy happy, because Enum doesn't allow subclassing [3].
So we end up with a bunch of `str`, `int` etc around the codebase,
instead of the appropriate Enum classes like `OverlapAction` or `_Scope` which are usually recorded separately in the doc strings.
This means we don't get proper static type checking on our Enums :( .
With stub files [4][5] we can lie to mypy about what type the Enum classes are[6],
so the type hints around the codebase are now correct be corrected.
Now we can have proper static type checking on the custom Enums, Yay!
So
```
class OverlapAction(FastEnum):
ERROR: str
WARNING: str
IGNORE: str
```
gets stubbed as
```
from enum import Enum
class OverlapAction(Enum):
ERROR: str
WARNING: str
IGNORE: str
```
[1] src/buildstream/types.py#L32
[2] https://mypy.readthedocs.io/en/stable/literal_types.html#enums
[3] https://docs.python.org/3/howto/enum.html#restricted-enum-subclassing
[4] https://typing.python.org/en/latest/guides/writing_stubs.html
[5] https://mypy.readthedocs.io/en/stable/stubgen.html
[6] python/mypy#3217
88a3419 to
6c72e88
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This enables users to add additional functionality such as debug tooling in the shell sandbox,
without needing to modify the target element. This is achived through introducing a new option to
shell.
All existing API and UX is maintained, to not break existing scripts. An alternative design was
considered, to have the additional elements as positional arguments similar to the existing element,
but this would need manual parsing to handle the cases where
--is present and notpresent, splitting based on a
.bstsuffix. This UX could be re-visited in future.Example usage:
bst shell --with base.bst example.bst -- cat example.txt
Where:
Changes:
test_with_other_targetsintegration test to the shell test suite.--withcli option to the shell subcommand and updates it's documentation.other_targets
towards: #422