llvmPackages_{12,13,14,15,16,17,18,git}: Allow overriding dependencies #320261
Conversation
|
@Artturin can this get the same backport treatment as #307211 please? :) Thanks for the speedy review @RossComputerGuy! |
ofborg
bot
added
10.rebuild-darwin: 0
|
can this be accomplished with a named attribute set argument? { ninja, foo, bar, ...} @args:
[...]
libclang callPackage ../common/clang (args // { });? i'm not sure but would be nice to not have to manually keep the union of all llvmPackges_xx.xxx args in llvm/default.nix |
Are you suggesting adding an I'm not sure how I feel about this. I would rather use the existing override mechanism than introduce yet-another-override pattern. Another side effect of the PR as it stands is that it results in all of the dependencies being listed in the top level. I note that the only harm of missing one out from the top level is that it cannot be overridden via |
no. https://nix.dev/tutorials/nix-language.html#functions using a named attribute set argument should allow you to pass down the top level args to all the llvm submodules, i think. in the top level llvm you'd use ellipses followed by then when calling the subpackage the attribute set to callPackage is setup using and the subpackage may need to be modified to take additional arguments so in libcxx say, you have to add an ellipses to the end of the argument set. |
This is already happening and I arranged it in #307211 exactly to allow this. Unfortunately, if the argument is not named in the parameter set, you cannot See for example Therefore, I currently conclude it's necessary to specify the parameters which we want to be able to override as inputs to llvmPackages as this PR does. |
thanks. i didn't see the
so it doesn't seem like there are any diff --git a/pkgs/development/compilers/llvm/12/default.nix b/pkgs/development/compilers/llvm/12/default.nix
index d3b823215c52..0c2267a68205 100644
--- a/pkgs/development/compilers/llvm/12/default.nix
+++ b/pkgs/development/compilers/llvm/12/default.nix
@@ -17,6 +17,7 @@
then null
else pkgs.bintools
, darwin
+, ...
}@args:
leti can run just fine. |
|
OK. Are you happy if I add |
|
pwaller
force-pushed
the
llvm-pass-all-deps
branch
from
629a2fd
to
ee94f72
Compare
|
Thanks @paparodeo, I've added it as well. |
... consistently. Further to NixOS#307211, allow overriding arguments through llvmPackages.override. This makes it possible to override any dependency of LLVM or clang by overriding it on `llvmPackages.override { <dependency> = ...; }`. This is useful in development or customization where sometimes it is desirable to turn features on or off. Without this patch the only way to for example override ncurses was to do `overriddenLLVM = llvmPackages.llvm.override { ncurses }`, but then you would have to thread `overriddenLLVM` as dependencies into clang and other packages, which results in quite a difficult expression to write correctly in cross compilation scenarios. Signed-off-by: Peter Waller <[email protected]>
pwaller
force-pushed
the
llvm-pass-all-deps
branch
from
ee94f72
to
40a7f21
Compare
pwaller
changed the title
|
Please note I won't have a chance to test this for a moment; I'm relying partially on the ofBorg package count rebuild result which should be zero and a good test. I'll also do a bit of testing over the coming days. |
There was a problem hiding this comment.
looks good. 0 builds for nixpkgs-review
Result of nixpkgs-review pr 320261 run on x86_64-linux 1
tried a few overrides and the correct things looked like they were being rebuilt.
$ nix-build -E 'with import ./. {}; llvmPackages_18.override { lua5_3 = lua5_3.overrideAttrs { foo =1; };}'
these 4 derivations will be built:
/nix/store/z7vxw6cgh4gzhk5w44x90jnmml3ih0n4-utils.sh.drv
/nix/store/784hhifl221ny531a8kzpbj3hmw7msdy-lua-5.3.6.drv
/nix/store/n8sr8zdj4g3gxlmjqg85zhknb1s203fp-lldb-18.1.7.drv
/nix/store/r7926xvs7abds7m2mjwrbyvy26g9nk6z-lldb-manpages-18.1.7.drv
$ nix-build -E 'with import ./. {}; llvmPackages_18.override { lit = lit.overrideAttrs { foo =1; };}'
these 4 derivations will be built:
/nix/store/9zg056a72jkwrhvg2zz8yjpfp5751i0c-python3.11-lit-17.0.6.drv
/nix/store/58qg67s9a06ci66r77sjsn8174i4s9zb-lldb-18.1.7.drv
/nix/store/aizdk1y7ksv4pd93dwhd0kp43h4fk4d0-lldb-manpages-18.1.7.drv
/nix/store/gp963g3pbn4a1z9fgxhaam2fiafnl6sm-openmp-18.1.7.drv
wegank
added
the
12.approvals: 3+
|
Successfully created backport PR for |
Pull NixOS#320261 introduced the possibility to consistently override dependencies within an llvm package set. I think NixOS#325175 accidentally dropped this, so reinstate it. Signed-off-by: Peter Waller <[email protected]>
cmake flags have a 'last flag wins' logic, so by appending to the end of the flags it is possible to override any cmake flag. It also ignores (and warns) if a flag is unused, so passing flags across all packages should be safe if you want to target one package. In combination with NixOS#320261, this PR allows consistently overriding all packages within LLVM with additional cmake arguments. Consistency here means for example 'if you override LLVM, then all dependencies on it are also see the overridden LLVM in their input'. Consistency is hard to achieve with the other obvious way of implementing this as a user: if you use overrideAttrs then you have to write a big mess of override code in order to override all dependents, and this can be very difficult in a cross-compilation scenario using crossSystem and useLLVM, for example. With this PR it is possible to write an overlay which overlays `llvmPackages` with `llvmPackage.override { devExtraCmakeFlags = [ ... ]; }`, and then the toolchain used with useLLVM in effect should respect these flags. This is useful in development for experimenting with the effect of various flags, hence the chosen name `devCmakeFlags`. This won't work out of the box without NixOS#341855 applied, which fixes override passthrough. See-Also: NixOS#320261, NixOS#341855 Signed-off-by: Peter Waller <[email protected]>
Pull NixOS#320261 introduced the possibility to consistently override dependencies within an llvm package set. I think NixOS#325175 accidentally dropped this, so reinstate it. Signed-off-by: Peter Waller <[email protected]>
Pull NixOS#320261 introduced the possibility to consistently override dependencies within an llvm package set. This is useful for development and testing exotic configurations. Go one step further and enable overriding targetLlvmLibraries. This makes it possible to write an overlay such as: ```nix overlays = [ (self: super: { llvmPackages = super.llvmPackages.override (prev: { targetLlvmLibraries = super.targetPackages.llvmPackages.libraries // { compiler-rt = super.targetPackages.llvmPackages.libraries.compiler-rt.override { ... } }; }); }) ]; ``` ... where the overridden compiler-rt will be used in a pkgsLLVM build. As a straw man, I've done the minimally invasive thing to the code structure: `targetLlvmLibraries` is not an explicitly named parameter for llvmPackages; but it is available in `packageSetArgs` if passed. This makes it slightly less discoverable, but this seems like a reasonable tradeoff considered that modifying this would be a fairly advanced/esoteric thing to need to do. In some ways it would be better to have as an explicit parameter with a default, but the obvious thing won't work because the default needs to be a non-trivial expression. Potentially we could instead have it as a defaulted parameter with the value of 'null', and if it's null, then compute the current thing. Signed-off-by: Peter Waller <[email protected]>
Pull NixOS#320261 introduced the possibility to consistently override dependencies within an llvm package set. This is useful for development and testing exotic configurations. Go one step further and enable overriding targetLlvmLibraries. This makes it possible to write an overlay such as: ```nix overlays = [ (self: super: { llvmPackages = super.llvmPackages.override (prev: { targetLlvmLibraries = super.targetPackages.llvmPackages.libraries // { compiler-rt = super.targetPackages.llvmPackages.libraries.compiler-rt.override { ... } }; }); }) ]; ``` ... where the overridden compiler-rt will be used in a pkgsLLVM build. As a straw man, I've done the minimally invasive thing to the code structure: `targetLlvmLibraries` is not an explicitly named parameter for llvmPackages; but it is available in `packageSetArgs` if passed. This makes it slightly less discoverable, but this seems like a reasonable tradeoff considered that modifying this would be a fairly advanced/esoteric thing to need to do. In some ways it would be better to have as an explicit parameter with a default, but the obvious thing won't work because the default needs to be a non-trivial expression. Potentially we could instead have it as a defaulted parameter with the value of 'null', and if it's null, then compute the current thing. Signed-off-by: Peter Waller <[email protected]>
... consistently.
This should require 0 package rebuilds.
Description of changes
Further to #307211, allow overriding arguments through llvmPackages.override.
This makes it possible to override any dependency of LLVM or
clang by overriding it on
llvmPackages.override { <dependency> = ...; }.This is useful in development or customization where sometimes it is
desirable to turn features on or off.
Without this patch the only way to for example override ncurses was to
do
overriddenLLVM = llvmPackages.llvm.override { ncurses }, but thenyou would have to thread
overriddenLLVMas dependencies into clang andother packages, which results in quite a difficult expression to write
correctly in cross compilation scenarios.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.