Typed nixpkgs.config with Gentoo-like use-flags#56227
Typed nixpkgs.config with Gentoo-like use-flags#56227oxij wants to merge 12 commits intoNixOS:masterfrom
nixpkgs.config with Gentoo-like use-flags#56227Conversation
|
On #56110 you got a 👎 from @edolstra and I suggested we should probably make an RFC out of it. In #56158 you said:
It may be conceptually not much, or code-wise not much, but it is a big change -- one which has been discussed in the past. I still think an RFC might be in order. |
a3de4f9 to
c805ccd
Compare
|
@grahamc I see why @edolstra would 👎 #56110, it needs a huge change to the rest of nixpkgs to work. This thing is small and pretty trivial. If there is a consensus that this needs an RFC then, if somebody else wants to do it, I'm not against it, if you want me to do it, then it won't happen soon, as my free time ends tomorrow and I won't have a long span I can commit to nixpkgs for a while. |
|
Let's see what other people think :) |
c805ccd to
ccea0f8
Compare
|
Still catching up on things but this looks great! Tiny minor nit: |
pkgs/top-level/config.nix
Outdated
There was a problem hiding this comment.
Can't you save half of the lines here by calling mkMassRebuild? (and half of the wondering whether there is a subtle difference)
pkgs/top-level/config.nix
Outdated
There was a problem hiding this comment.
These are not used anywhere, right? Maybe add a comment explaining the argument order swap?
There was a problem hiding this comment.
Oops, sorry, only whenEnabled is not used — and I see the currying benefits of whenDisable — which might still be worth a comment.
|
I think this is cheap (unlike overlays…), relies on the code already there and makes the use of the flags more consistent. Maybe it should be explicitly mentioned in some comment that non-default values of USE flags are not tested, not provided by Hydra and might not be well maintained; especially if one has two non-default flags set… |
ccea0f8 to
315f483
Compare
pkgs/top-level/default.nix
Outdated
There was a problem hiding this comment.
| # experince here.) | |
| # experience here.) |
315f483 to
8adad43
Compare
|
Some of this stuff looks good, but I disagree with the high level goal of adding use-flags. While I previously thought it was a good goal for Nixpkgs to have, I now think it would make the end user experience worse. The two main reasons are:
These are mainly pragmatic reasons and I definitely set the allure of USE flags. I just think the same thing could be accomplished with overlays, with much more specificity, and less mass rebuilds. |
pkgs/top-level/config.nix
Outdated
There was a problem hiding this comment.
It's a bit weird to have one feature attribute getting put into Whether to enable ${args.feature} support for nixpkgs packages., accepts values like "X11", and another feature attribute getting put into Whether to ${args.feature} while evaluating nixpkgs., accepting values like "run foobar". It might make sense to give them a different name, the latter one could be renamed to action.
In addition I fear it might end up like mkEnableOption "foo" where people don't know that Whether to enable ${arg}. gets put around their argument. But I guess writing such config options isn't as common as NixOS modules.
pkgs/top-level/config.nix
Outdated
There was a problem hiding this comment.
I think we should change nixpkgs to use a single variant throughout, but this could be done in a future PR.
lib/options.nix
Outdated
There was a problem hiding this comment.
This is an unnecessarily hacky way to get the functionality you're implementing with it. If you need additional module settings when options have a certain value, use a proper module for that instead of abusing mkOption's arguments.
Do to this you'll need to do a data transformation for config.nix, changing all the foo = mkUse { ... } into a list of modules that each define the option and do the extra things.
Or just write it as a normal module, with additional config being applied in the config section.
pkgs/top-level/config.nix
Outdated
There was a problem hiding this comment.
I don't think we need to introduce preliminary aliases. We can always add them later if they're really wanted.
pkgs/top-level/default.nix
Outdated
There was a problem hiding this comment.
Same as modules = [ ./config.nix ...
|
1. Any non-trivial USE flags are going to cause mass rebuilds.
Any non-trivial use of overlays is also causing a mass rebuild.
It also leads to fragmentation, where we now need to know what USE flags you have set when you report that something is broken.
Sure, so do overlays. The `~/.config/nixpkgs/config.nix` and similar influence nixpkgs evaluation in meaningful ways without USE flags anyway, which is why I insert the contents of $NIXPKGS_CONFIG into every single PR I make.
Unlike Gentoo, very few users expect to have to rebuild everything from source.
We have a hard enough time getting things to work correctly in vanilla Nixpkgs without having to worry about every permutation of Nixpkgs a user might have (this of course applies to overlays too, but i think there is less of an expectation that an overlay will work).
1. I don't see a problem if this is clearly documented. And it is documented in the source. Moreover, I can even suggest _not_ documenting USE flags in the nixpkgs manual so that the average user wouldn't even know any of this exist.
2. In practice, options _are_ usually orthogonal, which means most permutations, when applied via `extraScope` (see below), _will_ actually work. I have been using the previous iteration of this to apply a bunch of overrides to `extraScope` in my config for years (like 5 or so):
```nix
config.common = {
gpgSupport = true;
x11Support = graphical;
sdlSupport = graphical;
graphicsSupport = graphical;
xineramaSupport = graphical;
xvSupport = graphical;
screenSaverSupport = graphical;
jackaudioSupport = true;
... and many more ...
};
...
```
(`graphical` is a custom NixOS option that influences a bunch of stuff). I have never had any problems with this.
If this orthogonality was not present then sequential overrides already common in nixpkgs and hence overlays also wouldn't make any sense and would be constantly broken. But they are not.
More importantly, note that the big advantage of this `extraScope` technique over overrides and hence overlays is that `extraScope` comes before attribute defaults in package expression (as applied by `callPackage`), which means that if you have
```nix
{ stdenv
, someOption ? value
, otherOption ? someOption }: ...
```
then overriding `someOption` via `extraScope` will also override `otherOption`, while overriding via `.override` will not because `makeOverridable` will already have fixed the old value inside.
This makes a huge difference as you don't even need to look at the package expression to get the correct result, package expression will manage it for you automagically (and if it doesn't you just fix one broken expression, not all your overrides).
(Yep, I should have documented this in the commit messages. Sorry, after 5 years all this is common sense to me, I forgot, will do later.)
For this same reason, I think we should start discouraging config flags like ```config.pulseaudio``` or ```config.cudaSupport``` as they are almost guaranteed to be broken without having a whole new jobset on Hydra.
2. You can accomplish the exact same thing as USE flags with your own overlay.
As long as you know which packages you are using, you just have to do an override for each package with the flag.
Except with overlays for `cudaSupport` or `pulseaudio` I will have to carefully read the source of all the derivations I use to see if any options depend on each other, then carefully override all of them, then follow changes to nixpkgs to monitor changes to dependencies between said opinions and fix my overrides on updates, and tolerate the 2x memory consumption introduced by `.override`for those derivations for the rest of my life.
Similarly, to make a consistent fully headless system with overlays I will have to carefully read and override, follow nikpkgs changes and tolerate 2x memory consumption for about half of all the packages I use. This is pretty much equivalent to maintaining a separate fork of nixpkgs all by myself. Thanks, but no thanks.
Meanwhile, with `extraScope` overriding either takes a single line of config, everything else is automagical.
I just think the same thing could be accomplished with overlays, with much more specificity,
Sure, with inhuman amounts of work and 2x more memory.
and less mass rebuilds.
[Citation needed] Overriding most core packages will cause a mass rebuild anyway, AFAICS.
|
|
It's a bit weird to have one `feature` attribute getting put into `Whether to enable ${args.feature} support for nixpkgs packages.`, accepts values like "X11", and another `feature` attribute getting put into `Whether to ${args.feature} while evaluating nixpkgs.`, accepting values like "run foobar". It might make sense to give them a different name, the latter one could be renamed to `action`.
All of them are defined and used in a single file, so their definitions are pretty hard to miss, IMHO. In any case, `action` doesn't sound quite right either in this context, IMHO.
+ gtk3Support = false;
+ gtk2 = null;
+ gtk3 = null;
I think we should change nixpkgs to use a single variant throughout, but this could be done in a future PR.
Yep.
+ # Anything extra.
+ extra ? null
This is an unnecessarily hacky way to get the functionality you're implementing with it. If you need additional module settings when options have a certain value, use a proper module for that instead of abusing mkOption's arguments.
Or just write it as a normal module, with additional config being applied in the config section.
If I were to generate those things in `config` part of the module then the whole thing will get really ugly real soon because use-flags will become non-local to their effects.
Do to this you'll need to do a data transformation for `config.nix`, changing all the `foo = mkUse { ... }` into a list of modules that each define the option and do the extra things.
If you were to apply the same logic to the rest of NixOS then you can argue that `mkOption` needs to shed half of its arguments. Say `apply`, for instance. Why have `apply` when you can define another option and `apply` from one to the other in `config` part of some NixOS module somewhere? Why have `default` when you can have `mkDefault` in `config`? Why have `internal` when you can filter them out in manual's generator?
I think one `extra` argument buys a lot of clarity and locality there. If you think `mkOption` MUST not have such an `extra` thing, then, sure, I can just add `_type = "option"` by hand in `config.nix`. Though I see no benefit to that.
changing all the `foo = mkUse { ... }` into a list of modules that each define the option and do the extra things
Just imagining such code makes me shiver.
+ imports = [
+ (mkAliasOptionModule [ "use" "x11" ] [ "use" "X" ])
I don't think we need to introduce preliminary aliases. We can always add them later if they're really wanted.
This was added for testing purposes, you'd be surprised how many bugs this single line uncovered. :) I can remove it from the final version if desired, but I'd rather keep it, `x11` is a cute alias, IMHO.
+ modules = [
+ (import ./config.nix)
Same as `modules = [ ./config.nix ... `
Yep, that would be better. Will do.
|
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/flag-packages-out-of-date/12612/13 |
|
What is currently holding this PR back? |
|
I marked this as stale due to inactivity. → More info |
This comment was marked as off-topic.
This comment was marked as off-topic.
It has too many changes, one of which is not viable: use flags. See start of discussion. Some changes are viable, but are better applied in separate PRs. Furthermore, the author seems to have stopped contributing to Nixpkgs. As such, it seems better to close this PR than to leave it open. It can be reopened if the author reduces the scope of the changes, although a split into smaller PRs is to be preferred. |
Definitely understandable, although for the case of use flags, aren't they essentially just global overrides? Seems like an easier way to deal with options than package by package if you want to apply something globally. Eh, this is probably a discussion for a more focused PR. |
|
Honestly, part of the reason is trying to engineer things such that we don't override as much, making binary caches more effective and reducing support load. Support which is handled by passionate people in their spare time. So yes, your argumentation is correct, but does not override the sustainability of the project. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
| default = builtins.getEnv "NIXPKGS_ALLOW_BROKEN" == "1"; | ||
| defaultText = ''builtins.getEnv "NIXPKGS_ALLOW_BROKEN" == "1"''; | ||
| feature = "permit evaluation of broken packages"; | ||
| }; |
There was a problem hiding this comment.
We have some of these in pkgs/top-level/config.nix now, but they're not nearly as complete, so they're still worth adding.
There was a problem hiding this comment.
(except for the new ones that don't exist yet)
|
Eh, I'm not dead, it's just that I got so much pushback on my changes that I found it much easier to push a tiny minimal thing useful for other not-use-flags things, describe the rest, and hope other people reimplement it. I tried pushing use-flags into master since at least 2015, and the best I could get in was `pkgs/top-level/config.nix`... Which, while nice, is still inferior to splicing-callPackages. But, yes, splicing is not particularly compatible with "traditional" overrides and overlays. Which is sad. On the other hand, today I ran `nix-env -qaP -f . --out-path` on Nixpkgs and it took >24G of memory to evaluate 😱 SLNOS switched to splicing in 2019, and it takes at least 2x less memory to evaluate now (in fact, I didn't know Nixpkgs took that much before today, since I wasn't using Nixpkgs much for a while).
So, if you are working on this without me, Thank You! Please, continue.
I do plan to return to this eventually, I think, because I now have some projects that would benefit from using more vanilla Nixpkgs with public build caches and such.
In other news, "wait for it to be implemented by others" also worked in a bit ironic way for #49862. Unfortunately the actual thing I wanted it for is still not implemented in master 5 years later, and having it in SLNOS and not having in NixOS is really annoying, so I updated it today to hopefully reach a compromise there. Wink-wink, it would be nice if somebody looked at it and eventually merged it.
Reviving of #75389 would probably be next.
|
Not at all relevant to the PR, but have you thought of making SLNOS public, and usable as a nix flake import? If it's still the same structure as NixOS users should be able to replace the nixpkgs input with the SLNOS one to use SLNOS. |
|
Also, worth noting that nix modules would be a huge quality of life improvement to getting this implemented |
I don't think this has developed beyond a proof of concept and Eelco is currently participating in the package modules working group instead. |
I think, personally, use-flags would be extremely useful, so I look forward to this being potentially revived in the future. |
What?
This patchset does two things:
firstly, it checks types and produces defaults for
configattribute (akanixpkgs.config),secondly, it introduces Gentoo-like use-flags of Standard gentoo-like "use" flags across nixpkgs #12877 for the cheap on top of that.
The first part is inspired by and is similar to #43672 by @matthewbauer (but it is simpler than #43672 module-mechanics-wise and less comprehensive option-wise, that latter part and removal of
or falsefromconfig\..* or falseand similar is left for future work if this gets merged).The second part is inspired by #56109 by me and #56110 (comment) by @danbst, and is similar in function to #39580 by @matthewbauer (but it doesn't use overlays, which makes this very cheap (this patchset is unnoticeable in metrics, the impact of ~0.02% on allocations and something unmeasurable on eval time), and without
pkgsscope pollution with any new attributes).Why?
See commit messages below, #43672, #12877, #39580.
git logpkgs/top-level/stage.nix: don't override
overlaysandconfiginnixpkgsFunnixpkgsFunalready sets them. The next patch will also makenixpkgsFunuse the originalconfigwhilestage.nixwill get theprocessed one. Mixing them up can cause unexpected errors.
pkgs/top-level: check types of nixpkgs.config
This patch simply introduces a plain simple NixOS module and passes
nixpkgs.configthrough it viaevalModules(with some temporary hackery topassthru undefined options).
lib: add
extraproperty tomkOptiontop-level: introduce Gentoo-like global USE flags
This uses the above
nixpkgs.configinfra to makenixpkgs.config.useoptionset that influences the default scope of
callPackageand etc, thusimplementing Gentoo-like use flags.
Note that the whole point of use-flags is that they cover use cases not covered
by overlays and overrides. For instance, it is very hard to maintain a working
overlay of headless packages (as
minimal.nixprofile of NixOS dutifullydemonstrates, see Top-level nixpkgs-headless package set #29135 for more discussion), but it is almost trivial to make
such a thing using use-flags.
Moreover, influencing the default scope of
callPackageand etc allows one tooverride all packages in
pkgsall at once while keepingpkgsscope as-is.For instance, by setting
x11Support = falseincallPackagescope onewill (in the ideal world, Nixpkgs' reality is much more cruel, see the actual
patch) disable X11 support in all the packages while keeping, e.g.
xlibavailable. The combination of these two properties
(influencing all the packages while keeping the
pkgsscope) is impossible toachieve with overlays without manually overriding almost every single package in
scope.
nix-instantiateenvironmentnix-env -qaPdiffsNotes
Note that the above diff shows that this adds some packages on Darwin. "WTF?" you may rightfully ask after reading the changes.
What happens there is the above infra has
config.use.alsadisabled on Darwin by default, which "overrides" all packages with corresponding scope changes on Darwin, thus removingalsaLibdependencies from the above packages, thus allowing above packages to be evaluated on Darwin. If stuff like this isn't cool, then I don't know what is.I also wanted to disable PulseAudio on Darwin, because AFAIK nobody runs it on Darwin anyway, but that changes a lot of packages, so I decided against it for now.
What's next?
If this gets merged:
Most of Create config builder to handle default config values #43672 should be ported on top of this to simplify
confighandling in the rest of nixpkgs.Old
configoptions, including those defined by packages, need to be added, a lot of them can then be depricated using normal NixOS module mechanisms likemkRenamedOptionModuleand etc.Ideally, package options need to be cleaned up.
If not: