llvmPackages.libclang: add enableClangToolsExtra flag#446207
llvmPackages.libclang: add enableClangToolsExtra flag#446207emilazy merged 1 commit intoNixOS:masterfrom
Conversation
rocmPackages needs a clang at runtime but has no need to include clang-tidy. The approach in this change avoids rebuilds and is messy, I will make a followup in staging that changes this to more idiomatic optionalString usage if this is merged
9d67791 to
1f46266
Compare
|
Is a separate output not an option here? That would generally be cleaner, I think. (But of course require going through |
That seems like it should be possible but is non-trivial, clang-tools-extra ends up with both headers in darwin's bootstrap may be somewhat faster to iterate on with the option to skip building clang-tools-extra as it could be disabled for all but the final stage. I think the ideal option if we want to do something non-trivial might be going further and trying to put clang-tools-extra in a separate package in llvmPackages. |
Yeah, that would be great. But if it’s a pain I don’t want to block this if others think it’s a good idea. Maybe an incremental step would be to disable building the tools in the first place when this flag is off, since that would be desirable for a split package anyway? |
clang-tools-extra doesn't have any of the
The diff in this PR should already disable them when that flag's false unless I messed it up. ~600 less build steps and the following files/folders are no longer installed: |
|
@emilazy how do you feel about this attempt at splitting it up? 5a5ad6f#diff-e418126c856bf1baf0bb8e13be497b81adec5a2c76efae030ca666c369c33c4b |
|
Wow, amazing work! That would certainly be great for us if upstream is open to it – but until then I’m happy to go with the current approach here if you still think it’s the best short‐term option, especially with the potential wins for Darwin bootstrap 😅 (I thought we did some funny thing with the monorepo sources that avoided relying on the standalone build infrastructure, but I could be totally wrong there.) |
|
Landing the simple flag to turn off clang-tools-extra for now and later trying to upstream proper support for splitting it into a separate project makes the most sense to me. |
rocmPackagesneeds a clang at runtime, it would be nice to avoid including clang-tools-extra/clang-tidy related files in clang to reduce the runtime closure size of packages that support ROCm.The approach in this change avoids rebuilds and is messy, I will make a followup in staging that changes this to more idiomatic
optionalStringusage if this is mergedI've been working on closure size reduction for
rocmPackagesin #444860 and figured it'd be nicer on reviewers to separate out this clang change.nixpkgs-reviewresultGenerated using
nixpkgs-review.Command:
nixpkgs-review pr 446207 --extra-nixpkgs-config '{ allowAliases = true; }' --package llvmPackages.libclang.tests.withoutOptionalFeatures --package llvmPackages_18.libclang.tests.withoutOptionalFeatures --package llvmPackages_21.libclang.tests.withoutOptionalFeatures --package llvmPackages_git.libclang.tests.withoutOptionalFeaturesCommit:
1f4626619139ff66e420b8446c25e93dc8903b3fx86_64-linux✅ 20 packages built:
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.