Skip to content

binutils: detect the "gold" and "headers" variants#40214

Merged
alalazo merged 2 commits intospack:developfrom
bernhardkaindl:binutils-external-find-no-headers
Jun 8, 2024
Merged

binutils: detect the "gold" and "headers" variants#40214
alalazo merged 2 commits intospack:developfrom
bernhardkaindl:binutils-external-find-no-headers

Conversation

@bernhardkaindl
Copy link
Copy Markdown
Contributor

@bernhardkaindl bernhardkaindl commented Sep 26, 2023 •
edited by alalazo
Loading

Fixes #29350:

Changes:

Add initial def determine_variants():

  • llvm needs binutils+headers for plugin-api.h which most distros don't install
    and therefore make spack external find binutils search for the header and set ~headers/+headers

  • Also check for installed gold linker and set +gold/~gold

@spackbot-app spackbot-app bot requested a review from alalazo September 26, 2023 20:10
@bernhardkaindl bernhardkaindl changed the title binutils: Make external find set ~headers to fix llvm build binutils: Make external find set ~headers to fix build of llvm Sep 26, 2023
@bernhardkaindl bernhardkaindl changed the title binutils: Make external find set ~headers to fix build of llvm binutils: Make external find set ~headers to fix building llvm Sep 26, 2023
@bernhardkaindl
binutils: Make external find set ~headers to fix building llvm
828fe66 
Fixes #29350:

- binutils: Make spack external find binutils set ~headers to fix build of llvm

Changes:

Add initial def determine_variants():

- llvm needs binutils+headers for plugin-api.h which most distros don't install
  and therefore make spack external find binutils detect binutils~headers.

  This forces spack to build binutils for llvm when binutils have been
  imported from the system. This fixes the build of llvm in #29350.

- Also check for installed gold linker and set +gold/~gold
@bernhardkaindl bernhardkaindl force-pushed the binutils-external-find-no-headers branch from 929f8c7 to 828fe66 Compare September 26, 2023 20:16
@naromero77
Copy link
Copy Markdown
Contributor

naromero77 commented Dec 21, 2023

I have run into this same issue with LLVM. I don't understand the patch though and will wait for @alalazo to provide a proper review.

This was referenced Mar 21, 2024
Closed
Closed
@bonachea
Copy link
Copy Markdown
Contributor

bonachea commented Jun 5, 2024 •
edited
Loading

Ping @alalazo is this ever going to be fixed?

binutils external detection remains broken in spack:develop, which breaks LLVM and other packages on important systems (e.g. HPE Cray EX, where the binutils provided by the SLES15 distro is missing the plugin-api.h header, and possibly other things).

Currently the external detection for binutils effectively lies to Spack about what variants are installed, without verifying anything, leading to breakage of packages who need specific binutils variants. We need binutils external detection to either be fixed to correctly detect which variants are actually installed, or otherwise disable the external detection in binutils entirely to make it stop lying about what's available.

CC: @arezaii

@bonachea bonachea mentioned this pull request Jun 5, 2024
Closed
4 tasks
alalazo
alalazo requested changes Jun 6, 2024
View reviewed changes
@alalazo alalazo changed the title binutils: Make external find set ~headers to fix building llvm binutils: detect the "gold" and "headers" variants Jun 7, 2024
@bonachea bonachea self-requested a review June 7, 2024 20:55
bonachea
bonachea approved these changes Jun 7, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@bonachea bonachea left a comment

Choose a reason for hiding this comment

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

@alalazo wrote:

...
I installed the sprinkler myself, in 8e1ac7c Hope nobody minds.

If you confirm this works I'll merge the PR.

Fix confirmed on NERSC Perlmutter:

perlmutter$ spack external find binutils
==> The following specs have been detected on this system and added to <redacted>/packages.yaml
[email protected]  [email protected]

perlmutter$ grep -A6 binutils $SPACK_USER_CONFIG_PATH/packages.yaml 
    externals:
    - spec: [email protected]~gold~headers
      prefix: /opt/cray/pe/cce/17.0.0/binutils/x86_64/x86_64-pc-linux-gnu
    - spec: [email protected]~gold~headers
      prefix: /usr

Thanks for the quick turnaround!

@alalazo alalazo enabled auto-merge (squash) June 7, 2024 21:00
@alalazo alalazo self-assigned this Jun 7, 2024
@alalazo alalazo merged commit 528c1ed into spack:develop Jun 8, 2024
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
@bernhardkaindl @alalazo @teaguesterling
binutils: detect the "gold" and "headers" variants (spack#40214)
61a5cf8 
Co-authored-by: Massimiliano Culpo <[email protected]>
hariharan-devarajan pushed a commit to hariharan-devarajan/spack that referenced this pull request Jul 10, 2024
@bernhardkaindl @alalazo @hariharan-devarajan
binutils: detect the "gold" and "headers" variants (spack#40214)
2bdac4f 
Co-authored-by: Massimiliano Culpo <[email protected]>
@greenc-FNAL greenc-FNAL mentioned this pull request Jul 30, 2024
Merged
FrederickDeny pushed a commit to FrederickDeny/spack that referenced this pull request Aug 26, 2024
@bernhardkaindl @alalazo @FrederickDeny
binutils: detect the "gold" and "headers" variants (spack#40214)
bc8761b 
Co-authored-by: Massimiliano Culpo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalazo alalazo alalazo approved these changes

+1 more reviewer

@bonachea bonachea bonachea approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alalazo alalazo

Labels

update-package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

llvm+gold with external binutils build failure because of missing plugin-api.h header

4 participants

@bernhardkaindl @naromero77 @bonachea @alalazo