Skip to content

alacritty: --add-rpath instead of --set-rpath#240773

Merged
NickCao merged 1 commit intomasterfrom
unknown repository
Jul 1, 2023
Merged

alacritty: --add-rpath instead of --set-rpath#240773
NickCao merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 30, 2023

Description of changes

As reported by @blucoat in #219213 alacritty's RPATH is missing many of the libraries which it links to, including for example glibc.

The problem was diagnosed by @kchibisov as being caused by alacritty's use of --set-rpath (which completely replaces the rpath) instead of --add-rpath (which adds additional entries to the rpath):

#219213 (comment)

This commit implements @kchibisov's idea to change --set-rpath to --add-rpath:

#219213 (comment)

Closes #219213

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

As reported by @blucoat in
#219213 alacritty's RPATH is
missing many of the libraries which it links to, including for
example glibc.

The problem was diagnosed by @kchibisov as being caused by
alacritty's use of `--set-rpath` (which completely replaces the
rpath) instead of `--add-rpath` (which adds additional entries to
the rpath):

  #219213 (comment)

This commit implements @kchibisov's idea to change `--set-rpath` to
`--add-rpath`:

  #219213 (comment)

Closes #219213
@ghost ghost requested review from Br1ght0ne, Mic92 and trofi June 30, 2023 18:29
@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 30, 2023

It looks like we have 537 occurrences of --set-rpath in nixpkgs ☹️

Some of those are part of --set-rpath $(patchelf --print-rpath), which is okay, but most of them are probably causing problems similar to this one. Ugh.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jun 30, 2023

Maybe our fixupPhase should fail if any binaries in the outpath have unresolved DT_NEEDED entries in their ELF structures. There would be a mechanism to override this, of course, if needed.

@kchibisov
Copy link
Copy Markdown

Acked-by: Kirill Chibisov contact@kchibisov.com

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jun 30, 2023
@NickCao NickCao merged commit e3a2a8e into NixOS:master Jul 1, 2023
@ghost ghost deleted the pr/alacritty/rpath branch July 1, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Alacritty does not include glibc in rpath

3 participants