Skip to content

Allowing environment variables to be set in a spack.yaml#47587

Merged
tgamblin merged 26 commits intospack:developfrom
fryeguy52:env-vars-in-spack-env
Jan 27, 2025
Merged

Allowing environment variables to be set in a spack.yaml#47587
tgamblin merged 26 commits intospack:developfrom
fryeguy52:env-vars-in-spack-env

Conversation

@fryeguy52
Copy link
Copy Markdown
Contributor

@fryeguy52 fryeguy52 commented Nov 13, 2024 •
edited by tgamblin
Loading

This adds a new configuration section called env_vars: that can be set in an environment.

It looks very similar to the existing environment: section that can be added to modules.yaml,
but it is global for an entire spack environment. It's called env_vars: to deconflate it with spack
environments (the term was too overloaded).

The syntax looks like this:

spack:
  specs:
    - cmake%gcc
  env_vars:
    set:
      ENVAR_SET_IN_ENV_LOAD: "True"

Any of our standard environment modifications can be added to the env_vars section, e.g.
prepend_path:, unset:, append_path:, etc. Operations in env_vars: are performed
on spack env activate and undone on spack env deactivate.

@spackbot-app spackbot-app bot added commands core PR affects Spack core functionality environments tests General test capability(ies) labels Nov 13, 2024
@fryeguy52 fryeguy52 requested a review from psakievich November 13, 2024 22:13
@psakievich psakievich added the snl-core-team Issue for SNL Spack developers label Nov 13, 2024
@tgamblin tgamblin added this to the v0.24 milestone Nov 14, 2024
tgamblin
tgamblin reviewed Nov 14, 2024
View reviewed changes
@becker33 becker33 modified the milestones: v1.0.0, v1.0.0 stretch goals Nov 25, 2024
@greenc-FNAL greenc-FNAL mentioned this pull request Dec 17, 2024
Open
8 tasks
psakievich
psakievich reviewed Dec 19, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@psakievich psakievich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. @tgamblin is the schema forwarding sufficient for your request to rename the current environment schema? Seems like this is minimally disruptive.

@psakievich
Copy link
Copy Markdown
Contributor

psakievich commented Dec 19, 2024

add an "environment" section to SECTION_SCHEMAS in spack.config, and to spack.schema.merged.properties.

@fryeguy52 can you do this as well? Running spack config list I don't see this listed as a complete config yet.

@fryeguy52
Copy link
Copy Markdown
Contributor Author

fryeguy52 commented Jan 13, 2025

add an "environment" section to SECTION_SCHEMAS in spack.config, and to spack.schema.merged.properties.

@fryeguy52 can you do this as well? Running spack config list I don't see this listed as a complete config yet.

@psakievich I think I have what you requested in now. "env_vars" shows in config list. Do we want "environment" to show up as well? right now there is a new file env_vars.py that does import spack.scheme.environment

$ spack config list
compilers concretizer definitions env_vars view develop mirrors repos packages modules config upstreams bootstrap ci cdash

@psakievich
Copy link
Copy Markdown
Contributor

psakievich commented Jan 13, 2025 •
edited
Loading

I think this is good. I imagine env_var and environment are now equivalent and should just be an alias. I don't know it the environment scheme needs to be adjusted as part of this PR or not. @tgamblin are you happy with this state?

psakievich
psakievich previously approved these changes Jan 13, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@psakievich psakievich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this as is. I'd like to confirm we hit all of @tgamblin's requests before merging.

tgamblin
tgamblin requested changes Jan 15, 2025
View reviewed changes
fryeguy52
fryeguy52 commented Jan 15, 2025
View reviewed changes
tgamblin
tgamblin reviewed Jan 16, 2025
View reviewed changes
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Jan 27, 2025 •
edited
Loading

@fryeguy52: thanks! Can you submit a follow-on PR to update the docs with this new section? We had also talked about allowing env_vars: where environment: is currently allowed, and eventually deprecating environment:. I think we should at least allow both and deprecate environment: for now.

Existing docs are here: https://spack.readthedocs.io/en/latest/module_file_support.html#setting-environment-variables-for-selected-packages-in-config

@fryeguy52 fryeguy52 mentioned this pull request Feb 4, 2025
Open
6 tasks
@greenc-FNAL greenc-FNAL mentioned this pull request Feb 25, 2025
Merged
@haampie
Copy link
Copy Markdown
Member

haampie commented Feb 28, 2025

@fryeguy52 can you submit that follow-up PR with documentation? People are looking for it.

@psakievich psakievich mentioned this pull request Mar 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

@psakievich psakievich psakievich approved these changes

@RikkiButler20 RikkiButler20 Awaiting requested review from RikkiButler20

Assignees

No one assigned

Labels

commands core PR affects Spack core functionality environments shell-support snl-core-team Issue for SNL Spack developers tests General test capability(ies)

Projects

None yet

Milestone

v1.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@fryeguy52 @psakievich @tgamblin @haampie @becker33 @wdconinc