configuration: remove platform-specific scopes by tgamblin · Pull Request #50603 · spack/spack

tgamblin · 2025-05-21T23:40:13Z

Warning

This is a breaking change for v1.0. See below for how to address it.

Spack has historically allowed a platform-specific subdirectory for each configuration scope. e.g., in Spack's own defaults we use this for platform-specific overrides:

spack/etc/spack/
    packages.yaml
spack/etc/spack/linux/
    packages.yaml
spack/etc/spack/darwin/
    packages.yaml

Generifying platform includes

This adds specialized packages.yaml overrides for linux and darwin.

In #48784, we added a much more general facility for an include: section in configuration, so you can add your own much more specialized config directories if you need them. People had asked for OS scopes, target scopes, etc., and we did not want to support a bunch of rigidly defined subdirectories in the config system for all of these needs.

This PR addresses a couple lingering issues, namely:

We still hard-code the platform includes in Spack
Platform includes are at higher precedence than the including scope, while include: includes are lower. This makes it impossible for something like:
```
include:
- path: "${os}" 
```
to override the linux subdirectory in a config scope.

To solve this, we're removing the older, inflexible platform-specific config scopes in favor of allowing users to add their own such scopes if they need them.

Refactoring your config

If you were using platform-specific config. If you want platform configuration, you can still have it by adding this to the scope that needs it:

include:
   - include: "${platform}" 
     optional: true

If you want platform to have lower precedence than OS, you can do this:

include:
   - include: "${os}" 
     optional: true
   - include: "${platform}" 
     optional: true

And now OS config can override platform. Likewise, you could reverse the list and have platform with higher precedence than OS.

