nixos/logind: migrate to settings option by Stunkymonkey · Pull Request #435407 · NixOS/nixpkgs

Stunkymonkey · 2025-08-20T21:48:22Z

add services.logind.settings

Things done

Add a 👍 reaction to pull requests you find important.

aanderse · 2025-08-20T22:08:39Z

oh... i thought we had a settings option here now

dang 😞

Stunkymonkey · 2025-08-21T17:50:31Z

@aanderse maybe i should provide the settings option instead...?

aanderse · 2025-08-21T18:47:47Z

wow! what a generous offer @Stunkymonkey! i bet @LordGrimmauld appreciates the effort... i certainly do. thanks! 🚀

Stunkymonkey · 2025-08-21T19:14:09Z

@aanderse i am not an expert in deprecating options. But this is my first draft. So please help me out, what should be improved.

LordGrimmauld · 2025-08-21T20:15:53Z

Oh hey, more rfc 42 in systemd things, love to see it! I'll take a closer look tomorrow, but the over all approach looks reasonable enough. Care needs to be taken to ensure things like nixos VM tests still evaluate, though the rename redirects already do help a lot in that regard. I'll take a closer look once i have the time.

Stunkymonkey · 2025-08-21T20:17:32Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 435407
Commit: c35ec3a8bdaba2cff3542ee9d9d03de5fd135824

`x86_64-linux`

⏩ 2 packages blacklisted:

nixos-install-tools
tests.nixos-functions.nixos-test

✅ 5 packages built:

tests.devShellTools.nixos
tests.testers.lycheeLinkCheck.network
tests.testers.nixosTest-example
tests.testers.runNixOSTest-example (tests.testers.runNixOSTest-extendNixOS)
tests.trivial-builders.references

LordGrimmauld · 2025-08-21T20:18:51Z

side note, not relevant for this PR: seems i joined the systemd team, but am not listed in the systemd-related code owners listings. I'll have to open a PR to add myself to those too, then i'd have gotten a ping at the very beginning.

ElvishJerricco

This should probably be done the way systemd.settings.Manager was done:

nixpkgs/nixos/modules/system/boot/systemd.nix

