Skip to content

stdenv/darwin: set NIX_COREFOUNDATION_RPATH via hook#230870

Closed
eliasnaur wants to merge 1 commit intoNixOS:stagingfrom
eliasnaur:nix-corefoundation-rpath-hook
Closed

stdenv/darwin: set NIX_COREFOUNDATION_RPATH via hook#230870
eliasnaur wants to merge 1 commit intoNixOS:stagingfrom
eliasnaur:nix-corefoundation-rpath-hook

Conversation

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented May 9, 2023

This will unset the variable in the cross stdenv where CF is taken out of extraBuildInputs. This is useful for cross compilation to linux where setting RPATH on glibc will break it in runtime.

The hook needs to be turned off during the bootstrapping to prevent unwanted references between the stages.

This is a updated version of #111385 authored by @veprbl.

CC @LnL7 who reviewed the original PR.

Closes #111385

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

This will unset the variable in the cross stdenv where CF is taken out
of extraBuildInputs. This is useful for cross compilation to linux
where setting RPATH on glibc will break it in runtime.

The hook needs to be turned off during the bootstrapping to prevent
unwanted references between the stages.
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label May 9, 2023
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label May 9, 2023
@eliasnaur eliasnaur marked this pull request as draft May 9, 2023 13:59
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 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 May 9, 2023
@eliasnaur eliasnaur marked this pull request as ready for review May 9, 2023 19:25
@reckenrode
Copy link
Contributor

I’m working on a bump of the Darwin stdenv to LLVM 15 in #229786. As part of that, I’m refactoring the build process and cleaning things up a bit. I made setting NIX_COREFOUNDATION_RPATH conditional based on the target platform. I am able to successfully nix build .#pkgsCross.gnu64.hello and run it on a Linux NixOS machine. Does that cover your use case?

Note that I’m not trying to step on the toes of any other PRs, but if the stdenv update results in other issues being resolved, I’d like to identify and address those.

@eliasnaur
Copy link
Contributor Author

eliasnaur commented May 10, 2023

I’m working on a bump of the Darwin stdenv to LLVM 15 in #229786. As part of that, I’m refactoring the build process and cleaning things up a bit. I made setting NIX_COREFOUNDATION_RPATH conditional based on the target platform. I am able to successfully nix build .#pkgsCross.gnu64.hello and run it on a Linux NixOS machine. Does that cover your use case?

Great work, thank you!

My particular use-case is to build bit-for-bit reproducible disk images for raspberry pi. NIX_COREFOUNDATION_RPATH lengthens binary rpaths so that not even pathelf can salvage reproducibility.

I wrote a test for a similar problem with lib64 directories that can be used to verify whether this issue is fixed:

$ export MY_NIXPKGS_DIR=...
# before this PR
$ nix build --rebuild --override-input nixpkgs "$MY_NIXPKGS_DIR" -L github:eliasnaur/nix-lib64-rpath 2>&1 |rg NIX_COREFOUNDATION_RPATH
demonstrate-lib64-rpath-armv6l-unknown-linux-musleabihf> NIX_COREFOUNDATION_RPATH: /nix/store/6g3l83hj6j4snkk3y3m2i9yv1i93p914-apple-framework-CoreFoundation-11.0.0/Library/Frameworks
# after this PR
$ nix build --rebuild --override-input nixpkgs "$MY_NIXPKGS_DIR" -L github:eliasnaur/nix-lib64-rpath 2>&1 |rg NIX_COREFOUNDATION_RPATH
demonstrate-lib64-rpath-armv6l-unknown-linux-musleabihf> NIX_COREFOUNDATION_RPATH: 

Let me know, and I'll close this PR.

Note that I’m not trying to step on the toes of any other PRs, but if the stdenv update results in other issues being resolved, I’d like to identify and address those.

This PR is not even mine, and I don't understand the stdenv machinery well. I'm just trying to obtain reproducibility, and if you're doing the hard work, then great!

@reckenrode
Copy link
Contributor

Thanks for the test. That really helped. The approach I was taking didn’t work. I ended up adding a hook in final extraBuildInputs that gets dropped in a cross build. I’m building x86_64-darwin and aarch64-darwin now to confirm the hook works as expected and also plays nicely with the impure CoreFoundation’s own hook on x86_64-darwin.

@eliasnaur
Copy link
Contributor Author

Thanks for the test. That really helped. The approach I was taking didn’t work. I ended up adding a hook in final extraBuildInputs that gets dropped in a cross build. I’m building x86_64-darwin and aarch64-darwin now to confirm the hook works as expected and also plays nicely with the impure CoreFoundation’s own hook on x86_64-darwin.

What a coincidence; I didn't want to burden you with extra work so I just now completed a build with your PR. I reached the same conclusions as you, that the targetPlatform approach didn't work. Thanks for looking into this.

@zhaofengli
Copy link
Member

This is probably a better solution than manual hacks like #196564.

@veprbl
Copy link
Member

veprbl commented May 18, 2023

This is probably a better solution than manual hacks like #196564.

Yes, we might want to revert things like that as a part of PR with a global fix.

reckenrode added a commit to reckenrode/nixpkgs that referenced this pull request May 26, 2023
Closes NixOS#230870. Thanks to @eliasnaur for the test case and for rasining
awareness and to @veprbl for the work done on NixOS#111385.

This takes a slightly different approach from those two PRs. The hook is
set unconditionally. The stdenv bootstrap doesn’t really need CF at all,
so setting the hook is harmless. This simplifies things.
reckenrode added a commit to reckenrode/nixpkgs that referenced this pull request May 29, 2023
Closes NixOS#230870. Thanks to @eliasnaur for the test case and for rasining
awareness and to @veprbl for the work done on NixOS#111385.

This takes a slightly different approach from those two PRs. The hook is
set unconditionally. The stdenv bootstrap doesn’t really need CF at all,
so setting the hook is harmless. This simplifies things.
reckenrode added a commit to reckenrode/nixpkgs that referenced this pull request May 31, 2023
Closes NixOS#230870. Thanks to @eliasnaur for the test case and for rasining
awareness and to @veprbl for the work done on NixOS#111385.

This takes a slightly different approach from those two PRs. The hook is
set unconditionally. The stdenv bootstrap doesn’t really need CF at all,
so setting the hook is harmless. This simplifies things.
@eliasnaur
Copy link
Contributor Author

Superseded by #234861.

@eliasnaur eliasnaur closed this Jun 11, 2023
@eliasnaur eliasnaur deleted the nix-corefoundation-rpath-hook branch June 11, 2023 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 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.

4 participants

Comments