From 6a3af2624397bd385f02e37efbdc819d88c892f2 Mon Sep 17 00:00:00 2001 From: Nathan Williams Date: Mon, 13 Jul 2026 16:38:12 +0100 Subject: [PATCH 1/2] Static Type Checking: Enums 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] https://github.com/python/mypy/issues/3217 --- src/buildstream/_elementproxy.py | 6 +- src/buildstream/_overlapcollector.py | 6 +- src/buildstream/_pipeline.py | 9 +- src/buildstream/_stream.py | 16 +-- src/buildstream/element.py | 21 +++- src/buildstream/types.py | 8 +- src/buildstream/types.pyi | 177 +++++++++++++++++++++++++++ 7 files changed, 218 insertions(+), 25 deletions(-) create mode 100644 src/buildstream/types.pyi diff --git a/src/buildstream/_elementproxy.py b/src/buildstream/_elementproxy.py index 3425b6cc3..dc7f03090 100644 --- a/src/buildstream/_elementproxy.py +++ b/src/buildstream/_elementproxy.py @@ -95,7 +95,7 @@ def stage_artifact( sandbox: "Sandbox", *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True @@ -120,7 +120,7 @@ def stage_dependency_artifacts( selection: Optional[Sequence["Element"]] = None, *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True @@ -168,7 +168,7 @@ def _stage_artifact( sandbox: "Sandbox", *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True, diff --git a/src/buildstream/_overlapcollector.py b/src/buildstream/_overlapcollector.py index 79f62b156..d1f5dc0fc 100644 --- a/src/buildstream/_overlapcollector.py +++ b/src/buildstream/_overlapcollector.py @@ -66,7 +66,7 @@ def __init__(self, element: "Element"): # location (str): The Sandbox relative location this session was created for # @contextmanager - def session(self, action: str, location: Optional[str]): + def session(self, action: OverlapAction, location: Optional[str]): assert self._session is None, "Stage session already started" if location is None: @@ -108,13 +108,13 @@ def collect_stage_result(self, element: "Element", result: FileListResult): # location (str): The Sandbox relative location this session was created for # class OverlapCollectorSession: - def __init__(self, element: "Element", action: str, location: str): + def __init__(self, element: "Element", action: OverlapAction, location: str): # The Element we are staging for, on which we'll issue warnings self._element = element # type: Element # The OverlapAction for this session - self._action = action # type: str + self._action = action # type: OverlapAction # The Sandbox relative directory this session was created for self._location = location # type: str diff --git a/src/buildstream/_pipeline.py b/src/buildstream/_pipeline.py index 8a8a9e989..da54de206 100644 --- a/src/buildstream/_pipeline.py +++ b/src/buildstream/_pipeline.py @@ -44,7 +44,7 @@ # Yields: # Elements in the scope of the specified target elements # -def dependencies(targets: List[Element], scope: int, *, recurse: bool = True) -> Iterator[Element]: +def dependencies(targets: List[Element], scope: _Scope, *, recurse: bool = True) -> Iterator[Element]: # Keep track of 'visited' in this scope, so that all targets # share the same context. visited = (BitMap(), BitMap()) @@ -73,7 +73,12 @@ def dependencies(targets: List[Element], scope: int, *, recurse: bool = True) -> # A list of Elements appropriate for the specified selection mode # def get_selection( - context: Context, targets: List[Element], mode: str, *, silent: bool = True, depth_sort: bool = False + context: Context, + targets: List[Element], + mode: _PipelineSelection, + *, + silent: bool = True, + depth_sort: bool = False ) -> List[Element]: def redirect_and_log() -> List[Element]: # Redirect and log if permitted diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index 913bc3f17..91971073f 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -153,7 +153,7 @@ def load_selection( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, except_targets: Iterable[str] = (), load_artifacts: bool = False, connect_artifact_cache: bool = False, @@ -259,7 +259,7 @@ def query_cache(self, elements, *, sources_of_cached_elements=False, only_source def shell( self, target: str, - scope: int, + scope: _Scope, prompt: Callable[[Element], str], *, unique_id: Optional[str] = None, @@ -382,7 +382,7 @@ def build( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, ignore_junction_targets: bool = False, artifact_remotes: Iterable[RemoteSpec] = (), source_remotes: Iterable[RemoteSpec] = (), @@ -456,7 +456,7 @@ def fetch( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, except_targets: Iterable[str] = (), source_remotes: Iterable[RemoteSpec] = (), ignore_project_source_remotes: bool = False, @@ -579,7 +579,7 @@ def pull( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, ignore_junction_targets: bool = False, artifact_remotes: Iterable[RemoteSpec] = (), ignore_project_artifact_remotes: bool = False, @@ -633,7 +633,7 @@ def push( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, ignore_junction_targets: bool = False, artifact_remotes: Iterable[RemoteSpec] = (), ignore_project_artifact_remotes: bool = False, @@ -688,7 +688,7 @@ def checkout( *, location: Optional[str] = None, force: bool = False, - selection: str = _PipelineSelection.RUN, + selection: _PipelineSelection = _PipelineSelection.RUN, integrate: bool = True, hardlinks: bool = False, compression: str = "", @@ -1668,7 +1668,7 @@ def _load( self, targets: Iterable[str], *, - selection: str = _PipelineSelection.NONE, + selection: _PipelineSelection = _PipelineSelection.NONE, except_targets: Iterable[str] = (), ignore_junction_targets: bool = False, dynamic_plan: bool = False, diff --git a/src/buildstream/element.py b/src/buildstream/element.py index 6ac1ff1eb..209b9e9a9 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -90,7 +90,7 @@ from .sandbox import _SandboxFlags, SandboxCommandError from .sandbox._config import SandboxConfig from .sandbox._sandboxremote import SandboxRemote -from .types import _Scope, _CacheBuildTrees, _KeyStrength, OverlapAction, _DisplayKey +from .types import _HostMount, _Scope, _CacheBuildTrees, _KeyStrength, OverlapAction, _DisplayKey from ._artifact import Artifact from ._elementproxy import ElementProxy from ._elementsources import ElementSources @@ -604,7 +604,7 @@ def stage_artifact( sandbox: "Sandbox", *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True, @@ -664,7 +664,7 @@ def stage_dependency_artifacts( selection: Optional[Sequence["Element"]] = None, *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True, @@ -864,7 +864,7 @@ def subsandbox(self, sandbox: "Sandbox") -> Iterator["Sandbox"]: # Yields: # (Element): The dependencies in `scope`, in deterministic staging order # - def _dependencies(self, scope, *, recurse=True, visited=None): + def _dependencies(self, scope: _Scope, *, recurse=True, visited=None): # The format of visited is (BitMap(), BitMap()), with the first BitMap # containing element that have been visited for the `_Scope.BUILD` case @@ -971,7 +971,7 @@ def _stage_artifact( sandbox: "Sandbox", *, path: Optional[str] = None, - action: str = OverlapAction.WARNING, + action: OverlapAction = OverlapAction.WARNING, include: Optional[List[str]] = None, exclude: Optional[List[str]] = None, orphans: bool = True, @@ -2060,7 +2060,16 @@ def _push(self): # usebuildtree (bool): Use the buildtree as its source # # Returns: Exit code - def _shell(self, scope=None, *, mounts=None, isolate=False, prompt=None, command=None, usebuildtree=False): + def _shell( + self, + scope: _Scope | None = None, + *, + mounts: List[_HostMount] | None = None, + isolate: bool = False, + prompt: str | None = None, + command: List[str] | None = None, + usebuildtree: bool = False, + ): with self._prepare_sandbox(scope, shell=True, usebuildtree=usebuildtree) as sandbox: environment = sandbox._get_configured_environment() or self.get_environment() diff --git a/src/buildstream/types.py b/src/buildstream/types.py index 0cc2106b3..1e3ddbe63 100644 --- a/src/buildstream/types.py +++ b/src/buildstream/types.py @@ -35,6 +35,9 @@ class FastEnum(metaclass=MetaFastEnum): :class:`enum.Enum` attributes accesses can be really slow, and slow down the execution noticeably. This reimplementation doesn't suffer the same problems, but also does not reimplement everything. + + For mypy Enum static type checking support, all FastEnum should be stubbed as inheriting from enum.Enum. + Use `stubgen src/buildstream/types.py --include-docstrings` to generate the stubs, add `from enum import Enum` and replace all `(FastEnum)` with `(Enum)` """ name = None @@ -61,7 +64,7 @@ def __new__(cls, value): try: return cls._value_to_entry[value] except KeyError: - if type(value) is cls: # pylint: disable=unidiomatic-typecheck + if isinstance(value, cls): # pylint: disable=unidiomatic-typecheck return value raise ValueError("Unknown enum value: {}".format(value)) @@ -169,7 +172,6 @@ class OverlapAction(FastEnum): # Defines the scope of dependencies to include for a given element # when iterating over the dependency graph in APIs like # Element._dependencies(). -# class _Scope(FastEnum): # All elements which the given element depends on, following @@ -384,7 +386,7 @@ def new_from_node(cls, node: MappingNode) -> "_SourceMirror": alias_node: MappingNode = node.get_mapping("aliases") for alias, uris in alias_node.items(): - assert type(uris) is SequenceNode # pylint: disable=unidiomatic-typecheck + assert isinstance(uris, SequenceNode) # pylint: disable=unidiomatic-typecheck aliases[alias] = uris.as_str_list() return cls(name, aliases) diff --git a/src/buildstream/types.pyi b/src/buildstream/types.pyi new file mode 100644 index 000000000..8a75b9f13 --- /dev/null +++ b/src/buildstream/types.pyi @@ -0,0 +1,177 @@ +""" +Foundation type stubs +================ + +See src/buildstream/types.py + +These stubs are used to replace FastEnum with Enum when doing static type checking. + +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] https://github.com/python/mypy/issues/3217 + +""" + +from ._types import MetaFastEnum as MetaFastEnum +from .node import MappingNode as MappingNode, SequenceNode as SequenceNode +from _typeshed import Incomplete +from typing import Any +from enum import Enum + +class FastEnum(metaclass=MetaFastEnum): + """ + A reimplementation of a subset of the `Enum` functionality, which is far quicker than `Enum`. + + :class:`enum.Enum` attributes accesses can be really slow, and slow down the execution noticeably. + This reimplementation doesn't suffer the same problems, but also does not reimplement everything. + + For mypy Enum static type checking support, all FastEnum should be stubbed as inheriting from enum.Enum. + Use `stubgen src/buildstream/types.py --include-docstrings` to generate the stubs, add `from enum import Enum` and replace all `(FastEnum)` with `(Enum)` + """ + + name: Incomplete + value: Incomplete + @classmethod + def values(cls): + """Get all the possible values for the enum. + + Returns: + list: the list of all possible values for the enum + """ + + def __new__(cls, value): ... + def __eq__(self, other): ... + def __ne__(self, other): ... + def __hash__(self): ... + def __reduce__(self): ... + +class CoreWarnings: + """CoreWarnings() + + Some common warnings which are raised by core functionalities within BuildStream are found in this class. + """ + + OVERLAPS: str + UNSTAGED_FILES: str + REF_NOT_IN_TRACK: str + UNALIASED_URL: str + UNAVAILABLE_SOURCE_INFO: str + +class OverlapAction(Enum): + """OverlapAction() + + Defines what action to take when files staged into the sandbox overlap. + + .. note:: + + This only dictates what happens when functions such as + :func:`Element.stage_artifact() ` and + :func:`Element.stage_dependency_artifacts() ` + are called multiple times in an Element's :func:`Element.stage() ` + implementation, and the files staged from one function call result in overlapping files staged + from previous invocations. + + If multiple staged elements overlap eachother within a single call to + :func:`Element.stage_dependency_artifacts() `, + then the :ref:`overlap whitelist ` will be ovserved, and warnings will + be issued for overlapping files, which will be fatal warnings if + :attr:`CoreWarnings.OVERLAPS ` is specified + as a :ref:`fatal warning `. + """ + + ERROR: str + WARNING: str + IGNORE: str + +class _Scope(Enum): + ALL: int + BUILD: int + RUN: int + NONE: int + +class _KeyStrength(Enum): + STRONG: int + WEAK: int + +class _DisplayKey: + full: str + brief: str + strict: bool + def __init__(self, full: str, brief: str, strict: bool) -> None: ... + +class _SchedulerErrorAction(Enum): + CONTINUE: str + QUIT: str + TERMINATE: str + +class _CacheBuildTrees(Enum): + ALWAYS: str + AUTO: str + NEVER: str + +class _SourceUriPolicy(Enum): + ALL: str + ALIASES: str + MIRRORS: str + USER: str + +class _PipelineSelection(Enum): + NONE: str + REDIRECT: str + ALL: str + BUILD: str + RUN: str + +class _ProjectInformation: + project: Incomplete + provenance: Incomplete + duplicates: Incomplete + internal: Incomplete + def __init__(self, project, provenance_node, duplicates, internal) -> None: ... + +class _HostMount: + path: str + host_path: str + optional: bool + def __init__(self, path: str, host_path: str | None = None, optional: bool = False) -> None: ... + +class _SourceMirror: + name: str + aliases: dict[str, list[str]] + def __init__(self, name: str, aliases: dict[str, list[str]]) -> None: ... + @classmethod + def new_from_node(cls, node: MappingNode) -> _SourceMirror: ... + +SourceRef = None | int | str | list[Any] | dict[str, Any] From 6115da15640c911f71a85bd62eacf07f8251d1e5 Mon Sep 17 00:00:00 2001 From: Nathan Williams Date: Mon, 13 Jul 2026 16:39:54 +0100 Subject: [PATCH 2/2] docs: Add details about static type checking --- doc/source/hacking/using_the_testsuite.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/source/hacking/using_the_testsuite.rst b/doc/source/hacking/using_the_testsuite.rst index 418bdb7d9..9302b013a 100644 --- a/doc/source/hacking/using_the_testsuite.rst +++ b/doc/source/hacking/using_the_testsuite.rst @@ -190,6 +190,13 @@ consists of running the ``pylint`` tool, run the following:: .. _contributing_formatting_code: +Running Static Type Checkers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Static Type Checking is performed separately from testing. In order to run the static type checking step which +consists of running the ``mypy`` tool, run the following:: + + tox -e mypy + Formatting code ~~~~~~~~~~~~~~~ Similar to linting, code formatting is also done via a ``tox`` environment. To @@ -197,6 +204,8 @@ format the code using the ``black`` tool, run the following:: tox -e format +In CI `tox -e format-check` is used to ensure formatting has been run. + Observing coverage ~~~~~~~~~~~~~~~~~~ Once you have run the tests using `tox` (or `detox`), some coverage reports will