FIX run_cmd bug with shell=False

easybuild/tools/run.py

@@ -146,9 +146,12 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True
         runLog.write(cmd + "\n\n")
     else:
         runLog = None
+
+    exec_cmd = "/bin/bash"
 
     if not shell:
         if isinstance(cmd, list):
+            exec_cmd = None
             cmd.insert(0, '/usr/bin/env')
         elif isinstance(cmd, basestring):
             cmd = '/usr/bin/env %s' % cmd
@@ -159,7 +162,7 @@ def run_cmd(cmd, log_ok=True, log_all=False, simple=False, inp=None, regexp=True
     _log.info('running cmd: %s ' % cmd)
     try:
         p = subprocess.Popen(cmd, shell=shell, stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-                             stdin=subprocess.PIPE, close_fds=True, executable="/bin/bash")
+                             stdin=subprocess.PIPE, close_fds=True, executable=exec_cmd)
     except OSError, err:
         raise EasyBuildError("run_cmd init cmd %s failed:%s", cmd, err)
     if inp:

test/framework/package.py

@@ -44,86 +44,99 @@
 
 DEBUG = False
 DEBUG_FPM_FILE = "debug_fpm_mock"
-MOCKED_FPM = """#!/bin/bash
-
-DEBUG=%(debug)s  #put something here if you want to debug
-
-debug_echo () {
-    if [ -n "$DEBUG" ]; then
-        echo "$@" >> %(debug_fpm_file)s
-    fi
-}
-
-debug_echo "$@"
-
-#an array of excludes (probably more than one)
-excludes=()
-# only parse what we need to spit out the expected package file, ignore the rest
-while true
-do
-    debug_echo "arg: $1"
-    case "$1" in
-        "--workdir")
-            workdir="$2"
-            debug_echo "workdir"
-            debug_echo "$workdir"
-            ;;
-        "--name")
-            name="$2"
-            ;;
-        "--version")
-            version="$2"
-            debug_echo "version"
-            debug_echo "$version"
-            ;;
-        "--description")
-            description="$2"
-            ;;
-        "--url")
-            url="$2"
-            ;;
-        "--iteration")
-            iteration="$2"
-            ;;
-        "-t")
-            target="$2"
-            ;;
-        "-s")
-            source="$2"
-            ;;
-        "--exclude")
-            # pushing this onto an array
-            debug_echo "an exclude being pushed" $2
-            excludes+=("$2")
-            ;;
-        --*)
-            debug_echo "got a unhandled option"
-            ;;
-        *)
-            debug_echo "got the rest of the output"
-            installdir="$1"
-            modulefile="$2"
-            break
-            ;;
-    esac
-    shift 2
-done
-
-pkgfile=${workdir}/${name}-${version}.${iteration}.${target}
-echo "thisisan$target" > $pkgfile
-echo "$@" >> $pkgfile
-echo "STARTCONTENTS of installdir $installdir:" >> $pkgfile
-for exclude in ${excludes[*]}; do
-    exclude_str+=" -not -path /${exclude} "
-done
-find_cmd="find $installdir  $exclude_str "
-debug_echo "trying: $find_cmd"
-$find_cmd >> $pkgfile
-echo "ENDCONTENTS" >> $pkgfile
-echo "Contents of module file $modulefile:" >> $pkgfile
-cat $modulefile >> $pkgfile
-echo "I found excludes "${excludes[*]} >> $pkgfile
-echo "DESCRIPTION: $description" >> $pkgfile
+
+# purposely using non-bash script, to detect issues with shebang line being ignored (run_cmd with shell=False)
+MOCKED_FPM = """#!/usr/bin/env python
+import os, sys
+
+def debug(msg):
+    if '%(debug)s':
+        fp = open('%(debug_fpm_file)s', 'a')
+        fp.write(msg +'\\n')
+        fp.close()
+
+description, iteration, name, source, target, url, version, workdir = '', '', '', '', '', '', '', ''
+excludes = []
+
+debug(' '.join(sys.argv[1:]))
+
+idx = 1
+while idx < len(sys.argv):
+
+    if sys.argv[idx] == '--workdir':
+        idx += 1
+        workdir = sys.argv[idx]
+        debug('workdir'); debug(workdir)
+
+    elif sys.argv[idx] == '--name':
+        idx += 1
+        name = sys.argv[idx]
+
+    elif sys.argv[idx] == '--version':
+        idx += 1
+        version = sys.argv[idx]
+        debug('version'); debug(version)
+
+    elif sys.argv[idx] == '--description':
+        idx += 1
+        description = sys.argv[idx]
+
+    elif sys.argv[idx] == '--url':
+        idx += 1
+        url = sys.argv[idx]
+
+    elif sys.argv[idx] == '--iteration':
+        idx += 1
+        iteration = sys.argv[idx]
+
+    elif sys.argv[idx] == '-t':
+        idx += 1
+        target = sys.argv[idx]
+
+    elif sys.argv[idx] == '-s':
+        idx += 1
+        source = sys.argv[idx]
+
+    elif sys.argv[idx] == '--exclude':
+        idx += 1
+        excludes.append(sys.argv[idx])
+
+    elif sys.argv[idx].startswith('--'):
+        debug("got a unhandled option: " + sys.argv[idx] + ' ' + sys.argv[idx+1])
+        idx += 1
+
+    else:
+        installdir = sys.argv[idx]
+        modulefile = sys.argv[idx+1]
+        break
+
+    idx += 1
+
+pkgfile = os.path.join(workdir, name + '-' + version + '.' + iteration + '.' + target)
+
+fp = open(pkgfile, 'w')
+
+fp.write('thisisan' + target + '\\n')
+fp.write(' '.join(sys.argv[1:]) + '\\n')
+fp.write("STARTCONTENTS of installdir " + installdir + ':\\n')
+
+find_cmd = 'find ' + installdir + '  ' + ''.join([" -not -path /" + x + ' ' for x in excludes])
+debug("trying: " + find_cmd)
+fp.write(find_cmd + '\\n')
+
+fp.write('ENDCONTENTS\\n')
+
+fp.write("Contents of module file " + modulefile + ':')
+
+
+fp.write('modulefile: ' + modulefile + '\\n')
+#modtxt = open(modulefile).read()
+#fp.write(modtxt + '\\n')
+
+fp.write("I found excludes " + ' '.join(excludes) + '\\n')
+fp.write("DESCRIPTION: " + description + '\\n')
+
+fp.close()
 """
 
 
@@ -207,8 +220,14 @@ def test_package(self):
 
         # package using default packaging configuration (FPM to build RPM packages)
         pkgdir = package(easyblock)
-
         pkgfile = os.path.join(pkgdir, 'toy-0.0-gompi-1.3.12-test-eb-%s.1.rpm' % EASYBUILD_VERSION)
+
+        if DEBUG:
+            print "The FPM script debug output"
+            print read_file(os.path.join(self.test_prefix, DEBUG_FPM_FILE))
+            print "The Package File"
+            print read_file(pkgfile)
+
         self.assertTrue(os.path.isfile(pkgfile), "Found %s" % pkgfile)
 
         pkgtxt = read_file(pkgfile)
@@ -218,12 +237,6 @@ def test_package(self):
         no_logfiles_regex = re.compile(r'STARTCONTENTS.*\.(log|md)$.*ENDCONTENTS', re.DOTALL|re.MULTILINE)
         self.assertFalse(no_logfiles_regex.search(pkgtxt), "Pattern not '%s' found in: %s" % (no_logfiles_regex.pattern, pkgtxt))
 
-        if DEBUG:
-            print "The FPM script debug output"
-            print read_file(os.path.join(self.test_prefix, DEBUG_FPM_FILE))
-            print "The Package File"
-            print read_file(pkgfile)
-
         toy_txt = read_file(os.path.join(test_easyconfigs, 't', 'toy', 'toy-0.0-gompi-1.3.12-test.eb'))
         replace_str = '''description = """Toy C program. Now with `backticks'\n'''
         replace_str += '''and newlines"""'''

test/framework/run.py

@@ -32,13 +32,14 @@
 import os
 import re
 import signal
+import stat
 import sys
 from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config
 from unittest import TextTestRunner
 from vsc.utils.fancylogger import setLogLevelDebug, logToScreen
 
 from easybuild.tools.build_log import EasyBuildError
-from easybuild.tools.filetools import read_file
+from easybuild.tools.filetools import adjust_permissions, read_file, write_file
 from easybuild.tools.run import run_cmd, run_cmd_qa, parse_log_for_error
 from easybuild.tools.run import _log as run_log
 
@@ -172,6 +173,31 @@ def test_dry_run(self):
         ]))
         self.assertTrue(expected_regex.match(txt), "Pattern %s matches with: %s" % (expected_regex.pattern, txt))
 
+    def test_run_cmd_list(self):
+        """Test run_cmd with command specified as a list rather than a string"""
+        (out, ec) = run_cmd(['/bin/sh', '-c', "echo hello"], shell=False)
+        self.assertEqual(out, "hello\n")
+        # no reason echo hello could fail
+        self.assertEqual(ec, 0)
+
+    def test_run_cmd_script(self):
+        """Testing use of run_cmd with shell=False to call external scripts"""
+        py_test_script = os.path.join(self.test_prefix, 'test.py')
+        write_file(py_test_script, '\n'.join([
+            '#!/usr/bin/python',
+            'print("hello")',
+        ]))
+        adjust_permissions(py_test_script, stat.S_IXUSR)
+
+        (out, ec) = run_cmd(py_test_script)
+        self.assertEqual(ec, 0)
+        self.assertEqual(out, "hello\n")
+
+        (out, ec) = run_cmd([py_test_script], shell=False)
+        self.assertEqual(ec, 0)
+        self.assertEqual(out, "hello\n")
+
+
 
 def suite():
     """ returns all the testcases in this module """