-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
Add '--allowed-rpath-prefixes' option to '--shrink-rpath' … #98
Conversation
We're going to need this logic in another place, so make a function of this.
Fixes NixOS#97. In essence, the problem is that some packages in Nixpkgs have RPATHs pointing to both $NIX_BUILD_TOP and $out, e.g.: /tmp/nix-build-openldap-2.4.44.drv-0/openldap-2.4.44/libraries/libldap_r/.libs /tmp/nix-build-openldap-2.4.44.drv-0/openldap-2.4.44/libraries/liblber/.libs /nix/store/bfkmdxmv3a3f0g3d2q8jkdz2wam93c5z-openldap-2.4.44/lib /nix/store/bfkmdxmv3a3f0g3d2q8jkdz2wam93c5z-openldap-2.4.44/lib64 Currently, running `patchelf --shrink-rpath` does the wrong thing by keeping the /tmp/ paths and deleting the /nix/store ones. Now we can fix the problem by using patchelf --shrink-rpath --allowed-rpath-prefixes $NIX_STORE_DIR in the Nixpkgs fixupPhase instead.
Nice, this looks like it could prevent some of the issues we had in the past. cc @edolstra |
} | ||
|
||
static bool hasAllowedPrefix(const string & s, const vector<string> & allowedPrefixes){ | ||
for (vector<string>::const_iterator it = allowedPrefixes.begin(); it != allowedPrefixes.end(); ++it) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
=> for (auto & it : allowedPrefixes)
.
NixOS/patchelf#98 (cherry picked from commit 14b833e)
Is there any reason this was never used in the patchelf shell-hook? It sounds like it was implemented for that purpose. Edit: When I tried to use it ( |
Not sure. I know we do scan the output for references to the build path, but that's not quite the same.
Try using patchelf built from this repo or So maybe it was never used because no new release of patchelf was made? |
Yeah I think this PR was intended to fix those issues detected by the scanning (or at least some of them).
Oh that makes sense, I just kinda assumed it was already in the release because the PR is so long ago. I guess patchelf is "feature complete" enough not to warrant regular releases. Maybe cutting a new patchelf release would be a good idea?
Makes sense. |
So perhaps we can apply this in nixpkgs after all those years? |
Fixes #97. In essence, the problem is that some packages in Nixpkgs have
RPATHs pointing to both
$NIX_BUILD_TOP
and$out
, e.g.:Currently, running
patchelf --shrink-rpath
does the wrong thing bykeeping the /tmp/ paths and deleting the /nix/store ones. Now we can fix
the problem by using
in the Nixpkgs
fixupPhase
instead.I haven't yet tested the Nixpkgs part and whether this really fixes it, but hopefully will soon.