Skip to content

adapt for compiler as dependencies in spack #215

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

Merged
merged 9 commits into from
Jun 24, 2025

Conversation

albestro
Copy link
Contributor

@albestro albestro commented Apr 2, 2025

Relevant changes are:

  • drop compilers.yaml in favour of packages.yaml
  • use variants to distinguish bootstrap compiler (aka bootstrap) from bootstrapped compiler (aka gcc);
  • use +profiled for bootstrapped compiler (it was left out from Some minor fixes and cleanups #212)

TODO:

Notes

Distinguishing compilers from various steps

Currently the tool build a bootstrapped gcc compiler explicitly via these steps:

  1. gcc@7 look for system compiler
  2. gcc@12 % gcc@7 build a bootstrap compiler using the system one
  3. gcc@12 % gcc@12 build a bootstrapped compiler using the bootstrapp one from previous step

Before [email protected] this was doable, because compilers were separate from packages. Indeed, we can simplify saying that % was looking into compilers.yaml. Unfortunately, this is not possible anymore since compilers have become packages, so % looks into packages.yaml.

Previously compilers information from one step to the next were passed via compilers.yaml, while now it is passed via packages.yaml. This turns out to be a problem when compilers in two steps are of the same version, e.g. step 2 and step3, where with spack compiler find we get something like

# packages.yaml
packages:
  gcc:
    external:
      spec: gcc@12
      ...

and when we do spack concretize gcc@12 we get it concretized to that one already specified as external, no matter what variants: we ask for.

The current workaround (see 5a0f1e2) is to:

  • make the external spec more specific (using yq)
  • make the requested spec more specific (variants are not enough)

@albestro albestro marked this pull request as draft April 2, 2025 13:09
Find compilers in `packages.yaml`
@@ -45,9 +45,9 @@ all:{% for env in environments %} {{ env }}/generated/build_cache{% endfor %}

# Create the compilers.yaml configuration for each environment
{% for env, config in environments.items() %}
{{ env }}_PREFIX = {% for C in config.compiler %} $$($(SPACK_HELPER) -e ../compilers/{{ C.toolchain }} find --format '{prefix}' {{ C.spec }}){% endfor %}
{{ env }}_PREFIX = {% for C in config.compiler %} $$($(SPACK_HELPER) -e ../compilers/{{ C.toolchain }} find --explicit --format '{prefix}' {{ C.spec }}){% endfor %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed a89c7e3 which adds --explicit to most spack finds. This is a(n incomplete) workaround for the following problem, and needs further changes.

With compilers as nodes, spack find gcc etc. for other compilers now finds more compilers than we wish for. Previously, since we specify compilers in a separate config file, compilers would not generally show up as packages with spack find. Only a compiler installed in the current environment would show up with spack find, so we could safely do spack find gcc and only find the gcc we want. With compilers as nodes, spack find gcc in an environment finds the external gcc (which has been defined in alps-cluster-config, converted from compilers.yaml to packages.yaml), the bootstrap compiler, and the actual gcc we've just installed for users.

spack find --explicit slightly restricts the compilers that are found by only listing compilers that have been explicitly installed, e.g. through an environment. This excludes externals.

What I was hoping to use here is --only-roots, but that seems to return no packages. It's not clear if this is a bug, but I'm trying to find out.


Longer term, it may even be preferable to get rid of the spack compiler find altogether in the various makefiles. It should in theory not be needed, since spack knows that gcc provides c, cxx, and fortran. I would probably leave this for a future cleanup though, as it seems like it's probably a bigger refactoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

This can also further be worked around by specifiying a version (that's hopefully unique enough) in the environments.yaml file, e.g. here: https://github.com/eth-cscs/alps-uenv/blob/79a0dc5a84758aaaef0216f723a21434265be6b3/recipes/prgenv-gnu/24.11/gh200/environments.yaml#L4. Some recipes already specify the version, but pre-compilers-as-deps it didn't make a difference to specify a version. It's obviously nicer if one doesn't have to specify the version, since it always has to be the same as what is specified in compilers.yaml.

Copy link
Contributor

Choose a reason for hiding this comment

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

Response on slack regarding --only-roots:

This is likely a very long term bug. I can look at fixing it, but not until next week

That might be a sufficiently quick timeline for this PR, or we try to find another solution (I'm not quite sure using --only-roots is anyway sufficient/necessary/a good solution).

@msimberg
Copy link
Contributor

Opened eth-cscs/alps-cluster-config#39 for removing compilers.yaml in alps-cluster-config.

@msimberg msimberg marked this pull request as ready for review June 19, 2025 08:37
@msimberg
Copy link
Contributor

Linter is happy now.

@@ -447,13 +447,19 @@ def view_impl(args):

with open(args.compilers, "r") as file:
data = yaml.safe_load(file)
compilers = [c["compiler"] for c in data["compilers"]]

compilers = []
Copy link
Member

Choose a reason for hiding this comment

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

Note: I might be going off on a tangent here.

If we now treat compilers as normal dependencies, would it be possible to have them included in views by Spack by simply adding the compiler (e.g. [email protected]) to the list of explicit specs provided in the generated environments/$envname/spack.yaml file?

The list of explicit specs in the generated spack.yaml is currnently the list of specs in environments.yaml plus the spec for cray-mpich if cray-mpich was enabled, which is inserted here:
https://github.com/eth-cscs/stackinator/blob/main/stackinator/recipe.py#L301

If we add gcc to the list of specs, then it will be automatically added without having to do anything in this script.

The possible downside of this is that nvhpc might inject a lot more binaries and other rubbish into the view than we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note: I might be going off on a tangent here.

Not too tangential, but can be left for a follow-up. It's definitely a relevant question.

If we now treat compilers as normal dependencies, would it be possible to have them included in views by Spack by simply adding the compiler (e.g. [email protected]) to the list of explicit specs provided in the generated environments/$envname/spack.yaml file?

I need a bit of time to think about the consequences, but fundamentally it sounds like an improvement and simplification, so potentially win-win.

This gives me another extension of the idea: Nowadays spack injects its own compiler through the compiler-wrapper package. That package depends on the compiler, and sets up cc etc. with link, include directories, rpaths etc. injected. Instead of just adding gcc to the list of specs, add compiler-wrapper?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe... I can't possibly speculate about compiler-wrapper without first trying it out.
If we use a mixed toolchain (gcc, g++, nvfortran), compiler-wrapper might be the best way to inject "just the bits we want".

Copy link
Contributor

Choose a reason for hiding this comment

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

I had a quick look at compiler-wrapper and it's possible that it won't help. It seems to only have setup_dependent_build_environment to set the compiler wrappers for packages depending on compiler-wrapper. It doesn't actually directly depend on the compiler itself.

Could possibly be hacked for our purpose, but I retract my idea and think it's probably not worth looking into it further right now.

gcc/packages.yaml: bootstrap/generated/env
$(SPACK) compiler find --scope=user $(call compiler_bin_dirs, $$($(SPACK_HELPER) -e ./bootstrap find --explicit --format '{prefix}' {{ compilers.gcc.requires }}))
# add ~bootstrap variant to the detected compiler (built by previous bootstrap step)
yq write -i $@ 'packages.gcc.externals[0].spec' "$$(yq read $@ 'packages.gcc.externals[0].spec') ~bootstrap"
Copy link
Contributor

Choose a reason for hiding this comment

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

This currently leads to this issue: #239. I think it's not a blocker for merging this PR, but should be fixed at some point.

@msimberg msimberg merged commit c91ea98 into eth-cscs:main Jun 24, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in stackinator recipe v2 work Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants