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/network-interfaces: remove network-interfaces.target #272169

Merged
Merged
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
2 changes: 2 additions & 0 deletions nixos/doc/manual/release-notes/rl-2405.section.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m

- `paperless`' `services.paperless.extraConfig` setting has been removed and converted to the freeform type and option named `services.paperless.settings`.

- The legacy and long deprecated systemd target `network-interfaces.target` has been removed. Use `network.target` instead.

- `mkosi` was updated to v19. Parts of the user interface have changed. Consult the
[release notes](https://github.com/systemd/mkosi/releases/tag/v19) for a list of changes.

Expand Down
15 changes: 15 additions & 0 deletions nixos/modules/system/boot/systemd.nix
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,21 @@ in
cfg.services
);

assertions = concatLists (
mapAttrsToList
(name: service:
map (message: {
assertion = false;
inherit message;
}) (concatLists [
Copy link
Contributor

Choose a reason for hiding this comment

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

This assertion is very confusingly written. Why is there a concatLists for a list with a single element? Why is it mapping a list made with optional to an assertion with assertion = false instead of just putting the condition in the assertion field?

Copy link
Member

Choose a reason for hiding this comment

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

assertion = false

I don't know the impact of adding a lot of assertions (as there would be one for every system service)

concatLists

This is copied from the warnings above, to support multiple assertions later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any further objections? @ElvishJerricco

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think this was extremely weirdly written and should have been cleaned up, but I wasn't going to block the PR on it

(optional ((builtins.elem "network-interfaces.target" service.after) || (builtins.elem "network-interfaces.target" service.wants))
nyabinary marked this conversation as resolved.
Show resolved Hide resolved
"Service '${name}.service' is using the deprecated target network-interfaces.target, which no longer exists. Using network.target is recommended instead."
)
])
)
cfg.services
);

system.build.units = cfg.units;

system.nssModules = [ cfg.package.out ];
Expand Down
10 changes: 0 additions & 10 deletions nixos/modules/tasks/network-interfaces.nix
Original file line number Diff line number Diff line change
Expand Up @@ -1449,16 +1449,6 @@ in
listToAttrs
];

# The network-interfaces target is kept for backwards compatibility.
# New modules must NOT use it.
systemd.targets.network-interfaces =
{ description = "All Network Interfaces (deprecated)";
wantedBy = [ "network.target" ];
before = [ "network.target" ];
after = [ "network-pre.target" ];
unitConfig.X-StopOnReconfiguration = true;
};

systemd.services = {
network-local-commands = {
description = "Extra networking commands.";
Expand Down