summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* test-systemctl-enable: disable the test for %a for nowZbigniew Jędrzejewski-Szmek2022-03-291-1/+3
|
* test-systemctl-enable: also use freshly-built systemd-id128Zbigniew Jędrzejewski-Szmek2022-03-292-11/+15
| | | | Tests were failing on centos7 because systemd-id128 is not in path.
* test-systemctl-enable: use magic syntax to allow inverted testsZbigniew Jędrzejewski-Szmek2022-03-291-37/+39
| | | | | | | | | | Inspired by 7910ec3bcde2ee0086b3e49f8aaa2a9f13f58d97. '! true' passes, because it's a conditional expression. But '( ! true )' fails, because '( … )' creates a subshell, i.e. a separate program, and '! true' becomes the return value of that program, and the whole thing apparently is not a conditional expression for the outer shell. This is shorter, so let's just do this.
* shared/install: when creating symlinks, accept different but equivalent symlinksZbigniew Jędrzejewski-Szmek2022-03-292-17/+81
| | | | | | | | | | | | | | We would only accept "identical" links, but having e.g. a symlink /usr/lib/systemd/system/foo-alias.service → /usr/lib/systemd/system/foo.service when we're trying to create /usr/lib/systemd/system/foo-alias.service → ./foo.service is OK. This fixes an issue found in ubuntuautopkg package installation, where we'd fail when enabling systemd-resolved.service, because the existing alias was absolute, and (with the recent patches) we were trying to create a relative one. A test is added. (For .wants/.requires symlinks we were already doing OK. A test is also added, to verify.)
* test-systemctl-enable: make shellcheck happyZbigniew Jędrzejewski-Szmek2022-03-291-38/+38
| | | | | Quoting is not necessary in many places, but I think it's nicer to use it consistently.
* shared/install: fix handling of a linked unit fileZbigniew Jędrzejewski-Szmek2022-03-292-5/+11
| | | | | | When we have a symlink that goes outside of our search path, we should just ignore the target file name. But we were verifying it, and rejecting in the case where a symlink was created manually.
* shared/install: split UNIT_FILE_SYMLINK into two statesZbigniew Jędrzejewski-Szmek2022-03-293-8/+15
| | | | | | The two states are distinguished, but are treated everywhere identically, so there is no difference in behaviour except for slighlty different log output.
* basic/unit-file: reverse negative conditionalZbigniew Jędrzejewski-Szmek2022-03-291-16/+16
| | | | | Having the reverse condition first makes changes that I want to do later awkward, so reverse it as a separate step first.
* shared/install: stop passing duplicate root argument to install_name_printf()Zbigniew Jędrzejewski-Szmek2022-03-294-11/+9
| | | | All callers were just passing info + info->root, we can simplify this.
* shared/install: when looking for symlinks in .wants/.requires, ignore ↵Zbigniew Jędrzejewski-Szmek2022-03-291-33/+60
| | | | | | | | | | | | | | | | | symlink target We'd say that file is enabled indirectly if we had a symlink like: foo@.service ← bar.target.wants/foo@one.service but not when we had foo@one.service ← bar.target.wants/foo@one.service The effect of both link types is the same. In fact we don't care about the symlink target. (We'll warn if it is mismatched, but we honour it anyway.) So let's use the original match logic only for aliases. For .wants/.requires we instead look for a matching source name, or a source name that matches after stripping of instance.
* shared/install: create relative symlinks for enablement and aliasingZbigniew Jędrzejewski-Szmek2022-03-293-102/+105
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a fairly noticable change, but I think it needs to be done. So far we'd create an absolute symlink to the target unit file: .wants/foo.service → /usr/lib/systemd/system/foo.service or alias.service → /etc/systemd/system/aliased.service. This works reasonably well, except in one case: where the unit file is linked. When we look at a file link, the name of the physical file isn't used, and we only take the account the symlink source name. (In fact, the destination filename may not even be a well-formed unit name, so we couldn't use it, even if we wanted to.) But this means that if a file is linked, and specifies aliases, we'd create absolute links for those aliases, and systemd would consider each "alias" to be a separate unit. This isn't checked by the tests here, because we don't have a running systemd instance, but it is easy enough to check manually. The most reasonable way to fix this is to create relative links to the unit file: .wants/foo.service → ../foo.service alias.service → aliased.service. I opted to use no prefix for aliases, both normal and 'default.target', and to add "../" for .wants/ and .requires/. Note that the link that is created doesn't necessarily point to the file. E.g. if we're enabling a file under /usr/lib/systemd/system, and create a symlink in /etc/systemd/system, it'll still be "../foo.service", not "../../usr/lib/systemd/system/foo.service". For our unit loading logic this doesn't matter, and figuring out a path that actually leads somewhere would be more work. Since the user is allowed to move the unit file, or add a new unit file in a different location, and we don't actually follow the symlink, I think it's OK to create a dangling symlink. The prefix of "../" is useful to give a hint that the link points to files that are conceptually "one level up" in the directory hierarchy. With the relative symlinks, systemd knows that those are aliases. The tests are adjusted to use the new forms. There were a few tests that weren't really testing something useful: 'test -e x' fails if 'x' is a a dangling symlink. Absolute links in the chroot would be dangling, even though the target existed in the expected path, but become non-dangling when made relative and the test fails. This should be described in NEWS, but I'm not adding that here, because it'd likely result in conflicts.
* shared/install: also remove symlinks like .wants/foo@one.service → ↵Zbigniew Jędrzejewski-Szmek2022-03-292-3/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ../foo@one.service So far 'systemctl enable' would create absolute links to the target template name. And we would remove such symlinks just fine. But the user may create symlinks manually in a different form. In particular, symlinks for instanced units *must* have the instance in the source name, and then it is natural to also include it in the target name (.wants/foo@one.service → ../foo@one.service rather than .wants/foo@one.service → ../foo@.service). We would choke on such links, or not remove them at all. A test is added: before: + build-rawhide/systemctl --root=/tmp/systemctl-test.001xda disable templ1@.service Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@seven.service". Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@six.service". Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@five.service". Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@four.service". Removed "/tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@three.service". Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service. Failed to disable unit, refusing to operate on linked unit file /tmp/systemctl-test.001xda/etc/systemd/system/services.target.wants/templ1@two.service. after: + build-rawhide/systemctl --root=/tmp/systemctl-test.QVP0ev disable templ1@.service Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service". Removed "/tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service". + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@one.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@two.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@three.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@four.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@five.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@six.service + test '!' -h /tmp/systemctl-test.QVP0ev/etc/systemd/system/services.target.wants/templ1@seven.service
* shared/install: skip unnecessary chasing of symlinks in disableZbigniew Jędrzejewski-Szmek2022-03-291-18/+25
| | | | | | | We use the symlink source name and destination names to decide whether to remove the symlink. But if the source name is enough to decide to remove the symlink, we'd still look up the destination for no good reason. This is a slow operation, let's skip it.
* test-systemctl-enable: enhance the test for unit file linkingZbigniew Jędrzejewski-Szmek2022-03-291-3/+36
| | | | | Current behaviour is wrong, but it cannot be shown in this test, because we don't have a running systemd instance here.
* shared/install: do not try to resolve symlinks outside of root directoryZbigniew Jędrzejewski-Szmek2022-03-291-1/+1
| | | | | | | | | | | | | | | I linked a file as root, so I had a symlink /root/test.service ← /etc/systemd/system/test.service. To my surpise, when running test-systemctl-enable, it failed with a cryptic EACCES. The previous commit made the logs a bit better. Strace shows that we were trying to follow the symlink without taking --root into account. It seems that this bug was introduced in 66a19d85a533b15ed32f4066ec880b5a8c06babd: before it, we'd do readlink_malloc(), which returned a path relative to root. But we only used that path for checking if the path is in remove_symlinks_to set, which contains relative paths. So if the path was relative, we'd get a false-negative answer, but we didn't go outside of the root. (We need to canonicalize the symlink to get a consistent answer.) But after 66a19 we use chase_symlinks(), without taking root into account which is completely bogus.
* shared/install: when we fail to chase a symlink, show some logsZbigniew Jędrzejewski-Szmek2022-03-291-0/+3
| | | | | | | | | | | | | | | | When chase_symlinks() fails, we'd get the generic error: Failed to disable: Permission denied. Let's at least add the failure to changes list, so the user gets a slightly better message. Ideally, we'd say where exactly the permission failure occured, but chase_symlinks() is a library level function and I don't think we should add logging there. The output looks like this now: Failed to resolve symlink "/tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias2.service": Permission denied Failed to resolve symlink "/tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias.service": Permission denied Failed to disable unit, file /tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias2.service: Permission denied. Failed to disable unit, file /tmp/systemctl-test.1r7Roj/etc/systemd/system/link5alias.service: Permission denied.
* test-systemctl-enable: extend the test for repeated WantedBy/RequiredByZbigniew Jędrzejewski-Szmek2022-03-291-4/+18
| | | | | | | | | | | 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.
* shared/install: fix reenable on linked unit filesZbigniew Jędrzejewski-Szmek2022-03-293-15/+68
|
* shared/install: split unit_file_{disable,enable}() so _reenable doesn't do ↵Zbigniew Jędrzejewski-Szmek2022-03-291-33/+72
| | | | | | setup twice It was pretty ugly that we were creating LookupPaths twice.
* install: when linking a file, create the link first or abortZbigniew Jędrzejewski-Szmek2022-03-292-11/+10
| | | | | | 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.
* man: fix invalid description of template handling in WantedBy=Zbigniew Jędrzejewski-Szmek2022-03-292-28/+74
| | | | | | | | | 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.
* shared/install: also check for self-aliases during installation and ignore themZbigniew Jędrzejewski-Szmek2022-03-293-20/+21
| | | | | | | | | 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.
* systemctl: fix silent failure when --root is not foundZbigniew Jędrzejewski-Szmek2022-03-2914-105/+88
| | | | | | | | | | | | | | | | | | | | Some calls to lookup_path_init() were not followed by any log emission. E.g.: $ SYSTEMD_LOG_LEVEL=debug systemctl --root=/missing enable unit; echo $? 1 Let's add a helper function and use it in various places. $ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/missing enable unit; echo $? Failed to initialize unit search paths for root directory /missing: No such file or directory 1 $ SYSTEMCTL_SKIP_SYSV=1 build/systemctl --root=/missing enable unit; echo $? Failed to initialize unit search paths for root directory /missing: No such file or directory Failed to enable: No such file or directory. 1 The repeated error in the second case is not very nice, but this is a niche case and I don't think it's worth the trouble to trying to avoid it.
* shared/install: return failure when enablement fails, but process as much as ↵Zbigniew Jędrzejewski-Szmek2022-03-292-23/+26
| | | | | | | | | | | | | | | | | | possible 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 aa0f357fd833feecbea6c3e9be80b643e433bced (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.
* shared/install: propagate errors about invalid aliases and such tooZbigniew Jędrzejewski-Szmek2022-03-296-56/+88
| | | | | | | If an invalid arg appears in [Install] Alias=, WantedBy=, RequiredBy=, we'd warn in the logs, but not propagate this information to the caller, and in particular not over dbus. But if we call "systemctl enable" on a unit, and the config if invalid, this information is quite important.
* shared/install: simplify unit_file_dump_changes()Zbigniew Jędrzejewski-Szmek2022-03-291-36/+27
| | | | No functional change.
* shared/specifier: fix %u/%U/%g/%G when called as unprivileged userZbigniew Jędrzejewski-Szmek2022-03-2911-104/+155
| | | | | | | | | | | | | | | | 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.
* shared/install: move scope into InstallContextZbigniew Jędrzejewski-Szmek2022-03-291-195/+191
| | | | | | | | | This makes it easier to pass it around in preparation for future changes. While at it, let's rename InstallContext c → ctx, and InstallInfo i → info. 'c' and 'i' are bad names for variables that are passed through multiple layers of functions calls. It's easier to follow what is happening with a meaningful variable names.
* shared/install: provide proper error messages when invalid specifiers are usedZbigniew Jędrzejewski-Szmek2022-03-292-31/+20
| | | | | $ build/systemctl --root=/tmp/systemctl-test.KXY8fu enable some-some-link6@.socket Failed to enable unit, invalid specifier in "target@C:%C.socket".
* shared/specifier: provide proper error messages when specifiers fail to read ↵Zbigniew Jędrzejewski-Szmek2022-03-294-39/+82
| | | | | | | | | | | | | | | files 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".
* shared/specifier: clarify and add test for missing dataZbigniew Jędrzejewski-Szmek2022-03-292-1/+17
| | | | | | In systemd.unit we document that unset fields resolve to "". But we didn't directly test this, so let's do that. Also, we return -ENOENT if the file is missing, which we didn't document or test.
* man/os-release: add a note about repeating entriesZbigniew Jędrzejewski-Szmek2022-03-291-0/+4
| | | | | | | | We didn't actually say that keys should not be repeated. At least the examples in docs (both python and shell) would do that, and any simple parser that builds a dictionary would most likely behave the same way. But let's document this expectation, but also say how to deal with malformed files.
* basic/env-file: make load-env-file deduplicate entries with the same keyZbigniew Jędrzejewski-Szmek2022-03-293-13/+26
| | | | | We generally assume parsing like the shell would do it, so the last value should win when there are repeats.
* test-os-util: add basic tests for os-release parsingZbigniew Jędrzejewski-Szmek2022-03-291-0/+62
|
* basic: add new variable $SYSTEMD_OS_RELEASE to override location of os-releaseZbigniew Jędrzejewski-Szmek2022-03-293-7/+26
| | | | | | 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.
* man: clarify the descriptions of aliases and linked unit filesZbigniew Jędrzejewski-Szmek2022-03-291-15/+43
| | | | | This just describes the rules that are implemented by the manager, and this pull request does not change any of them.
* tests: add helper for creating tempfiles with contentZbigniew Jędrzejewski-Szmek2022-03-294-73/+44
| | | | | I put it in tests because I think we're most likely to use it in tests. If necessary, it can be moved somewhere else later.
* test: add test for systemctl link & enableZbigniew Jędrzejewski-Szmek2022-03-293-1/+532
| | | | | | 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.
* shared/install: add a bit more quotingZbigniew Jędrzejewski-Szmek2022-03-291-11/+11
| | | | | | | | | When we are printing a valid unit name, quoting isn't necessary, because unit names cannot contain whitespace or other confusing characters. In particular if the unit name is prefixed by " unit " or something else that clearly identifies the string as a unit name, quoting would just add unnecessary noise. But when we're printing paths or invalid names, it's better to add quotes for clarity.
* shared/install: reuse the standard symlink verification subroutineZbigniew Jędrzejewski-Szmek2022-03-293-77/+75
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We save a few lines, but the important thing is that we don't have two different implementations with slightly different rules used for enablement and loading. Fixes #22000. Tested with: - the report in #22000, it now says: $ SYSTEMD_LOG_LEVEL=debug systemctl --root=/ enable test.service Suspicious symlink /etc/systemd/system/test.service→/etc/systemd/system/myown.d/test.service, treating as alias. unit_file_resolve_symlink: self-alias: /etc/systemd/system/test.service → test.service, ignoring. running_in_chroot(): Permission denied Suspicious symlink /etc/systemd/system/test.service→/etc/systemd/system/myown.d/test.service, treating as alias. unit_file_resolve_symlink: self-alias: /etc/systemd/system/test.service → test.service, ignoring. Failed to enable unit, refusing to operate on linked unit file test.service - a symlink to /dev/null: ... unit_file_resolve_symlink: linked unit file: /etc/systemd/system/test3.service → /dev/null Failed to enable unit, unit /etc/systemd/system/test3.service is masked. - the same from the host: ... unit_file_resolve_symlink: linked unit file: /var/lib/machines/rawhide/etc/systemd/system/test3.service → /var/lib/machines/rawhide/dev/null Failed to enable unit, unit /var/lib/machines/rawhide/etc/systemd/system/test3.service is masked. - through the manager: $ sudo systemctl enable test.service Failed to enable unit: Refusing to operate on alias name or linked unit file: test.service $ sudo systemctl enable test3.service Failed to enable unit: Unit file /etc/systemd/system/test3.service is masked. As seen in the first example, the warning is repeated. This is because we call the lookup logic twice: first for sysv-compat, and then again for real. I think that since this is only for broken setups, and when sysv-compat is enabled, and in an infrequent manual operation, at debug level, this is OK.
* basic/stat-util: add null_or_empty_path_with_root()Zbigniew Jędrzejewski-Szmek2022-03-293-6/+39
|
* basic/unit-file: split out the subroutine for symlink verificationZbigniew Jędrzejewski-Szmek2022-03-291-63/+96
| | | | | | The old logs used __func__, but this doesn't make sense now, because the low-level function will be used in other places. So those are adjusted to be more generic.
* meson: Detect python instead of hard-coding python3Heiko Becker2022-03-231-1/+2
| | | | | It allows to specify the desired python executable (and version) via meson's native file if there are multiple versions available.
* Revert "lgtm: disable cpp/missing-return"Frantisek Sumsal2022-03-231-2/+0
| | | | | | This reverts commit 6f4bffb586dfb0ce8db4e02ccb7f076a45bca419. Should be, hopefully, fixed by https://github.com/github/codeql/issues/8409.
* doc: two markdown markup fixesLennart Poettering2022-03-231-2/+2
|
* doc: add a bunch of missing <br>Lennart Poettering2022-03-231-6/+6
|
* Merge pull request #22835 from keszybz/foreach_string-inline-iteratorYu Watanabe2022-03-2353-147/+67
|\ | | | | Inline the iterator declaration in FOREACH_STRING
| * systemctl: use the right name in error messageZbigniew Jędrzejewski-Szmek2022-03-231-1/+1
| |
| * strv: declare iterator of FOREACH_STRING() in the loopZbigniew Jędrzejewski-Szmek2022-03-2353-146/+66
| | | | | | | | | | | | | | | | | | | | | | Same idea as 03677889f0ef42cdc534bf3b31265a054b20a354. No functional change intended. The type of the iterator is generally changed to be 'const char*' instead of 'char*'. Despite the type commonly used, modifying the string was not allowed. I adjusted the naming of some short variables for clarity and reduced the scope of some variable declarations in code that was being touched anyway.
* | Merge pull request #22836 from poettering/more-build-image-docsZbigniew Jędrzejewski-Szmek2022-03-237-37/+79
|\ \ | | | | | | docs: more tweaks for the image building docs
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-03 12:18:39 +0100