Revert "llvmPackages: do not include static archives when shared…" #178007
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
Reverts #162607 / 1748887. Reason for revert: This change caused llvm-config{,-native} to be unable to find static archives bundled with LLVM, as has been [reported]. Ever since #152944 using moveToOutput in LLVM is _evil_ because llvm-config obtains it knowledge about the installation locations from the CMake configure step. Consequently a change like #162607 will need to be implemented by making LLVM itself install the static archives to the correct location or by adding yet another patch which updates llvm-config's knowledge of the location. The latter is not desireable in my opinion, though, since it is just asking for this sort of trouble: Before #152944 we had an outputs.patch that did this sort of things which broke spectacularly in edge cases. Fixes #148117. [reported]: #148117 (comment)
|
I got bitten by this while trying to make a derivation for https://antelang.org/, so I would be happy to see this merged. |
|
I think everyone had enough time to comment. |
Merged
1 task
|
Successfully created backport PR #180233 for |
rrbutani
added
the
6.topic: llvm/clang
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.
Reverts #162607 / 1748887.
Reason for revert: This change caused llvm-config{,-native} to be unable
to find static archives bundled with LLVM, as has been reported. Ever
since #152944 using moveToOutput in LLVM is evil because llvm-config
obtains it knowledge about the installation locations from the CMake
configure step.
Consequently a change like #162607 will need to be implemented by making
LLVM itself install the static archives to the correct location or by
adding yet another patch which updates llvm-config's knowledge of the
location. The latter is not desireable in my opinion, though, since it
is just asking for this sort of trouble: Before #152944 we had an
outputs.patch that did this sort of things which broke spectacularly in
edge cases.
Fixes #148117.
Tested with
for the default LLVM version.
Note that this PR will increase the closure size of LLVM, but reduced closure size is not worth sacrifizing correctness for. There is no trivial way to fix @waldheinz's original patch, so reverting until a better solution is found is the better option.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes