LLVM 13 introduced `-Wunused-but-set-variable` diagnostic flag, which
trips over some intentionally set-but-not-used variables or variables
attached to cleanup handlers with side effects (`_cleanup_umask_`,
`_cleanup_(notify_on_cleanup)`, `_cleanup_(restore_sigsetp)`, etc.):
```
../src/basic/process-util.c:1257:46: error: variable 'saved_ssp' set but not used [-Werror,-Wunused-but-set-variable]
_cleanup_(restore_sigsetp) sigset_t *saved_ssp = NULL;
^
1 error generated.
```
The function returns non-negative UnitNameFlags on success, and negative
errno on error. In the past we kept the return type as int because of those
negative return values. But nowadays _UNIT_NAME_INVALID == -EINVAL. And if
we tried to actually return something that doesn't fit in the return type,
the compiler would throw an error. By changing to the "real" return type,
we allow the debugger to use symbolic representation for the variables.
We had the following scenario:
under /etc/systemd/system/
- foo@.service
- bar@tty12.service → foo@tty12.service
- multi-user.target.wants/foo@tty12.service
Existing code did not "know" that foo@tty12.service has alias bar@tty12.service:
$ systemctl show -P Names foo@tty12.service
foo@tty12.service
Since multi-user.target is always loaded, we would load foo@tty12.service.
When trying to load bar@tty12.service, it would (correctly) detect that
bar@tty12.service is an alias for foo@tty12.service, and try to merge the
bar@tty12.service unit into the foo@tty12.service. This would fail, because
foo@tty12.service was already loaded, and only about-to-be-loaded units can
be merged.
With the patch we consider bar@tty12.service an alias of foo@tty12.service
immediately, so the issue does not occur:
$ systemctl show -P Names foo@tty12.service
foo@tty12.service bar@tty12.service
Fixes#19409.
This turned in a bigger rewrite. The logic add "the main name and all aliases"
was implemented twice, slightly different in both cases. I split that part out
to a new function. The result about the same length, but hopefully a bit easier
to read.
Logging output is also improved a bit. Some left-over debug logs have been
removed or cleaned up.
This is a fairly big change, but (with the addition in the following commit),
we have pretty good coverage of this logic.
Hidden and backup files cannot be valid unit name (we reject anything
starting with a dot, and we require type suffixes). So let's not iterate
over those at all.
The general idea is that when a unit file is "linked" (i.e. installed by
symlinking from outside of the search paths), the *destination* name is
irrelevant. It doesn't even have to be a valid unit name, or to match the type
or instance value. The obvious collorary is that we shouldn't look at the
symlink destination name to derive the unit name, instance value, or anything
else at all.
When building the name map, when we find a linked unit (possibly at the end
of a series of alias redirects), store the *source* of the final symlink as the
fragment path. This has two effects:
- we stop looking at the *target* file name to derive unit info, i.e. actually
implement the stuff described in the first paragraph.
- we load the unit fragment through the symlink. If someone were to remove the
symlink, we'll not load the unit. This seems like the right thing.
Fixes#18058.
Before this change, we were generally quite confused about unit alises for
linked units. Fortunately most poeple use the same symlink source and target,
so in practice we wouldn't hit this too often.
In unit_load_fragment() a comment is added to explain what we're doing there.
