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

nixos/modules/programs/dconf: redesign profiles #189099

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/dconf.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{ lib }:

# This module contains helpers for the `programs.dconf` NixOS module

rec {
types = {
tuple = "_tuple";
};

mkTuple = _elements: {
inherit _elements;

_type = types.tuple;
};
}
Comment on lines +5 to +15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need this here? Why not put it into the module.

Copy link
Member

@jtojnar jtojnar Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This incomplete implementation of GVariant type system. Ideally, we would have a full implementation like home-manager has and having it separately is nicer for using it in different context, including tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is the full one, we should keep it separate. Maybe add a comment, that it isn't complete and maybe with a ref to hm.

1 change: 1 addition & 0 deletions lib/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ let

# serialization
cli = callLibs ./cli.nix;
dconf = callLibs ./dconf.nix;
generators = callLibs ./generators.nix;

# misc
Expand Down
44 changes: 31 additions & 13 deletions lib/generators.nix
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,18 @@ rec {
+ "\n")
+ (toINI { inherit mkSectionName mkKeyValue listsAsDuplicateKeys; } sections);

# converts { a.b.c = 5; } to { "a.b".c = 5; } for toINI
flattenAttrs = with builtins; extraIsLeafPredicate: sep: let
recurse = path: value:
if isAttrs value && !lib.isDerivation value && !extraIsLeafPredicate value then
lib.mapAttrsToList (name: value: recurse ([ name ] ++ path) value) value
else if length path > 1 then {
${concatStringsSep sep (lib.reverseList (tail path))}.${head path} = value;
} else {
${head path} = value;
};
in attrs: lib.foldl lib.recursiveUpdate { } (lib.flatten (recurse [ ] attrs));

/* Generate a git-config file from an attrset.
*
* It has two major differences from the regular INI format:
Expand Down Expand Up @@ -213,21 +225,27 @@ rec {
let mkKeyValue = mkKeyValueDefault { } " = " k;
in concatStringsSep "\n" (map (kv: "\t" + mkKeyValue kv) (lib.toList v));

# converts { a.b.c = 5; } to { "a.b".c = 5; } for toINI
gitFlattenAttrs = let
recurse = path: value:
if isAttrs value && !lib.isDerivation value then
lib.mapAttrsToList (name: value: recurse ([ name ] ++ path) value) value
else if length path > 1 then {
${concatStringsSep "." (lib.reverseList (tail path))}.${head path} = value;
} else {
${head path} = value;
};
in attrs: lib.foldl lib.recursiveUpdate { } (lib.flatten (recurse [ ] attrs));

toINI_ = toINI { inherit mkKeyValue mkSectionName; };
in
toINI_ (gitFlattenAttrs attrs);
toINI_ (flattenAttrs (_: false) "." attrs);

/* mkValueStringDefault wrapper that handles dconf INI quirks. */
mkValueStringDconf = v:
if builtins.isList v then
"[${lib.concatMapStringsSep ", " mkValueStringDconf v}]"
else if lib.types.isType lib.dconf.types.tuple v then
"(${lib.concatMapStringsSep ", " mkValueStringDconf v._elements})"
else if builtins.isString v then
"'${v}'"
else mkValueStringDefault {} v;

/* mkKeyValueDefault wrapper that handles dconf INI quirks. */
mkKeyValueDconf = mkKeyValueDefault { mkValueString = mkValueStringDconf; } "=";

/* Generates INI in dconf keyfile style.
* The main differences of the format is that it requires strings to be quoted and has a tuple type (`lib.dconf.mkTuple`).
*/
toDconfINI = attrs: toINI { mkKeyValue = mkKeyValueDconf; } (flattenAttrs (x: lib.types.isType lib.dconf.types.tuple x) "/" attrs);

