lib/types: dont warn loaOf for home-manager namespace #77575
Conversation
This option namespace is not a part of NixOS so we shouldn't provide this warning for it.
worldofpeace
requested review from
edolstra,
infinisil and
nbp
as code owners
ofborg
bot
added
10.rebuild-darwin: 0
|
I understand but I don't quite agree on this. Sure, home-manager is not a NixOS project, so it should be their responsibility to handle this. However it's a popular application among NixOS users and, I say, for the sake of UX we should provide clearer errors (especially if home-manager can't implement this, or can it?). |
|
They can create their own elemType: types.loaOf elemType // {
merge = /* copy of our merge with attrNames replaced */;
}However, I wonder if we should expose the merge function parametrizable by the elemType: let
origType = types.loaOf elemType;
in origType // {
merge = origType._mergeWithAttrNames [
/* home-manager attrNames */
];
}cc @rycee |
We should, as @jtojnar suggests, allow home-manager to utilize NixOS |
|
Do not have time to test it right now but the following might be sufficient: --- a/lib/types.nix
+++ b/lib/types.nix
@@ -329,57 +329,18 @@ rec {
# List or attribute set of ...
loaOf = elemType:
let
- convertAllLists = loc: defs:
+ convertAllLists = nameAttrs: loc: defs:
let
padWidth = stringLength (toString (length defs));
unnamedPrefix = i: "unnamed-" + fixedWidthNumber padWidth i + ".";
in
- imap1 (i: convertIfList loc (unnamedPrefix i)) defs;
- convertIfList = loc: unnamedPrefix: def:
+ imap1 (i: convertIfList nameAttrs loc (unnamedPrefix i)) defs;
+ anyString = placeholder "name";
+ convertIfList = nameAttrs: loc: unnamedPrefix: def:
if isList def.value then
let
padWidth = stringLength (toString (length def.value));
unnamed = i: unnamedPrefix + fixedWidthNumber padWidth i;
- anyString = placeholder "name";
- nameAttrs = [
- { path = [ "environment" "etc" ];
- name = "target";
- }
- { path = [ "containers" anyString "bindMounts" ];
- name = "mountPoint";
- }
- { path = [ "programs" "ssh" "knownHosts" ];
- # hostNames is actually a list so we would need to handle it only when singleton
- name = "hostNames";
- }
- { path = [ "fileSystems" ];
- name = "mountPoint";
- }
- { path = [ "boot" "specialFileSystems" ];
- name = "mountPoint";
- }
- { path = [ "services" "znapzend" "zetup" ];
- name = "dataset";
- }
- { path = [ "services" "znapzend" "zetup" anyString "destinations" ];
- name = "label";
- }
- { path = [ "services" "geoclue2" "appConfig" ];
- name = "desktopID";
- }
- { path = [ "home-manager" "users" anyString "programs" "ssh" "matchBlocks" ];
- name = "host"; # https://github.com/rycee/home-manager/blob/e8dbc3561373b68d12decb3c0d7c1ba245f138f7/modules/programs/ssh.nix#L265
- }
- { path = [ "home-manager" "users" anyString "home" "file" ];
- name = "target"; # https://github.com/rycee/home-manager/blob/0e9b7aab3c6c27bf020402e0e2ef20b65c040552/modules/files.nix#L33
- }
- { path = [ "home-manager" "users" anyString "xdg" "configFile" ];
- name = "target"; # https://github.com/rycee/home-manager/blob/54de0e1d79a1370e57a8f23bef89f99f9b92ab67/modules/misc/xdg.nix#L41
- }
- { path = [ "home-manager" "users" anyString "xdg" "dataFile" ];
- name = "target"; # https://github.com/rycee/home-manager/blob/54de0e1d79a1370e57a8f23bef89f99f9b92ab67/modules/misc/xdg.nix#L58
- }
- ];
matched = let
equals = a: b: b == anyString || a == b;
fallback = { name = "name"; };
@@ -430,12 +391,45 @@ rec {
lib.warn msg res
else
def;
+
+ mergeLoaOf = rec {
+ _anyString = anyString;
+ __nameAttrs = [
+ { path = [ "environment" "etc" ];
+ name = "target";
+ }
+ { path = [ "containers" anyString "bindMounts" ];
+ name = "mountPoint";
+ }
+ { path = [ "programs" "ssh" "knownHosts" ];
+ # hostNames is actually a list so we would need to handle it only when singleton
+ name = "hostNames";
+ }
+ { path = [ "fileSystems" ];
+ name = "mountPoint";
+ }
+ { path = [ "boot" "specialFileSystems" ];
+ name = "mountPoint";
+ }
+ { path = [ "services" "znapzend" "zetup" ];
+ name = "dataset";
+ }
+ { path = [ "services" "znapzend" "zetup" anyString "destinations" ];
+ name = "label";
+ }
+ { path = [ "services" "geoclue2" "appConfig" ];
+ name = "desktopID";
+ }
+ ];
+ _mergeWithNameAttrs = nameAttrs: loc: defs: attrOnly.merge loc (convertAllLists nameAttrs loc defs);
+ __functor = self: _mergeWithNameAttrs __nameAttrs;
+ };
attrOnly = attrsOf elemType;
in mkOptionType rec {
name = "loaOf";
description = "list or attribute set of ${elemType.description}s";
check = x: isList x || isAttrs x;
- merge = loc: defs: attrOnly.merge loc (convertAllLists loc defs);
+ merge = mergeLoaOf;
emptyValue = { value = {}; };
getSubOptions = prefix: elemType.getSubOptions (prefix ++ ["<name?>"]);
getSubModules = elemType.getSubModules; |
|
There is already a fix for this in home-manager: nix-community/home-manager#984 |
|
@misuzu that only fixes the warning producing definitions inside home-manager modules. If user sets any of those options in their @rycee will need to decide which of these variants they want:
|
|
All internal use of the list form of For the SSH hosts option I will have to ponder a bit. Most likely I'll try to figure out some nice way to introduce a |
|
Well, {
a = { arrows = [ "b" "c" "d" "e" ]; };
b = { arrows = [ "c" ]; };
c = { arrows = [ "d" "e" ]; };
d = { arrows = [ "e" ]; };
e = { arrows = [ ]; };
} |
This option namespace is not a part of NixOS
so we shouldn't provide this warning for it.
Motivation for this change
#77501 #77501 (comment).
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)