Skip to content

binary_distribution: stop relocating tarballs with relative rpaths#48488

Merged
alalazo merged 1 commit intodevelopfrom
hs/fix/drop-rel-buildcache-code
Jan 10, 2025
Merged

binary_distribution: stop relocating tarballs with relative rpaths#48488
alalazo merged 1 commit intodevelopfrom
hs/fix/drop-rel-buildcache-code

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Jan 9, 2025

Spack used to have a flag spack buildcache create --rel which was removed in v0.20.0.

This PR removes the install part of that feature, meaning that binary cache entries created with --rel will only work if they are installed to a store with identical projections. A warning is issued if users install an old tarball that was created with --rel.


The function of --rel was to take absolute rpaths like

$ readelf -d /path/to/store/linux-aarch64/gcc-10.3.0/pkg-1.2.3-abcdef/bin/example
... Library rpath: [/path/to/store/linux-aarch64/gcc-10.3.0/dep-1.2.3-abcdef/lib/libdep.so]

and make them relative to /path/to/store using $ORIGIN:

$ORIGIN/../../../dep-1.2.3-abcdef/lib/libdep.so

so that under identical projections, there is no need to patch rpaths. In practice however:

  1. projections change (and we are going to change it in etc/defaults/config.yaml), so upon install $ORIGIN/../../dep-1.2.3-abcdef/lib/libdep.so had to be expanded to an absolute path, and nothing was gained.
  2. use of $ORIGIN shrinks the rpath, meaning users had to run patchelf to create a whole new dynamic section, which sometimes breaks binaries if they were stripped.
  3. we continue to scan the entire binary for non-rpath strings to relocate, so there's no performance gain either
  4. there are bugs when dependencies are in upstreams during build or upon install.

In other words, it's unlikely that users benefited from and used --rel, and it's been removed long enough to stop supporting the install side of it.

So, this PR removes the code that did the dance of expanding $ORIGIN rpaths to absolute paths, and simply leaves these rpaths relative.

@spackbot-app spackbot-app bot added binary-packages core PR affects Spack core functionality tests General test capability(ies) labels Jan 9, 2025
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Jan 10, 2025

Can you add an entry here: #30634 ?

@alalazo alalazo self-assigned this Jan 10, 2025
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

This clean up should help maintaining the relocation module in the future 💯

@alalazo alalazo merged commit 93cd216 into develop Jan 10, 2025
@alalazo alalazo deleted the hs/fix/drop-rel-buildcache-code branch January 10, 2025 10:30
mtaillefumier pushed a commit to mtaillefumier/spack that referenced this pull request Jan 20, 2025
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binary-packages breaking-change core PR affects Spack core functionality tests General test capability(ies)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants