lib/modules: implement a way to override a property based on its previous value

[nrabulinski]

Description of changes

This is by no means meant to be a complete implementation. It's in fact suboptimal and not even fully functional, but I got it done to show that it's possible and that it could prove useful.
My idea is to introduce mkOverrideWith (name subject to change) which would work like the current mkOverride with one caveat: instead of taking an attrset it takes a lambda, which accepts the next highest priority value as its argument. This allows to set a new value for a given property while reusing the previous value.

Why would this be useful

Personally I see one big upside of having a mechanism like this: not having to introduce IFD or having to resort to extendModules.
Examples:

• stylix works by generating the palette in a derivation, then imports said derivation and lets the user use the colors in their config. With this approach, one could simply assign placeholders values to properties, and the colors would be substituted at build time. The code for that could look something like this

# helix.nix
{
	programs.helix = {
		enable = true;
		settings.themes = {
			stylix = {
				"ui.background" = {bg = "@base00@";};
				"ui.virtual" = "@base03@";
				"ui.menu" = {
					fg = "@base05@";
					bg = "@base01@";
				};
			};
		};
	};
}
# stylix.nix
{lib, pkgs, ...}: {
	xdg.configFile."helix/themes/stylix.toml".source =
		lib.mkForceWith
		(prev: pkgs.runCommand "helix-theme" {} ''
			substituteColors ${prev} > $out
		'');
}

• Secrets in configuration files: Currently the only way to have secrets in configuration files is to write the whole configuration by hand, encrypt it, and then give path to it with sops or agenix. The alternative is to use scalpel (or handroll a similar solution) but that requires using extendModules or other suboptimal solutions. While what I'm proposing here still wouldn't be ideal (I'm happy for someone more knowledgable about the module system to come up with a better solution based on this idea), you could potentially do something like this

# config.nix
{
	services.mosquitto = {
		enable = true;
		bridges.br1.settings = {
			remote_password = "!!BR1_PASSWORD!!";
		};
	};
}
# secrets.nix
{config, lib, pkgs, ...}: {
	config = lib.mkOverrideWith 20 (prev: lib.recursiveUpdate prev (let
		prevVal = prev.systemd.services.mosquitto.serviceConfig.ExecStart;
		prevFile = builtins.head (builtins.match ".*-c ([^[:space:]]+)" prevVal;
	in {
		systemd.services.mosquitto.serviceConfig.ExecStart =
			builtins.replaceStrings
			[ prevFile ]
			[ "${config.scalpel.trafos."mosquitto.conf".destination} "]
			prevVal;
		scalpel.trafos."mosquitto.conf" = {
			source = prevFile;
			matchers."BR1_PASSWORD".secret = config.sops.secrets.br1passwd.path;
			owner = "mosquitto";
			group = "mosquitto";
			mode = "0440";
		};
	}));
}

• Patching packages for modules which themselves apply some patches. Now, this is probably a sign that a module has subpar API, but I think we've all seen that happen in nixpkgs and having a way of easily dealing with that could be beneficial. Instead of replicating what the module does with the package one could simply do

{lib, ...}: {
	programs.foo.package = lib.mkForceWith (pkg: pkg.overrideAttrs { ... });
}

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[infinisil]

This looks like #157070 or #155903, please take a look at those threads

[nrabulinski]

@infinisil I read through those PRs and while similar, I don't think they are quite the same and, in my humble opinion, my proposal, assuming someone would come in and fully implement it, could prove more useful and composable. From what I understand it, neither of those previously suggested approaches work with priorities. Instead, the one @roberth suggested transforms a default value, while the one by @babbaj transforms the value of the current priority, which introduces more complexity and problems than it solves.
This is why I think if we were to introduce a function like this, the easiest solution while still being plenty powerful is to have it transform the merged value of priority below the one we're assigning (i.e. mkForceWith takes as an argument the merged value if priority was restricted to 51, etc)

[nrabulinski]

Also, neither contains real-world usecases except for removing a value from a list, which could easily be solved with other solutions, while the ones I showed I can't really see how else one could solve. And while mkPostMerge could be used to solve that, as I already mentioned, I think utilizing priorities here would simplify the implementation and make things clearer

[roberth]

I'm not sure that we should have this. Encoding logic into priority numbers is ultimately just not an easy to understand concept, and it is not declarative. I've closed my own PR for this reason.

neither of those previously suggested approaches work with priorities. Instead, the one @roberth suggested transforms a default value

Did you try it? I'm pretty sure it uses the next priority in line; not just mkDefault priority or something like that. I might have used "default" as a shortcut in an explanation.

Not to give you false hope, but in general I would recommend to split out refactorings from the actual change, into separate commits, because there's a lot going on in the diff that's not essential.

[nrabulinski]

Not to give you false hope, but in general I would recommend to split out refactorings from the actual change, into separate commits, because there's a lot going on in the diff that's not essential.

Naturally, which is why the commit message says it's a proof-of-concept implementation and why I repeatedly state that the implementation is not complete and, in fact, it's broken 😃

[nrabulinski]

Did you try it? I'm pretty sure it uses the next priority in line; not just mkDefault priority or something like that. I might have used "default" as a shortcut in an explanation.

You're right, I didn't try it, I just read your PR and saw that you moved to #157070 and that's what I based my opinion on.

I'm not sure that we should have this. Encoding logic into priority numbers is ultimately just not an easy to understand concept, and it is not declarative. I've closed my own PR for this reason.

And what makes you feel like mkPostMerge is better? In my opinion it's more restrictive (thus being less useful, and the situations it is useful for have better solutions) while arguably being more complicated to explain, if anything. I agree that the concept is not declarative or "pure", but I don't understand what's more complicated about it than override already is. Unless you believe we shouldn't have override as a whole?

[roberth]

but I don't understand what's more complicated about it than override already is.

By making the overriding feature more capable and complex, we remove an incentive to write good declarative modules that properly support the use cases that might also be achievable with mkForceWith (or mkPostMerge or whatever).
Users will be worse off, for being required to program rather than treat configuration.nix as a configuration file. Configuration UIs can not feasibly support it either.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/remove-from-list-in-nixos-module-or-homemanager/54761/6

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/from-a-nixos-module-is-it-possible-to-delete-an-entry-from-an-attrset-defined-by-a-different-module/73289/15