Revert "run: disable --expand-environment by default for --scope"

This reverts commit 8167c56bfa97525a7b12e7c5685576657364e3cf.

We've announced the breaking change during v254-v257. Let's actually
apply it for v258.

NEWS

@@ -2,7 +2,14 @@ systemd System and Service Manager
 
 CHANGES WITH 258 in spe:
 
-       Announcements of Future Feature Removals:
+        Incompatible changes:
+
+        * systemd-run's --expand-environment= switch, which was disabled
+          by default when combined with --scope, has been changed to to be
+          enabled by default. This brings cmdline expansion of transient
+          scopes on par with services.
+
+        Announcements of Future Feature Removals:
 
         * The D-Bus method org.freedesktop.systemd1.StartAuxiliaryScope() is
           deprecated because accounting data and such cannot be reasonably
@@ -41,10 +48,6 @@ CHANGES WITH 258 in spe:
           altogether at a later point, but this might be revisited based on
           user feedback.
 
-        * systemd-run's switch --expand-environment= which currently is disabled
-          by default when combined with --scope, will be changed in a future
-          release to be enabled by default.
-
         — <place>, <date>
 
 CHANGES WITH 257:

man/systemd-run.xml

@@ -185,19 +185,15 @@
       <varlistentry>
         <term><option>--expand-environment=<replaceable>BOOL</replaceable></option></term>
 
-        <listitem><para>Expand environment variables in command arguments. If enabled, environment variables
+        <listitem><para>Expand environment variables in command arguments. If enabled (the default),
-        specified as <literal>${<replaceable>VARIABLE</replaceable>}</literal> will be expanded in the same
+        environment variables specified as <literal>${<replaceable>VARIABLE</replaceable>}</literal> will be
-        way as in commands specified via <varname>ExecStart=</varname> in units. With
+        expanded in the same way as in commands specified via <varname>ExecStart=</varname> in units. With
         <varname>--scope</varname>, this expansion is performed by <command>systemd-run</command> itself, and
         in other cases by the service manager that spawns the command. Note that this is similar to, but not
         the same as variable expansion in
         <citerefentry project='man-pages'><refentrytitle>bash</refentrytitle><manvolnum>1</manvolnum></citerefentry>
         and other shells.</para>
 
-        <para>The default is to enable this option in all cases, except for <varname>--scope</varname> where
-        it is disabled by default, for backward compatibility reasons. Note that this will be changed in a
-        future release, where it will be switched to enabled by default as well.</para>
-
         <para>See
         <citerefentry><refentrytitle>systemd.service</refentrytitle><manvolnum>5</manvolnum></citerefentry>
         for a description of variable expansion. Disabling variable expansion is useful if the specified

src/run/run.c

@@ -55,7 +55,7 @@ static const char *arg_unit = NULL;
 static char *arg_description = NULL;
 static const char *arg_slice = NULL;
 static bool arg_slice_inherit = false;
-static int arg_expand_environment = -1;
+static bool arg_expand_environment = true;
 static bool arg_send_sighup = false;
 static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
 static const char *arg_host = NULL;
@@ -394,17 +394,11 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_slice_inherit = true;
                         break;
 
-                case ARG_EXPAND_ENVIRONMENT: {
+                case ARG_EXPAND_ENVIRONMENT:
-                        bool b;
+                        r = parse_boolean_argument("--expand-environment=", optarg, &arg_expand_environment);
-
-                        r = parse_boolean_argument("--expand-environment=", optarg, &b);
                         if (r < 0)
                                 return r;
-
-                        arg_expand_environment = b;
-
                         break;
-                }
 
                 case ARG_SEND_SIGHUP:
                         arg_send_sighup = true;
@@ -1086,6 +1080,7 @@ static int transient_cgroup_set_properties(sd_bus_message *m) {
         _cleanup_free_ char *name = NULL;
         _cleanup_free_ char *slice = NULL;
         int r;
+
         assert(m);
 
         if (arg_slice_inherit) {
@@ -1154,7 +1149,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p
         /* We disable environment expansion on the server side via ExecStartEx=:.
          * ExecStartEx was added relatively recently (v243), and some bugs were fixed only later.
          * So use that feature only if required. It will fail with older systemds. */
-        bool use_ex_prop = arg_expand_environment == 0;
+        bool use_ex_prop = !arg_expand_environment;
 
         assert(m);
         assert(pty_path || pty_fd < 0);
@@ -1324,7 +1319,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p
                         _cleanup_strv_free_ char **opts = NULL;
 
                         r = exec_command_flags_to_strv(
-                                        (arg_expand_environment > 0 ? 0 : EXEC_COMMAND_NO_ENV_EXPAND)|(arg_ignore_failure ? EXEC_COMMAND_IGNORE_FAILURE : 0),
+                                        (arg_expand_environment ? 0 : EXEC_COMMAND_NO_ENV_EXPAND)|(arg_ignore_failure ? EXEC_COMMAND_IGNORE_FAILURE : 0),
                                         &opts);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to format execute flags: %m");
@@ -1789,7 +1784,7 @@ static int bus_call_with_hint(
         if (r < 0) {
                 log_error_errno(r, "Failed to start transient %s unit: %s", name, bus_error_message(&error, r));
 
-                if (arg_expand_environment == 0 &&
+                if (!arg_expand_environment &&
                     sd_bus_error_has_names(&error,
                                            SD_BUS_ERROR_UNKNOWN_PROPERTY,
                                            SD_BUS_ERROR_PROPERTY_READ_ONLY))
@@ -2344,7 +2339,7 @@ static int start_transient_scope(sd_bus *bus) {
                         return r;
         }
 
-        if (arg_expand_environment > 0) {
+        if (arg_expand_environment) {
                 _cleanup_strv_free_ char **expanded_cmdline = NULL, **unset_variables = NULL, **bad_variables = NULL;
 
                 r = replace_env_argv(arg_cmdline, env, &expanded_cmdline, &unset_variables, &bad_variables);
@@ -2612,18 +2607,6 @@ static int run(int argc, char* argv[]) {
                         return log_oom();
         }
 
-        /* For backward compatibility reasons env var expansion is disabled by default for scopes, and
-         * enabled by default for everything else. Try to detect it and print a warning, so that we can
-         * change it in the future and harmonize it. */
-        if (arg_expand_environment < 0) {
-                arg_expand_environment = !arg_scope;
-
-                if (!arg_quiet && arg_scope && strchr(arg_description, '$'))
-                        log_warning("Scope command line contains environment variable, which is not expanded"
-                                    " by default for now, but will be expanded by default in the future."
-                                    " Use --expand-environment=yes/no to explicitly control it as needed.");
-        }
-
         r = connect_bus(&bus);
         if (r < 0)
                 return r;