install: when disabling units, do so even if the unit is missing

In some cases there might be unit symlinks in .wants/ or .requires/
directories even though the unit is otherwise fully removed. In this
case, don't fail removal, but still remove the symlinks.

This reworks the symlink marking logic to always add unit files that we
are missing to the changes list, but proceed with any symlink removal
for them. This way we'll still generate useful hints that a unit is
missing if you invoke "systemctl disable idontexist.service", but also
still remove any link to it.

Fixes: #4995

2 files changed, 118 insertions, 81 deletions

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index b4489ea935..27f948cbdd 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1912,63 +1912,6 @@ static int send_unit_files_changed(sd_bus *bus, void *userdata) {
         return sd_bus_send(bus, message, NULL);
 }
 
-static int reply_unit_file_changes_and_free(
-                Manager *m,
-                sd_bus_message *message,
-                int carries_install_info,
-                UnitFileChange *changes,
-                unsigned n_changes) {
-
-        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        unsigned i;
-        int r;
-
-        if (unit_file_changes_have_modification(changes, n_changes)) {
-                r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
-        }
-
-        r = sd_bus_message_new_method_return(message, &reply);
-        if (r < 0)
-                goto fail;
-
-        if (carries_install_info >= 0) {
-                r = sd_bus_message_append(reply, "b", carries_install_info);
-                if (r < 0)
-                        goto fail;
-        }
-
-        r = sd_bus_message_open_container(reply, 'a', "(sss)");
-        if (r < 0)
-                goto fail;
-
-        for (i = 0; i < n_changes; i++)
-                if (changes[i].type >= 0) {
-                        const char *change = unit_file_change_type_to_string(changes[i].type);
-                        assert(change != NULL);
-
-                        r = sd_bus_message_append(
-                                        reply, "(sss)",
-                                        change,
-                                        changes[i].path,
-                                        changes[i].source);
-                        if (r < 0)
-                                goto fail;
-                }
-
-        r = sd_bus_message_close_container(reply);
-        if (r < 0)
-                goto fail;
-
-        unit_file_changes_free(changes, n_changes);
-        return sd_bus_send(NULL, reply, NULL);
-
-fail:
-        unit_file_changes_free(changes, n_changes);
-        return r;
-}
-
 /* Create an error reply, using the error information from changes[]
  * if possible, and fall back to generating an error from error code c.
  * The error message only describes the first error.
@@ -1982,12 +1925,14 @@ static int install_error(
                 unsigned n_changes) {
         int r;
         unsigned i;
-        assert(c < 0);
 
         for (i = 0; i < n_changes; i++)
+
                 switch(changes[i].type) {
+
                 case 0 ... INT_MAX:
                         continue;
+
                 case -EEXIST:
                         if (changes[i].source)
                                 r = sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS,
@@ -1998,29 +1943,106 @@ static int install_error(
                                                       "File %s already exists.",
                                                       changes[i].path);
                         goto found;
+
                 case -ERFKILL:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_MASKED,
                                               "Unit file %s is masked.", changes[i].path);
                         goto found;
+
                 case -EADDRNOTAVAIL:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_GENERATED,
                                               "Unit %s is transient or generated.", changes[i].path);
                         goto found;
+
                 case -ELOOP:
                         r = sd_bus_error_setf(error, BUS_ERROR_UNIT_LINKED,
                                               "Refusing to operate on linked unit file %s", changes[i].path);
                         goto found;
+
+                case -ENOENT:
+                        r = sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_UNIT, "Unit file %s does not exist.", changes[i].path);
+                        goto found;
+
                 default:
                         r = sd_bus_error_set_errnof(error, changes[i].type, "File %s: %m", changes[i].path);
                         goto found;
                 }
 
-        r = c;
+        r = c < 0 ? c : -EINVAL;
+
  found:
         unit_file_changes_free(changes, n_changes);
         return r;
 }
 
+static int reply_unit_file_changes_and_free(
+                Manager *m,
+                sd_bus_message *message,
+                int carries_install_info,
+                UnitFileChange *changes,
+                unsigned n_changes,
+                sd_bus_error *error) {
+
+        _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        bool bad = false, good = false;
+        unsigned i;
+        int r;
+
+        if (unit_file_changes_have_modification(changes, n_changes)) {
+                r = bus_foreach_bus(m, NULL, send_unit_files_changed, NULL);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to send UnitFilesChanged signal: %m");
+        }
+
+        r = sd_bus_message_new_method_return(message, &reply);
+        if (r < 0)
+                goto fail;
+
+        if (carries_install_info >= 0) {
+                r = sd_bus_message_append(reply, "b", carries_install_info);
+                if (r < 0)
+                        goto fail;
+        }
+
+        r = sd_bus_message_open_container(reply, 'a', "(sss)");
+        if (r < 0)
+                goto fail;
+
+        for (i = 0; i < n_changes; i++) {
+
+                if (changes[i].type < 0) {
+                        bad = true;
+                        continue;
+                }
+
+                r = sd_bus_message_append(
+                                reply, "(sss)",
+                                unit_file_change_type_to_string(changes[i].type),
+                                changes[i].path,
+                                changes[i].source);
+                if (r < 0)
+                        goto fail;
+
+                good = true;
+        }
+
+        /* If there was a failed change, and no successful change, then return the first failure as proper method call
+         * error. */
+        if (bad && !good)
+                return install_error(error, 0, changes, n_changes);
+
+        r = sd_bus_message_close_container(reply);
+        if (r < 0)
+                goto fail;
+
+        unit_file_changes_free(changes, n_changes);
+        return sd_bus_send(NULL, reply, NULL);
+
+fail:
+        unit_file_changes_free(changes, n_changes);
+        return r;
+}
+
 static int method_enable_unit_files_generic(
                 sd_bus_message *message,
                 Manager *m,
@@ -2057,7 +2079,7 @@ static int method_enable_unit_files_generic(
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, carries_install_info ? r : -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, carries_install_info ? r : -1, changes, n_changes, error);
 }
 
 static int method_enable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2126,7 +2148,7 @@ static int method_preset_unit_files_with_mode(sd_bus_message *message, void *use
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, r, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, r, changes, n_changes, error);
 }
 
 static int method_disable_unit_files_generic(
@@ -2161,7 +2183,7 @@ static int method_disable_unit_files_generic(
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_disable_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2196,7 +2218,7 @@ static int method_revert_unit_files(sd_bus_message *message, void *userdata, sd_
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_set_default_target(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2227,7 +2249,7 @@ static int method_set_default_target(sd_bus_message *message, void *userdata, sd
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_preset_all_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2270,7 +2292,7 @@ static int method_preset_all_unit_files(sd_bus_message *message, void *userdata,
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_add_dependency_unit_files(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -2310,7 +2332,7 @@ static int method_add_dependency_unit_files(sd_bus_message *message, void *userd
         if (r < 0)
                 return install_error(error, r, changes, n_changes);
 
-        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes);
+        return reply_unit_file_changes_and_free(m, message, -1, changes, n_changes, error);
 }
 
 static int method_get_unit_file_links(sd_bus_message *message, void *userdata, sd_bus_error *error) {
diff --git a/src/shared/install.c b/src/shared/install.c
index f25ed685f6..c027f8ecd3 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -389,6 +389,12 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang
                                         verb, changes[i].path);
                         logged = true;
                         break;
+
+                case -ENOENT:
+                        log_error_errno(changes[i].type, "Failed to %s unit, unit %s does not exist.", verb, changes[i].path);
+                        logged = true;
+                        break;
+
                 default:
                         assert(changes[i].type < 0);
                         log_error_errno(changes[i].type, "Failed to %s unit, file %s: %m.",
@@ -1807,7 +1813,9 @@ static int install_context_mark_for_removal(
                 InstallContext *c,
                 const LookupPaths *paths,
                 Set **remove_symlinks_to,
-                const char *config_path) {
+                const char *config_path,
+                UnitFileChange **changes,
+                unsigned *n_changes) {
 
         UnitFileInstallInfo *i;
         int r;
@@ -1833,19 +1841,26 @@ static int install_context_mark_for_removal(
 
                 r = install_info_traverse(scope, c, paths, i, SEARCH_LOAD|SEARCH_FOLLOW_CONFIG_SYMLINKS, NULL);
                 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);
+                        log_debug_errno(r, "Name %s leads to a dangling symlink, removing name.", i->name);
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_DANGLING, i->path ?: i->name, NULL);
+                } else if (r == -ENOENT) {
+
+                        if (i->auxiliary)  /* some unit specified in Also= or similar is missing */
+                                log_debug_errno(r, "Auxiliary unit of %s not found, removing name.", i->name);
+                        else {
+                                log_debug_errno(r, "Unit %s not found, removing name.", i->name);
+                                unit_file_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                        }
 
-                if (i->type != UNIT_FILE_TYPE_REGULAR) {
-                        log_debug("Unit %s has type %s, ignoring.",
-                                  i->name,
-                                  unit_file_type_to_string(i->type) ?: "invalid");
+                } else if (r < 0) {
+                        log_debug_errno(r, "Failed to find unit %s, removing name: %m", i->name);
+                        unit_file_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                } else if (i->type == UNIT_FILE_TYPE_MASKED) {
+                        log_debug("Unit file %s is masked, ignoring.", i->name);
+                        unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_MASKED, i->path ?: i->name, NULL);
+                        continue;
+                } else if (i->type != UNIT_FILE_TYPE_REGULAR) {
+                        log_debug("Unit %s has type %s, ignoring.", i->name, unit_file_type_to_string(i->type) ?: "invalid");
                         continue;
                 }
 
@@ -2401,7 +2416,7 @@ int unit_file_disable(
                         return r;
         }
 
-        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path);
+        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path, changes, n_changes);
         if (r < 0)
                 return r;
 
@@ -2790,7 +2805,7 @@ static int execute_preset(
         if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) {
                 _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
 
-                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path);
+                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes);
                 if (r < 0)
                         return r;