@GrahamcOfBorg eval

Just played around with this locally and pushed a commit that documents this in the release notes. As soon as ofborg has finished its evaluations, we can merge this 👍

I broke ofborg. Rolling back, deploying, and then will re-evaluate.

@GrahamcOfBorg eval

I don't like this idea for multiple reasons:

• When a different nixpkgs module sets e.g. envVariables, and the user wants to override the configFile, you can't do this with this change, as you don't have control over that module to unset envVariables.
• We have many more options in NixOS already that "silently" defuse other options, but that's just how the module system works with priorities. You provide some special options that write a config file in a special way, but you also allow configFile if you want to override everything. To me it's clear that such configFile options override other options, and when we put this in the option description, it's even clearer.
• These assertions are very bulky and written manually. I don't want to see this all over NixOS modules just for a tiny advantage (which might not even be an advantage, see point 1)

@infinisil I think you're right. I'm closing this PR.

When a different nixpkgs module sets e.g. envVariables, and the user wants to override the configFile, you can't do this with this change, as you don't have control over that module to unset envVariables.

Isn't it possible to use priorities like this here:

{ lib, ... }:

{
  programs.less = {
    envVariables = lib.mkForce {};
  };
}

We have many more options in NixOS already that "silently" defuse other options, but that's just how the module system works with priorities

Well it makes perfect sense with the concept of priorities, but this isn't used in the less module, right?

To me it's clear that such configFile options override other options, and when we put this in the option description, it's even clearer.

When I remember me while trying to learn NixOS two years ago I screwed a lot of stuff up at first due to some misunderstandings, so I have to admit that I don't understand why we should save 4 options that are potentially helpful, especially for beginners :)

Ah yeah, using mkForce would work, but it's still weird to have to explicitly mkForce {} if you didn't set this option in your own configuration to get rid of an assertion failure.

Well it makes perfect sense with the concept of priorities, but this isn't used in the less module, right?

Oh you're right, the module doesn't use priorities as I thought it would:

Usually you'd implement this by using configFile = mkDefault generatedConfigFile, then you wouldn't need nullOr in its type, and people could nixos-option ..configFile to see what the config file is in any case. (I also just noticed that this module uses builtins.readFile on the configFile, which is bad because it disallows config files that depend on derivations)

I have to admit that I don't understand why we should save 4 options that are potentially helpful, especially for beginners :)

In addition to the first point (which should be enough reason on its own), I don't think we should have this because these priority overrides are all over the place in NixOS, and we can't guard against them all (well we could, but that would be a pain). If we had something builtin in the module system to warn us (not error out at least) that would be better (but then again, we have no way to turn off warnings, and you sometimes want to override other options without getting a warning).

Here's some more examples of options overriding other options: systemd.sevices.<name>.serviceConfig.ExecStart overrides systemd.services.<name>.script (and there's many more of such in systemd), many configFile/passwordFile options, environment.etc.<name>.source overrides environment.etc.<name>.text, programs.bash.shellAliases overrides environment.shellAliases, and more.

Ah yeah, using mkForce would work, but it's still weird to have to explicitly mkForce {} if you didn't set this option in your own configuration to get rid of an assertion failure.

I'm afraid I have to disagree here: a hypothetical third-party module for less expects those options to have a certain value to function properly. IMHO explicitly overriding values basically means that you leave a supported "path" and you'll probably experience edge-cases.

I always think twice before using mkForce to achieve something, so I consider such a behavior actually a good thing 😅

Here's some more examples of options overriding other options: systemd.sevices..serviceConfig.ExecStart overrides systemd.services..script (and there's many more of such in systemd), many configFile/passwordFile options, environment.etc..source overrides environment.etc..text, programs.bash.shellAliases overrides environment.shellAliases, and more.

I'm probably missing something, but having i.e. ExecStart and script set causes an evaluation error as I'd expect:

error: The unique option `systemd.services.foo.serviceConfig.ExecStart' is defined multiple times, in `<unknown-file>' and `/home/ma27/Projects/nixpkgs/nixos/modules/system/boot/systemd.nix'.

{programs.bash,environment}.shellAliases seem to be merged together, but you're right, single keys might override each other.

Usually you'd implement this by using configFile = mkDefault generatedConfigFile, then you wouldn't need nullOr in its type, and people could nixos-option ..configFile to see what the config file is in any case. (I also just noticed that this module uses builtins.readFile on the configFile, which is bad because it disallows config files that depend on derivations)

Hmm that sounds in fact way better. Another option that came to my mind was to provide some kind of extraConfig, but that would make merging inside the module system harder again.

Yeah the mkForce point is a good one upon second thought. Ah and yeah the script and ExecStart options I didn't check.

If people really want to see more such assertions, I wouldn't mind seeing a small RFC for this that lists advantages/disadvantages and such.