Mpi launch fix

[wxtim]

closes #2573

Summary of problem

Rose MPI launch fails if ROSE_LAUNCHER contains spaces.

[oliver-sanders]

Possibly broken by shellcheck changes?

[oliver-sanders]

I'm not sure I understand this solution:

    for command in ${ROSE_LAUNCHER}; do
        if  command="$(type -P "$command")"

I'm not sure each item in $ROSE_LAUNCHER is supposed to be a separate command?

I think this was probably broken by shellcheck changes (my bad) because otherwise this script is more or less unchanged.

Here's the full diff to Rose 2019:

--- mpi.2019.01.x	2022-03-31 15:36:09.771016000 +0100
+++ mpi.master	        2022-03-31 15:36:40.450043000 +0100
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #-------------------------------------------------------------------------------
-# Copyright (C) 2012-2020 British Crown (Met Office) & Contributors.
+# Copyright (C) British Crown (Met Office) & Contributors.
 #
 # This file is part of Rose, a framework for meteorological suites.
 #
@@ -54,7 +54,8 @@
 #
 # CONFIGURATION
 #     The command reads from the `[rose-mpi-launch]` section in
-#     `$ROSE_HOME/etc/rose.conf` and `$HOME/.metomi/rose.conf`.
+#     the Rose configuration.
+#
 #     Valid settings are:
 #
 #     launcher-list=LIST
@@ -111,14 +112,23 @@
 # DIAGNOSTICS
 #     Return 0 on success, 1 or exit code of the launcher program on failure.
 #-------------------------------------------------------------------------------
-. $(dirname $0)/../lib/bash/rose_init
-rose_init rose_log
+set -eu
+# shellcheck source=metomi/rose/etc/lib/bash/rose_log
+. "$(rose resource lib/bash/rose_log)"
+# shellcheck source=metomi/rose/etc/lib/bash/rose_usage
+. "$(rose resource lib/bash/rose_usage)"
+
+ROSE_CMD="${ROSE_NS}-${ROSE_UTIL}"
 
 # ------------------------------------------------------------------------------
 ROSE_COMMAND=
 ROSE_COMMAND_FILE=
 while (($# > 0)); do
     case $1 in
+    --help)
+        rose_help
+        exit
+        :;;
     --command-file=*)
         ROSE_COMMAND_FILE=${1#*=}
         shift 1
@@ -134,6 +144,9 @@
     --inner)
         shift 1
         if [[ -n ${ROSE_LAUNCHER_ULIMIT_OPTS:-} ]]; then
+            # shellcheck disable=SC2086
+            # permit word splitting in ROSE_LAUNCHER_ULIMIT_OPTS
+            # alternative would be to use an array
             while getopts 'HST:ab:c:d:e:f:i:l:m:n:p:q:r:s:t:u:v:x:' \
                 OPT $ROSE_LAUNCHER_ULIMIT_OPTS
             do
@@ -189,14 +202,14 @@
     if (($# < 1)); then
         rose_usage 1
     fi
-    if ! ROSE_COMMAND=$(type -P $1); then
+    if ! ROSE_COMMAND="$(type -P "$1")"; then
         err "$1: COMMAND not found"
     fi
     shift 1
 fi
 
 ROSE_LAUNCHER_LIST=${ROSE_LAUNCHER_LIST:-$( \
-    rose config --default= $ROSE_NS launcher-list)}
+    rose config --default= "$ROSE_CMD" launcher-list)}
 export NPROC=${NPROC:-1}
 
 #-------------------------------------------------------------------------------
@@ -211,7 +224,7 @@
 if ! printenv ROSE_LAUNCHER >/dev/null; then
     ROSE_LAUNCHER=
     for LAUNCHER in $ROSE_LAUNCHER_LIST; do
-        if type -P $LAUNCHER >/dev/null; then
+        if type -P "$LAUNCHER" >/dev/null; then
             ROSE_LAUNCHER=$LAUNCHER
             break
         fi
@@ -224,11 +237,11 @@
 ROSE_LAUNCHER_BASE=
 if [[ -n $ROSE_LAUNCHER ]]; then
     # Path
-    if ! ROSE_LAUNCHER_PATH=$(type -P $ROSE_LAUNCHER); then
+    if ! ROSE_LAUNCHER_PATH="$(type -P "$ROSE_LAUNCHER")"; then
         err "ROSE_LAUNCHER: $ROSE_LAUNCHER: command not found"
     fi
     ROSE_LAUNCHER=$ROSE_LAUNCHER_PATH
-    ROSE_LAUNCHER_BASE=$(basename $ROSE_LAUNCHER)
+    ROSE_LAUNCHER_BASE="$(basename "$ROSE_LAUNCHER")"
 fi
 
 #-------------------------------------------------------------------------------
@@ -239,9 +252,10 @@
         err "ROSE_LAUNCHER not defined, command file not supported."
     fi
     ROSE_LAUNCHER_FILEOPTS=${ROSE_LAUNCHER_FILEOPTS:-$( \
-        rose config --default= $ROSE_NS launcher-fileopts.$ROSE_LAUNCHER_BASE)}
-    eval "info 2 exec $ROSE_LAUNCHER $ROSE_LAUNCHER_FILEOPTS $@"
-    eval "exec $ROSE_LAUNCHER $ROSE_LAUNCHER_FILEOPTS $@"
+        rose config --default= \
+        "$ROSE_CMD" "launcher-fileopts.$ROSE_LAUNCHER_BASE")}
+    eval "info 2 exec $ROSE_LAUNCHER $ROSE_LAUNCHER_FILEOPTS $*"
+    eval "exec $ROSE_LAUNCHER $ROSE_LAUNCHER_FILEOPTS $*"
 else
     if [[ -n $ROSE_LAUNCHER_BASE ]]; then
         ROSE_LAUNCH_INNER=
@@ -250,9 +264,9 @@
         fi
         # Options
         ROSE_LAUNCHER_PREOPTS=${ROSE_LAUNCHER_PREOPTS:-$(rose config -E \
-            --default= $ROSE_NS launcher-preopts.$ROSE_LAUNCHER_BASE)}
+            --default= "$ROSE_CMD" "launcher-preopts.$ROSE_LAUNCHER_BASE")}
         ROSE_LAUNCHER_POSTOPTS=${ROSE_LAUNCHER_POSTOPTS:-$(rose config -E \
-            --default= $ROSE_NS launcher-postopts.$ROSE_LAUNCHER_BASE)}
+            --default= "$ROSE_CMD" "launcher-postopts.$ROSE_LAUNCHER_BASE")}
     else
         ROSE_LAUNCH_INNER=
         ROSE_LAUNCHER_PREOPTS=
@@ -261,8 +275,10 @@
     if ((ROSE_VERBOSITY >= 3)); then
         info 3 printenv
         printenv | sort
-        run ldd $ROSE_COMMAND || true 2>/dev/null
+        run ldd "$ROSE_COMMAND" || true 2>/dev/null
     fi
+    # shellcheck disable=SC2086
+    # permit word splitting in command / argument strings
     info 2 exec \
         $ROSE_LAUNCHER \
         $ROSE_LAUNCHER_PREOPTS \
@@ -270,6 +286,8 @@
         "$ROSE_COMMAND" \
         $ROSE_LAUNCHER_POSTOPTS \
         "$@"
+    # shellcheck disable=SC2086
+    # permit word splitting in command / argument strings
     exec \
         $ROSE_LAUNCHER \
         $ROSE_LAUNCHER_PREOPTS \

My guess would be the quotes added around LAUNCHER & ROSE_LAUNCHER variables should not have been added.

[wxtim]

I'm not sure each item in $ROSE_LAUNCHER is supposed to be a separate command?

This is the case with the examples in the issue : @dpmatthews - are the items in $ROSE_LAUNCHER separate commands?

[wxtim]

My guess would be the quotes added around LAUNCHER & ROSE_LAUNCHER variables should not have been added.

I think you are right about ROSE_LAUNCHER, but after talking with @dpmatthews I'm not removing quotes from LAUNCHER because I don't think that we want to split launcher - It is set in a for loop which appears to rely on word splitting from ROSE_LAUNCHER_LIST.

[dpmatthews]

Happy with the fix but I haven't checked the new test

[oliver-sanders]

Do we have a real-world example we can try this out against?

[dpmatthews]

The example problems I gave in #2573 were both taken from real world examples. This change fixes those problems.

[oliver-sanders]

Ok going in.