I was considering deduplicating the list of target units in
WantedBy/RequiredBy. But to do this meaningfully, we'd need to do alias
expansion first, i.e. after the initial parsing is done. This seems to be
more trouble than it would be worth.
Instead, I added tests that we're doing the right thing and creating symlinks
as expected. For duplicate links, we create the link, and on the second time we
see that the link is already there, so the output is correct.
We'd create aliases and other symlinks first, and only then try to create
the main link. Since that can fail, let's do things in opposite order, and
abort immediately if we can't link the file itself.
We don't need to talk about Alias=. The approach of using Alias= to enable
units is still supported, but hasn't been advertised as the way to do thing
for many years. Using it as an explanation is just confusing.
Also, the description of templated units did not take DefaultInstance=
into account. It is updated and extended.
We had a check that was done in unit_file_resolve_symlink(). Let's move
the check to unit_validate_alias_symlink_or_warn(), which makes it available
to the code in install.c.
With this, unit_file_resolve_symlink() behaves almost the same. The warning
about "suspicious symlink" is done a bit later. I think this should be OK.
So far we'd issue a warning (before this series, just in the logs on the server
side, and before this commit, on stderr on the caller's side), but return
success. It seems that successfull return was introduced by mistake in
aa0f357fd8 (my fault :( ), which was supposed to
be a refactoring without a functional change. I think it's better to fail,
because if enablement fails, the user will most likely want to diagnose the
issue.
Note that we still do partial enablement, as far as that is possible. So if
e.g. we have [Install] Alias=foo.service foobar, we'll create the symlink
'foo.service', but not 'foobar', since that's not a valid unit name. We'll
print info about the action taken, and about 'foobar' being invalid, and return
failure.
We would resolve those specifiers to the calling user/group. This is mostly OK
when done in the manager, because the manager generally operates as root
in system mode, and a non-root in user mode. It would still be wrong if
called with --test though. But in systemctl, this would be generally wrong,
since we can call 'systemctl --system' as a normal user, either for testing
or even for actual operation with '--root=…'.
When operating in --global mode, %u/%U/%g/%G should return an error.
The information whether we're operating in system mode, user mode, or global
mode is passed as the data pointer to specifier_group_name(), specifier_user_name(),
specifier_group_id(), specifier_user_id(). We can't use userdata, because
it's already used for other things.
ENOENT is easily confused with the file that we're working on not being
present, e.g. when the file contains %o or something else that requires
os-release to be present. Let's use -EUNATCH instead to reduce that chances of
confusion if the context of the error is lost.
And once we have pinpointed the reason, let's provide a proper error message:
+ build/systemctl --root=/tmp/systemctl-test.TO7Mcb enable some-some-link6@.socket
/tmp/systemctl-test.TO7Mcb/etc/systemd/system/some-some-link6@.socket: Failed to resolve alias "target@A:%A.socket": Protocol driver not attached
Failed to enable unit, cannot resolve specifiers in "target@A:%A.socket".
The test for the variable is added in test-systemctl-enable because there we
can do it almost for free, and the variable is most likely to be used with
'systemctl enable --root' anyway.
This test has overlap with test-install-root, but it tests things at a
different level, so I think it's useful to add. It immediately shows various
bugs which will be fixed in later patches.
