Best practices for config flags to packages

lilyball · September 17, 2019, 12:46am

Various packages take config flags that change things about how they’re built. For example, the unison package has a flag enableX11 ? true and the ffsend package has both x11Support and preferXsel.

When adding new config flags to packages, my worry here is always the fact that the config flag namespace is exactly the same as the package namespace, meaning I could add a config flag today and down the road some package could be added with the same name as my flag, and suddenly my package breaks because the boolean it was expecting is now a derivation.

Does anyone have any suggestions for the proper way to deal with config flags? I can think of two options:

Try to pick a name that doesn’t sound like a package would use it. For example preferXsel doesn’t really sound like something a package would use, but on the other hand x11Support absolutely could end up as the name of a package some day.
Reserve a name like flags that we’ll never turn into a package (maybe enforced by an assertion somewhere) and say all flags should be passed there. This option is awkward since we lose the syntax for defining defaults and we’d have to do something like
```
{ …, flags ? {}}:
with {
  x11Support = stdenv.isLinux || stdenv.hostPlatform.isBSD
, preferXsel = false
} // flags;
…
```
For this option I’d prefer config except that already refers to nixpkgs.config.

Is there a third reasonable option I’m missing?

qyliss · September 17, 2019, 1:21am

Try to pick a name that doesn’t sound like a package would use
it. For example preferXsel doesn’t really sound like something a
package would use, but on the other hand x11Support absolutely could
end up as the name of a package some day.

I don’t see a package named x11Support existing in the foreseeable
future. If it were, say, a library, it would be x11-support. I think
we should consider the “.Support" and "enable[A-Z0-9].” namespaces to
be for options only, since that’s what the vast majority of packages
use.

For what it’s worth, I have seen a package name / option collision once,
but I don’t remember the context. I think it might have been a boolean
option that was just the name of the functionality, no “enable” or
“Support” or anything.

Reserve a name like flags that we’ll never turn into a package
(maybe enforced by an assertion somewhere) and say all flags should be
passed there.

I’ve been wondering for a while whether it might be nice to introduce a
separate argument for build options. So, rather than a default.nix file
for a package being Dependencies | Options → Derivation, it would be
Options → Dependencies → Derivation or something. (Other way round?
Not sure.)

That would also make it possible to reflect on the package, and at least
extract a list of option names (although there’d still be no way to
attach documentation).

Either way, I think the best thing you can do for now when adding
packages to Nixpkgs is to use options named either ".Support" and
"enable[A-Z0-9]., and to stick to one or the other for each package.
That way, we won’t end up with even more fragmentation.

If you think that introducing a flags argument, or some other method,
might be better, the best thing to do would be to create an RFC, and
then we can start changing packages all over to a new, unified
interface. It’s also worth noting that I think @edolstra’s new module
proposal (the one with the angle bracket module syntax) addresses this,
including option documentation. But, as I understand it, that’s a long
way off, and some intermediate change may well be justified.

eraserhd · September 17, 2019, 2:07am

Reserve a name like flags that we’ll never turn into a package

I’m all for this. I’ve been using “configure”, which I picked up from weechat and NeoVim and applied to Kakoune. A quick and possibly inaccurate grep suggests these are actually the only three packages using this, which surprises me! I’d assumed it was a convention.

It would be nice to have a consensus on this.

qyliss · September 17, 2019, 2:22am

It would be nice to have a consensus on this.

That’s what the RFC process is for.

Every time we introduce yet another way of doing this, no matter how
cool it is, Nixpkgs becomes harder to use. We need to standardize.

lilyball · September 17, 2019, 3:02am

My worry about configure is it’s really close to config which would be confusing.

Regarding a separate function level for options, we’d have to do { deps }: { options }: otherwise we have no good way of actually detecting the presence of the options from callPackage. This would be a possibility, except this version would be hard to use without callPackage (e.g. invoking the package directly from nix-build using --arg to provide the arguments). But I don’t know how important that is; certainly I haven’t tried to directly use a package from nix-build myself outside of accessing it via the nixpkgs root.

My concern here is twofold:

Guaranteeing this is safe with an assert is impractical (do we really want to evaluate every key in nixpkgs and do string manipulation just to assert that it doesn’t conflict with our naming scheme?), which would be unfortunate because someone could accidentally screw this up, especially in an overlay.
This naming scheme isn’t terribly flexible. For example, the preferXsel flag to unison means "use xsel if it’s available, otherwise fall back to xclip) and enableXsel doesn’t really have the same connotations.

qyliss · September 17, 2019, 12:42pm

My worry about configure is it’s really close to config which
would be confusing.

You mean it would be confusing with the “config” in NixOS?

Regarding a separate function level for options, we’d have to do
{ deps }: { options }: otherwise we have no good way of actually
detecting the presence of the options from callPackage. This would
be a possibility, except this version would be hard to use without
callPackage (e.g. invoking the package directly from nix-build
using --arg to provide the arguments). But I don’t know how
important that is; certainly I haven’t tried to directly use a package
from nix-build myself outside of accessing it via the nixpkgs
root.

That would be a shame, but it’s already the case that Nix frequently
evolves to suit Nixpkgs!

My concern here is twofold:

Guaranteeing this is safe with an assert is impractical (do we
really want to evaluate every key in nixpkgs and do string
manipulation just to assert that it doesn’t conflict with our naming
scheme?), which would be unfortunate because someone could
accidentally screw this up, especially in an overlay.

Absolutely! But those problems already exist, and without a strong
rationale for changing the existing packages that use this method, it
will continue to be a problem not matter what we do for new packages.
That’s why I think it’s best to go with consistency for now, and do an
RFC to change it to something better.

I’m in no way defending the current system – it sorely needs to
change. But changing it piecemeal isn’t going to do much to make things
better, especially if 10 different people each introduce their own new
way of doing it.

This naming scheme isn’t terribly flexible. For example, the
preferXsel flag to unison means "use xsel if it’s available,
otherwise fall back to xclip) and enableXsel doesn’t really have
the same connotations.

In this case, it might make sense to use preferXsel, then, although I
think enableXsel would be fine too.

lilyball · September 18, 2019, 6:44am

config exists in nixpkgs too. It contains the contents of ~/.config/nixpkgs/config.nix plus 2 options doCheckByDefault (false by default) and warnings ([] by default).