directives: lazy execution for package metadata #51881

haampie · 2026-01-22T12:54:32Z

This refactors the directive system to execute directives lazily. Directives
are now queued during class definition and only executed when the corresponding
dictionary on the package class is accessed (e.g., pkg.dependencies,
pkg.versions). This reduces overhead during cache population, and in non-forking
build sub-processes where spec.package is used and metadata isn't needed.

DirectiveDictDescriptor is a singleton descriptor shared across package
classes. It handles the lazy initialization of package attributes.
For each dictionary, the set of directives affecting it is computed
automatically. For example, accessing pkg.extendees automatically triggers
the execution of both extends and depends_on directives.
The actual dictionary is now stored at pkg._dependencies and so on. It is
initially set to None to indicate the directives have not run yet.
_execute_depends_on now applies nested patches immediately via
_execute_patch instead of going through patch(...) internally. This prevents
state corruption in the global directive queue when depends_on is evaluated
lazily.
maintainers directives remain eager to ensure backward compatibility with
packages that define maintainers as a simple class-level list.
PackageBase._patches_dependencies = True/False is set without evaluation of
depends_on and extends directives. This allows us to optimize patch indexing by
avoiding evaluation of these directives if none of them produce patches.

In the current form, the PR has two "breaking" changes:

patches=... is kwarg-only in the depends_on and extends directives.
Previously this was a positional argument.
Nested directives are only deleted from the patches kwarg of depends_on
and extends. Previously all args and kwarg values where searched for nested
directives.

In principle (1) is a breaking change to the package API, but pragmatically speaking,
I would argue (a) it was an oversight from us to keep patches as a positional argument,
and (b) it's not breaking in practice for all packages in spack/spack-packages. If it
breaks an internal repo, the change of adding patches=[...] is trivial.

The descriptor indirection does not seem to impact the "setup" phase of the
solver.

The first use of Spack is significantly faster. Below are results from a best of 3.

Before (855e07a):

$ spack clean -m && time spack list
0m15.371s

After (5ee3706):

$ spack clean -m && time spack list
0m8.452s

The list(spack.repo.PATH.all_package_classes()) benchmark is now useless. With
a few other PRs combined (#51875, #51836, #51879, #48771), the first-use time reduces
further to under 5 seconds, or 3x faster than reference.

haampie · 2026-01-23T12:13:38Z

As a possible next step, not included in this PR, also with when(...) could be made lazy by passing the when-stack-tuple as a kwarg like directive(..., when=(<original when kwarg>, <when 1>, ..., <when n>). Probably not as impactful.

Edit: submitted #51884 nonetheless, because it simplifies the when stack to Tuple[str, ...], instead of some odd mix Tuple[Optional[str], Tuple[Spec, ...]].

haampie · 2026-01-26T16:38:34Z

Looks like it doesn't affect hashes in spack/spack-packages#3139; combined with "lazy when" from #51884

This refactors the directive system to execute directives lazily. Directives are now queued during class definition and only executed when the corresponding dictionary on the package class is accessed (e.g., `pkg.dependencies`, `pkg.versions`). This reduces overhead during cache population, and in non-forking build sub-processes where `spec.package` is used and metadata isn't needed. - `DirectiveDictDescriptor` is a singleton descriptor shared across package classes. It handles the lazy initialization of package attributes. - For each dictionary, the set of directives affecting it is computed automatically. For example, accessing `pkg.extendees` automatically triggers the execution of both `extends` and `depends_on` directives. - The actual dictionary is now stored at `pkg._dependencies` and so on. It is initially set to `None` to indicate the directives have not run yet. - `_execute_depends_on` now applies nested patches immediately via `_execute_patch` instead of going through `patch(...)` internally. This prevents state corruption in the global directive queue when `depends_on` is evaluated lazily. - `maintainers` directives remain eager to ensure backward compatibility with packages that define maintainers as a simple class-level list. - `PackageBase._patches_dependencies = True/False` is set without evaluation of `depends_on` and `extends` directives. This allows us to optimize patch indexing by avoiding evaluation of these directives if none of them produce patches. Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

alalazo

LGTM. The case that seems most complicated is the one of patched dependencies, but we are still placing information in the same place as before, so ✔️

alalazo · 2026-01-29T12:50:41Z

In principle (1) is a breaking change to the package API, but pragmatically speaking,
I would argue (a) it was an oversight from us to keep patches as a positional argument,
and (b) it's not breaking in practice for all packages in spack/spack-packages. If it
breaks an internal repo, the change of adding patches=[...] is trivial.

I agree with that. I'd say I would not see as breaking a change that can be fixed in a way that is also backward compatible.

alalazo · 2026-01-29T13:32:47Z

Waiting on @tgamblin to give ✔️ before merging

haampie · 2026-01-29T16:09:27Z

FWIW, I can follow up with an audit that explicitly executes directives, like:

for pkg in repo.all_package_classes():
  for attr in directive_dicts:
    getattr(pkg, attr)

tgamblin · 2026-01-30T02:26:06Z

I'm ok with this (enthusiastic, even) if we can add an audit like the one @haampie mentions. I think we need that soon though -- otherwise I think we're going to start getting a lot of packages with invalid constraints.

tgamblin · 2026-01-30T02:29:45Z

lib/spack/spack/package_base.py

+    #: Set to true when this package has directives that specify patches on dependencies. Can be
+    #: used as an optimization to avoid execution of :func:`~spack.directives.depends_on` and
+    #: :func:`~spack.directives.extends` directives when looking for all patches defined by this
+    #: package.


I think the comment should be clear that the directive does this automatically, so that users (or future developers) don't think that packagers need to set this. I had to dig around to verify this.

tgamblin · 2026-01-30T02:31:07Z

lib/spack/spack/directives_meta.py

+    _descriptor_cache: Dict[str, "DirectiveDictDescriptor"] = {}
    #: Set of all known directive dictionary names from `@directive(dicts=...)`
    _directive_dict_names: Set[str] = set()
    #: List of directives to be executed at class initialization time


Describe what the str key is in this dictionary.

becker33 · 2026-01-26T01:37:47Z

lib/spack/spack/directives.py

 def _execute_depends_on(
    pkg: PackageType,
-    spec: spack.spec.Spec,
+    spec: Union[SpecType, spack.spec.Spec],


Why is SpecType set to str? If so, we should just use str. I would expect SpecType to be Union[str, spack.spec.Spec].

tgamblin

Minor stuff - otherwise LGTM.

tgamblin · 2026-01-30T02:32:09Z

lib/spack/spack/directives_meta.py

+    #: Whether the package being defined patches dependencies
+    _patches_dependencies: bool = False


say who sets this.

tgamblin · 2026-01-30T02:33:59Z

lib/spack/spack/directives_meta.py

-                        directives.remove(directive)  # iterations ends, so mutation is fine
+    def _remove_kwarg_value_directives_from_queue(value) -> None:
+        """Remove directives found in a kwarg value from the execution queue."""
+        # Certain keyword argument values of directives may themselve be (lists of) directives. An


Suggested change

# Certain keyword argument values of directives may themselve be (lists of) directives. An

# Certain keyword argument values of directives may themselves be (lists of) directives. An

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

haampie · 2026-01-30T10:23:59Z

Thanks, incorporated the feedback including an audit. Also avoided the duplication of DirectiveMeta._patches_dependencies in PackageBase, so it's only documented once. It wasn't needed, unlike the type hints for the dynamically defined dictionaries.

github-actions bot added the unit-tests label Jan 22, 2026

haampie force-pushed the hs/fix/speedup-first-spack-load branch 9 times, most recently from 176ca08 to 3ce6b09 Compare January 23, 2026 11:24

haampie force-pushed the hs/fix/speedup-first-spack-load branch from 3ce6b09 to d973407 Compare January 23, 2026 14:46

haampie mentioned this pull request Jan 24, 2026

directives: lazy with when(...) #51884

Open

haampie force-pushed the hs/fix/speedup-first-spack-load branch 4 times, most recently from d359abf to ed2a9aa Compare January 24, 2026 21:19

tgamblin requested review from becker33, Copilot and tgamblin and removed request for Copilot January 27, 2026 07:53

Copilot started reviewing on behalf of tgamblin January 27, 2026 07:53 View session

haampie added 2 commits January 27, 2026 15:12

add a few more tests

dd4bb2c

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

haampie force-pushed the hs/fix/speedup-first-spack-load branch from ed2a9aa to dd4bb2c Compare January 27, 2026 14:19

alalazo previously approved these changes Jan 29, 2026

View reviewed changes

alalazo self-assigned this Jan 29, 2026

tgamblin requested changes Jan 30, 2026

View reviewed changes

becker33 reviewed Jan 30, 2026

View reviewed changes

tgamblin requested changes Jan 30, 2026

View reviewed changes

haampie added 2 commits January 30, 2026 10:40

Review comments

8488335

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

add audit

99787e1

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>

haampie dismissed alalazo’s stale review via 99787e1 January 30, 2026 10:19

		#: Whether the package being defined patches dependencies
		_patches_dependencies: bool = False

	# Certain keyword argument values of directives may themselve be (lists of) directives. An
	# Certain keyword argument values of directives may themselves be (lists of) directives. An

directives: lazy execution for package metadata #51881

Are you sure you want to change the base?

directives: lazy execution for package metadata #51881

Uh oh!

Conversation

haampie commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalazo left a comment

Choose a reason for hiding this comment

Uh oh!

alalazo commented Jan 29, 2026

Uh oh!

alalazo commented Jan 29, 2026

Uh oh!

haampie commented Jan 29, 2026

Uh oh!

tgamblin commented Jan 30, 2026

Uh oh!

tgamblin Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

tgamblin Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

becker33 Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

tgamblin Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

tgamblin Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

haampie commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

haampie commented Jan 22, 2026 •

edited

Loading

haampie commented Jan 23, 2026 •

edited

Loading

haampie commented Jan 26, 2026 •

edited

Loading

haampie commented Jan 30, 2026 •

edited

Loading