llvmPackages_{38,39}: musl compat, minor cleanup #42452
Merged
Conversation
This file contains 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
| @@ -81,6 +85,14 @@ in stdenv.mkDerivation rec { | |||
| ++ stdenv.lib.optionals ( isDarwin) [ | |||
| "-DLLVM_ENABLE_LIBCXX=ON" | |||
| "-DCAN_TARGET_i386=false" | |||
| ] ++ stdenv.lib.optionals stdenv.hostPlatform.isMusl [ | |||
| "-DLLVM_HOST_TRIPLE=${stdenv.hostPlatform.config}" | |||
There was a problem hiding this comment.
Can we handle these without the condition? I guess that could break things but I would much prefer to have these 3 flags be unconditional.
There was a problem hiding this comment.
I would too -- but previously was asked to keep them musl-only for now so I was following along here. How about I make a new PR doing this across LLVM versions containing this?
There was a problem hiding this comment.
Okay that's definitely fine to make it musl-only. I just want to minimize conditionals in Nixpkgs as much as possible.
There was a problem hiding this comment.
I assume since the nativeBuildInput stuff is done in here, doing that too wouldn't make it a mass rebuild, right? I'd ping whoever asked before, either in this PR, or a 3rd master PR (not the 4 5 6 one).
There was a problem hiding this comment.
I'll make a new PR if that's alright with you-- rather keep unrelated things separate even if it looks like we have 3 PR's on that subject now but oh well :).
There was a problem hiding this comment.
(since that PR will conflict with this one if not based on top, I'll wait to put that together until this is merged since I think there's little controversial/risky here, esp in comparison :))
9 tasks
|
Failure on x86_64-darwin (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
dtzWill
added a commit
to dtzWill/nixpkgs
that referenced
this pull request
Jun 23, 2018
Previously we only set these when using musl. See NixOS#42452 (comment) Same should be done for changes in NixOS#42452, I'll add a follow-up commit doing so in the PR containing this or wherever makes sense-- depending on how review/merging of that goes. Sending similar changes for LLVM 4 to master, as suggested.
|
Success on aarch64-linux (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
|
Darwin failure looks transient-- builder overloaded with processes apparently: |
|
Rebuild script says these are the jobs that will be rebuilt: (I wanted to get clearer idea of the 11-100 darwin tag): |
Mostly same as used with LLVM 4.0
(as we do on other LLVM builds)
Don't have a fix for this handy and don't need it yet so for now just disable.
Same as those added to llvm_39 a few commits ago.
dtzWill
force-pushed
the
feature/llvm-38-39-musl
branch
from
789313f
to
db7041a
Compare
|
Rebasing for some fixes on master to make for cleaner nox-review (not sure I have enough build power to process them but the first one I tried failed so I investigated :)) |
|
Success on aarch64-linux (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
|
Failure on x86_64-darwin (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
|
Alright LGTM on Linux and such-- post-merge review welcome, and will be monitoring hydra a bit to see how non-borg Darwin likes the changes (since they appear to be builder-specific). |
|
Success on x86_64-linux (full log) Attempted: clang_38, clang_39, llvm_38, llvm_39, llvm_4 Partial log (click to expand)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
"cleanup" is really few instances of moving cmake to nativeBuildInput.
Patches are all the ones already used for LLVM 4, which this moves
a level up so they can share the same files.
Generally seems good, although libc++ doesn't seem to work due to not
having fixes that made it into later releases.
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)