External package detection for compilers by becker33 · Pull Request #43464 · spack/spack

becker33 · 2024-04-03T00:59:51Z

This PR creates shared infrastructure for compiler packages to implement the detailed search capabilities from the spack compiler find command for the spack external find command.

This PR updates the aocc, apple-clang, arm, gcc, and nag packages to use the new functionality. PRs for other compilers to come.

With this PR, spack compiler find can be replaced with spack external find --tag compiler as long as only the five covered compilers are relevant.

Once the rest of the packages are updated, future work will replace the spack compiler find command with an alias to spack external find --tag compiler.

spackbot-app · 2024-04-03T01:02:12Z

@ravil-mobile can you review this PR?

This PR modifies the following package(s), for which you are listed as a maintainer:

aocc
apple-clang
dpcpp
gcc
nag

var/spack/repos/builtin/packages/apple-clang/detection_test.yaml

var/spack/repos/builtin/packages/gcc/package.py

alalazo

I checked the mixin and left a few comments. Overall I don't understand the technical reason why we can't split this in multiple PRs:

One to push common logic into the mixin
Others to add detection methods to other compilers

Since this PR both refactors existing code and changes the logic, it's more difficult to spot issues by just reviewing code.

That said, a very basic test seems to have regressed:

$ spack external find gcc

finds gcc@9 in this PR on my laptop, and gcc@9.4.0 on develop.

The newly added compilers:

aocc
arm
nag

all miss a detection_test.yaml file to perform detection tests.

lib/spack/spack/build_systems/compiler.py

var/spack/repos/builtin/packages/xlf/package.py

var/spack/repos/builtin/packages/xl/detection_test.yaml

var/spack/repos/builtin/packages/xl/package.py

alalazo

I checked why this is reporting gcc@9. It's because we changed the flag being used from -dumpfullversion to -dumpversion:

$ gcc -dumpversion
9
$ gcc -dumpfullversion
9.4.0

It also seems we skip checking if:

$ gcc --version
gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

contains the Apple string to discard whether gcc is apple-clang in disguise?

Can we please revert that?

I still think this would be much easier to review if split in smaller PRs doing a single change, rather than a big PR that overhauls code and regexes and flags together.

I'll submit a separate change to the detection test that makes this fail, so we also test which flag is supposed to be used.

var/spack/repos/builtin/packages/gcc/package.py

lib/spack/spack/test/compilers/detection.py

var/spack/repos/builtin/packages/arm/package.py

Instead, leave either a debug message or a loud warning, depending on the entry being missing. Delete a unit test that was detecting `gcc-8` as being part of a `gcc@7.7.7` spec, with previous auto-detection.

lib/spack/spack/test/cmd/compiler.py

alalazo · 2024-04-30T10:22:32Z

Relevant changes that we should note:

xlf and xlc have been merged in a unique xl package. That prevent us from being able to detect old versions of the compiler suite, where xlf was 2 major version ahead of xlc.
We changed a regex, see External package detection for compilers #43464 (comment) I'm leaving a trace here in case we come back to this PR later
Auto-detection of compiler prefixes, when converting a packages.yaml entry to a compilers.yaml entry, have been removed. Missing attributes now would print a warning or a debug message to users.

alalazo · 2024-04-30T10:23:15Z

@becker33 Let me know if there's anything in the latest commits you don't agree with.

Review points have been addressed

amd-toolchain-support · 2024-05-01T07:58:40Z

Testing for AOCC support I have noticed that spack external find {aocc,gcc,...} will detect and re-add Spack provided compilers if they are loaded at the point of searching. Is this intended behaviour?

$ spack install aocc@4.2+license-agreed
...
$ spack load aocc
$ spack external find aocc
==> The following specs have been detected on this system and added to /home/$USER/repos/spack/etc/spack/packages.yaml
aocc@4.2.0

Otherwise AOCC detection is behaving as I would expect.

alalazo · 2024-05-01T08:08:08Z

Is this intended behaviour?

Yes, this is how external detection for packages works at the moment. It's just based on PATH inspection.

alalazo · 2024-05-02T21:05:38Z

(To be solved in a following PR) There is still one thing that is missing for:

spack external find -t compiler

to be a full substitute of spack compiler find, i.e. we need some way to have an equivalent of:

spack compiler find --mixed-toolchain

See #40902

This creates shared infrastructure for compiler packages to implement the detailed search capabilities from the `spack compiler find` command for the `spack external find` command. After this commit, `spack compiler find` can be replaced with `spack external find --tag compiler`, with the exception of mixed toolchains.

spackbot-app bot added build-systems commands compilers core PR affects Spack core functionality documentation Improvements or additions to documentation environments new-package new-version tests General test capability(ies) update-package labels Apr 3, 2024

spackbot-app bot requested review from alalazo, amd-toolchain-support, michaelkuhn and skosukhin April 3, 2024 01:02

becker33 force-pushed the features/compiler-find-external-based-on-PR branch 5 times, most recently from 231ef1f to d9d6b24 Compare April 4, 2024 02:58

alalazo requested changes Apr 5, 2024

View reviewed changes

var/spack/repos/builtin/packages/apple-clang/detection_test.yaml Show resolved Hide resolved

becker33 force-pushed the features/compiler-find-external-based-on-PR branch from 944ba46 to ab29485 Compare April 5, 2024 14:38

alalazo mentioned this pull request Apr 11, 2024

Allow compilers to be configured in packages.yaml #42016

Merged

3 tasks

alalazo reviewed Apr 11, 2024

View reviewed changes

var/spack/repos/builtin/packages/gcc/package.py Outdated Show resolved Hide resolved

alalazo requested changes Apr 11, 2024

View reviewed changes

spackbot-app bot added the intel label Apr 11, 2024

alalazo reviewed Apr 12, 2024

View reviewed changes

alalazo previously requested changes Apr 12, 2024

View reviewed changes

var/spack/repos/builtin/packages/gcc/package.py Outdated Show resolved Hide resolved

alalazo reviewed Apr 12, 2024

View reviewed changes

lib/spack/spack/test/compilers/detection.py Outdated Show resolved Hide resolved

alalazo reviewed Apr 12, 2024

View reviewed changes

var/spack/repos/builtin/packages/arm/package.py Outdated Show resolved Hide resolved

alalazo added 4 commits April 29, 2024 14:19

pgi: specify platforms for external detection

cfec7c4

oneapi: specify platforms for external detection

b7570c7

intel: specify platforms for external detection

2f45ece

llvm-amdgpu: specify platforms for external detection

8400bb3

alalazo force-pushed the features/compiler-find-external-based-on-PR branch from 80291c0 to 8400bb3 Compare April 29, 2024 12:20

alalazo added 2 commits April 29, 2024 18:21

Improve debug message for failing version detection

a55b698

Restore compiler unit tests to previous state

46db4f8

alalazo added this to the v0.22.0 milestone Apr 29, 2024

tgamblin assigned becker33 Apr 29, 2024

alalazo added 4 commits April 29, 2024 22:19

Add checks for extra attributes

5d7bad5

Format YAML detection homogeneously

47ce7f5

Update cce detection test

26ed93e

Do not auto-detect missing entries from packages.yaml

805f787

Instead, leave either a debug message or a loud warning, depending on the entry being missing. Delete a unit test that was detecting `gcc-8` as being part of a `gcc@7.7.7` spec, with previous auto-detection.

alalazo reviewed Apr 30, 2024

View reviewed changes

lib/spack/spack/test/cmd/compiler.py Show resolved Hide resolved

alalazo approved these changes Apr 30, 2024

View reviewed changes

haampie approved these changes May 6, 2024

View reviewed changes

alalazo merged commit 1f31c33 into develop May 6, 2024

alalazo deleted the features/compiler-find-external-based-on-PR branch May 6, 2024 08:33

greenc-FNAL mentioned this pull request May 14, 2024

Merge from fnal/v0.22.0-fermi FNALssi/spack#352

Merged

This was referenced Jun 5, 2024

Move compiler detection logic to packages #17525

Closed

pgi: added external detection capabilities #18148

Closed

alalazo mentioned this pull request Jul 4, 2024

spack compiler find: write into packages.yaml #44419

Closed

Conversation

becker33 commented Apr 3, 2024

Uh oh!

spackbot-app bot commented Apr 3, 2024

Uh oh!

Uh oh!

Uh oh!

alalazo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo commented Apr 30, 2024

Uh oh!

alalazo commented Apr 30, 2024

Uh oh!

amd-toolchain-support commented May 1, 2024

Uh oh!

alalazo commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalazo commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alalazo left a comment •

edited

Loading

alalazo left a comment •

edited

Loading

alalazo commented May 1, 2024 •

edited

Loading

alalazo commented May 2, 2024 •

edited

Loading