Skip to content

Fix meta-modules preferred compiler bug - #2084

Open
eap wants to merge 5 commits into
JCSDA:developfrom
eap:bugfix/preferred-compiler
Open

Fix meta-modules preferred compiler bug#2084
eap wants to merge 5 commits into
JCSDA:developfrom
eap:bugfix/preferred-compiler

Conversation

@eap

@eap eap commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

The motivation for this PR comes from the the function custom_sort_key which was broken, leading to some bugs when generating meta-modules with compilers that were dependent on the preferred compiler.

When sorting, Spec.__contains__, checks the dependency tree of a spec, since llvm was built with gcc it got the same sort order as gcc in the list.

Other fixes

The for compiler in compilers loop had a whole bunch of meta-module logic that only applied to the preferred compiler, this was gated with a "continue" clause. I just dropped that clause and moved everything into the function context. That saved some variable scope bookkeeping and (along with the sort fixes) guarantees that it runs every time.

@eap
eap requested a review from climbfuji August 13, 2026 21:08
@eap eap changed the title Fix preferred compiler bug Fix meta-modules preferred compiler bug Aug 13, 2026
@eap
eap marked this pull request as ready for review August 13, 2026 21:12
Comment thread spack-ext/lib/jcsda-emc/spack-stack/stack/meta_modules.py
@climbfuji

Copy link
Copy Markdown
Collaborator

Thanks for this cleanup. I'll need to give this a thorough test on a few systems.

@eap eap mentioned this pull request Aug 14, 2026
19 tasks
@climbfuji

Copy link
Copy Markdown
Collaborator

@eap I am finally testing this PR - stay tuned

@eap eap left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

review notes

Comment thread spack-ext/lib/jcsda-emc/spack-stack/stack/meta_modules.py
Comment thread spack-ext/lib/jcsda-emc/spack-stack/stack/meta_modules.py
@climbfuji

Copy link
Copy Markdown
Collaborator

I think this PR is good the way it's now. I tested it on 2/3 platforms, just finishing the third.

@climbfuji climbfuji left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Works on the three sites I tested (HPCs / devbox, lmod / tcl, intel / gcc / llvm)

Comment thread spack-ext/lib/jcsda-emc/spack-stack/stack/meta_modules.py
@climbfuji

Copy link
Copy Markdown
Collaborator

@AlexanderHrabski-NOAA @AlexanderRichert-NOAA I'd like to merge this soon, if possible. Any desire to review and/or test? Thanks!

@rickgrubin-noaa rickgrubin-noaa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Successfully tested on two EPIC hosts.

@climbfuji

Copy link
Copy Markdown
Collaborator

Successfully tested on two EPIC hosts.

Thanks, Rick for testing! Appreciate it.

@AlexanderHrabski-NOAA

Copy link
Copy Markdown
Collaborator

@climbfuji I'm not too familiar with the meta-module piece of spack-stack (though I am learning), and I'll need to shift my attention elsewhere for a couple days. I can try to find time on Friday to review this if it's still open!

@climbfuji

Copy link
Copy Markdown
Collaborator

@climbfuji I'm not too familiar with the meta-module piece of spack-stack (though I am learning), and I'll need to shift my attention elsewhere for a couple days. I can try to find time on Friday to review this if it's still open!

We can wait until Friday, no problem. We have plenty of other PRs in the pipeline :-)

Co-authored-by: Dom Heinzeller <dom.heinzeller@icloud.com>
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.

4 participants