fix handling of quotes in rpath wrapper

In some software the compiler must be invoked with an argument
containing a literal single quote which requires escaping in the call.
E.g.: `gcc -DFOO=\'value\'`

However the current rpath wrapper would remove those single quotes
causing errors during compilation or erronous behavior of the compiled binary.

Fix by replacing the `eval` of the `rpath_args.py` output by `readarray -t`.
An array asignment could be used (`CMD_ARGS=( $(rpath_args.py ...) )`)
but that would break up arguments containing spaces.
Quotes from the output are kept literally, so quoting to avoid this is not possible.
`readarray -t` works and is widely available.

Also some minor fixes related to quoting in the bash script template.

Author: Flamefire

easybuild/scripts/rpath_args.py

@@ -25,11 +25,10 @@
 ##
 """
 Utility script used by RPATH wrapper script;
-output is statements that define the following environment variables
-* $CMD_ARGS: new list of command line arguments to pass
-* $RPATH_ARGS: command line option to specify list of paths to RPATH
+output is a list of arguments to be passed to the wrapped command, one per line
 
 author: Kenneth Hoste (HPC-UGent)
+author: Alexander Grund (TU Dresden)
 """
 import os
 import re
@@ -161,8 +160,12 @@ def is_new_existing_path(new_path, paths):
     # add -rpath flags in front
     cmd_args = cmd_args_rpath + cmd_args
 
-# wrap all arguments into single quotes to avoid further bash expansion
-cmd_args = ["'%s'" % a.replace("'", "''") for a in cmd_args]
 
-# output: statement to define $CMD_ARGS and $RPATH_ARGS
-print("CMD_ARGS=(%s)" % ' '.join(cmd_args))
+def quote(arg):
+    """Quote the argument for use in a shell to avoid further expansion"""
+    # Wrap in single quotes and replace every single quote by '"'"'
+    # This terminates the single-quoted string, inserts a literal single quote and starts a new single-quotest-string
+    return "'" + arg.replace("'", "'\"'\"'") + "'"
+
+
+print('\n'.join(cmd_args))

easybuild/scripts/rpath_wrapper_template.sh.in

@@ -30,20 +30,21 @@
 # before actually calling the original compiler/linker command.
 #
 # author: Kenneth Hoste (HPC-UGent)
+# author: Alexander Grund (TU Dresden)
 
 set -e
 
 # logging function
 function log {
     # escape percent signs, since this is a template script
     # that will templated using Python string templating
-    echo "($$) [$(date "+%%Y-%%m-%%d %%H:%%M:%%S")] $1" >> %(rpath_wrapper_log)s
+    echo "($$) [$(date "+%%Y-%%m-%%d %%H:%%M:%%S")] $1" >> '%(rpath_wrapper_log)s'
 }
 
 # command name
-CMD=`basename $0`
+CMD=$(basename $0)
 
-log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: '$(echo \"$@\")'"
+log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: $*)"
 
 # rpath_args.py script spits out statement that defines $CMD_ARGS
 # options for 'python' command (see https://docs.python.org/3/using/cmdline.html#miscellaneous-options)
@@ -52,18 +53,12 @@ log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: '$(echo \"$@\
 # * -s: don’t add the user site-packages directory to sys.path;
 # * -S: disable the import of the module site and the site-dependent manipulations of sys.path that it entails;
 # (once we only support Python 3, we can (also) use -I (isolated mode)
-log "%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' $(echo \"$@\")'"
-rpath_args_out=$(%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' "$@")
-
-log "rpath_args_out:
-$rpath_args_out"
-
-# define $CMD_ARGS by evaluating output of rpath_args.py script
-eval $rpath_args_out
+log "%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' $*"
+readarray -t CMD_ARGS < <( %(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' "$@" )
 
 # exclude location of this wrapper from $PATH to avoid other potential wrappers calling this wrapper
-export PATH=$(echo $PATH | tr ':' '\n' | grep -v "^%(wrapper_dir)s$" | tr '\n' ':')
+export PATH=$(echo "$PATH" | tr ':' '\n' | grep -v "^%(wrapper_dir)s$" | tr '\n' ':')
 
 # call original command with modified list of command line arguments
-log "running '%(orig_cmd)s $(echo ${CMD_ARGS[@]})'"
-%(orig_cmd)s "${CMD_ARGS[@]}"
+log "running: %(orig_cmd)s $(echo "${CMD_ARGS[@]}")"
+'%(orig_cmd)s' "${CMD_ARGS[@]}"

test/framework/toolchain.py

@@ -2744,9 +2744,12 @@ def test_rpath_args_script(self):
     def test_toolchain_prepare_rpath(self):
         """Test toolchain.prepare under --rpath"""
 
+        # Code for a bash script that prints each passed argument on a new line
+        BASH_SCRIPT_PRINT_ARGS = '#!/bin/bash\nfor arg in "$@"; do echo "$arg"; done'
+
         # put fake 'g++' command in place that just echos its arguments
         fake_gxx = os.path.join(self.test_prefix, 'fake', 'g++')
-        write_file(fake_gxx, '#!/bin/bash\necho "$@"')
+        write_file(fake_gxx, BASH_SCRIPT_PRINT_ARGS)
         adjust_permissions(fake_gxx, stat.S_IXUSR)
         os.environ['PATH'] = '%s:%s' % (os.path.join(self.test_prefix, 'fake'), os.getenv('PATH', ''))
 
@@ -2789,7 +2792,7 @@ def test_toolchain_prepare_rpath(self):
         # Check that we can create a wrapper for a toolchain for which self.compilers() returns 'None' for the Fortran
         # compilers (i.e. Clang)
         fake_clang = os.path.join(self.test_prefix, 'fake', 'clang')
-        write_file(fake_clang, '#!/bin/bash\necho "$@"')
+        write_file(fake_clang, BASH_SCRIPT_PRINT_ARGS)
         adjust_permissions(fake_clang, stat.S_IXUSR)
         tc_clang = Clang(name='Clang', version='1')
         tc_clang.prepare_rpath_wrappers()
@@ -2852,18 +2855,25 @@ def test_toolchain_prepare_rpath(self):
             '-L%s/foo' % self.test_prefix,
             '-L/bar',
             "'$FOO'",
-            '-DX="\\"\\""',
+            # C/C++ preprocessor value including a quotes
+            r'-DX1="\"\""',
+            r'-DX2="\"I\""',
+            r"-DY1=\'\'",
+            r"-DY2=\'J\'",
         ])
         out, ec = run_cmd(cmd)
         self.assertEqual(ec, 0)
-        expected = ' '.join([
+        expected = '\n'.join([
             '-Wl,--disable-new-dtags',
             '-Wl,-rpath=%s/foo' % self.test_prefix,
             '%(user)s.c',
             '-L%s/foo' % self.test_prefix,
             '-L/bar',
             '$FOO',
-            '-DX=""',
+            '-DX1=""',
+            '-DX2="I"',
+            "-DY1=''",
+            "-DY2='J'",
         ])
         self.assertEqual(out.strip(), expected % {'user': os.getenv('USER')})
 
@@ -2888,35 +2898,43 @@ def test_toolchain_prepare_rpath(self):
         for path in paths:
             mkdir(path, parents=True)
         args = ['-L%s' % x for x in paths]
+        path_with_spaces = os.path.join(self.test_prefix, 'prefix/with spaces')
+        mkdir(path_with_spaces)
+        args.append('-L"%s"' % path_with_spaces)
 
         cmd = "g++ ${USER}.c %s" % ' '.join(args)
         out, ec = run_cmd(cmd, simple=False)
         self.assertEqual(ec, 0)
 
-        expected = ' '.join([
+        expected = '\n'.join([
             '-Wl,--disable-new-dtags',
-            '-Wl,-rpath=%s/tmp/foo/' % self.test_prefix,
-            '-Wl,-rpath=%s/prefix/software/stubs/1.2.3/lib' % self.test_prefix,
-            '-Wl,-rpath=%s/prefix/software/foobar/4.5/notreallystubs' % self.test_prefix,
-            '-Wl,-rpath=%s/prefix/software/zlib/1.2.11/lib' % self.test_prefix,
-            '-Wl,-rpath=%s/prefix/software/foobar/4.5/stubsbutnotreally' % self.test_prefix,
+            '-Wl,-rpath=%(libdir)s/tmp/foo/',
+            '-Wl,-rpath=%(libdir)s/prefix/software/stubs/1.2.3/lib',
+            '-Wl,-rpath=%(libdir)s/prefix/software/foobar/4.5/notreallystubs',
+            '-Wl,-rpath=%(libdir)s/prefix/software/zlib/1.2.11/lib',
+            '-Wl,-rpath=%(libdir)s/prefix/software/foobar/4.5/stubsbutnotreally',
+            '-Wl,-rpath=%(path_with_spaces)s',
             '%(user)s.c',
-            '-L%s/prefix/software/CUDA/1.2.3/lib/stubs/' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/stubs/lib/' % self.test_prefix,
-            '-L%s/tmp/foo/' % self.test_prefix,
-            '-L%s/prefix/software/stubs/1.2.3/lib' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/lib/stubs' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/stubs/lib' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/lib64/stubs/' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/stubs/lib64/' % self.test_prefix,
-            '-L%s/prefix/software/foobar/4.5/notreallystubs' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/lib64/stubs' % self.test_prefix,
-            '-L%s/prefix/software/CUDA/1.2.3/stubs/lib64' % self.test_prefix,
-            '-L%s/prefix/software/zlib/1.2.11/lib' % self.test_prefix,
-            '-L%s/prefix/software/bleh/0/lib/stubs' % self.test_prefix,
-            '-L%s/prefix/software/foobar/4.5/stubsbutnotreally' % self.test_prefix,
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib/stubs/',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib/',
+            '-L%(libdir)s/tmp/foo/',
+            '-L%(libdir)s/prefix/software/stubs/1.2.3/lib',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib/stubs',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib64/stubs/',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib64/',
+            '-L%(libdir)s/prefix/software/foobar/4.5/notreallystubs',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib64/stubs',
+            '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib64',
+            '-L%(libdir)s/prefix/software/zlib/1.2.11/lib',
+            '-L%(libdir)s/prefix/software/bleh/0/lib/stubs',
+            '-L%(libdir)s/prefix/software/foobar/4.5/stubsbutnotreally',
+            '-L%(path_with_spaces)s',
         ])
-        self.assertEqual(out.strip(), expected % {'user': os.getenv('USER')})
+        self.assertEqual(out.strip(), expected % {'libdir': self.test_prefix,
+                                                  'user': os.getenv('USER'),
+                                                  'path_with_spaces': path_with_spaces,
+                                                  })
 
         # calling prepare() again should *not* result in wrapping the existing RPATH wrappers
         # this can happen when building extensions