Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying compiler version for 'spack stack create env' #1294

Merged

Conversation

AlexanderRichert-NOAA
Copy link
Collaborator

Summary

This PR borrows the logic from #1213 that allows a compiler version to be specified in spack stack create env. It's a couple of tweaks to stack_env.py. This isn't strictly urgent, but I'm hoping to put this in the 1.8.0 release in anticipation of supporting installations by NCO (plus maybe others will find it useful; I know it will be useful for Acorn, and maybe also for NRL?).

Testing

Tested on Acorn and personal machine.

  • spack stack create env --compiler gcc correctly yields prefer: ['%gcc'] in spack.yaml
  • spack stack create env --compiler [email protected] correctly yields prefer: ['%[email protected]'] in spack.yaml
  • spack stack create env --compiler gcc with unspecified name correctly yields env directory with name empty.linux.default.gcc
  • spack stack create env --compiler [email protected] with unspecified name correctly yields env directory with name empty.linux.default.gcc-8.5.0

Applications affected

n/a

Systems affected

All

Dependencies

none

Issue(s) addressed

none

Checklist

  • This PR addresses one issue/problem/enhancement, or has a very good reason for not doing so.
  • These changes have been tested on the affected systems and applications.
  • All dependency PRs/issues have been resolved and this PR can be merged.

@climbfuji climbfuji changed the base branch from develop to release/1.8.0 September 11, 2024 18:32
Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

I am fine with merging this into the release branch. I haven't tested this myself, but I will when I move to the next platform to roll out 1.8.0.

Note that there's no need to go back to existing 1.8.0 installs and redo them. Thanks for making these changes.

if not definitions[i]["compilers"][j] == target_compiler:
definitions[i]["compilers"].pop(j)
j -= 1
definitions[i] = {"compilers": [target_compiler]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is so much easier; do you remember why we put that complicated logic in place originally (I don't)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No I don't remember... Maybe something to do with supporting multiple compilers in an environment..?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right. This is something we need to test before we merge. The Intel or oneAPI environments require some packages to be built with gcc, therefore the compilers line in the spack config must contain both compilers. See

We need to make sure that this still works (i.e. a spack module [tcl|lmod] refresh followed by spack stack setup-meta-modules still works as expected. I think it does, but better to check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. I think we would expect it to work though, on account of there being only one value for target_compiler (based on self.compiler)? Unless users can specify multiple...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@climbfuji climbfuji merged commit 4614f87 into JCSDA:release/1.8.0 Sep 11, 2024
8 checks passed
@climbfuji climbfuji mentioned this pull request Sep 12, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants