From dd6b325a05c4caccd1a17dd4147f48a916eee386 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 19 Jun 2024 18:44:26 +0200 Subject: [PATCH 1/4] shared/install: drop unneeded initialization --- src/shared/install.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/install.c b/src/shared/install.c index 812f7991ab..421266eb36 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2009,7 +2009,7 @@ static int install_info_symlink_wants( UnitNameFlags valid_dst_type = UNIT_NAME_ANY; const char *n; - int r = 0, q; + int r, q; assert(info); assert(lp); From a159aa07e1548367d2fde80cb0d45b869c591864 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 19 Jun 2024 18:59:15 +0200 Subject: [PATCH 2/4] shared/install: propagate all errors in install_info_apply() Currently, install_info_apply() only updates r if it's 0, meaning that if one of the earlier install_info_symlink_alias/wants() calls returns > 0, errors generated by later calls will be discarded. Fix that. --- src/shared/install.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 421266eb36..639372ae5b 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2152,15 +2152,15 @@ static int install_info_apply( r = install_info_symlink_alias(scope, info, lp, config_path, force, changes, n_changes); q = install_info_symlink_wants(scope, file_flags, info, lp, config_path, info->wanted_by, ".wants/", changes, n_changes); - if (r == 0) + if (q != 0 && r >= 0) r = q; q = install_info_symlink_wants(scope, file_flags, info, lp, config_path, info->required_by, ".requires/", changes, n_changes); - if (r == 0) + if (q != 0 && r >= 0) r = q; q = install_info_symlink_wants(scope, file_flags, info, lp, config_path, info->upheld_by, ".upholds/", changes, n_changes); - if (r == 0) + if (q != 0 && r >= 0) r = q; return r; From 4441cf330b3847d6c553fb230e8e4c86aa75ebb9 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 19 Jun 2024 18:45:14 +0200 Subject: [PATCH 3/4] shared/install: correctly report changes in install_info_symlink_alias() Follow-up for b2751cf0394d36c24590b5f7b33e9f864b57ba0d Also make the conditions consistent for install_info_symlink_wants(). Fixes #33411 --- src/shared/install.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 639372ae5b..ca8e7d2733 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1986,7 +1986,9 @@ static int install_info_symlink_alias( } broken = r == 0; /* symlink target does not exist? */ - RET_GATHER(ret, create_symlink(lp, alias_target ?: info->path, alias_path, force || broken, changes, n_changes)); + r = create_symlink(lp, alias_target ?: info->path, alias_path, force || broken, changes, n_changes); + if (r != 0 && ret >= 0) + ret = r; } return ret; @@ -2080,7 +2082,7 @@ static int install_info_symlink_wants( return -ENOMEM; q = create_symlink(lp, info->path, path, /* force = */ true, changes, n_changes); - if ((q < 0 && r >= 0) || r == 0) + if (q != 0 && r >= 0) r = q; if (unit_file_exists(scope, lp, dst) == 0) { From 9fb5a8ca24e677e10f8c2b8973b5e2a11676bda0 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 19 Jun 2024 21:28:05 +0200 Subject: [PATCH 4/4] test-install-root: introduce test case for #33411 --- src/test/test-install-root.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index fbfab6d69b..3a623a8761 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -23,12 +23,14 @@ TEST(basic_mask_and_enable) { InstallChange *changes = NULL; size_t n_changes = 0; - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "a.service", NULL) == -ENOENT); - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "b.service", NULL) == -ENOENT); - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "c.service", NULL) == -ENOENT); - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "d.service", NULL) == -ENOENT); - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "e.service", NULL) == -ENOENT); - assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "f.service", NULL) == -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "a.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "b.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "c.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "d.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "e.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "f.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "g.service", NULL), -ENOENT); + ASSERT_EQ(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "h.service", NULL), -ENOENT); p = strjoina(root, "/usr/lib/systemd/system/a.service"); assert_se(write_string_file(p, @@ -197,6 +199,24 @@ TEST(basic_mask_and_enable) { changes = NULL; n_changes = 0; assert_se(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "f.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + + /* Test enabling units with only Alias= (unit_file_enable should return > 0 to indicate we did + * something, #33411) */ + + p = strjoina(root, SYSTEM_CONFIG_UNIT_DIR "/g.service"); + ASSERT_OK(write_string_file(p, + "[Install]\n" + "Alias=h.service\n", WRITE_STRING_FILE_CREATE)); + + ASSERT_GT(unit_file_enable(RUNTIME_SCOPE_SYSTEM, 0, root, STRV_MAKE("g.service"), &changes, &n_changes), 0); + install_changes_free(changes, n_changes); + changes = NULL; n_changes = 0; + + ASSERT_OK(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "g.service", &state)); + ASSERT_EQ(state, UNIT_FILE_ENABLED); + + ASSERT_OK(unit_file_get_state(RUNTIME_SCOPE_SYSTEM, root, "h.service", &state)); + ASSERT_EQ(state, UNIT_FILE_ALIAS); } TEST(linked_units) {