From cd228002ccedb927b4531a4b7dd9ea7015fdb657 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 31 Mar 2021 10:08:31 +0200 Subject: [PATCH 1/3] test-install-root: create referenced targets --- src/test/test-install-root.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index bea196c0ce..3649bc232a 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -1244,6 +1244,12 @@ int main(int argc, char *argv[]) { p = strjoina(root, "/usr/lib/systemd/system-preset/"); assert_se(mkdir_p(p, 0755) >= 0); + p = strjoina(root, "/usr/lib/systemd/system/multi-user.target"); + assert_se(write_string_file(p, "# pretty much empty", WRITE_STRING_FILE_CREATE) >= 0); + + p = strjoina(root, "/usr/lib/systemd/system/graphical.target"); + assert_se(write_string_file(p, "# pretty much empty", WRITE_STRING_FILE_CREATE) >= 0); + test_basic_mask_and_enable(root); test_linked_units(root); test_default(root); From 8ae27441c2dcf585f58242991302b09778d4d710 Mon Sep 17 00:00:00 2001 From: Jan Synacek Date: Wed, 3 Jun 2020 10:33:21 +0200 Subject: [PATCH 2/3] install: warn if WantedBy targets don't exist Currently, if [Install] section contains WantedBy=target that doesn't exist, systemd creates the symlinks anyway. That is just user-unfriendly. Let's be nice and warn about installing non-existent targets. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1835351. Replaces: #15834 --- src/shared/install.c | 25 ++++++++++++++++++------- src/shared/install.h | 1 + 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index eb8d7c1c45..a8091cd071 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -350,6 +350,11 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang log_info("Unit %s is an alias to a unit that is not present, ignoring.", changes[i].path); break; + case UNIT_FILE_DESTINATION_NOT_PRESENT: + if (!quiet) + log_warning("Unit %s is added as a dependency to a non-existent unit %s.", + changes[i].source, changes[i].path); + break; case -EEXIST: if (changes[i].source) log_error_errno(changes[i].type_or_errno, @@ -1832,6 +1837,7 @@ static int install_info_symlink_alias( } static int install_info_symlink_wants( + UnitFileScope scope, UnitFileInstallInfo *i, const LookupPaths *paths, const char *config_path, @@ -1902,6 +1908,9 @@ static int install_info_symlink_wants( q = create_symlink(paths, i->path, path, true, changes, n_changes); if (r == 0) r = q; + + if (unit_file_exists(scope, paths, dst) == 0) + unit_file_changes_add(changes, n_changes, UNIT_FILE_DESTINATION_NOT_PRESENT, dst, i->path); } return r; @@ -1937,6 +1946,7 @@ static int install_info_symlink_link( } static int install_info_apply( + UnitFileScope scope, UnitFileInstallInfo *i, const LookupPaths *paths, const char *config_path, @@ -1955,11 +1965,11 @@ static int install_info_apply( r = install_info_symlink_alias(i, paths, config_path, force, changes, n_changes); - q = install_info_symlink_wants(i, paths, config_path, i->wanted_by, ".wants/", changes, n_changes); + q = install_info_symlink_wants(scope, i, paths, config_path, i->wanted_by, ".wants/", changes, n_changes); if (r == 0) r = q; - q = install_info_symlink_wants(i, paths, config_path, i->required_by, ".requires/", changes, n_changes); + q = install_info_symlink_wants(scope, i, paths, config_path, i->required_by, ".requires/", changes, n_changes); if (r == 0) r = q; @@ -2023,7 +2033,7 @@ static int install_context_apply( if (i->type != UNIT_FILE_TYPE_REGULAR) continue; - q = install_info_apply(i, paths, config_path, force, changes, n_changes); + q = install_info_apply(scope, i, paths, config_path, force, changes, n_changes); if (r >= 0) { if (q < 0) r = q; @@ -3457,10 +3467,11 @@ static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState); static const char* const unit_file_change_type_table[_UNIT_FILE_CHANGE_TYPE_MAX] = { - [UNIT_FILE_SYMLINK] = "symlink", - [UNIT_FILE_UNLINK] = "unlink", - [UNIT_FILE_IS_MASKED] = "masked", - [UNIT_FILE_IS_DANGLING] = "dangling", + [UNIT_FILE_SYMLINK] = "symlink", + [UNIT_FILE_UNLINK] = "unlink", + [UNIT_FILE_IS_MASKED] = "masked", + [UNIT_FILE_IS_DANGLING] = "dangling", + [UNIT_FILE_DESTINATION_NOT_PRESENT] = "destination not present", }; DEFINE_STRING_TABLE_LOOKUP(unit_file_change_type, int); diff --git a/src/shared/install.h b/src/shared/install.h index 948faa0ca7..7ab0a871e0 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -32,6 +32,7 @@ enum { UNIT_FILE_UNLINK, UNIT_FILE_IS_MASKED, UNIT_FILE_IS_DANGLING, + UNIT_FILE_DESTINATION_NOT_PRESENT, _UNIT_FILE_CHANGE_TYPE_MAX, _UNIT_FILE_CHANGE_TYPE_INVALID = -EINVAL, }; From 8adbad370f522831dd9246fe272caf37ce748d4a Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 31 Mar 2021 10:38:00 +0200 Subject: [PATCH 3/3] test-install-root: add test for unknown WantedBy= target --- src/test/test-install-root.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 3649bc232a..50ed23a4ee 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -25,6 +25,7 @@ static void test_basic_mask_and_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", NULL) == -ENOENT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", NULL) == -ENOENT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", NULL) == -ENOENT); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", NULL) == -ENOENT); p = strjoina(root, "/usr/lib/systemd/system/a.service"); assert_se(write_string_file(p, @@ -168,6 +169,31 @@ static void test_basic_mask_and_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", &state) >= 0 && state == UNIT_FILE_MASKED); assert_se(unlink(p) == 0); + + /* Test enabling with unknown dependency target */ + + p = strjoina(root, "/usr/lib/systemd/system/f.service"); + assert_se(write_string_file(p, + "[Install]\n" + "WantedBy=x.target\n", WRITE_STRING_FILE_CREATE) >= 0); + + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", NULL) >= 0); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + + assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("f.service"), &changes, &n_changes) == 1); + assert_se(n_changes == 2); + assert_se(changes[0].type_or_errno == UNIT_FILE_SYMLINK); + assert_se(streq(changes[0].source, "/usr/lib/systemd/system/f.service")); + p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR"/x.target.wants/f.service"); + assert_se(streq(changes[0].path, p)); + assert_se(changes[1].type_or_errno == UNIT_FILE_DESTINATION_NOT_PRESENT); + p = strjoina(root, "/usr/lib/systemd/system/f.service"); + assert_se(streq(changes[1].source, p)); + assert_se(streq(changes[1].path, "x.target")); + unit_file_changes_free(changes, n_changes); + changes = NULL; n_changes = 0; + + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_ENABLED); } static void test_linked_units(const char *root) {