New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement standardized "use flags" for Nixpkgs #39580
Conversation
This uses overlays to implement "user flags" for Nixpkgs. Note that this just allows packages to use these flags. No packages flags are renamed but that is planned (with backward compatibility). Eventually I want to generate some manual from the mkOptions but no time right now. Fixes NixOS#12877.
This should be solved by something like the module system, but for now I am just implementing this as an example.
pkgs/top-level/options.nix
Outdated
''; | ||
}; | ||
|
||
withGTK = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be withGtk
since other initialisms also use this style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that although I'm not quite sure if that's better. GTK is usually capitilized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is ALSA. See this question discussing the use of acronyms in identifiers. I do not really care which style we choose but the inconsistency is jarring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That only first letter capitalizes also bothered me in camelCase
.
But it is a trade-off for thisHTTPSUIFFMPEGNames
being readable.
Since I have a love towards descriptive names, I adopted this thisHttpsUiFfmpegNames
.
Nice! I like the idea of some standardized build options. Some initial thoughts:
|
So the global default will always be used. In fact it probably makes sense to not even provide a package default for any of the listed configss. Right now very few packages are using a "with*" so hopefully it will not lead to anything unexpected. |
Ok, so we might have things like |
pkgs/top-level/options.nix
Outdated
@@ -0,0 +1,128 @@ | |||
{lib, stdenv}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use hostPlatform
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah definitely! targetPlatform might actually make more sense though. In fact, one of the big reasons for doing this is getting rid of a lot of the stdenv.isDarwin
and stdenv.isLinux
that is spread accross Nixpkgs. Quite a few should be easily replaced with withApple
, withSystemd
and withAlsa
.
@edolstra has talked about at various points trying to avoid non-packages on the top level, all things equal. I'm somewhat sympathetic---and while various functions are sort of ad-hoc exception, use flags would be a whole class of non-packages on the top level. Maybe we could put these all in |
Yeah that would probably work! We could also have each package use the "config" attribute directly. The main reason I like the original though is it's pretty straightforward for maintainers to grok how to handle the new flags (in fact many are already using them). |
Still not sure how well this works- but POC for putting the options in splice.nix.
Yeah I don't really care. The names make pretty clear they aren't packages. |
The isKexecable flag treated Linux without kexec as just a normal variant, when it really should be treated as a special case incurring complexity debt to support.
This should make it easier to build lots of packages statically when you want.
Caveat: I'm not some kind of nix decision maker, just leaving my 2 cents. Technically / in terms of the code, this seems fine, but I think there are some policy/development strategy questions we need to answer before we mainline a change like this:
Overall, I like the idea, and I miss |
Most likely with some sort of profiles. This is basically what we do with cross now: there are tons of possible config system options, but we just provide a couple of "examples" in https://github.com/NixOS/nixpkgs/blob/master/lib/systems/examples.nix. I would like to get something like that and a release-options.nix file that build a few combinations of configuration (along with the defaults of course). Some possible ones:
Note that lots of packages are going to be the same between these profiles & not a ton of rebuilding will be needed. I'm hoping with time we can get better at structuring dependencies so there's a lot of shared infrastructure between them (this will improve other things as well). These examples don't all have to be built by Hydra yet. |
My idea is that plethora of configuration options can be integrated only if NixOS+IPFS ever takes into reality. Hydra needs to build main version of packages. Build/test of custom options versions of packages need to be on the shoulders of users, build and tested on user machine, and then cache of that package and testing info shared to the main CI through IPFS. Something like that. If package has custom options, but nobody uses some particular sets of them, what is the point to build and test all thet sets of options, until someone starts to use that particular sets. |
I don't know how you could ever trust that IPFS has valid binaries. Without the binary how would you know that the hash in IPFS is correct? It's definitely true we need more builders for Nixpkgs for this to work well - but I think we can improve this without ridiculous costs. We may need to constrain it to a subset of packages for some profiles - things like SDL games should be marked as "broken" when you're headless for example. |
Yes, verifying builds is the question I deliberately omitted. Because I was not talking about package cache, but about build and test logs to be shared to CI. So maintainers can see those logs, and use that information. |
And it is obvious at first IPFS would not have secure cache. You use IPFS packet cache at your own risk. |
If you recall from my original issue, I was extremely wary of getting too many of these options. I basically proposed very high-level "headless" ones and nothing else, because the chances of arbitrary combinations (with exponential possibilities) working is pretty low IMO, and will cause massive rebuilds where people will only realize they don't work after waiting 5 hours. Anyway, I like that you're doing this, but I'd be very careful and considered about which flags we want to put in there. I could imagine one or two maybe being supported nicely, but too many more and folks will just lose faith in the mechanism after it breaks too often. |
I guess what I mostly dislike with “headless” is that it’s pretty vague. There’s not really a headless configure flag - usually you just need to disable all of the GUI backends. My goal with these options is that they will make sense for packages to use without having to make a judgement call on what it means to be headless. I think a headless “profile” makes sense though. |
I'm uber-happy about this in general and `slnosLinux` in particular :) Huge thanks for doing something about this in nixpkgs! I have a couple of comments based on SLNOS experience.
Firstly, I feel like you push those options into the global namespace only because you want to avoid both making expression writers to write
```nix
{ config, stdenv, ...
, pulseAudioSupport ? config.withPulseAudio
```
and patching `callPackage`. While I agree that avoiding the former is a good thing, why don't just patch `callPackage` as follows instead?
```diff
- callPackage = pkgs.newScope {};
+ callPackage = pkgs.newScope config;
```
Secondly, as I mentioned several times before, neither this PR's implementation nor the one above would actually really work because most packages currently override options in arguments to `callPackage`. Those need to be moved to default attrset values in package themselves first (or later, but don't expect this to do any good before that).
Secondly, my main issue with use-flags is that I'd like them to be as close to "feeling like I'm in Gentoo" as possible. The problem with `withCamelCase` and `camelCaseSupport` is that both are ugly because they mangle package names. `calmelCaseSupport` mangles names less but `withSomething` is more often semantically correct (there are cases when `something` doesn't add "support" for anything but the package is linked against `something` for some other reason). However, sometimes even `withSomething` is not entirely correct (assuming "with" means "linked with"/"will refer to this in the closure", not just "built with"), for instance `withPulseAudio` for Firefox would, in fact, be incorrect in such interpretation since Firefox doesn't link against PulseAudio ever, even when built against it (it will `dlopen` it, and PA is added into the closure in the wrapper).
Making a separate namespace for this like `stdenv.use`/`config.use` solves both problems, but makes you write
```nix
{ stdenv, ...
, pulseAudioSupport ? stdenv.use.pulseAudio
```
which is exactly the issue discussed above. That wouldn't be a problem if you could do
```nix
{ stdenv, ...
, useFlags ?
{ pulseaudio ? false
, ...
}
```
instead, but, alas, you can't stack default attrset values in nix.
All of the above combined, I think, inevitably leads to the SLNOS way of applying use-flags separately like this
```nix
{ stdenv, ... }:
{ pulseaudio ? false }:
```
SLNOS uses a separate `callPackage` for this (hence my wish to autogenerate `all-packages.nix` to be able to use a different `callPackage` buy just `map`ing it over), but it just struck me that it could probably be done by just patching `callPackageWith` to apply the scope and then, if the result is still a function, apply the use-flags.
Thoughts?
|
Also forgot to mention. I'd love `headlessLinux` to be `nographicsLinux // nosoundLinux` or similar. I find those things to be useful by themselves too, e.g. a X11-less singe board computer (RPi, BBB) that has ALSA enabled and is connected to a USB sound card and a speaker set to make a standalone music player/announcement system/personal assistant (when connected to a mic).
|
This is a better name
Closing for now. I think custom overlays are the way to go for this. |
pkgs/top-level/config.nix provides some defaults for the config values. This allows global defaults to be provided for config options & prevents individual packages for having to interpret the defaults. Hopefully this will make way for better documentation of configuration options. Eventually I want to move this into a proper module system, see (NixOS#39580). Other squashed commits: - check-meta: use default values from pkgs/top-level/config.nix We no longer have to check that these exist - their defaults are configured there. - config: propagate original config Lots of times we will have custom config options. We want these to still be accessible in Nixpkgs. - config: add packageOverrides One config option I originally missed. - config: fix licenses loop blacklistedLicenses & whitelistedLicense cannot depend on each other in their apply function. Instead just use the original config’. - treewide: remove ‘or’ default for configs these are now provided by config.nix - config: make types enforced config.shouldCheckMeta -> config.checkMeta This was the original name.
This is something I worked up. I'm interested whether people think this is a good way to go. It basically uses the "overlay" system to implement standardized config options in Nixpkgs. To use them in your packages, you must have an argument with the same name as one of the standardized options. For example,
withPulseAudio and withAlsa will automatically be set to the appropriate value for your platform. You can override it by setting the config "withPulseAudio" or "withAlsa" to true.
This is basically what was discussed in #12877. The new overlay system makes this extremely straightforward. For this to work well, people need to be aware of it and use it where appropriate. I want to generate some documentation to go with this.
Are there other options that should be included?