nixos/systemd: Implement a packages option for tmpfiles

[dasJ]

Motivation for this change

#66856 (comment)

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

cc @flokli

[dasJ]

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]

Suggested change

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

[flokli]

Suggested change

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

[flokli]

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).

[ajs124]

Like this?

[xaverdh]

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

[ajs124]

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.

[xaverdh]

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

[flokli]

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.

[ajs124]

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.

[flokli]

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

[ajs124]

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

[flokli]

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?

[ajs124]

done

[flokli]

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

[flokli]

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)

[ajs124]

See comment below about x11

[flokli]

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

[ajs124]

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?

[flokli]

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 🤷

[flokli]

Can you reflow this?

[flokli]

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

[ajs124]

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).

[orivej]

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.)

[dasJ]

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

[ajs124]

@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.

[orivej]

@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 = .

[orivej]

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.

[orivej]

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.

[orivej]

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.

[flokli]

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?

[primeos]

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
  ''))
];

[orivej]

@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.