remove _add_platform_scope() from config`
refactor default config scope to account for new structure
~~- [ ] unit tests~~
Docs

lib/spack/docs/configuration.rst

greenc-FNAL · 2025-05-22T16:04:06Z

It appears that having defaults/include.yaml saying:

include:
  - path: "${platform}"
    optional: true
  - path: base

does not cause linux/include.yaml to be honored. Is this intentional? It would seem to be inconsistent with how packages.yaml, etc. are handled.

gartung · 2025-05-22T16:17:30Z

Correct. I needed to add $spack/etc/spack/include.yaml with

# include os specific config directory to support multi-platform package areas
include:
- path: "${os}/${platform}"
  optional: true
- path: "${platform}"
  optional: true

This was required to load $spack/etc/spack/linux/repos.yaml

tgamblin · 2025-05-22T17:03:44Z

It appears that having defaults/include.yaml saying:
include:
  - path: "${platform}"
    optional: true
  - path: base
does not cause linux/include.yaml to be honored. Is this intentional? It would seem to be inconsistent with how packages.yaml, etc. are handled.

include: includes files in the current scope, even in defaults (which is just the lowest scope). The above YAML says "below my scope, add another config scope using the contents of some directory called ${platform} relative to the file where include: is defined".

Putting an include in defaults shouldn't propagate to higher scopes -- e.g. if you did this:

include:
  - path: myfile.yaml

I do not think you would want all higher scopes to require a file called myfile.yaml -- just the one where you put the include:.

greenc-FNAL · 2025-05-22T18:24:47Z

@tgamblin @tldahlgren So I added include.yaml to $spack/etc/spack/:

[greenc@scisoftbuild02] spack (feature-fnal/remove-platform-scopes) (spack) $ spack config blame include
---                                                                                 include:
/scratch/greenc/art-develop-2025-05-15/spack_env/etc/spack/include.yaml:3           - path: ${platform}/${os}
/scratch/greenc/art-develop-2025-05-15/spack_env/etc/spack/include.yaml:4             optional: True
/scratch/greenc/art-develop-2025-05-15/spack_env/etc/spack/include.yaml:5           - path: ${platform}
/scratch/greenc/art-develop-2025-05-15/spack_env/etc/spack/include.yaml:6             optional: True
/scratch/greenc/art-develop-2025-05-15/spack_env/etc/spack/defaults/include.yaml:4  - path: base

and got:

[greenc@scisoftbuild02] spack (feature-fnal/remove-platform-scopes) (spack) $ ( unset SPACK_DISABLE_LOCAL_CONFIG; spack config scopes --file)
env:critic-develop-gcc-14-2-0-cxx20-prof-gcc-11 user site include:almalinux9 include:linux system defaults include:base

Is it expected and reasonable that (e.g.) etc/spack/packages.yaml has a higher priority than include:almalinux and include:linux?

tgamblin · 2025-05-22T21:18:42Z

@greenc-FNAL :

Is it expected and reasonable that (e.g.) etc/spack/packages.yaml has a higher priority than include:almalinux and include:linux?

Yes -- see the PR description:

Platform includes are at higher precedence than the including scope, while include: includes are lower.

We want the including scope to be higher precedence than what is included. Given A includes B, consider 3 use cases:

	A has higher precedence than B	A has lower precedence than B
A can override something in B	✅ define it in A or ✅ define it in C and include C from A at higher precedence than B	❌ can't do it
A wants to use B's value for something	✅ don't define it in A	✅ always the case
B is optional; A wants to define a default value but use B's value if present	✅ define it in C and include C from A at lower precedence than B	✅ define it in A

We went with the more expressive model where the including scope has full control.

greenc-FNAL · 2025-05-23T13:29:59Z

@tgamblin I guess I'm having a conceptual problem with the other-than-normal indirectness of "included by" here. If I'm understanding you correctly, if etc/spack/include.yaml says:

include:
- path: "${platform}/${os}"
  optional: true
- path: "${platform}"
  optional: true

then etc/spack/packages.yaml—regardless of whether it actually exists—"includes" linux/almalinus9/packages.yaml if it exists, and linux/packages.yaml (again, if it exists) at a lower precedence. Any actual content of etc/spack/packages.yaml overrides content in both of the other files, leading to a situation where OS-specific content overrides platform-specific content, but what was previously understood to be "generic, overridable" content now overrides both of the others.

It's also disconcerting to me that this happens at "global" rather than "section" level: one does not include one packages.yaml from another, but if the directories in which those files are located have the correct relationship in an include.yaml file, then that is the semantic they have. However, so do corresponding config.yaml files, and include.yaml files (potentially causing things to get really hairy), and, and ...

My worry is that this is not just a "breaking change," but a "brain-breaking change:" the docs will have to carefully explain the consequences of the new mechanism, how it differs from the old, and exactly what must be done in the new paradigm to replicate the semantics of the old paradigm.

If I've misunderstood or misstated anything in my analysis above, I'd appreciate correction: I'm perfectly willing to learn either that I've been understanding this wrongly, and/or that the fact that I'm having trouble conceptually is unique to me, and other people are fine with it.

tgamblin · 2025-05-23T16:11:06Z

@greenc-FNAL: can you say how you want things to behave and why?

Copilot

Pull Request Overview

This PR removes rigid platform‐specific configuration scopes in favor of a more flexible include mechanism. Key changes include:

Removing calls to _add_platform_scope and updating scope naming conventions (e.g. using "::" for environment scopes).
Adjusting tests, documentation, and completion scripts to align with the new naming syntax.
Refactoring bootstrap configuration and audit functions to reflect the removal of platform-specific scopes.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
share/spack/spack-completion.fish	Update completions to use new scope naming with "::" syntax
lib/spack/spack/test/config.py	Modify expected environment scope names from "env:" to "env::"
lib/spack/spack/main.py	Remove _add_platform_scope invocation
lib/spack/spack/environment/environment.py	Update environment scope naming to use "::"
lib/spack/spack/config.py	Refactor configuration scope handling and remove platform-dependent logic
lib/spack/spack/bootstrap/config.py	Remove platform-specific configuration scope addition
lib/spack/spack/audit.py	Adjust audit scope lookup and remove extraneous debug output
lib/spack/docs/configuration.rst	Update documentation for platform-specific configuration via includes
etc/spack/defaults/include.yaml	Define new include paths for platform configuration

lib/spack/spack/config.py

lib/spack/spack/audit.py

tgamblin · 2025-05-23T16:50:59Z

rebased to deal with #50612

greenc-FNAL · 2025-05-23T18:00:10Z

@greenc-FNAL: can you say how you want things to behave and why?

I don't think this approach is bad, I'm just having trouble wrapping my head around it and worry that it's going to be hard for other people too, and existing Spack installations, when upgraded, are going to behave differently that before in ways that might be hard to trace.

If I was thinking about the problem de novo-ish, I would expect a system where the "scope"s would be command_line [env] user site defaults, with any given file representing a section in a given scope able to have an include: clause whose array elements would describe a descending hierarchy of precedence, all of which would be overridden by the remaining contents of the including file.

This would seem to be more in keeping with file-based include semantics in other languages, and the concepts of "includes," is explicit rather than indirect. If I understood the motives behind include as its own section (except in environments) specifying directories rather than specific include: clauses in section-specific files, it might be easier to get my head around the idea.

tgamblin · 2025-05-23T22:51:34Z

@tldahlgren: I'm bailing on the :defaults syntax we discussed for scopes b/c it's complicating this PR. Specifically, windows paths. I think all config scope names need to be identifiers without / or :, and we can use @greenc-FNAL's #41455 to associate short names with filesystem paths. We can still use : to indicate inclusion, but I don't think we can do that and long paths in the same expression without confusing everyone. We can do the fix in a separate PR, maybe after #41455.

Here, I just added a private _merged_scope option to get_config to tide us over, which is all we need for the tests here.

tgamblin · 2025-05-23T22:55:50Z

@greenc-FNAL:

If I was thinking about the problem de novo-ish, I would expect a system where the "scope"s would be command_line [env] user site defaults, with any given file representing a section in a given scope able to have an include: clause whose array elements would describe a descending hierarchy of precedence, all of which would be overridden by the remaining contents of the including file.

We don't have this -- we just have scopes and sections, and merging is done at scope granularity, not section granularity. However, if you think of the entire scope as a split out spack.yaml, it makes a lot more sense. Every file in a directory scope is a section in a single file scope:

spack:
    include:
        - ...
    packages:
        all:
            ...
    repos:
        ...

is equivalent to:

config_dir/
    include.yaml
    packages.yaml
    repos.yaml

Spack has historically allowed a platform-specific subdirectory for each configuration scope. e.g., in Spack's own defaults we use this for platform-specific overrides: ``` spack/etc/spack/ packages.yaml spack/etc/spack/linux/ packages.yaml spack/etc/spack/darwin/ packages.yaml ``` This adds specialized `packages.yaml` overrides for linux and darwin. In #48784, we added a much more general facility for an `include:` section in configuration, so you can add your own much more specialized config directories. This PR addresses a couple issues with that, namely: 1. We still hard-code the platform includes in Spack 2. Platform includes are at *higher* precedence than the including scope, while `include:` includes are lower. This makes it impossible for something like: ```yaml include: - include: "${os}" ``` to override the linux subdirectory in a config scope. This PR removes the older platform-specific config scopes in favor of allowing users to add their own such scopes if they need them. So, if you want platform configuration, you can add this to the scope that needs it: ```yaml include: - include: "${platform}" ``` If you want platform to have lower precedence than OS, you can do this: ```yaml include: - include: "${os}" include: - include: "${platform}" ``` And now OS config can override platform. Likewise, you could reverse the list and have platofrm with higher precedence than OS. - [x] remove `_add_platform_scope() from `config` - [x] refactor default config scope to account for new structure - [ ] TBD: docs Signed-off-by: Todd Gamblin <[email protected]>

