-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
llvmPackages.libcxx: use hasSharedLibraries instead of isStatic #362848
llvmPackages.libcxx: use hasSharedLibraries instead of isStatic #362848
Conversation
Huh — what's the difference between isStatic and hasSharedLibraries? When would one be true but not the other? |
@alyssais see this comment: nixpkgs/lib/systems/default.nix Lines 196 to 203 in a435bac
|
Ah, I see! So really we should basically always be checking hasSharedLibraries instead of isStatic? |
Not sure in what other cases it needs to be checked, but yes, I believe hasSharedLibraries should often be more correct. |
Lots of packages check it to e.g. tell CMake not to try to build shared libraries. |
I think a rebase would fix OfBorg. |
5a52d92
to
cefa795
Compare
This can go to master — it doesn't really rebuild anything. |
cefa795
to
d2b4629
Compare
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.
Would welcome PRs to update more isStatic checks to hasSharedLibraries. :)
Split from #352629, as this should be a relatively uncontroversial bugfix that also makes sense on its own.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.