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: remove dependencies on local-fs.target #67858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! 👍
ae82f4c
to
2417a5c
Compare
@GrahamcOfBorg test transmission |
Since NixOS#61321, local-fs.target is part of sysinit.target again, meaning units without DefaultDependencies=no will automatically depend on it, and the manual set dependencies can be dropped.
2417a5c
to
f74735c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 We should merge this pretty as soon as we can with the upcoming freeze.
Since NixOS#61321, local-fs.target is part of sysinit.target again, meaning units without DefaultDependencies=no will automatically depend on it, and the manual set dependencies can be dropped. (cherry picked from commit f74735c)
@@ -102,7 +102,6 @@ in | |||
|
|||
systemd.services.triggerhappy = { | |||
wantedBy = [ "multi-user.target" ]; | |||
after = [ "local-fs.target" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the after
clause? Won't this change the semantics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - as described in the PR description:
Since #61321, local-fs.target is
part of sysinit.target again, meaning units without
DefaultDependencies=no
will automatically depend on it, and the manually set dependencies can be dropped.
triggerhappy.service
doesn't set DefaultDependencies=no
(ususally only some low-level, mostly systemd-internal services do), so it depends on sysinit.target
, which pulls in local-fs.target
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But IIUC After=
is not about setting dependencies, it only regulates ordering whenever units are started/stopped concurrently. From the manual:
Note that this setting is independent of and orthogonal to the requirement dependencies as configured by Requires=, Wants= or BindsTo=
sysinit.target gets added to both After= and Requires=:
… sysinit.target
systemd automatically adds dependencies of the types Requires= and After= for this target unit to all services (except for those with DefaultDependencies=no).
|
Thank you, that makes sense now! |
This change was re-introduced when updating to systemd 243. Also see: NixOS#67858 (cherry picked from commit 53fb1c5)
This change was re-introduced when updating to systemd 243. Also see: #67858
Since #61321, local-fs.target is
part of sysinit.target again, meaning units without
DefaultDependencies=no will automatically depend on it, and the manual
set dependencies can be dropped.
Motivation for this change
There's still a lot of now-redundant dependencies on
local-fs.target
in the NixOS module system.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)