Merge pull request #4390 from keszybz/install-specifiers

Various install-related tweaks

3 files changed, 67 insertions, 33 deletions

diff --git a/src/shared/install.c b/src/shared/install.c
index f70b3e3dd5..d33a658d0a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -934,11 +934,14 @@ static int install_info_may_process(
 /**
  * Adds a new UnitFileInstallInfo entry under name in the InstallContext.will_process
  * hashmap, or retrieves the existing one if already present.
+ *
+ * Returns negative on error, 0 if the unit was already known, 1 otherwise.
  */
 static int install_info_add(
                 InstallContext *c,
                 const char *name,
                 const char *path,
+                bool auxiliary,
                 UnitFileInstallInfo **ret) {
 
         UnitFileInstallInfo *i = NULL;
@@ -955,6 +958,8 @@ static int install_info_add(
 
         i = install_info_find(c, name);
         if (i) {
+                i->auxiliary = i->auxiliary && auxiliary;
+
                 if (ret)
                         *ret = i;
                 return 0;
@@ -968,6 +973,7 @@ static int install_info_add(
         if (!i)
                 return -ENOMEM;
         i->type = _UNIT_FILE_TYPE_INVALID;
+        i->auxiliary = auxiliary;
 
         i->name = strdup(name);
         if (!i->name) {
@@ -990,7 +996,7 @@ static int install_info_add(
         if (ret)
                 *ret = i;
 
-        return 0;
+        return 1;
 
 fail:
         install_info_free(i);
@@ -1039,7 +1045,7 @@ static int config_parse_also(
                 void *data,
                 void *userdata) {
 
-        UnitFileInstallInfo *i = userdata;
+        UnitFileInstallInfo *info = userdata, *alsoinfo = NULL;
         InstallContext *c = data;
         int r;
 
@@ -1048,7 +1054,7 @@ static int config_parse_also(
         assert(rvalue);
 
         for (;;) {
-                _cleanup_free_ char *word = NULL;
+                _cleanup_free_ char *word = NULL, *printed = NULL;
 
                 r = extract_first_word(&rvalue, &word, NULL, 0);
                 if (r < 0)
@@ -1056,15 +1062,22 @@ static int config_parse_also(
                 if (r == 0)
                         break;
 
-                r = install_info_add(c, word, NULL, NULL);
+                r = install_full_printf(info, word, &printed);
+                if (r < 0)
+                        return r;
+
+                if (!unit_name_is_valid(printed, UNIT_NAME_ANY))
+                        return -EINVAL;
+
+                r = install_info_add(c, printed, NULL, true, &alsoinfo);
                 if (r < 0)
                         return r;
 
-                r = strv_push(&i->also, word);
+                r = strv_push(&info->also, printed);
                 if (r < 0)
                         return r;
 
-                word = NULL;
+                printed = NULL;
         }
 
         return 0;
@@ -1187,7 +1200,7 @@ static int unit_file_load(
                          config_item_table_lookup, items,
                          true, true, false, info);
         if (r < 0)
-                return r;
+                return log_debug_errno(r, "Failed to parse %s: %m", info->name);
 
         info->type = UNIT_FILE_TYPE_REGULAR;
 
@@ -1425,7 +1438,7 @@ static int install_info_traverse(
                                 bn = buffer;
                         }
 
-                        r = install_info_add(c, bn, NULL, &i);
+                        r = install_info_add(c, bn, NULL, false, &i);
                         if (r < 0)
                                 return r;
 
@@ -1464,9 +1477,9 @@ static int install_info_add_auto(
 
                 pp = prefix_roota(paths->root_dir, name_or_path);
 
-                return install_info_add(c, NULL, pp, ret);
+                return install_info_add(c, NULL, pp, false, ret);
         } else
-                return install_info_add(c, name_or_path, NULL, ret);
+                return install_info_add(c, name_or_path, NULL, false, ret);
 }
 
 static int install_info_discover(
@@ -1475,7 +1488,9 @@ static int install_info_discover(
                 const LookupPaths *paths,
                 const char *name,
                 SearchFlags flags,
-                UnitFileInstallInfo **ret) {
+                UnitFileInstallInfo **ret,
+                UnitFileChange **changes,
+                unsigned *n_changes) {
 
         UnitFileInstallInfo *i;
         int r;
@@ -1485,10 +1500,12 @@ static int install_info_discover(
         assert(name);
 
         r = install_info_add_auto(c, paths, name, &i);
-        if (r < 0)
-                return r;
+        if (r >= 0)
+                r = install_info_traverse(scope, c, paths, i, flags, ret);
 
-        return install_info_traverse(scope, c, paths, i, flags, ret);
+        if (r < 0)
+                unit_file_changes_add(changes, n_changes, r, name, NULL);
+        return r;
 }
 
 static int install_info_symlink_alias(
@@ -1700,8 +1717,10 @@ static int install_context_apply(
                         return q;
 
                 r = install_info_traverse(scope, c, paths, i, flags, NULL);
-                if (r < 0)
+                if (r < 0) {
+                        unit_file_changes_add(changes, n_changes, r, i->name, NULL);
                         return r;
+                }
 
                 /* We can attempt to process a masked unit when a different unit
                  * that we were processing specifies it in Also=. */
@@ -1759,10 +1778,15 @@ static int install_context_mark_for_removal(
                         return r;
 
                 r = install_info_traverse(scope, c, paths, i, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, NULL);
-                if (r == -ENOLINK)
-                        return 0;
-                else if (r < 0)
-                        return r;
+                if (r == -ENOLINK) {
+                        log_debug_errno(r, "Name %s leads to a dangling symlink, ignoring.", i->name);
+                        continue;
+                } else if (r == -ENOENT && i->auxiliary) {
+                        /* some unit specified in Also= or similar is missing */
+                        log_debug_errno(r, "Auxiliary unit %s not found, ignoring.", i->name);
+                        continue;
+                } else if (r < 0)
+                        return log_debug_errno(r, "Failed to find unit %s: %m", i->name);
 
                 if (i->type != UNIT_FILE_TYPE_REGULAR) {
                         log_debug("Unit %s has type %s, ignoring.",
@@ -2198,7 +2222,8 @@ int unit_file_add_dependency(
 
         config_path = runtime ? paths.runtime_config : paths.persistent_config;
 
-        r = install_info_discover(scope, &c, &paths, target, SEARCH_FOLLOW_CONFIG_SYMLINKS, &target_info);
+        r = install_info_discover(scope, &c, &paths, target, SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                  &target_info, changes, n_changes);
         if (r < 0)
                 return r;
         r = install_info_may_process(target_info, &paths, changes, n_changes);
@@ -2210,7 +2235,8 @@ int unit_file_add_dependency(
         STRV_FOREACH(f, files) {
                 char ***l;
 
-                r = install_info_discover(scope, &c, &paths, *f, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+                r = install_info_discover(scope, &c, &paths, *f, SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                          &i, changes, n_changes);
                 if (r < 0)
                         return r;
                 r = install_info_may_process(i, &paths, changes, n_changes);
@@ -2263,7 +2289,8 @@ int unit_file_enable(
         config_path = runtime ? paths.runtime_config : paths.persistent_config;
 
         STRV_FOREACH(f, files) {
-                r = install_info_discover(scope, &c, &paths, *f, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+                r = install_info_discover(scope, &c, &paths, *f, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                          &i, changes, n_changes);
                 if (r < 0)
                         return r;
                 r = install_info_may_process(i, &paths, changes, n_changes);
@@ -2309,7 +2336,7 @@ int unit_file_disable(
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
-                r = install_info_add(&c, *i, NULL, NULL);
+                r = install_info_add(&c, *i, NULL, false, NULL);
                 if (r < 0)
                         return r;
         }
@@ -2376,7 +2403,7 @@ int unit_file_set_default(
         if (r < 0)
                 return r;
 
-        r = install_info_discover(scope, &c, &paths, name, 0, &i);
+        r = install_info_discover(scope, &c, &paths, name, 0, &i, changes, n_changes);
         if (r < 0)
                 return r;
         r = install_info_may_process(i, &paths, changes, n_changes);
@@ -2406,7 +2433,8 @@ int unit_file_get_default(
         if (r < 0)
                 return r;
 
-        r = install_info_discover(scope, &c, &paths, SPECIAL_DEFAULT_TARGET, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+        r = install_info_discover(scope, &c, &paths, SPECIAL_DEFAULT_TARGET, SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                  &i, NULL, NULL);
         if (r < 0)
                 return r;
         r = install_info_may_process(i, &paths, NULL, 0);
@@ -2438,7 +2466,8 @@ static int unit_file_lookup_state(
         if (!unit_name_is_valid(name, UNIT_NAME_ANY))
                 return -EINVAL;
 
-        r = install_info_discover(scope, &c, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+        r = install_info_discover(scope, &c, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                  &i, NULL, NULL);
         if (r < 0)
                 return r;
 
@@ -2525,7 +2554,7 @@ int unit_file_exists(UnitFileScope scope, const LookupPaths *paths, const char *
         if (!unit_name_is_valid(name, UNIT_NAME_ANY))
                 return -EINVAL;
 
-        r = install_info_discover(scope, &c, paths, name, 0, NULL);
+        r = install_info_discover(scope, &c, paths, name, 0, NULL, NULL, NULL);
         if (r == -ENOENT)
                 return 0;
         if (r < 0)
@@ -2743,7 +2772,8 @@ static int preset_prepare_one(
         if (install_info_find(plus, name) || install_info_find(minus, name))
                 return 0;
 
-        r = install_info_discover(scope, &tmp, paths, name, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+        r = install_info_discover(scope, &tmp, paths, name, SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                  &i, changes, n_changes);
         if (r < 0)
                 return r;
         if (!streq(name, i->name)) {
@@ -2756,7 +2786,8 @@ static int preset_prepare_one(
                 return r;
 
         if (r > 0) {
-                r = install_info_discover(scope, plus, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+                r = install_info_discover(scope, plus, paths, name, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                          &i, changes, n_changes);
                 if (r < 0)
                         return r;
 
@@ -2764,7 +2795,8 @@ static int preset_prepare_one(
                 if (r < 0)
                         return r;
         } else
-                r = install_info_discover(scope, minus, paths, name, SEARCH_FOLLOW_CONFIG_SYMLINKS, &i);
+                r = install_info_discover(scope, minus, paths, name, SEARCH_FOLLOW_CONFIG_SYMLINKS,
+                                          &i, changes, n_changes);
 
         return r;
 }
diff --git a/src/shared/install.h b/src/shared/install.h
index c6aa4f6ef1..b1f220693b 100644
--- a/src/shared/install.h
+++ b/src/shared/install.h
@@ -119,10 +119,10 @@ struct UnitFileInstallInfo {
         char **also;
 
         char *default_instance;
+        char *symlink_target;
 
         UnitFileType type;
-
-        char *symlink_target;
+        bool auxiliary;
 };
 
 static inline bool UNIT_FILE_INSTALL_INFO_HAS_RULES(UnitFileInstallInfo *i) {
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index db1c928660..1686054d2a 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -326,7 +326,9 @@ static void test_default(const char *root) {
         assert_se(unit_file_get_default(UNIT_FILE_SYSTEM, root, &def) == -ENOENT);
 
         assert_se(unit_file_set_default(UNIT_FILE_SYSTEM, root, "idontexist.target", false, &changes, &n_changes) == -ENOENT);
-        assert_se(n_changes == 0);
+        assert_se(n_changes == 1);
+        assert_se(changes[0].type == -ENOENT);
+        assert_se(streq_ptr(changes[0].path, "idontexist.target"));
         unit_file_changes_free(changes, n_changes);
         changes = NULL; n_changes = 0;