Skip to content
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

Closed
wants to merge 7 commits into from

Conversation

matthewbauer
Copy link
Member

@matthewbauer matthewbauer commented Apr 26, 2018

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,

{ stdenv, pkgconfig, fetchurl
, withPulseAudio, libpulseaudio
, withAlsa, alsaLib
}:

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.

nix-build -A foo --arg config '{ withPulseAudio = false; }'

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?

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.
'';
};

withGTK = mkOption {
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

@jtojnar jtojnar Apr 27, 2018

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.

Copy link
Contributor

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.

https://github.com/NixOS/nixpkgs/search?utf8=%E2%9C%93&q=withgtk+OR+usegtk+OR+withqt+OR+useqt+OR+withalsa+OR+usealsa&type=

@xeji
Copy link
Contributor

xeji commented Apr 26, 2018

Nice! I like the idea of some standardized build options. Some initial thoughts:

  • It would have to be well documented.
  • I'm not sure how to get consensus on the global default values. They are important because they can influence what is built on Hydra and available in the binary cache.
  • Whats the interaction with default values in a package expresson? For instance, what happens if the global option withGTK=false but the qemu expression has withGTK ? true ? IMO defaults in the package should have priority over global option defaults but I'm not sure your implementation does that.

@matthewbauer
Copy link
Member Author

Whats the interaction with default values in a package expresson? For instance, what happens if the global option withGTK=false but the qemu expression has withGTK ? true ? IMO defaults in the package should have priority over global option defaults but I'm not sure your implementation does that.

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.

@xeji
Copy link
Contributor

xeji commented Apr 27, 2018

Ok, so we might have things like withGTK to use the global option and another name gtkSupport ? true if we think the package should use a different default. That will work (but may create a little confusion).

@@ -0,0 +1,128 @@
{lib, stdenv}:
Copy link
Member

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?

Copy link
Member Author

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.

@Ericson2314
Copy link
Member

@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 options.*, and to a trick like we do with xorg in splice.nix so callPackage sees them?

@matthewbauer
Copy link
Member Author

matthewbauer commented Apr 27, 2018

@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 options.*, and to a trick like we do with xorg in splice.nix so callPackage sees them?

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.
@Ericson2314
Copy link
Member

Yeah I don't really care. The names make pretty clear they aren't packages.

matthewbauer referenced this pull request May 7, 2018
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.
@maurer
Copy link
Member

maurer commented Jun 7, 2018

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:

  • Currently, most packages have one configuration. There are exceptions, but usually if a package builds and runs on hydra, we know it's put together correctly. How are we going to approach testing changes if there's an explosion of configuration options?
    Having a robust testing infrastructure in Hydra is one of the reasons we are able to get by with so few developers and still remain reasonably up to date.
    I am concerned that adding a wide variety of untested configurations would end up generating a large number of (valid) bug reports that need to be triaged and fixed.
    Approaches I can see are:
    • Only test the "defaults", e.g. with the options record empty.
    • Test every global set of options which is "compatible" (I think this is computationally implausible)
    • Have some set of endorsed options records which are tested
  • If we permit different options for a package than the global setting, is it broken/incorrect behavior for a package to fail in that case? For example, if we had a withKrb flag for Kerberos support, and I have it on globally, but off for a package via an override, it could receive a header from the dependency which referenced a Kerberos header. Depending on the build system, this could do any of:
    • Error out, if it's not using pkg-config or similar to inherit search paths
    • End up linking Kerberos in anyways, but not have support on the interface layer
    • End up with full Kerberos support
      Are any of these allowed? All of them? How do we deal with this?
  • Which packages will be present in the NixOS cache? The Gentoo USEflags approach largely makes sense because everyone is going to be building locally on a heterogeneous background anyways, so configuration explosion was always a problem. In our case, most users use the cache for most packages, manually building only the more esoteric packages or ones they need bleeding edge versions of.
    This is not necessarily a problem that needs a complete solution, but again, we'd need some kind of policy on which versions of packages would be cached.

Overall, I like the idea, and I miss USE from Gentoo. However, I would hope that in getting the flags ourselves, we don't inherit Gentoo's testing difficulties.

@matthewbauer
Copy link
Member Author

Currently, most packages have one configuration. There are exceptions, but usually if a package builds and runs on hydra, we know it's put together correctly. How are we going to approach testing changes if there's an explosion of configuration options?

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:

  • headlessLinux - all X11 and stuff disabled
  • freeDarwin - all proprietary Apple SDK removed
  • waylandLinux - Linux built with Wayland APIs
  • staticLinux - everything statically compiled
  • staticDarwin - everything statically compiled
  • slnosLinux - Linux without Systemd or Pulseaudio

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.

@Anton-Latukha
Copy link
Contributor

Anton-Latukha commented Jun 7, 2018

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.

@matthewbauer
Copy link
Member Author

So if someone builds package with custom options, it get shared through IPFS, as also it's log to the central CI.

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.

@Anton-Latukha
Copy link
Contributor

Anton-Latukha commented Jun 7, 2018

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.

@Anton-Latukha
Copy link
Contributor

Anton-Latukha commented Jun 7, 2018

And it is obvious at first IPFS would not have secure cache. You use IPFS packet cache at your own risk.

@copumpkin
Copy link
Member

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.

@matthewbauer
Copy link
Member Author

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.

@oxij
Copy link
Member

oxij commented Jun 8, 2018 via email

@oxij
Copy link
Member

oxij commented Jun 8, 2018 via email

@matthewbauer
Copy link
Member Author

Closing for now. I think custom overlays are the way to go for this.

matthewbauer added a commit to matthewbauer/nixpkgs that referenced this pull request Aug 6, 2018
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.
@infinisil infinisil added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0 significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants