Revert "systemctl: when removing enablement or mask symlinks, cover both /run and...

We currently have packages in the archive which use
"systemctl --runtime unmask" and are broken by this change.
This is a intermediate step until it is clear whether upstream will
revert this commit or whether we will have to update affected packages
to deal with this changed behaviour.

See #902287 and https://github.com/systemd/systemd/issues/9393

This reverts commit 4910b35078ad24dcbc63f372b2fee087640201d0.

Gbp-Pq: Topic debian
Gbp-Pq: Name Revert-systemctl-when-removing-enablement-or-mask-symlink.patch

diff --git a/man/systemctl.xml b/man/systemctl.xml
index d95d3726affb9825d20ef19b6f176f0f65f89e37..abf3ea49b2b13b0f51fab87c473ac61435bdf922 100644 (file)
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -557,19 +557,19 @@
         <term><option>--runtime</option></term>
 
         <listitem>
-          <para>When used with <command>set-property</command>, make changes only
-          temporarily, so that they are lost on the next reboot.</para>
-
-          <para>Similarily, when used with <command>enable</command>, <command>mask</command>,
-          <command>edit</command> and related commands, make temporary changes, which are lost on
-          the next reboot. Changes are not made in subdirectories of <filename>/etc</filename>, but
-          in <filename>/run</filename>. The immediate effect is identical, however since the latter
+          <para>When used with <command>enable</command>,
+          <command>disable</command>, <command>edit</command>,
+          (and related commands), make changes only temporarily, so
+          that they are lost on the next reboot. This will have the
+          effect that changes are not made in subdirectories of
+          <filename>/etc</filename> but in <filename>/run</filename>,
+          with identical immediate effects, however, since the latter
           is lost on reboot, the changes are lost too.</para>
 
-          <para>Note: this option cannot be used with <command>disable</command>,
-          <command>unmask</command>, <command>preset</command>, or <command>preset-all</command>,
-          because those operations sometimes need to remove symlinks under <filename>/etc</filename>
-          to have the desired effect, which would cause a persistent change.</para>
+          <para>Similarly, when used with
+          <command>set-property</command>, make changes only
+          temporarily, so that they are lost on the next
+          reboot.</para>
         </listitem>
       </varlistentry>
 

