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
audit: fix static linking #109112
audit: fix static linking #109112
Conversation
#pragma weak audit_strsplit_r | ||
EOF | ||
'' + stdenv.lib.optionalString stdenv.targetPlatform.isStatic '' | ||
export LDFLAGS=-Wl,--whole-archive |
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.
I think we should use NIX_LDFLAGS instead.
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.
Why? As I was told in 100906 by @nh2, NIX_LDFLAGS should be used as last resort.
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.
@nh2 Does the same apply to LDFLAGS?
Does this overwrite LDFLAGS here?
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.
Does the same apply to LDFLAGS?
Sorry, I saw this only now.
The same does not apply to LDFLAGS
; setting LDFLAGS
is fine because it's picked up by the build system (autoconf
here, whose main configuration method is via these environment variables).
Only the NIX_*FLAGS
are sneaked past the build systems directly to the linker.
/rebase-staging |
I rebased onto staging due to the number of rebuilds |
patches = [ ./patches/weak-symbols.patch ] ++ | ||
stdenv.lib.optional stdenv.hostPlatform.isMusl [ |
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.
patches = [ ./patches/weak-symbols.patch ] ++ | |
stdenv.lib.optional stdenv.hostPlatform.isMusl [ | |
patches = [ ./patches/weak-symbols.patch ] | |
++ stdenv.lib.optional stdenv.hostPlatform.isMusl [ |
# --whole-archive linker flag is required to be sure that linker | ||
# correctly chooses strong version of symbol regardless of order of | ||
# object files at command line. | ||
+ stdenv.lib.optionalString stdenv.targetPlatform.isStatic '' |
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.
This should have been hostPlatform
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.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)