Lines 409 to 419 in 874be15

    
               settings.Manager = mkOption { 
        
                 default = { }; 
        
                 defaultText = lib.literalExpression '' 
        
                   { 
        
                     DefaultIOAccounting = true; 
        
                     DefaultIPAccounting = true; 
        
                   } 
        
                 ''; 
        
                 type = lib.types.submodule { 
        
                   freeformType = types.attrsOf unitOption; 
        
                 };

For one thing, systemd conf files are not exactly the same as INI, as close as they might be. So the utils.systemdUtils.unitOptions.unitOption type, and the utils.systemdUtils.lib.attrsToSection function are the correct ones to use.

And we decided with settings.Manager that it was probably better to just explicitly have settings.Manager as the option, rather than having settings as an attrsOf of sections, because that config file allows only the one section. It's just a little more specific, leaves less room for people to mistakenly use the wrong section name, and still allows us to switch to attrsOf for more sections in the future if systemd ever adds more sections to that conf file. I suggest we do the same here for logind.

Stunkymonkey · 2025-08-21T22:04:06Z

@ElvishJerricco i adjusted the code. But the type lib.types.attrsOf utils.systemdUtils.unitOptions.unitOption; might be off? Appart from that. Is the new version in the way you like?

Stunkymonkey · 2025-08-24T13:14:38Z

hey @LordGrimmauld,
please look at it now again. I was just going through them one-by-one to not miss anything. One thread is not perfectly done, but i commented on it.

LordGrimmauld

I'd still like to have a proper discussion with the systemd folks about whether we should align KillUserProcesses with upstream systemd defaults, but as a 1:1 migration this PR looks pretty good. That discussion can happen elsewhere. Thank you :)

LordGrimmauld · 2025-08-24T17:28:38Z

I forgot to say, i did run the login nixos test successfully. That is probably the only one that is immediately relevant here.

Addressed

r-vdp

LGTM, I'll leave it to the systemd team to merge.

flokli · 2025-08-25T10:29:55Z

nixos/modules/system/boot/systemd/logind.nix

      '';
+      type = lib.types.submodule {
+        freeformType = lib.types.attrsOf utils.systemdUtils.unitOptions.unitOption;
+        options.KillUserProcesses = lib.mkOption {


This workaround might not be needed anymore at all, and we should probably drop this entirely (and add/update release notes).

tmux now spawns panes in separate cgroups, see tmux/tmux#3514.

flokli · 2025-08-25T10:32:08Z

Let's deal with not overriding the upstream default in our defaults in a followup.

Stunkymonkey · 2025-08-25T19:17:27Z

@flokli i am all for it. I first wanted migrate to the rfc42 style without changing the behavior.

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

There is a breaking change that landed in the upstream from this PR NixOS/nixpkgs#435407

K900 · 2025-08-26T09:38:48Z

No backwards compatibility, seriously?

LordGrimmauld · 2025-08-26T09:54:58Z

No backwards compatibility, seriously?

Hm, what do you mean? There is rename modules for the things that were renamed, just extraConfig was removed because that one just does not have an equivalent

K900 · 2025-08-26T09:56:13Z

It would be nice to keep extraConfig at least for a time.

LordGrimmauld · 2025-08-26T10:13:12Z

We did not retain extraConfig in #426692 either, so i did not assume it would be necessary. Further, jovians usage of extraConfig to set HandlePowerKey should have been replaced long ago: 520150f introduced the services.logind.powerKey option for that two years ago. I am not sure what retaining extraConfig would have helped if two years were not enough to migrate to the dedicated option there.

That said, yes, there is different things that did not have their dedicated options before. In those cases, retaining extraConfig could have helped. Is there any place where retaining obsolete extraConfig for backwards-compat is documented? I certainly didn't know that is something we can/should do (frankly, i disagree with letting anti-patterns live and be forgotten about).

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

There is a breaking change that landed in the upstream from this PR NixOS/nixpkgs#435407

Stunkymonkey marked this pull request as ready for review August 20, 2025 21:48

nix-owners bot requested review from ElvishJerricco, aanderse, arianvp and flokli August 20, 2025 21:50

Stunkymonkey changed the title ~~nixos/logind: add lidSwitchIgnoreInhibited option~~ nixos/logind: migrate to settings option Aug 21, 2025

Stunkymonkey force-pushed the nixos/logind-LidSwitchIgnoreInhibited branch from b4e3726 to 68eee75 Compare August 21, 2025 18:44

Stunkymonkey marked this pull request as draft August 21, 2025 18:47

Stunkymonkey force-pushed the nixos/logind-LidSwitchIgnoreInhibited branch 3 times, most recently from e38fdcc to c32d4d9 Compare August 21, 2025 19:08

Stunkymonkey force-pushed the nixos/logind-LidSwitchIgnoreInhibited branch from c32d4d9 to c35ec3a Compare August 21, 2025 19:17

Stunkymonkey marked this pull request as ready for review August 21, 2025 19:20

LordGrimmauld self-requested a review August 21, 2025 20:16

ElvishJerricco previously requested changes Aug 21, 2025

View reviewed changes

Stunkymonkey force-pushed the nixos/logind-LidSwitchIgnoreInhibited branch from c35ec3a to c562c50 Compare August 21, 2025 22:00

Stunkymonkey requested a review from ElvishJerricco August 21, 2025 22:00

nixos/logind: migrate to settings option

89526e1

Stunkymonkey force-pushed the nixos/logind-LidSwitchIgnoreInhibited branch from 3111154 to 89526e1 Compare August 24, 2025 16:52

LordGrimmauld approved these changes Aug 24, 2025

View reviewed changes

nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 24, 2025

r-vdp approved these changes Aug 24, 2025

View reviewed changes

nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 24, 2025

flokli reviewed Aug 25, 2025

View reviewed changes

flokli merged commit 6d3383c into NixOS:master Aug 25, 2025
32 of 34 checks passed

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Aug 26, 2025

logind: has moved to rfc42 style settings, adjust

54088ed

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Aug 26, 2025

logind: has moved to rfc42 style settings, adjust

cdd415c

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

DaRacci added a commit to DaRacci/Jovian-NixOS that referenced this pull request Aug 26, 2025

fix: migrate logind extraConfig to attribute

af20f69

There is a breaking change that landed in the upstream from this PR NixOS/nixpkgs#435407

DaRacci mentioned this pull request Aug 26, 2025

fix: migrate logind extraConfig to attribute Jovian-Experiments/Jovian-NixOS#515

Merged

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Aug 28, 2025

logind: has moved to rfc42 style settings, adjust

9e9776a

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Aug 31, 2025

logind: has moved to rfc42 style settings, adjust

4e7bf99

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Sep 1, 2025

logind: has moved to rfc42 style settings, adjust

eae9236

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Sep 1, 2025

logind: has moved to rfc42 style settings, adjust

db50c41

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Sep 2, 2025

logind: has moved to rfc42 style settings, adjust

e96b1db

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to brianmcgillion/ghaf that referenced this pull request Sep 3, 2025

logind: has moved to rfc42 style settings, adjust

9c92439

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

brianmcgillion added a commit to tiiuae/ghaf that referenced this pull request Sep 3, 2025

logind: has moved to rfc42 style settings, adjust

2e6e699

NixOS/nixpkgs#435407 Signed-off-by: Brian McGillion <bmg.avoin@gmail.com>

unlsycn pushed a commit to unlsycn/Jovian-NixOS that referenced this pull request Sep 19, 2025

fix: migrate logind extraConfig to attribute

256ca4b

There is a breaking change that landed in the upstream from this PR NixOS/nixpkgs#435407

	settings.Manager = mkOption {
	default = { };
	defaultText = lib.literalExpression ''
	{
	DefaultIOAccounting = true;
	DefaultIPAccounting = true;
	}
	'';
	type = lib.types.submodule {
	freeformType = types.attrsOf unitOption;
	};

Uh oh!

Conversation

Stunkymonkey commented Aug 20, 2025 • edited by LordGrimmauld Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

aanderse commented Aug 20, 2025

Uh oh!

Stunkymonkey commented Aug 21, 2025

Uh oh!

aanderse commented Aug 21, 2025

Uh oh!

Stunkymonkey commented Aug 21, 2025

Uh oh!

LordGrimmauld commented Aug 21, 2025

Uh oh!

Stunkymonkey commented Aug 21, 2025

nixpkgs-review result

x86_64-linux

Uh oh!

LordGrimmauld commented Aug 21, 2025

Uh oh!

ElvishJerricco left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Stunkymonkey commented Aug 21, 2025

Uh oh!

Stunkymonkey commented Aug 24, 2025

Uh oh!

LordGrimmauld left a comment

Choose a reason for hiding this comment

Uh oh!

LordGrimmauld commented Aug 24, 2025

Uh oh!

r-vdp left a comment

Choose a reason for hiding this comment

Uh oh!

flokli Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flokli commented Aug 25, 2025

Uh oh!

Stunkymonkey commented Aug 25, 2025

Uh oh!

K900 commented Aug 26, 2025

Uh oh!

LordGrimmauld commented Aug 26, 2025

Uh oh!

K900 commented Aug 26, 2025

Uh oh!

LordGrimmauld commented Aug 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Stunkymonkey commented Aug 20, 2025 •

edited by LordGrimmauld

Loading

`nixpkgs-review` result

`x86_64-linux`

ElvishJerricco left a comment •

edited

Loading