diff --git a/src/shared/install.c b/src/shared/install.c
index 77ae812878ef7557ca04fbd755af5ddd4a68c513..e53f09fa865c1df5319176380343944c424d6e8e 100644 (file)
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1911,6 +1911,7 @@ static int install_context_mark_for_removal(
                 InstallContext *c,
                 const LookupPaths *paths,
                 Set **remove_symlinks_to,
+                const char *config_path,
                 UnitFileChange **changes,
                 size_t *n_changes) {
 
@@ -1919,6 +1920,7 @@ static int install_context_mark_for_removal(
 
         assert(c);
         assert(paths);
+        assert(config_path);
 
         /* Marks all items for removal */
 
@@ -2028,7 +2030,7 @@ int unit_file_unmask(
         size_t n_todo = 0, n_allocated = 0;
         const char *config_path;
         char **i;
-        bool dry_run = !!(flags & UNIT_FILE_DRY_RUN);
+        bool dry_run;
         int r, q;
 
         assert(scope >= 0);
@@ -2038,71 +2040,73 @@ int unit_file_unmask(
         if (r < 0)
                 return r;
 
+        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
+
+        dry_run = !!(flags & UNIT_FILE_DRY_RUN);
+
         STRV_FOREACH(i, files) {
+                _cleanup_free_ char *path = NULL;
+
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
 
-                FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
-                        _cleanup_free_ char *path = NULL;
-
-                        path = path_make_absolute(*i, config_path);
-                        if (!path)
-                                return -ENOMEM;
+                path = path_make_absolute(*i, config_path);
+                if (!path)
+                        return -ENOMEM;
 
-                        r = null_or_empty_path(path);
-                        if (r == -ENOENT)
-                                continue;
-                        if (r < 0)
-                                return r;
-                        if (r == 0)
-                                continue;
+                r = null_or_empty_path(path);
+                if (r == -ENOENT)
+                        continue;
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        continue;
 
-                        if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2))
-                                return -ENOMEM;
+                if (!GREEDY_REALLOC0(todo, n_allocated, n_todo + 2))
+                        return -ENOMEM;
 
-                        todo[n_todo] = strdup(*i);
-                        if (!todo[n_todo])
-                                return -ENOMEM;
+                todo[n_todo] = strdup(*i);
+                if (!todo[n_todo])
+                        return -ENOMEM;
 
-                        n_todo++;
-                }
+                n_todo++;
         }
 
         strv_uniq(todo);
 
         r = 0;
-        FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
-                STRV_FOREACH(i, todo) {
-                        _cleanup_free_ char *path = NULL;
-                        const char *rp;
-
-                        path = path_make_absolute(*i, config_path);
-                        if (!path)
-                                return -ENOMEM;
+        STRV_FOREACH(i, todo) {
+                _cleanup_free_ char *path = NULL;
+                const char *rp;
 
-                        if (!dry_run && unlink(path) < 0) {
-                                if (errno != ENOENT) {
-                                        if (r >= 0)
-                                                r = -errno;
-                                        unit_file_changes_add(changes, n_changes, -errno, path, NULL);
-                                }
+                path = path_make_absolute(*i, config_path);
+                if (!path)
+                        return -ENOMEM;
 
-                                continue;
+                if (!dry_run && unlink(path) < 0) {
+                        if (errno != ENOENT) {
+                                if (r >= 0)
+                                        r = -errno;
+                                unit_file_changes_add(changes, n_changes, -errno, path, NULL);
                         }
 
-                        unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
-
-                        rp = skip_root(&paths, path);
-                        q = mark_symlink_for_removal(&remove_symlinks_to, rp ?: path);
-                        if (q < 0)
-                                return q;
+                        continue;
                 }
 
-                q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
-                if (r >= 0)
-                        r = q;
+                unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
+
+                rp = skip_root(&paths, path);
+                q = mark_symlink_for_removal(&remove_symlinks_to, rp ?: path);
+                if (q < 0)
+                        return q;
         }
 
+        q = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
+        if (r >= 0)
+                r = q;
+
         return r;
 }
 
@@ -2494,7 +2498,6 @@ int unit_file_disable(
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(install_context_done) InstallContext c = {};
         _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
-        bool dry_run = !!(flags & UNIT_FILE_DRY_RUN);
         const char *config_path;
         char **i;
         int r;
@@ -2506,6 +2509,10 @@ int unit_file_disable(
         if (r < 0)
                 return r;
 
+        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
+
         STRV_FOREACH(i, files) {
                 if (!unit_name_is_valid(*i, UNIT_NAME_ANY))
                         return -EINVAL;
@@ -2515,17 +2522,11 @@ int unit_file_disable(
                         return r;
         }
 
-        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, changes, n_changes);
+        r = install_context_mark_for_removal(scope, &c, &paths, &remove_symlinks_to, config_path, changes, n_changes);
         if (r < 0)
                 return r;
 
-        FOREACH_STRING(config_path, paths.runtime_config, paths.persistent_config) {
-                r = remove_marked_symlinks(remove_symlinks_to, config_path, &paths, dry_run, changes, n_changes);
-                if (r < 0)
-                        return r;
-        }
-
-        return 0;
+        return remove_marked_symlinks(remove_symlinks_to, config_path, &paths, !!(flags & UNIT_FILE_DRY_RUN), changes, n_changes);
 }
 
 int unit_file_reenable(
@@ -2909,45 +2910,45 @@ int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char
 
 static int execute_preset(
                 UnitFileScope scope,
-                UnitFileFlags flags,
                 InstallContext *plus,
                 InstallContext *minus,
                 const LookupPaths *paths,
+                const char *config_path,
                 char **files,
                 UnitFilePresetMode mode,
+                bool force,
                 UnitFileChange **changes,
                 size_t *n_changes) {
 
-        const char *config_path;
-        bool force = !!(flags & UNIT_FILE_FORCE);
-        bool runtime = !!(flags & UNIT_FILE_RUNTIME);
-        int r = 0, q;
+        int r;
 
         assert(plus);
         assert(minus);
         assert(paths);
+        assert(config_path);
 
         if (mode != UNIT_FILE_PRESET_ENABLE_ONLY) {
                 _cleanup_set_free_free_ Set *remove_symlinks_to = NULL;
 
-                q = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, changes, n_changes);
-                if (q < 0)
-                        return q;
+                r = install_context_mark_for_removal(scope, minus, paths, &remove_symlinks_to, config_path, changes, n_changes);
+                if (r < 0)
+                        return r;
 
-                FOREACH_STRING(config_path, paths->runtime_config, paths->persistent_config) {
-                        q = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes);
-                        if (r == 0)
-                                r = q;
-                }
-        }
+                r = remove_marked_symlinks(remove_symlinks_to, config_path, paths, false, changes, n_changes);
+        } else
+                r = 0;
 
         if (mode != UNIT_FILE_PRESET_DISABLE_ONLY) {
+                int q;
+
                 /* Returns number of symlinks that where supposed to be installed. */
-                q = install_context_apply(scope, plus, paths,
-                                          runtime ? paths->runtime_config : paths->persistent_config,
-                                          force, SEARCH_LOAD, changes, n_changes);
-                if (r == 0)
-                        r = q;
+                q = install_context_apply(scope, plus, paths, config_path, force, SEARCH_LOAD, changes, n_changes);
+                if (r >= 0) {
+                        if (q < 0)
+                                r = q;
+                        else
+                                r += q;
+                }
         }
 
         return r;
@@ -3011,6 +3012,7 @@ int unit_file_preset(
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(presets_freep) Presets presets = {};
+        const char *config_path;
         char **i;
         int r;
 
@@ -3022,6 +3024,10 @@ int unit_file_preset(
         if (r < 0)
                 return r;
 
+        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
+
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
                 return r;
@@ -3032,7 +3038,7 @@ int unit_file_preset(
                         return r;
         }
 
-        return execute_preset(scope, flags, &plus, &minus, &paths, files, mode, changes, n_changes);
+        return execute_preset(scope, &plus, &minus, &paths, config_path, files, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes);
 }
 
 int unit_file_preset_all(
@@ -3046,6 +3052,7 @@ int unit_file_preset_all(
         _cleanup_(install_context_done) InstallContext plus = {}, minus = {};
         _cleanup_(lookup_paths_free) LookupPaths paths = {};
         _cleanup_(presets_freep) Presets presets = {};
+        const char *config_path = NULL;
         char **i;
         int r;
 
@@ -3057,6 +3064,10 @@ int unit_file_preset_all(
         if (r < 0)
                 return r;
 
+        config_path = (flags & UNIT_FILE_RUNTIME) ? paths.runtime_config : paths.persistent_config;
+        if (!config_path)
+                return -ENXIO;
+
         r = read_presets(scope, root_dir, &presets);
         if (r < 0)
                 return r;
@@ -3074,6 +3085,7 @@ int unit_file_preset_all(
                 }
 
                 FOREACH_DIRENT(de, d, return -errno) {
+
                         if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
                                 continue;
 
@@ -3097,7 +3109,7 @@ int unit_file_preset_all(
                 }
         }
 
-        return execute_preset(scope, flags, &plus, &minus, &paths, NULL, mode, changes, n_changes);
+        return execute_preset(scope, &plus, &minus, &paths, config_path, NULL, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes);
 }
 
 static void unit_file_list_free_one(UnitFileList *f) {

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index f072ad0c31f9ee9d5b7955bc2b0349636b2606e9..fc0baa8cca502a7fbf4c78fe2cbcbdac74f0574e 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -7683,11 +7683,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
                 return -EINVAL;
         }
 
-        if (arg_runtime && STRPTR_IN_SET(argv[optind], "disable", "unmask", "preset", "preset-all")) {
-                log_error("--runtime cannot be used with %s", argv[optind]);
-                return -EINVAL;
-        }
-
         return 1;
 }