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

bug: Specialization does not work with nixos modules #3716

Closed
2 tasks done
zoriya opened this issue Feb 28, 2023 · 12 comments · Fixed by #3718
Closed
2 tasks done

bug: Specialization does not work with nixos modules #3716

zoriya opened this issue Feb 28, 2023 · 12 comments · Fixed by #3718
Assignees
Labels
bug triage Issues or feature request that have not been triaged yet

Comments

@zoriya
Copy link

zoriya commented Feb 28, 2023

Are you following the right branch?

  • My Nixpkgs and Home Manager versions are in sync

Is there an existing issue for this?

  • I have searched the existing issues

Issue description

Hello, I am trying to set up specializations for light/dark theme switching, but creating a specialization result in a build error.

error: The option `home-manager.users.zoriya.specialization.test.configuration._module.args.name' is defined multiple times while it's expected to be unique.

       Definition values:
       - In `/nix/store/3jhxmigz3b15xdcdhj0pryq2bg6qyhcv-source/modules/misc/specialization.nix': "zoriya"
       - In `<unknown-file>': "configuration"
       Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions.
(use '--show-trace' to show detailed location information)`

The snippets that make my build crash is the specialization test"

nixpkgs.lib.nixosSystem {
  inherit system;
  specialArgs = inputs;
  modules = [
	{
	  home-manager = {
	    useGlobalPkgs = true;
	    useUserPackages = true;
	    extraSpecialArgs = inputs;
	    users.${user} = {
		    specialization.test.configuration = {
		      home.file.testfile.text = "very special";
		    };
		    home.stateVersion = "22.11";
	    };
	  };
	}
  ];
}

Link to the full config

Maintainer CC

@rycee

System information

- system: `"x86_64-linux"`
 - host os: `Linux 6.2.0, NixOS, 23.05 (Stoat), 23.05.20230222.988cc95`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.13.2`
 - channels(root): `"nixos"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`
@zoriya zoriya added bug triage Issues or feature request that have not been triaged yet labels Feb 28, 2023
@ncfavier
Copy link
Member

ncfavier commented Feb 28, 2023

That's a fun one! cc @roberth @infinisil

Minimal reproducer:

with import <nixpkgs/lib>;
let eval = evalModules {
  modules = [ {
    options.hm = mkOption {
      type = types.submoduleWith {
        modules = [ ({ moduleType, ... }: {
          options.foo = mkOption {
            type = types.str;
          };
          options.special = mkOption {
            type = moduleType;
          };
        }) ];
      };
    };

    config.hm = { name, ... }: {
      foo = name;
      special = {};
    };
  } ];
};
in eval.config.hm.special.foo
error: The option `hm.special._module.args.name' is defined multiple times while it's expected to be unique.

       Definition values:
       - In `<unknown-file>': "hm"
       - In `<unknown-file>': "special"
       Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions.

_module.args.name is defined here, and this ends up being evaluated twice with different values for loc (hm, then hm.special).

A way to solve this would be to add a nameArgument argument to submoduleWith and define _module.args.${nameArgument} instead. The HM NixOS module could set that to userName, and this wouldn't interfere with specialisations because moduleType does not inherit this argument:

config.hm = { userName, name, ... }: {
      foo = "${userName} / ${name}";
      special = {};
    };
  } ];
};
in eval.config.hm.special.foo
"hm / special"

EDIT NixOS/nixpkgs#218812

@roberth
Copy link
Contributor

roberth commented Feb 28, 2023

hm. I've been at this problem a couple of times now. I'd say name is rather unprincipled in how it's implemented.
I reinvented dependentAttrsOf the other day

I've also gone through some iterations of this in the NixOS vm test test matrix PR

Eventually I settled on a solution that didn't need name, avoiding submodule directly inside attrsOf.

@roberth
Copy link
Contributor

roberth commented Feb 28, 2023

Eventually I settled on a solution that didn't need name, avoiding submodule directly inside attrsOf.

Home-manager could structure its specialisations module in the same way NixOS does, which happens to satisfy the same criteria.

The non-moduleType submodule inside attrsOf can forward the username to the moduleType submodule as usual.

That's a breaking change though, which is a bit un-fun.

@roberth
Copy link
Contributor

roberth commented Feb 28, 2023

The breaking change does solve the lack of extensibility in the specialisations attribute. There's currently no way to add a per-specialisation option that isn't inside the home-manager config. NixOS has used such an option to specify whether to inherit the regular config, and other options may come up in the future.

@ncfavier
Copy link
Member

The non-moduleType submodule inside attrsOf can forward the username to the moduleType submodule as usual.

How? I don't see where the NixOS specialisation module does this.

@ncfavier
Copy link
Member

The breaking change does solve the lack of extensibility in the specialisations attribute. There's currently no way to add a per-specialisation option that isn't inside the home-manager config.

Not sure what you mean here, we could already add options besides configuration here.

@roberth
Copy link
Contributor

roberth commented Feb 28, 2023

How? I don't see where the NixOS specialisation module does this.

Right, it doesn't, which could be a good thing. I assumed from the example that the name had to be used for some reason. The goal, for this solution, is to not evaluate the invalid name at all.

@roberth
Copy link
Contributor

roberth commented Feb 28, 2023

Not sure what you mean here, we could already add options besides configuration here.

I must have inferred too much / incorrectly from the example. The trace would be helpful. I wish people just posted traces, but I guess they consider them too verbose...

EDIT: more reason for NixOS/nix#7553 I guess.

@ncfavier
Copy link
Member

Right, I simplified the failing example down to the bare minimum, so I removed the intermediate layer of submodule.

The goal, for this solution, is to not evaluate the invalid name at all.

Aha, the following seems to work:

diff --git a/modules/misc/specialization.nix b/modules/misc/specialization.nix
index 67e593e2..1f93ef00 100644
--- a/modules/misc/specialization.nix
+++ b/modules/misc/specialization.nix
@@ -1,4 +1,4 @@
-{ config, extendModules, lib, ... }:
+{ config, extendModules, name, lib, ... }:
 
 with lib;
 
@@ -8,7 +8,7 @@ with lib;
       options = {
         configuration = mkOption {
           type = let
-            stopRecursion = { specialization = mkOverride 0 { }; };
+            stopRecursion = { _module.args.name = mkForce name; specialization = mkOverride 0 { }; };
             extended = extendModules { modules = [ stopRecursion ]; };
           in extended.type;
           default = { };

@ncfavier
Copy link
Member

And if name isn't passed, that means we're not in a NixOS module, so name isn't evaluated either, so the problem fixes itself by laziness.

ncfavier added a commit to ncfavier/home-manager that referenced this issue Feb 28, 2023
If used inside the NixOS/nix-darwin module, we get conflicting definitions
of `name` inside the specialization: one is the user name coming from the
NixOS module definition and the other is `configuration`, the name of this
option. Thus we need to explicitly wire the former into the module arguments.

See discussion at nix-community#3716
ncfavier added a commit to ncfavier/home-manager that referenced this issue Feb 28, 2023
If used inside the NixOS/nix-darwin module, we get conflicting definitions
of `name` inside the specialization: one is the user name coming from the
NixOS module definition and the other is `configuration`, the name of this
option. Thus we need to explicitly wire the former into the module arguments.

See discussion at nix-community#3716
@ncfavier
Copy link
Member

#3718

ncfavier added a commit that referenced this issue Feb 28, 2023
If used inside the NixOS/nix-darwin module, we get conflicting definitions
of `name` inside the specialization: one is the user name coming from the
NixOS module definition and the other is `configuration`, the name of this
option. Thus we need to explicitly wire the former into the module arguments.

See discussion at #3716
@zoriya
Copy link
Author

zoriya commented Mar 1, 2023

Wow, that was fast. Thanks a lot!

15cm pushed a commit to 15cm/home-manager that referenced this issue Apr 22, 2023
…y#3718)

If used inside the NixOS/nix-darwin module, we get conflicting definitions
of `name` inside the specialization: one is the user name coming from the
NixOS module definition and the other is `configuration`, the name of this
option. Thus we need to explicitly wire the former into the module arguments.

See discussion at nix-community#3716
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Issues or feature request that have not been triaged yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants