nixos/dconf: support generating from attrs

[linsui]

Description of changes

This implementation is partly based on #189099 and home-manager's dconf implementation and addresses some related reviews. A main difference between this and #189099 is that this implementation is completely backward-compatible and has lockdown support.

Example:

with lib.gvariant;
{
  enable = true;
  profiles.user.databases = [
    {
      settings = {
        "com/raggesilver/BlackBox" = {
          opacity = mkUint32 100;
          theme-dark = "Tommorow Night";
          scrollback-lines = mkUint32 10000;
        };
      };
      locks = [
        "/com/raggesilver/BlackBox/theme-dark"
      ];
    }
  ];
}

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 23.05 Release Notes (or backporting 22.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.

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/2319

[jtojnar]

Thanks. Will try to review this tomorrow or on Monday (if I do not forget or get sidetracked).

[jtojnar]

Also reading discourse.nixos.org/t/write-key-value-using-lib-hm-gvariant-for-home-manager/31234, I realized we might eventually want mkValue to turn Nix attrsets into GVariant dictionaries with string keys. If we allow nested keys, it would not be possible.

I thought we need to convert the attrs to a GVariant dictionaries first anyway.

Right but we might still implement it with something like the following:

@@ -68,6 +69,11 @@ rec {
       else mkArray v
     else if isGVariant v then
       v
+    else if builtins.isAttrs v then
+      lib.throwIf
+        (v == {})
+        "Please create an empty dictionary with something like `lib.gvariant.mkEmptyArray (lib.gvariant.type.dictionaryEntryOf lib.gvariant.type.string lib.gvariant.type.int32)`"
+        (lib.mapAttrsToList mkDictionaryEntry v)
     else
       throw "The GVariant type of ${v} can't be inferred.";
 

But I have not considered all the downsides yet so it is probably best left for future PRs as well.

---

Yes, NixOS modules use nested attribute sets for their options but dconf is not NixOS. When modelling a domain, we should stick as closely as possible to it.

But I thought the whole point of toINI is expressing ini files with native nix stype attrs. It's reasonable to me to convert the dconf path as a nested attrs.

The use of toINI is implementation detail IMO – the point is modelling dconf database contents in Nix. With that in mind, I think automatically converting leaf values makes sense for convenience, just like we do with RFC 42 settings . But I am still not convinced replicating the hierarchy is needed.

In INI-based settings, you often have two levels: sections and keys. dconf appears hierarchical but in practice, people tend to think in two levels as well: GSettings schemas (analogous to INI sections, as they are used as such in keyfiles) and keys. This is also evidenced in the way you write the examples:

test.not.locked = mkInt32 1;
test.is.locked = "locked";

not

test = {
  not.locked = mkInt32 1;
  is.locked = "locked";
};

(Though I admit nesting would feel natural in some use cases, e.g. with relocatable schemas. But those are comparatively rare so I am still not convinced the added possibilities for silent misconfiguration is worth it.)

(Actually, dconf describes itself as a key database, which is also shown in commands dconf read /org/gnome/desktop/wm/keybindings/move-to-center and lockfiles. So maybe we should go even further and have the settings be single level, and leave the nesting for future GSettings schema-based higher-level module.)

See also the environment.etc NixOS option for another instance of this pattern.

I am not saying I will never be convinced the other way but for now I think the paths as strings close fewer doors. It can always be extended with nesting-attributes paths support or mkValue for dictionaries later, while the other way is not possible.

[linsui]

I removed all features related to the nested format.

[jtojnar]

Looks perfect to me now. Thanks for bearing with me.

[jtojnar]

Looks like the tests do not actually run.

Also, I am thinking of getting rid of mkEmptyArray since it needs to be replaced mkArray when one decides to to add elements. Something like the following.

diff --git a/lib/gvariant.nix b/lib/gvariant.nix
index 3142ffc5f149..06bd3ef6a297 100644
--- a/lib/gvariant.nix
+++ b/lib/gvariant.nix
@@ -66,7 +66,11 @@ rec {
     else if builtins.isString v then
       mkString v
     else if builtins.isList v then
-      mkArray v
+      let
+        vs = map mkValue v;
+        head = lib.throwIf (v == [ ]) "Attempting to create an empty array but mkValue does not allow specifying type. Please create empty array with mkArray and pass the type explicitly." (builtins.head vs);
+      in
+      mkArray head.type vs
     else if isGVariant v then
       v
     else
@@ -75,15 +79,15 @@ rec {
   /* Returns the GVariant array from the given type of the elements and a Nix list.
 
      Type:
-       mkArray :: [Any] -> gvariant
+       mkArray :: gvariant.type -> [Any] -> gvariant
 
      Example:
        # Creating a string array
-       lib.gvariant.mkArray [ "a" "b" "c" ]
+       lib.gvariant.mkArray  [ "a" "b" "c" ]
   */
-  mkArray = elems:
+  mkArray = elemType: elems:
     let
-      vs = map mkValue (lib.throwIf (elems == [ ]) "Please create empty array with mkEmptyArray." elems);
+      vs = map mkValue elems;
       elemType = lib.throwIfNot (lib.all (t: (head vs).type == t) (map (v: v.type) vs))
         "Elements in a list should have same type."
         (head vs).type;
@@ -93,19 +97,6 @@ rec {
         "@${self.type} [${concatMapStringsSep "," toString self.value}]";
     };
 
-  /* Returns the GVariant array from the given empty Nix list.
-
-     Type:
-       mkEmptyArray :: gvariant.type -> gvariant
-
-     Example:
-       # Creating an empty string array
-       lib.gvariant.mkEmptyArray (lib.gvariant.type.string)
-  */
-  mkEmptyArray = elemType: mkPrimitive (type.arrayOf elemType) [ ] // {
-    __toString = self: "@${self.type} []";
-  };
-
 
   /* Returns the GVariant variant from the given Nix value. Variants are containers
      of different GVariant type.
diff --git a/lib/tests/modules/gvariant.nix b/lib/tests/modules/gvariant.nix
index a792ebf85b74..d2bbbbcc88b2 100644
--- a/lib/tests/modules/gvariant.nix
+++ b/lib/tests/modules/gvariant.nix
@@ -31,12 +31,12 @@ in {
         { uint64 = mkUint64 42; }
 
         { array1 = [ "one" ]; }
-        { array1 = mkArray [ "two" ]; }
-        { array2 = mkArray [ (mkInt32 1) ]; }
-        { array2 = mkArray [ (nkUint32 2) ]; }
+        { array1 = mkArray type.string [ "two" ]; }
+        { array2 = mkArray type.int32 [ (mkInt32 1) ]; }
+        { array2 = mkArray type.uint32 [ (nkUint32 2) ]; }
 
         { emptyArray1 = [ ]; }
-        { emptyArray2 = mkEmptyArray type.uint32; }
+        { emptyArray2 = mkArray type.uint32 []; }
 
         { string = "foo"; }
         { string = "foo"; }

[linsui]

So when the type can be inferred the user don't need to use mkArray and mkArray is only used for empty list?

[jtojnar]

Yes.

…

On Tue, 12 Sept 2023, 04:19 linsui, ***@***.***> wrote: So when the type can be infered the user don't need to use mkArray and mkArray is only used for empty list? — Reply to this email directly, view it on GitHub <#234615 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFMEY4MAKUS27TXX4MDRNTXZ7BDZANCNFSM6AAAAAAYR2S26Y> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[linsui]

Then I thought the situation doesn't change. Currently if the user want to add elements to an empty array they need to remove the mkEmptyArray and type. With your proposal, the user needs to remove the mkArray and type.

[jtojnar]

They do not need to, the explicit constructor would be fine to use.

[linsui]

How about making mkArray only require the type when creating an empty array?

[linsui]

Looks like the tests do not actually run.

How can I fix this?

[jtojnar]

How about making mkArray only require the type when creating an empty array?

I think having functions that take different number of arguments depending what those arguments are can be confusing (e.g. it is not possible to express it using type annotations).

Looks like the tests do not actually run.

How can I fix this?

One way would be adding assertions to https://github.com/NixOS/nixpkgs/blob/fff294ce97764ee8ec20aeb24b20fff114aebbd2/lib/tests/modules.sh

The other would be creating file like https://github.com/NixOS/nixpkgs/blob/fff294ce97764ee8ec20aeb24b20fff114aebbd2/lib/tests/misc.nix and then adding it to

nixpkgs/lib/tests/check-eval.nix

Line 3 in 55ec5ae

let tests = [ "misc" "systems" ];

The latter might be cleaner and faster (no need to involve the module system for anything but merging) but might require more changes.

[nixos-discourse]

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

https://discourse.nixos.org/t/configuration-of-gnome-extensions/33337/2

[nixos-discourse]

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

https://discourse.nixos.org/t/need-help-for-nixos-gnome-scaling-settings/24590/12