Co-authored-by: Chris Green <[email protected]>

We can rework scope names in a future PR Signed-off-by: Todd Gamblin <[email protected]>

Signed-off-by: Todd Gamblin <[email protected]>

@tldahlgren

* New command list-scopes [--file] [--non-platform] - Add missing `_non_platform_scopes()` - Update shell completion scripts - Scope is "defaults," not, "default." - Reorder tests to look for OS-dependent test hysteresis - Disable output suppression for pipeline failure diagnosis - Remove problematic test criteria - `defaults` apparently not available in test environment * `--file` and `--non-platform` are no longer mutually-exclusive * Add `-f` alias for `--file` option * Remove test dependence on external shell environment * Add tests for platform-specific scopes * Add test for `--file --non-platform` * Update shell completion files * Fix key error in test * Use test platform in tests * Remove faulty list-scopes assertions * New `list-scopes` option `--included` * Update command completion * Remove non-working test code * `list-scopes` -> `scopes` Per #41455 (comment). * Remove `--non-platform` in antipication of #50603 * file -> path * Rationalize options and add ability to specify sections * Improve help text for `spack config scopes` * New option `--show-paths|-s` to show paths where scopes have the * Add shortcut `-i` to `--include` * Add non-option argument to specify section for which to show paths * `--path` -> `--path-scopes` to avoid confusion with `--show-paths` * Remove unhelpful type hint * Per @tldahlgren review * Improve tests per @tldahlgren suggestions * Attempt to accommodate Windows™ * Non-option argument `<section>` implies `-s|--show-paths`

Spack has historically allowed a platform-specific subdirectory for each configuration scope. e.g., in Spack's own defaults we use this for platform-specific overrides: ``` spack/etc/spack/ packages.yaml spack/etc/spack/linux/ packages.yaml spack/etc/spack/darwin/ packages.yaml ``` This adds specialized `packages.yaml` overrides for linux and darwin. In spack#48784, we added a much more general facility for an `include:` section in configuration, so you can add your own much more specialized config directories if you need them. People had asked for OS scopes, target scopes, etc., and we did not want to support a bunch of rigidly defined subdirectories in the config system for all of these needs. This PR addresses a couple lingering issues, namely: 1. We still hard-code the platform includes in Spack 2. Platform includes are at *higher* precedence than the including scope, while `include:` includes are lower. This makes it impossible for something like: ```yaml include: - path: "${os}" ``` to override the linux subdirectory in a config scope. To solve this, we're removing the older, inflexible platform-specific config scopes in favor of allowing users to add their own such scopes if they need them. If you were using platform-specific config. If you want platform configuration, you can still have it by adding this to the scope that needs it: ```yaml include: - include: "${platform}" optional: true ``` If you want platform to have lower precedence than OS, you can do this: ```yaml include: - include: "${os}" optional: true - include: "${platform}" optional: true ``` And now OS config can override platform. Likewise, you could reverse the list and have platform with higher precedence than OS. - [x] remove `_add_platform_scope() from `config` - [x] refactor default config scope to account for new structure - [x] Docs --------- Signed-off-by: Todd Gamblin <[email protected]> Co-authored-by: Tamara Dahlgren <[email protected]> Co-authored-by: Chris Green <[email protected]>

@tldahlgren

* New command list-scopes [--file] [--non-platform] - Add missing `_non_platform_scopes()` - Update shell completion scripts - Scope is "defaults," not, "default." - Reorder tests to look for OS-dependent test hysteresis - Disable output suppression for pipeline failure diagnosis - Remove problematic test criteria - `defaults` apparently not available in test environment * `--file` and `--non-platform` are no longer mutually-exclusive * Add `-f` alias for `--file` option * Remove test dependence on external shell environment * Add tests for platform-specific scopes * Add test for `--file --non-platform` * Update shell completion files * Fix key error in test * Use test platform in tests * Remove faulty list-scopes assertions * New `list-scopes` option `--included` * Update command completion * Remove non-working test code * `list-scopes` -> `scopes` Per spack#41455 (comment). * Remove `--non-platform` in antipication of spack#50603 * file -> path * Rationalize options and add ability to specify sections * Improve help text for `spack config scopes` * New option `--show-paths|-s` to show paths where scopes have the * Add shortcut `-i` to `--include` * Add non-option argument to specify section for which to show paths * `--path` -> `--path-scopes` to avoid confusion with `--show-paths` * Remove unhelpful type hint * Per @tldahlgren review * Improve tests per @tldahlgren suggestions * Attempt to accommodate Windows™ * Non-option argument `<section>` implies `-s|--show-paths`

Spack has historically allowed a platform-specific subdirectory for each configuration scope. e.g., in Spack's own defaults we use this for platform-specific overrides: ``` spack/etc/spack/ packages.yaml spack/etc/spack/linux/ packages.yaml spack/etc/spack/darwin/ packages.yaml ``` This adds specialized `packages.yaml` overrides for linux and darwin. In spack#48784, we added a much more general facility for an `include:` section in configuration, so you can add your own much more specialized config directories if you need them. People had asked for OS scopes, target scopes, etc., and we did not want to support a bunch of rigidly defined subdirectories in the config system for all of these needs. This PR addresses a couple lingering issues, namely: 1. We still hard-code the platform includes in Spack 2. Platform includes are at *higher* precedence than the including scope, while `include:` includes are lower. This makes it impossible for something like: ```yaml include: - path: "${os}" ``` to override the linux subdirectory in a config scope. To solve this, we're removing the older, inflexible platform-specific config scopes in favor of allowing users to add their own such scopes if they need them. If you were using platform-specific config. If you want platform configuration, you can still have it by adding this to the scope that needs it: ```yaml include: - include: "${platform}" optional: true ``` If you want platform to have lower precedence than OS, you can do this: ```yaml include: - include: "${os}" optional: true - include: "${platform}" optional: true ``` And now OS config can override platform. Likewise, you could reverse the list and have platform with higher precedence than OS. - [x] remove `_add_platform_scope() from `config` - [x] refactor default config scope to account for new structure - [x] Docs --------- Signed-off-by: Todd Gamblin <[email protected]> Co-authored-by: Tamara Dahlgren <[email protected]> Co-authored-by: Chris Green <[email protected]>