/* Generates JSON from an arbitrary (non-function) value.
* For more information see the documentation of the builtin.
Expand Down
20 changes: 20 additions & 0 deletions nixos/doc/manual/from_md/release-notes/rl-2211.section.xml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@
would in NixOS 22.05 and earlier.
</para>
</listitem>
<listitem>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer do xml files in nixos/* . Also please carry over the other changelog to 23.05

<para>
The <literal>programs.dconf</literal> module now supports
configuration using the Nix language. This allows you to
configure settings of apps and DEs (if they use
<literal>dconf</literal>) declaratively without having to use
<literal>extraGSettingsOverrides</literal> which has some
problems noted in
<link xlink:href="https://github.com/NixOS/nixpkgs/issues/54150">this
issue</link>.
</para>
</listitem>
<listitem>
<para>
PHP now defaults to PHP 8.1, updated from 8.0.
Expand Down Expand Up @@ -651,6 +663,14 @@
<literal>emacs-gtk</literal>.
</para>
</listitem>
<listitem>
<para>
The option <literal>programs.dconf.packages</literal> has been
removed, use
<literal>programs.dconf.profiles.user.databases</literal>
instead.
</para>
</listitem>
<listitem>
<para>
riak package removed along with
Expand Down
7 changes: 7 additions & 0 deletions nixos/doc/manual/release-notes/rl-2211.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ In addition to numerous new and upgraded packages, this release has the followin
Alternatively, you can remove the `hostPlatform` line and use NixOS like you
would in NixOS 22.05 and earlier.

- The `programs.dconf` module now supports configuration using the Nix language.
This allows you to configure settings of apps and DEs (if they use `dconf`)
declaratively without having to use `extraGSettingsOverrides` which has some
problems noted in [this issue](https://github.com/NixOS/nixpkgs/issues/54150).

- PHP now defaults to PHP 8.1, updated from 8.0.

- `protonup` has been aliased to and replaced by `protonup-ng` due to upstream not maintaining it.
Expand Down Expand Up @@ -212,6 +217,8 @@ Available as [services.patroni](options.html#opt-services.patroni.enable).
- Emacs now uses the Lucid toolkit by default instead of GTK because of stability and compatibility issues.
Users who still wish to remain using GTK can do so by using `emacs-gtk`.

- The option `programs.dconf.packages` has been removed, use `programs.dconf.profiles.user.databases` instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can definitely be a mkRenamedOption


- riak package removed along with `services.riak` module, due to lack of maintainer to update the package.

- ppd files in `pkgs.cups-drv-rastertosag-gdi` are now gzipped. If you refer to such a ppd file with its path (e.g. via [hardware.printers.ensurePrinters](options.html#opt-hardware.printers.ensurePrinters)) you will need to append `.gz` to the path.
Expand Down
2 changes: 1 addition & 1 deletion nixos/modules/i18n/input-method/ibus.nix
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ in
# Without dconf enabled it is impossible to use IBus
programs.dconf.enable = true;

programs.dconf.packages = [ ibusPackage ];
programs.dconf.profiles.ibus.databases = [ (pkgs.dconf-utils.mkDconfDb "${ibusPackage}/etc/dconf/db/ibus.d") ];

services.dbus.packages = [
ibusPackage
Expand Down
80 changes: 40 additions & 40 deletions nixos/modules/programs/dconf.nix
Original file line number Diff line number Diff line change
@@ -1,56 +1,55 @@
{ config, lib, pkgs, ... }:

with lib;

let
cfg = config.programs.dconf;
cfgDir = pkgs.symlinkJoin {
name = "dconf-system-config";
paths = map (x: "${x}/etc/dconf") cfg.packages;
postBuild = ''
mkdir -p $out/profile
mkdir -p $out/db
'' + (
concatStringsSep "\n" (
mapAttrsToList (
name: path: ''
ln -s ${path} $out/profile/${name}
''
) cfg.profiles
)
) + ''
${pkgs.dconf}/bin/dconf update $out/db
'';
};

asFileDb = val:
let db =
if lib.isAttrs val && !lib.isDerivation val then
pkgs.dconf-utils.mkDconfDb "${pkgs.writeTextDir "dconf/db" (lib.generators.toDconfINI val)}/dconf"
else val;
in "file-db:${db}";
in
{
###### interface
imports = [
(lib.mkRemovedOptionModule [ "programs" "dconf" "packages" ] "This option is not supported anymore, you should use `programs.dconf.profiles.<profile>.databases` instead.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We canno use mkRenamedOption here?

];

options = {
programs.dconf = {
enable = mkEnableOption (lib.mdDoc "dconf");
enable = lib.mkEnableOption (lib.mdDoc "dconf");

profiles = lib.mkOption {
type = with lib.types; attrsOf (submodule {
options = {
enableUserDb = lib.mkOption {
type = bool;
default = true;
description = lib.mdDoc "Add `user-db:user` at the beginning of the profile.";
};

profiles = mkOption {
type = types.attrsOf types.path;
default = {};
description = lib.mdDoc "Set of dconf profile files, installed at {file}`/etc/dconf/profiles/«name»`.";
internal = true;
databases = lib.mkOption {
type = with lib.types; listOf (oneOf [ attrs str path package ]);
default = [];
description = lib.mdDoc "List of data sources for the profile. An element can be an attrset, or the path of an already compiled database.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing explanation of string argument.

};
};
});
description = lib.mdDoc "Attrset of dconf profiles.";
};

packages = mkOption {
type = types.listOf types.package;
default = [];
description = lib.mdDoc "A list of packages which provide dconf profiles and databases in {file}`/etc/dconf`.";
defaultProfile = lib.mkOption {
type = with lib.types; nullOr str;
default = null;
description = lib.mdDoc "The default dconf profile.";
};
};
};

###### implementation

config = mkIf (cfg.profiles != {} || cfg.enable) {
environment.etc.dconf = mkIf (cfg.profiles != {} || cfg.packages != []) {
source = cfgDir;
};
config = lib.mkIf cfg.enable {
environment.etc = lib.attrsets.mapAttrs' (name: value: lib.nameValuePair "dconf/profile/${name}" {
text = lib.concatMapStrings (x: "${x}\n") ((lib.optional value.enableUserDb "user-db:user") ++ (map asFileDb value.databases));
}) cfg.profiles;

services.dbus.packages = [ pkgs.dconf ];

Expand All @@ -59,8 +58,9 @@ in
# For dconf executable
environment.systemPackages = [ pkgs.dconf ];

# Needed for unwrapped applications
environment.sessionVariables.GIO_EXTRA_MODULES = mkIf cfg.enable [ "${pkgs.dconf.lib}/lib/gio/modules" ];
environment.sessionVariables = {
# Needed for unwrapped applications
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we rather fix those or is the overhead so big, that we rather not wrap this at all?

GIO_EXTRA_MODULES = [ "${pkgs.dconf.lib}/lib/gio/modules" ];
} // (if cfg.defaultProfile != null then { DCONF_PROFILE = cfg.defaultProfile; } else {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} // (if cfg.defaultProfile != null then { DCONF_PROFILE = cfg.defaultProfile; } else {});
} // lib.optionalAttrs (cfg.defaultProfile != null) { DCONF_PROFILE = cfg.defaultProfile; };

};

}
38 changes: 6 additions & 32 deletions nixos/modules/services/x11/display-managers/gdm.nix
Original file line number Diff line number Diff line change
Expand Up @@ -229,39 +229,13 @@ in

systemd.user.services.dbus.wantedBy = [ "default.target" ];

programs.dconf.profiles.gdm =
let
customDconf = pkgs.writeTextFile {
name = "gdm-dconf";
destination = "/dconf/gdm-custom";
text = ''
${optionalString (!cfg.gdm.autoSuspend) ''
[org/gnome/settings-daemon/plugins/power]
sleep-inactive-ac-type='nothing'
sleep-inactive-battery-type='nothing'
sleep-inactive-ac-timeout=0
sleep-inactive-battery-timeout=0
''}
'';
programs.dconf.profiles.gdm.databases = [ "${gdm}/share/gdm/greeter-dconf-defaults" ] ++ lib.lists.optional cfg.gdm.autoSuspend {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
programs.dconf.profiles.gdm.databases = [ "${gdm}/share/gdm/greeter-dconf-defaults" ] ++ lib.lists.optional cfg.gdm.autoSuspend {
programs.dconf.profiles.gdm.databases = [ "${gdm}/share/gdm/greeter-dconf-defaults" ]
++ lib.lists.optional cfg.gdm.autoSuspend {

org.gnome.settings-daemon.plugins.power = {
sleep-inactive-ac-type = "nothing";
sleep-inactive-battery-type = "nothing";
sleep-inactive-ac-timeout = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should require an explicit integer type (eg. lib.gvariant.mkInt). For example, org.gnome.desktop.session’s idle-delay requires uint32 and people regularly trip on it, passing it a Nix number in home-manager, which implicitly gets turned into an int.

Alternative would be making the dconf module internal and creating a separate GSettings module that would also perform validation against schema.

sleep-inactive-battery-timeout = 0;
};

customDconfDb = pkgs.stdenv.mkDerivation {
name = "gdm-dconf-db";
buildCommand = ''
${pkgs.dconf}/bin/dconf compile $out ${customDconf}/dconf
'';
};
in pkgs.stdenv.mkDerivation {
name = "dconf-gdm-profile";
buildCommand = ''
# Check that the GDM profile starts with what we expect.
if [ $(head -n 1 ${gdm}/share/dconf/profile/gdm) != "user-db:user" ]; then
echo "GDM dconf profile changed, please update gdm.nix"
exit 1
fi
# Insert our custom DB behind it.
sed '2ifile-db:${customDconfDb}' ${gdm}/share/dconf/profile/gdm > $out
'';
};

# Use AutomaticLogin if delay is zero, because it's immediate.
Expand Down
10 changes: 10 additions & 0 deletions pkgs/development/libraries/dconf/utils.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{ runCommand, dconf }:

{
# Builds a dconf database from a keyfile directory
mkDconfDb = dir: runCommand "dconf-db" {
nativeBuildInputs = [ dconf ];
} ''
dconf compile $out ${dir}
'';
}
2 changes: 2 additions & 0 deletions pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -3703,6 +3703,8 @@ with pkgs;

dconf = callPackage ../development/libraries/dconf { };

dconf-utils = callPackage ../development/libraries/dconf/utils.nix { };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very tiny, please move it into dconf itself.

Copy link
Member

@jtojnar jtojnar Apr 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is only tiny because it is very incomplete. We will eventually want something like https://github.com/nix-community/home-manager/blob/ec06f419af79207b33d797064dfb3fc9dbe1df4a/modules/lib/gvariant.nix Never mind, was looking at a wrong file.


ddate = callPackage ../tools/misc/ddate { };

ddosify = callPackage ../development/tools/ddosify { };
Expand Down