Skip to content
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

Respect NIX_DONT_SET_RPATH when .so appears in the command line #27831

Merged
merged 1 commit into from
Aug 1, 2017

Conversation

orivej
Copy link
Contributor

@orivej orivej commented Aug 1, 2017

Motivation for this change

Unified processing of command line arguments in ld-wrapper broke support for NIX_DONT_SET_RPATH and revealed that ld-wrapper adds the directory of its -plugin argument to runpath. This pull request fixes that. It treats dir/libname.so as -L dir -l name, because this is how ld.so interprets resulting binary: with dir in RUNPATH and the bare libname.so (without dir) in NEEDED, it looks for libname.so in each RUNPATH and chooses the first, even when the linker was invoked with an absolute path to .so.

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@orivej, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @fpletz and @vcunat to be potential reviewers.

@orivej orivej changed the base branch from master to staging August 1, 2017 08:54
@orivej orivej mentioned this pull request Aug 1, 2017
8 tasks
Unified processing of command line arguments in ld-wrapper broke support for
`NIX_DONT_SET_RPATH` and revealed that ld-wrapper adds the directory of its
`-plugin` argument to runpath. This pull request fixes that. It treats
`dir/libname.so` as `-L dir -l name`, because this is how `ld.so` interprets
resulting binary: with `dir` in `RUNPATH` and the bare `libname.so` (without
`dir`) in `NEEDED`, it looks for `libname.so` in each `RUNPATH` and chooses the
first, even when the linker was invoked with an absolute path to `.so`.
@globin globin self-assigned this Aug 1, 2017
@globin globin merged commit 0767478 into NixOS:staging Aug 1, 2017
@Ericson2314
Copy link
Member

Fixed the bug and made it smaller, nice!

orivej added a commit to orivej/nixpkgs that referenced this pull request Aug 7, 2017
This fixes a bug introduced in NixOS#27831: `for path in "$dir"/lib*.so` assumed that
all libs match `lib*.so`, but 0767478 started
adding libs that match `*.so` and `*.so.*`.
orivej added a commit to orivej/nixpkgs that referenced this pull request Aug 8, 2017
This fixes a bug introduced in NixOS#27831: `for path in "$dir"/lib*.so` assumed that
all libs match `lib*.so`, but 0767478 started
adding libs that match `*.so` and `*.so.*`.
globin pushed a commit that referenced this pull request Aug 8, 2017
This fixes a bug introduced in #27831: `for path in "$dir"/lib*.so` assumed that
all libs match `lib*.so`, but 0767478 started
adding libs that match `*.so` and `*.so.*`.
@orivej orivej deleted the dont-set-rpath branch August 26, 2017 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants