treewide: Allow the linker to be chosen independently#122778
Merged
Ericson2314 merged 2 commits intoNixOS:stagingfrom May 14, 2021
Merged
treewide: Allow the linker to be chosen independently#122778Ericson2314 merged 2 commits intoNixOS:stagingfrom
Ericson2314 merged 2 commits intoNixOS:stagingfrom
Conversation
|
Result of 38 packages skipped due to time constraints:
3 packages built successfully:
|
Member
|
This breaks the firefox build on x86_64-linux. |
Member
Author
|
Yeah i found I think workarounds for everything but Firefox. Hmmm. |
168194c to
2c99ee7
Compare
Member
Author
|
Firefox now has a (somewhat ugly) workaround. I don't really like the override, but writing down every possible combination is also a bit much. That's when I wish we had a relational language / solver :D. |
Member
Author
|
@matthewbauer It's been a minute, but I'd love for you to way on this. I first mentioned doing something like this quite some time ago :). |
Add a comment with explanation, which I should have done all along.
ddde8a3 to
96f73d6
Compare
Member
Author
|
OK the fix for firefox is a lot less ugly now. |
lovesegfault
approved these changes
May 14, 2021
Member
lovesegfault
left a comment
There was a problem hiding this comment.
Left two small nits, but the changes look good overall to me. I suspect it may be worth it to do a separate hydra run of this just to be sure, even if this is going into staging and not master. Up to the author.
sternenseemann
approved these changes
May 14, 2021
Member
sternenseemann
left a comment
There was a problem hiding this comment.
LGTM. Not sure if bootBintools is the clearest terminology in this case, but OTOH I am also unable to express what I think “boot” is supposed to mean in that context.
This will begin the process of breaking up the `useLLVM` monolith. That is good in general, but I hope will be good for NetBSD and Darwin in particular. Co-authored-by: sterni <sternenseemann@systemli.org>
84a73a7 to
18c38f8
Compare
Member
Author
|
@sternenseemann Yeah I don't really like the name either, but I don't intend for it to be really stable so we can just change it later. I somewhat hope as the bootstrapping logic becomes more standardized across all the combinations, we can move it out of the default.nix anyways. |
sternenseemann
added a commit
to sternenseemann/nixpkgs
that referenced
this pull request
Aug 3, 2021
PR NixOS#122778 allowed the linker being chosen independently from useLLVM which also affected pkgsLLVM where we were relying on this behavior. For platform sets assembled from scratch useLLVM still implies linker == "lld", however in the case of pkgsLLVM we update the current platform via the set update operator which means that `linker` won't be re-evaluated. Using ld.bfd with pkgsLLVM is okay to a certain extent, but with C++ things begin to break. We fix this by setting linker explicitly.
11 tasks
sternenseemann
added a commit
that referenced
this pull request
Aug 3, 2021
PR #122778 allowed the linker being chosen independently from useLLVM which also affected pkgsLLVM where we were relying on this behavior. For platform sets assembled from scratch useLLVM still implies linker == "lld", however in the case of pkgsLLVM we update the current platform via the set update operator which means that `linker` won't be re-evaluated. Using ld.bfd with pkgsLLVM is okay to a certain extent, but with C++ things begin to break. We fix this by setting linker explicitly.
Artturin
reviewed
Feb 22, 2023
|
|
||
| stdenvNoLibs = mkStdenvNoLibs stdenv; | ||
| stdenvNoLibs = | ||
| if stdenv.hostPlatform != stdenv.buildPlatform && (stdenv.hostPlatform.isDarwin || stdenv.hostPlatform.isDarwin.useLLVM or false) |
Member
There was a problem hiding this comment.
if stdenv.hostPlatform != stdenv.buildPlatform && (stdenv.hostPlatform.isDarwin || stdenv.hostPlatform.isDarwin.useLLVM or false)
should be
if stdenv.hostPlatform != stdenv.buildPlatform && (stdenv.hostPlatform.isDarwin || stdenv.hostPlatform.useLLVM or false)
nix-repl> pkgsCross.x86_64-freebsd.stdenv.hostPlatform.useLLVM
true
Member
Author
There was a problem hiding this comment.
Oops! Thanks for catching!
Artturin
added a commit
to Artturin/nixpkgs
that referenced
this pull request
Dec 13, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Dec 14, 2023
#122778 (comment) (cherry picked from commit 55405ad)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This will begin the process of breaking up the
useLLVMmonolith. Thatis good in general, but I hope will be good for NetBSD and Darwin in
particular.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)