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

stdenv: move --enable-deterministic-archives flag into GNU wrapper #138289

Merged
merged 2 commits into from
Oct 7, 2021

Conversation

sternenseemann
Copy link
Member

Motivation for this change
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 via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 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.

@Ericson2314
Copy link
Member

TODO can we get rid of this NIX_LDFLAGS hack altogether?

Where is that?

We can at the moment, but that whole system is being reworked in #132343, so I'd rather not touch anything while that's ongoing.

I am not sure when I'll finish that (as much as I like it!) so without knowing the details, if there is some hack to get rid of now easily, by all means do so.

@sternenseemann
Copy link
Member Author

Where is that?

76c9a926e06afd07b2a263504f60e64b0675fea7 which Alyssa commented on (a fact that easily gets lost in the github UI…)

@Ericson2314
Copy link
Member

Oh I see. Yeah fine for now for sure.

@sternenseemann sternenseemann linked an issue Sep 17, 2021 that may be closed by this pull request
@sternenseemann sternenseemann force-pushed the fix-non-gnu-strip branch 3 times, most recently from 8f42223 to 5220c58 Compare September 18, 2021 12:16
@sternenseemann sternenseemann marked this pull request as ready for review September 18, 2021 12:18
@sternenseemann
Copy link
Member Author

I think this is good enough for now. Will need some additional testing though, as it is a full stdenv rebuild on all platforms now.

* Only try to link libgcc if GCC is actually used

* Link libgcc depending on libc (glibc vs. musl), rather than
  Linux/musl vs. Linux/!musl

This is a step towards fixing pkgsLLVM.python3.
`--enable-deterministic-archives` is a GNU specific strip flag and
causes other strip implementations (for example LLVM's, see NixOS#138013)
to fail. Since strip failures are ignored, this means that stripping
doesn't work at all in certain situation (causing unnecessary
dependencies etc.).

To fix this, no longer pass `--enable-deterministic-archives`
unconditionally, but instead add it in a GNU binutils specific strip
wrapper only.

`commonStripFlags` was only used for this flag, so we can remove
it altogether.

Future work could be to make a generic strip wrapper, with support for
nix-support/strip-flags-{before,after} and NIX_STRIP_FLAGS_{BEFORE,AFTER}.
This possibly overkill and unnecessary though -- also with the
additional challenge of incorporating the darwin strip wrapper somehow.
@sternenseemann
Copy link
Member Author

Checked stripping with stdenv, clangStdenv and pkgsLLVM.stdenv (pkgsLLVM.python3 works as well!). So it seems all is good.

@sternenseemann sternenseemann merged commit 5d0972c into NixOS:staging Oct 7, 2021
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Oct 7, 2021
PR NixOS#138289 stopped --enable-deterministic-archives being passed by
setup.sh, so we need to make sure we wrap strip and pass this flag if it
is supported (which is the case for GNU binutils).

This mirrors the behavior of the gnu strip wrapper for aarch64-darwin.
@sternenseemann sternenseemann deleted the fix-non-gnu-strip branch January 7, 2023 18:19
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.

clangStdenv does not strip on Linux
3 participants