@tldahlgren

* New command list-scopes [--file] [--non-platform] - Add missing `_non_platform_scopes()` - Update shell completion scripts - Scope is "defaults," not, "default." - Reorder tests to look for OS-dependent test hysteresis - Disable output suppression for pipeline failure diagnosis - Remove problematic test criteria - `defaults` apparently not available in test environment * `--file` and `--non-platform` are no longer mutually-exclusive * Add `-f` alias for `--file` option * Remove test dependence on external shell environment * Add tests for platform-specific scopes * Add test for `--file --non-platform` * Update shell completion files * Fix key error in test * Use test platform in tests * Remove faulty list-scopes assertions * New `list-scopes` option `--included` * Update command completion * Remove non-working test code * `list-scopes` -> `scopes` Per spack#41455 (comment). * Remove `--non-platform` in antipication of spack#50603 * file -> path * Rationalize options and add ability to specify sections * Improve help text for `spack config scopes` * New option `--show-paths|-s` to show paths where scopes have the * Add shortcut `-i` to `--include` * Add non-option argument to specify section for which to show paths * `--path` -> `--path-scopes` to avoid confusion with `--show-paths` * Remove unhelpful type hint * Per @tldahlgren review * Improve tests per @tldahlgren suggestions * Attempt to accommodate Windows™ * Non-option argument `<section>` implies `-s|--show-paths`

tgamblin requested a review from tldahlgren May 21, 2025 23:40

spackbot-app bot added core PR affects Spack core functionality defaults labels May 21, 2025

tgamblin added breaking-change configuration labels May 22, 2025

tgamblin added this to the v1.0.0 milestone May 22, 2025

spackbot-app bot added the documentation Improvements or additions to documentation label May 22, 2025

greenc-FNAL mentioned this pull request May 22, 2025

New subcommand spack config scopes [-hips] #41455

Merged

greenc-FNAL added a commit to greenc-FNAL/spack that referenced this pull request May 22, 2025

