sysupdate: Change disabling with --component-suggested/--feature-suggested

The disabling of features or components with the flag
--component-suggested/--feature-suggested didn't disable the suggested
ones but instead disabled all other ones. This is rather unintuitive due
to how the flags are named and also not really needed because the
intended reconciliation outcome can instead be done by first disabling
everything and then enabling the suggested ones again which is easier to
reason about. For components the tricky part is that they default to
enabled and thus it's better to have the disable/enable commands with
--component-suggested operate only on suggested ones instead of touching
others like "legacy" components that don't explicity say whether they
are enabled and suggested or not.

Make running disablement of components/features with
--component-suggested/--feature-suggested undo a previous enablement
with the same flags. Document how one can align the system to only use
suggested components/features and not anything else by doing it in two
steps, first disabling everything and then enabling suggested ones. This
also makes it clearer now that all components that are not explicitly
enabled nor suggested will be disabled then.

diff --git a/man/systemd-sysupdate.xml b/man/systemd-sysupdate.xml
index dcfcf9455962344b7accbfd3e88db0e05b09b221..49080f1e3657072eeb25c23f99a231a2ba0ae9fe 100644 (file)
--- a/man/systemd-sysupdate.xml
+++ b/man/systemd-sysupdate.xml
@@ -334,10 +334,19 @@
         are currently <emphasis>suggested</emphasis> for this system, as determined by the
         <varname>Suggest=</varname> and <varname>SuggestOn…=</varname> settings in the component definition
         files. This is supported for the <command>enable-component</command> and
-        <command>disable-component</command> commands.</para>
+        <command>disable-component</command> commands. Both commands operate on the same set of components,
+        thus <command>disable-component</command> undoes what <command>enable-component</command> did.</para>
 
         <para>This option may not be combined with <option>--component=</option>.</para>
 
+        <para>In order to reconcile the system according to suggestions, i.e., to also turn off the components
+        that are not (or no longer) suggested, first disable all components with <option>--component-all</option>,
+        then enable the suggested ones with this switch. Note that this has wider consequences than the equivalent
+        reconciliation of optional features because components are enabled by default, and a component counts as not
+        suggested unless <varname>Suggest=</varname> or one of the <varname>SuggestOn…=</varname> settings says otherwise.
+        Reconciling hence explicitly disables every component that is not suggested, including components that ship no
+        <filename>*.component</filename> file at all. The default component is not affected.</para>
+
         <xi:include href="version-info.xml" xpointer="v262"/></listitem>
       </varlistentry>
 
@@ -360,7 +369,12 @@
         that are currently <emphasis>suggested</emphasis> for this system, as determined by the
         <varname>Suggest=</varname> and <varname>SuggestOn…=</varname> settings in the feature definition
         files. This is supported for the <command>enable-feature</command> and
-        <command>disable-feature</command> commands.</para>
+        <command>disable-feature</command> commands. Both commands operate on the same set of features, thus
+        <command>disable-feature</command> undoes what <command>enable-feature</command> did.</para>
+
+        <para>In order to reconcile the system according to suggestions, i.e., to also turn off the features
+        that are not (or no longer) suggested, first disable all features with <option>--feature-all</option>,
+        then enable the suggested ones with this switch.</para>
 
         <xi:include href="version-info.xml" xpointer="v262"/></listitem>
       </varlistentry>

diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c
index 9ae80a2a8bf5df75d56a477c21b29881ea234992..0e1338140fb44ee91113b58b8a165fe1f8b28c29 100644 (file)
--- a/src/sysupdate/sysupdate.c
+++ b/src/sysupdate/sysupdate.c
@@ -2244,9 +2244,9 @@ static int context_enable_feature(
                 HASHMAP_FOREACH(f, c->features) {
                         r = feature_is_suggested(f);
                         if (r < 0)
-                                return log_error_errno(r, "Failed to determine if feature '%s' of component '%s' shall be enabled: %m", f->id, context_component_display(c));
-                        if (!!r != !!enable) {
-                                log_debug("Skipping feature '%s' of component '%s'.", f->id, context_component_display(c));
+                                return log_error_errno(r, "Failed to determine if feature '%s' of component '%s' is suggested: %m", f->id, context_component_display(c));
+                        if (r == 0) {
+                                log_debug("Feature '%s' of component '%s' is not suggested, skipping.", f->id, context_component_display(c));
                                 continue;
                         }
 
@@ -3245,15 +3245,11 @@ static int verb_enable_component(int argc, char *argv[], uintptr_t _data, void *
 
                         r = context_component_is_suggested(&cc);
                         if (r < 0) {
-                                log_warning_errno(r, "Failed to determine whether '%s' shall be enabled, skipping: %m", *name);
+                                log_warning_errno(r, "Failed to determine whether '%s' is suggested, skipping: %m", *name);
                                 continue;
                         }
-
-                        /* This reconciles the system with the suggestions: on 'enable-component' we act on
-                         * the components that are suggested, on 'disable-component' we act on the ones that
-                         * are not. Hence pick the components whose suggestion state matches the operation. */
-                        if (!!r != !!enable) {
-                                log_debug("Skipping '%s'.", *name);
+                        if (r == 0) {
+                                log_debug("Component '%s' is not suggested, skipping.", *name);
                                 continue;
                         }
 

diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh
index 1ddb2ac9b9df4a1025cfacae8380a6c90567b320..f35235ce8431f4f92840d9d434375098c4500931 100755 (executable)
--- a/test/units/TEST-72-SYSUPDATE.sh
+++ b/test/units/TEST-72-SYSUPDATE.sh
@@ -1717,12 +1717,20 @@ EOF
 assert_dropin "$(comp_enable_dropin compx)" yes
 test ! -e "$(comp_enable_dropin compy)"
 
-# 'disable-component --component-suggested' reconciles the other way around: it
-# acts on the components that are *not* suggested (i.e. compy).
+# 'disable-component --component-suggested' acts on the very same set, i.e. it
+# undoes what the enable above did. The non-suggested compy is left untouched.
 "$SYSUPDATE" --component-suggested disable-component
-assert_dropin "$(comp_enable_dropin compy)" no
-# compx must be left as it was (still enabled from above).
+assert_dropin "$(comp_enable_dropin compx)" no
+test ! -e "$(comp_enable_dropin compy)"
+
+# Reconciling the system with the current suggestions is a two step operation:
+# disable all components, then enable the suggested ones. Unlike the single
+# 'disable-component --component-suggested' above this makes the non-suggested
+# compy explicitly disabled.
+"$SYSUPDATE" --component-all disable-component
+"$SYSUPDATE" --component-suggested enable-component
 assert_dropin "$(comp_enable_dropin compx)" yes
+assert_dropin "$(comp_enable_dropin compy)" no
 
 # --component-suggested is not supported for the update verb.
 (! "$SYSUPDATE" --component-suggested --verify=no update) |& grep -F "not supported" >/dev/null
@@ -1816,6 +1824,12 @@ assert_dropin "$(feat_enable_dropin_default feata)" yes
 assert_dropin "$(feat_enable_dropin_default featb)" yes
 assert_dropin "$(feat_enable_dropin_default featc)" yes
 
+# ... and so does 'disable-feature --feature-all', in the other direction.
+"$SYSUPDATE" disable-feature --feature-all
+assert_dropin "$(feat_enable_dropin_default feata)" no
+assert_dropin "$(feat_enable_dropin_default featb)" no
+assert_dropin "$(feat_enable_dropin_default featc)" no
+
 # --feature-suggested (no machine tag): only the Suggest=yes feature is picked.
 rm -rf /etc/sysupdate.d
 set_machine_tags unrelated
@@ -1831,6 +1845,21 @@ set_machine_tags sysupdate-test-tag
 assert_dropin "$(feat_enable_dropin_default feata)" yes
 assert_dropin "$(feat_enable_dropin_default featc)" yes
 test ! -e "$(feat_enable_dropin_default featb)"
+
+# 'disable-feature --feature-suggested' selects the same features as the enable
+# above, i.e. it undoes it and leaves the non-suggested featb untouched.
+"$SYSUPDATE" disable-feature --feature-suggested
+assert_dropin "$(feat_enable_dropin_default feata)" no
+assert_dropin "$(feat_enable_dropin_default featc)" no
+test ! -e "$(feat_enable_dropin_default featb)"
+
+# The same two step reconciliation for features: disable all of them, then
+# enable the suggested ones, leaving the non-suggested featb explicitly off.
+"$SYSUPDATE" disable-feature --feature-all
+"$SYSUPDATE" enable-feature --feature-suggested
+assert_dropin "$(feat_enable_dropin_default feata)" yes
+assert_dropin "$(feat_enable_dropin_default featc)" yes
+assert_dropin "$(feat_enable_dropin_default featb)" no
 set_machine_tags ""
 
 # --component-all must still include the default component when all of its