nixos/systemd: Implement a packages option for tmpfiles #93073

dasJ · 2020-07-13T16:13:53Z

Motivation for this change

Things done

dasJ · 2020-07-13T16:14:06Z

This is a draft since my tests are still building.

flokli

At least kresd and colord could also be updated to make use of the new option, instead of using environment.etc by themselves.

flokli · 2020-07-13T19:23:30Z

nixos/modules/services/networking/kresd.nix

@@ -134,8 +134,7 @@ in {
      CacheDirectoryMode = "0750";
    };

-    environment.etc."tmpfiles.d/knot-resolver.conf".source =
-      "${package}/lib/tmpfiles.d/knot-resolver.conf";
+    systemd.tmpfiles.packages = package;


Suggested change

systemd.tmpfiles.packages = package;

systemd.tmpfiles.packages = [ package ];

flokli · 2020-07-13T19:23:40Z

nixos/modules/services/x11/colord.nix

@@ -26,7 +26,7 @@ in {

    systemd.packages = [ pkgs.colord ];

-    environment.etc."tmpfiles.d/colord.conf".source = "${pkgs.colord}/lib/tmpfiles.d/colord.conf";
+    systemd.tmpfiles.packages = pkgs.colord;


Suggested change

systemd.tmpfiles.packages = pkgs.colord;

systemd.tmpfiles.packages = [ pkgs.colord ];

flokli · 2020-07-13T19:25:56Z

nixos/modules/system/boot/systemd.nix

+      "tmpfiles.d".source = (pkgs.symlinkJoin {
+        name = "tmpfiles.d";
+        paths = cfg.tmpfiles.packages;
+      }) + "/lib/tmpfiles.d";


Could we check here every package in the list exposes a lib/tmpfiles.d and fail otherwise?

This should make it easier to spot accidentially passing in the wrong output, as described in #93006 (comment).

xaverdh · 2020-07-14T12:16:28Z

nixos/modules/system/boot/systemd.nix

+          for i in $(cat $pathsPath); do
+            test -d $i/lib/tmpfiles.d || (
+              echo "ERROR: The path $i was passed to systemd.tmpfiles.packages but does not contain the folder lib/tmpfiles.d"
+              exit 1


Is this supposed to exit the entire shell?
I think bash / sh will only exit the subshell here..

I'm >90% sure this works and tested it with systemd.tmpfiles.packages = [ pkgs.hello ];.
Actually… yeah, this probably only exits the subshell, but all phases run with set -e, so this does fail the whole build.

Ah right, set -e will make it fail. Sorry for the noise!

flokli · 2020-07-14T12:43:13Z

nixos/modules/system/boot/systemd.nix

+        <filename><replaceable>pkg</replaceable>/lib/tmpfiles.d</filename>
+        will be included.
+
+        If a <filename>lib</filename> output is available, rules are searched there.


This should somehow describe it then /only/ looks in the lib output

(and maybe explain the whole fallback depending on which outputs the package has (lib -> out -> default output)), but I'm struggling with how to write this in a more explicit and less misunderstandable way.

How about: "If a lib output is available, rules are searched there and only there. If there is lib it will fall back to out and if that does not exist either, the default output will be used."

Although this should maybe just be documented somewhere "generically". As in, explain getOutput in some part of the manual and reference if for options like this, because there are probably a bunch of them.

Yeah, let's refactor it to a more generic place as soon as there's more than 2 things trying to document it :-)

#93006 isn't trying to document it right now, but it also does apply = map getLib;

Can you change this to "If a lib output is available, rules are searched there and only there. If there is lib it will fall back to out and if that does not exist either, the default output will be used." for now?

flokli · 2020-07-14T12:46:01Z

nixos/modules/system/boot/systemd.nix

-
-      "tmpfiles.d/home.conf".source = "${systemd}/example/tmpfiles.d/home.conf";
-      "tmpfiles.d/journal-nocow.conf".source = "${systemd}/example/tmpfiles.d/journal-nocow.conf";
-      "tmpfiles.d/portables.conf".source = "${systemd}/example/tmpfiles.d/portables.conf";


Nice catch! This isn't there as we currently build systemd with -Dportabled=false, which is fine for now.

flokli · 2020-07-14T12:46:35Z

nixos/modules/system/boot/systemd.nix

+        ln -s "${systemd}/example/tmpfiles.d/static-nodes-permissions.conf"
+        ln -s "${systemd}/example/tmpfiles.d/systemd.conf"
+        ln -s "${systemd}/example/tmpfiles.d/systemd-nologin.conf"
+        ln -s "${systemd}/example/tmpfiles.d/systemd-nspawn.conf"


We we should add this during nixos/modules/system/boot/systemd-nspawn.nix (but unconditionally, as we want to have the tmpfiles set up when running systemd-nspawn imperatively)

See comment below about x11

flokli · 2020-07-14T12:49:13Z

nixos/modules/system/boot/systemd.nix

+        ln -s "${systemd}/example/tmpfiles.d/systemd-tmp.conf"
+        ln -s "${systemd}/example/tmpfiles.d/tmp.conf"
+        ln -s "${systemd}/example/tmpfiles.d/var.conf"
+        ln -s "${systemd}/example/tmpfiles.d/x11.conf"


This should be moved to nixos/modules/services/x11/xserver.nix

Why? It's part of the systemd package. Putting it in nixos/modules/services/x11/xserver.nix would just result in

systemd.tmpfiles.packages = [(pkgs.runCommand "x11-tmpfiles" {} '' mkdir -p $out/lib/tmpfiles.d cd $out/lib/tmpfiles.d ln -s "${systemd}/example/tmpfiles.d/x11.conf" '')];

Is that really better?

Hmm, it contains x11-specific tmpfile rules, which we probably don't really want on a nongraphical system. But eh, this specific one only removes files, so 🤷

nixos/modules/system/boot/systemd.nix

flokli · 2020-07-17T18:37:50Z

nixos/modules/system/boot/systemd.nix

+        <filename><replaceable>pkg</replaceable>/lib/tmpfiles.d</filename>
+        will be included.
+
+        If a <filename>lib</filename> output is available, rules are searched there and only there. If there is no <filename>lib</filename> output it will fall back to <filename>out</filename> and if that does not exist either, the default output will be used.


Can you reflow this?

flokli · 2020-07-17T18:38:43Z

nixos/modules/system/boot/systemd.nix

+
+        All found files in
+        <filename><replaceable>pkg</replaceable>/lib/tmpfiles.d</filename>
+        will be included.


I'd add "If none of these files can be found, this will return an error."

Hm, right now we actually check if this folder exist, not if any files are in there. Maybe we should change that check (in addition to documenting it).

Also drop the `portables` tmpfiles because the file is missing in the systemd derivation.

orivej · 2020-08-11T10:18:37Z

nixos/modules/system/boot/systemd.nix

-      "tmpfiles.d/systemd-tmp.conf".source = "${systemd}/example/tmpfiles.d/systemd-tmp.conf";
-      "tmpfiles.d/tmp.conf".source = "${systemd}/example/tmpfiles.d/tmp.conf";
-      "tmpfiles.d/var.conf".source = "${systemd}/example/tmpfiles.d/var.conf";
-      "tmpfiles.d/x11.conf".source = "${systemd}/example/tmpfiles.d/x11.conf";


Previously I could disable tmp.conf (to keep /tmp across reboots) with environment.etc."tmpfiles.d/tmp.conf".text = "# disabled by me";. This does not work anymore. How shall I achieve that now? May these .source lines be restored? (I do not see the benefit of the new way.)

Oof. @ajs124 Can files be overridden with a package that is later in the list?

@dasJ No, I don't think so. We could probably do this in nix instead of a derivation, then you could mkForce the option.
@orivej the benefit, like with other similar systemd module options, is that we can use tmpfile files from upstream packages easily.

@ajs124 I see the purpose of systemd.tmpfiles.packages in general, but I do not see the benefit of the new synthetic packages "systemd-default-tmpfiles" and "nixos-tmpfiles.d" over the old environment.etc."tmpfiles.d/00-nixos.conf".text = …; "tmpfiles.d/tmp.conf".source = .

I suppose that using solely systemd.tmpfiles.packages for managing /etc/tmpfiles.d is good for uniformity, but it looses the well thought out infrastructure for customizing /etc via environment.etc settings. I think that systemd.tmpfiles.packages should be empty by default to allow the user an easy choice between using unmodified upstream tmpfiles.d and custom alternatives.

A simple solution would be maintaining a exclude list of .conf files, which shouldn't be added to /etc in the end.

This sounds good if we would exclude all files in environment.etc.

You could also lib.mkForce the list to get rid of the default packages.

This is not acceptable because I want to keep all other systemd tmpfiles.d installed.

This sounds good if we would exclude all files in environment.etc.

Not sure if I understand. /etc/tmpfiles.d would still symlink into the nix store, but the code assembling it could have an exclusion list.

Also, what do you think about using buildEnv, and its priorities mechanism?

I've just opened #96755 to track this.

Also, what do you think about using buildEnv, and its priorities mechanism?

Sounds like an improvement to me as it doesn't require Nixpkgs patches anymore but it's likely still a pretty verbose solution that isn't that use friendly (though I would be ok with it and I'm not sure if we could easily do better).

I guess a pretty compact snipped to use this would look something like this:

systemd.tmpfiles.packages = [ (lib.hiPrio (pkgs.writeTextDir "lib/tmpfiles.d/tmp.conf" '' # Disabled '')) ];

@flokli See #96766, I'm using environment.etc as the exclusion list.

buildEnv seems incompatible with mutable /etc and the way environment.etc is implemented.

…nt.etc This allows the user to configure systemd tmpfiles.d via `environment.etc."tmpfiles.d/X.conf".text = "..."`, which after NixOS#93073 causes permission denied (with new X.conf): ``` ln: failed to create symbolic link '/nix/store/...-etc/etc/tmpfiles.d/X.conf': Permission denied builder for '/nix/store/...-etc.drv' failed with exit code 1 ``` or collision between environment.etc and systemd-default-tmpfiles packages (with existing X.conf, such as tmp.conf): ``` duplicate entry tmpfiles.d/tmp.conf -> /nix/store/...-etc-tmp.conf mismatched duplicate entry /nix/store/...-systemd-246/example/tmpfiles.d/tmp.conf <-> /nix/store/...-etc-tmp.conf builder for '/nix/store/...-etc.drv' failed with exit code 1 ``` Fixes NixOS#96755

…nt.etc (#96766) This allows the user to configure systemd tmpfiles.d via `environment.etc."tmpfiles.d/X.conf".text = "..."`, which after #93073 causes permission denied (with new X.conf): ``` ln: failed to create symbolic link '/nix/store/...-etc/etc/tmpfiles.d/X.conf': Permission denied builder for '/nix/store/...-etc.drv' failed with exit code 1 ``` or collision between environment.etc and systemd-default-tmpfiles packages (with existing X.conf, such as tmp.conf): ``` duplicate entry tmpfiles.d/tmp.conf -> /nix/store/...-etc-tmp.conf mismatched duplicate entry /nix/store/...-systemd-246/example/tmpfiles.d/tmp.conf <-> /nix/store/...-etc-tmp.conf builder for '/nix/store/...-etc.drv' failed with exit code 1 ``` Fixes #96755

dasJ changed the title ~~nixos/systemd: Implement a packages option~~ nixos/systemd: Implement a packages option for tmpfiles Jul 13, 2020

dasJ force-pushed the tmpfiles-packages branch from 64a2a1e to 74fd722 Compare July 13, 2020 16:14

ofborg bot added 6.topic: nixos 8.has: module (update) labels Jul 13, 2020

flokli added this to To Do in systemd via automation Jul 13, 2020

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jul 13, 2020

dasJ force-pushed the tmpfiles-packages branch from 74fd722 to 667fb68 Compare July 13, 2020 16:26

dasJ marked this pull request as ready for review July 13, 2020 16:27

dasJ requested review from Mic92, flokli, andir, ajs124 and fpletz July 13, 2020 16:28

flokli reviewed Jul 13, 2020

View reviewed changes

dasJ force-pushed the tmpfiles-packages branch from 667fb68 to 1f8d86b Compare July 13, 2020 16:35

systemd automation moved this from To Do to In Progress Jul 13, 2020

flokli requested changes Jul 13, 2020

View reviewed changes

ajs124 force-pushed the tmpfiles-packages branch 2 times, most recently from 0dfd7b6 to e4e2801 Compare July 14, 2020 00:12

xaverdh reviewed Jul 14, 2020

View reviewed changes

flokli reviewed Jul 14, 2020

View reviewed changes

Mic92 reviewed Jul 15, 2020

View reviewed changes

nixos/modules/system/boot/systemd.nix Show resolved Hide resolved

ajs124 force-pushed the tmpfiles-packages branch from e4e2801 to 037d81f Compare July 16, 2020 16:11

flokli reviewed Jul 17, 2020

View reviewed changes

nixos/systemd: Implement a packages option for tmpfiles

a44b2cd

Also drop the `portables` tmpfiles because the file is missing in the systemd derivation.

ajs124 force-pushed the tmpfiles-packages branch from 037d81f to a44b2cd Compare July 17, 2020 22:04

flokli merged commit f14799c into NixOS:master Jul 20, 2020

systemd automation moved this from In Progress to Done Jul 20, 2020

ajs124 deleted the tmpfiles-packages branch July 20, 2020 22:35

ajs124 mentioned this pull request Jul 25, 2020

nixos/tasks/lvm: fix systemd tmpfiles #93838

Merged

10 tasks

orivej reviewed Aug 11, 2020

View reviewed changes

flokli mentioned this pull request Aug 15, 2020

Use upstream tmpfiles.d #40594

Closed

primeos mentioned this pull request Aug 31, 2020

Overriding files in /etc/tmpfiles.d is now more difficult #96755

Closed

orivej mentioned this pull request Aug 31, 2020

nixos/systemd: support adding and overriding tmpfiles.d via environment.etc #96766

Merged

10 tasks

flokli mentioned this pull request Dec 2, 2020

systemd tooling does not pull in .link files defined packages #105689

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/systemd: Implement a packages option for tmpfiles #93073

nixos/systemd: Implement a packages option for tmpfiles #93073

dasJ commented Jul 13, 2020 •

edited

dasJ commented Jul 13, 2020

flokli left a comment

flokli Jul 13, 2020

flokli Jul 13, 2020

flokli Jul 13, 2020

ajs124 Jul 14, 2020

xaverdh Jul 14, 2020

ajs124 Jul 14, 2020

xaverdh Jul 14, 2020

flokli Jul 14, 2020

ajs124 Jul 14, 2020

flokli Jul 15, 2020

ajs124 Jul 15, 2020

flokli Jul 16, 2020

ajs124 Jul 16, 2020

flokli Jul 14, 2020

flokli Jul 14, 2020

ajs124 Jul 14, 2020

flokli Jul 14, 2020

ajs124 Jul 14, 2020

flokli Jul 14, 2020

flokli Jul 17, 2020

flokli Jul 17, 2020

ajs124 Jul 17, 2020

orivej Aug 11, 2020

dasJ Aug 12, 2020

ajs124 Aug 12, 2020

orivej Aug 12, 2020

orivej Aug 12, 2020

orivej Aug 15, 2020

orivej Aug 15, 2020

flokli Aug 15, 2020

primeos Aug 31, 2020

orivej Aug 31, 2020

	systemd.tmpfiles.packages = package;
	systemd.tmpfiles.packages = [ package ];

	systemd.tmpfiles.packages = pkgs.colord;
	systemd.tmpfiles.packages = [ pkgs.colord ];

nixos/systemd: Implement a packages option for tmpfiles #93073

nixos/systemd: Implement a packages option for tmpfiles #93073

Conversation

dasJ commented Jul 13, 2020 • edited

Motivation for this change

Things done

dasJ commented Jul 13, 2020

flokli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasJ commented Jul 13, 2020 •

edited