Remove --non-platform in antipication of spack#50603

437a449

greenc-FNAL reviewed May 22, 2025

View reviewed changes

lib/spack/docs/configuration.rst Outdated Show resolved Hide resolved

greenc-FNAL added a commit to FNALssi/spack that referenced this pull request May 22, 2025

Remove --non-platform in antipication of spack#50603

6b053f7

spackbot-app bot added environments shell-support tests General test capability(ies) labels May 23, 2025

tgamblin requested a review from Copilot May 23, 2025 16:23

Copilot AI reviewed May 23, 2025

View reviewed changes

lib/spack/spack/config.py Outdated Show resolved Hide resolved

lib/spack/spack/audit.py Outdated Show resolved Hide resolved

tgamblin force-pushed the remove-platform-scope branch from cf209d5 to b1adf78 Compare May 23, 2025 16:50

tgamblin force-pushed the remove-platform-scope branch 2 times, most recently from f7511c4 to 84873c8 Compare May 23, 2025 22:50

greenc-FNAL previously approved these changes May 27, 2025

View reviewed changes

tldahlgren previously approved these changes May 27, 2025

View reviewed changes

tldahlgren self-assigned this May 27, 2025

tgamblin and others added 7 commits May 27, 2025 10:30

remove additional platform scopes from config and bootstrap/config

fc8b925

configuration: snapshot of docs updates

673b378

configuration.rst: fix typo

a1bed79

Co-authored-by: Chris Green <[email protected]>

reword docs

e27e87d

do something far simpler than slicing to appease tests

e39020d

We can rework scope names in a future PR Signed-off-by: Todd Gamblin <[email protected]>

more path-like names for included scopes

84fdd9a

Signed-off-by: Todd Gamblin <[email protected]>

tldahlgren dismissed stale reviews from greenc-FNAL and themself via 84fdd9a May 27, 2025 17:36

tldahlgren force-pushed the remove-platform-scope branch from 2e0bf8d to 84fdd9a Compare May 27, 2025 17:36

tldahlgren approved these changes May 27, 2025

View reviewed changes

tldahlgren enabled auto-merge (squash) May 27, 2025 18:49

greenc-FNAL added a commit to greenc-FNAL/spack that referenced this pull request May 27, 2025

Remove --non-platform in antipication of spack#50603

e8a24d8

greenc-FNAL disabled auto-merge May 27, 2025 21:39

greenc-FNAL enabled auto-merge (squash) May 27, 2025 21:39

tgamblin disabled auto-merge May 28, 2025 01:14

tgamblin enabled auto-merge (squash) May 28, 2025 01:16

tgamblin merged commit f2ed3b1 into develop May 28, 2025
34 checks passed

tgamblin deleted the remove-platform-scope branch May 28, 2025 01:32

greenc-FNAL added a commit to greenc-FNAL/spack that referenced this pull request May 28, 2025

Remove --non-platform in antipication of spack#50603

304dec6

greenc-FNAL mentioned this pull request Jun 2, 2025

Back-port platform scope removal and spack config scopes from upstream develop FNALssi/spack#554

Merged

Conversation

tgamblin commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generifying platform includes

Refactoring your config

Uh oh!

Uh oh!

greenc-FNAL commented May 22, 2025

Uh oh!

gartung commented May 22, 2025

Uh oh!

tgamblin commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greenc-FNAL commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greenc-FNAL commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin commented May 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

tgamblin commented May 23, 2025

Uh oh!

greenc-FNAL commented May 23, 2025

Uh oh!

tgamblin commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgamblin commented May 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tgamblin commented May 21, 2025 •

edited

Loading

tgamblin commented May 22, 2025 •

edited

Loading

greenc-FNAL commented May 22, 2025 •

edited

Loading

tgamblin commented May 22, 2025 •

edited

Loading

greenc-FNAL commented May 23, 2025 •

edited

Loading

tgamblin commented May 23, 2025 •

edited

Loading