move logic out of async fn to avoid race, and fixups

Signed-off-by: William Woodall <[email protected]>

Author: wjwwood

launch/launch/actions/execute_local.py

@@ -578,17 +578,6 @@ async def __execute_process(self, context: LaunchContext) -> None:
             raise RuntimeError('process_event_args unexpectedly None')
 
         cmd = process_event_args['cmd']
-        if evaluate_condition_expression(context, self.__split_arguments):
-            if self.__shell:
-                self.__logger.debug("Ignoring 'split_arguments=True' because 'shell=True'.")
-            else:
-                self.__logger.debug("Splitting arguments because 'split_arguments=True'.")
-                expanded_cmd = []
-                for token in cmd:
-                    expanded_cmd.extend(shlex.split(token))
-                cmd = expanded_cmd
-                # Also update process_event_args so they reflect the splitting.
-                process_event_args['cmd'] = cmd
         cwd = process_event_args['cwd']
         env = process_event_args['env']
         if self.__log_cmd:
@@ -759,6 +748,18 @@ def execute(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEnti
             else:
                 self.__stdout_logger, self.__stderr_logger = \
                     launch.logging.get_output_loggers(name, self.__output)
+            # Update the cmd to respect split_arguments option.
+            if evaluate_condition_expression(context, self.__split_arguments):
+                if self.__shell:
+                    self.__logger.debug("Ignoring 'split_arguments=True' because 'shell=True'.")
+                else:
+                    self.__logger.debug("Splitting arguments because 'split_arguments=True'.")
+                    expanded_cmd = []
+                    assert self.__process_event_args is not None
+                    for token in self.__process_event_args['cmd']:
+                        expanded_cmd.extend(shlex.split(token))
+                    # Also update self.__process_event_args['cmd'] so it reflects the splitting.
+                    self.__process_event_args['cmd'] = expanded_cmd
             context.asyncio_loop.create_task(self.__execute_process(context))
         except Exception:
             for event_handler in event_handlers:

launch/launch/actions/execute_process.py

@@ -262,19 +262,21 @@ def _append_arg():
             nonlocal arg
             result_args.append(arg)
             arg = []
+
         for sub in parser.parse_substitution(cmd):
             if isinstance(sub, TextSubstitution):
                 try:
                     tokens = shlex.split(sub.text)
-                except:
+                except Exception:
                     logger = launch.logging.get_logger(cls.__name__)
                     logger.error(f"Failed to parse token '{sub.text}' of cmd '{cmd}'")
                     raise
-                if not tokens:
-                    # String with just spaces.
+                if len(tokens) == 0:
+                    # String with just spaces produces an empty list.
                     # Appending args allow splitting two substitutions
-                    # separated by a space.
-                    # e.g.: `$(subst1 asd) $(subst2 bsd)` will be two separate arguments.
+                    # separated by some amount of whitespace.
+                    # e.g.: `$(subst1 asd) $(subst2 bsd)` will be two separate arguments,
+                    # first `$(subst1 asd)`, then this case ` `, then the `$(subst2 bsd)`.
                     _append_arg()
                     continue
                 if sub.text[0].isspace():

launch/test/launch/test_execute_process.py

@@ -29,7 +29,6 @@
 from launch.actions.register_event_handler import RegisterEventHandler
 from launch.actions.shutdown_action import Shutdown
 from launch.actions.timer_action import TimerAction
-from launch.conditions import evaluate_condition_expression
 from launch.event_handlers.on_process_start import OnProcessStart
 from launch.events.shutdown import Shutdown as ShutdownEvent
 from launch.substitutions.launch_configuration import LaunchConfiguration