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.

Sign up for GitHub

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

Jump to bottom

NIX_LIB64_IN_SELF_RPATH leaks through to cross-builds #221350

Closed
eliasnaur opened this issue Mar 15, 2023 · 3 comments · Fixed by #223861
Closed

NIX_LIB64_IN_SELF_RPATH leaks through to cross-builds #221350

eliasnaur opened this issue Mar 15, 2023 · 3 comments · Fixed by #223861
Labels
0.kind: bug Something is broken

Comments

@eliasnaur
Copy link
Contributor

Describe the bug

lib64 directories are added to linker rpath flags for cross-compilation to platforms that don't need them.

Steps To Reproduce

Steps to reproduce the behavior:

  1. nix build -L github:eliasnaur/nix-lib64-rpath on a x86_64-linux system

output:

...
demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf> CC armv6l-unknown-linux-gnueabihf-gcc
demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf> NIX_LDFLAGS: -rpath /nix/store/lskdn4fvvjlz53dxkbkgb1x2dy4zkf7k-demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf/lib64 -rpath /nix/store/lskdn4fvvjlz53dxkbkgb1x2dy4zkf7k-demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf/lib
...

Note the lib64 directory added to NIX_LDFLAGS.

See full build log at https://github.com/eliasnaur/nix-lib64-rpath/actions/runs/4428890865/jobs/7768605092.

Running the same command on a aarch64-darwin configuration (with a aarch64-linux builder configured):

% nix build -L
...
demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf> CC armv6l-unknown-linux-gnueabihf-gcc
demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf> NIX_LDFLAGS: -rpath /nix/store/sw8ymyqpavw0v4hl01q0xb9c9wr50v9z-demonstrate-lib64-rpath-armv6l-unknown-linux-gnueabihf/lib
...

Note that NIX_LDFLAGS doesn't contain a lib64 directory.

Expected behavior

NIX_LDFLAGS should not contain lib64 directories when cross-compiling for non-x86_64-linux platforms.

Additional context

Adding lib64 to NIX_LDFLAGS is generally benign, except that I'm trying to achieve bit-for-bit reproducible builds. The extra lib64 entries end up in slightly larger ELF sections that can't be saved by careful use of patchelf.
@eliasnaur eliasnaur added the 0.kind: bug Something is broken label Mar 15, 2023
@eliasnaur
Copy link
Contributor Author

I believe this is the offending line:

nixpkgs/pkgs/stdenv/linux/default.nix

Line 118 in 4700c7a

${lib.optionalString (system == "x86_64-linux") "NIX_LIB64_IN_SELF_RPATH=1"}

but I'm not sure whether the x86_64-linux check should be moved elsewhere or whether the check should use something else than system.

@eliasnaur eliasnaur mentioned this issue Mar 30, 2023
Merged
12 tasks
eliasnaur added a commit to eliasnaur/nixpkgs that referenced this issue Mar 31, 2023
@eliasnaur
stdenv: remove the NIX_LIB*_IN_SELF_RPATH environment variables
88fd660 
The NIX_LIB64|32_IN_SELF_RPATH environment variables control whether
to add lib64 and lib32 to rpaths. However, they're set depending
on the build paltform, not the target platform and thus their values
are incorrect for for cross-builds.

On the other hand, setting them according to the build platform introduce
pointless differences in build outputs; see NixOS#221350 for details.

This change fixes the issues by boldly removes the NIX_LIB*_IN_SELF_RPATH
facility altogether, in the hope that it is no longer necessary. They
were introduced in 2009, long before nixpkgs had good support for
cross-builds.

Fixes NixOS#221350
@ghost
Copy link

ghost commented Apr 2, 2023

check should be moved elsewhere or whether the check should use something else than system.

The check should definitely be using stdenv.hostPlatform. But you would need to thread stdenv.hostPlatform.isx86_64 through as a new environment variable. As you point out in #223861, it would be better to just stop adding these -rpath flags if possible.

eliasnaur added a commit to eliasnaur/nixpkgs that referenced this issue Apr 8, 2023
@eliasnaur
stdenv: remove the NIX_LIB*_IN_SELF_RPATH environment variables
e210704 
The NIX_LIB64|32_IN_SELF_RPATH environment variables control whether
to add lib64 and lib32 to rpaths. However, they're set depending
on the build paltform, not the target platform and thus their values
are incorrect for for cross-builds.

On the other hand, setting them according to the build platform introduce
pointless differences in build outputs; see NixOS#221350 for details.

This change fixes the issues by boldly removes the NIX_LIB*_IN_SELF_RPATH
facility altogether, in the hope that it is no longer necessary. They
were introduced in 2009, long before nixpkgs had good support for
cross-builds.

Fixes NixOS#221350
@eliasnaur eliasnaur mentioned this issue Apr 13, 2023
Closed
12 tasks
eliasnaur added a commit to eliasnaur/nixpkgs that referenced this issue May 5, 2023
@eliasnaur
stdenv: remove the NIX_COREFOUNDATION_RPATH sledgehammer
4f103fb 
Similar to PR NixOS#223861 and the issue NixOS#221350, NIX_COREFOUNDATION_RPATH
is set unconditionally in the darwin stdenv. This is wrong for cross-
compiles and results in `rpaths` depending on the builder platform.

This change makes another bold move and deletes the
NIX_COREFOUNDATION_RPATH facility completely, in the hope it is no
longer needed, or that any breakage is manageable.

As an added bonus we can delete a darwin specific hack in glibc.
@abathur
Copy link
Member

abathur commented May 29, 2023

Closing since it sounds like the merge of #223861 resolved this. Happy to reopen if I'm mistaken.

@abathur abathur closed this as completed May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
0.kind: bug Something is broken
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

stdenv: don't set NIX_LIB*_IN_SELF_RPATH by default eliasnaur/nixpkgs
2 participants
@eliasnaur @abathur