Testdriver: don't use os.popen in config/ghc

Rewrite config/ghc to use getStdout (which use subprocess.Popen) instead
of os.popen, which is deprecated; this also avoids the use of shell

Also:
* Move getStdout to driver/testutil.py so both config/ghc and
  driver/runtests.py can use it

* Remove support for Python below 2.4, which doesn't have subprocess

Reviewed By: thomie

Differential Revision: https://phabricator.haskell.org/D908

3 files changed, 29 insertions, 46 deletions

diff --git a/testsuite/driver/runtests.py b/testsuite/driver/runtests.py
index 4e497e84e1..fcfad77d1a 100644
--- a/testsuite/driver/runtests.py
+++ b/testsuite/driver/runtests.py
@@ -18,10 +18,7 @@ import re
 # * If we import ctypes before subprocess on cygwin, then sys.exit(0)
 #   says "Aborted" and we fail with exit code 134.
 # So we import it here first, so that the testsuite doesn't appear to fail.
-try:
-    import subprocess
-except:
-    pass
+import subprocess
 
 PYTHON3 = sys.version_info >= (3, 0)
 if PYTHON3:
diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py
index e9beee406e..98a75e011e 100644
--- a/testsuite/driver/testlib.py
+++ b/testsuite/driver/testlib.py
@@ -17,13 +17,7 @@ import copy
 import glob
 from math import ceil, trunc
 import collections
-
-have_subprocess = False
-try:
-    import subprocess
-    have_subprocess = True
-except:
-    print("Warning: subprocess not found, will fall back to spawnv")
+import subprocess
 
 from testglobals import *
 from testutil import *
@@ -103,20 +97,14 @@ def _reqlib( name, opts, lib ):
     if lib in have_lib:
         got_it = have_lib[lib]
     else:
-        if have_subprocess:
-            # By preference we use subprocess, as the alternative uses
-            # /dev/null which mingw doesn't have.
-            cmd = strip_quotes(config.ghc_pkg)
-            p = subprocess.Popen([cmd, '--no-user-package-db', 'describe', lib],
-                                 stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE)
-            # read from stdout and stderr to avoid blocking due to
-            # buffers filling
-            p.communicate()
-            r = p.wait()
-        else:
-            r = os.system(config.ghc_pkg + ' --no-user-package-db describe '
-                                         + lib + ' > /dev/null 2> /dev/null')
+        cmd = strip_quotes(config.ghc_pkg)
+        p = subprocess.Popen([cmd, '--no-user-package-db', 'describe', lib],
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
+        # read from stdout and stderr to avoid blocking due to
+        # buffers filling
+        p.communicate()
+        r = p.wait()
         got_it = r == 0
         have_lib[lib] = got_it
 
@@ -1803,13 +1791,8 @@ def rawSystem(cmd_and_args):
     # with the Windows (non-cygwin) python. An argument "a b c"
     # turns into three arguments ["a", "b", "c"].
 
-    # However, subprocess is new in python 2.4, so fall back to
-    # using spawnv if we don't have it
     cmd = cmd_and_args[0]
-    if have_subprocess:
-        return subprocess.call([strip_quotes(cmd)] + cmd_and_args[1:])
-    else:
-        return os.spawnv(os.P_WAIT, cmd, cmd_and_args)
+    return subprocess.call([strip_quotes(cmd)] + cmd_and_args[1:])
 
 # When running under native msys Python, any invocations of non-msys binaries,
 # including timeout.exe, will have their arguments munged according to some
@@ -2293,20 +2276,5 @@ def printFailingTestInfosSummary(file, testInfos):
                           ' (' + ','.join(testInfos[directory][test][reason]) + ')\n')
     file.write('\n')
 
-def getStdout(cmd_and_args):
-    if have_subprocess:
-        p = subprocess.Popen([strip_quotes(cmd_and_args[0])] + cmd_and_args[1:],
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE)
-        (stdout, stderr) = p.communicate()
-        r = p.wait()
-        if r != 0:
-            raise Exception("Command failed: " + str(cmd_and_args))
-        if stderr != '':
-            raise Exception("stderr from command: " + str(cmd_and_args))
-        return stdout
-    else:
-        raise Exception("Need subprocess to get stdout, but don't have it")
-
 def modify_lines(s, f):
     return '\n'.join([f(l) for l in s.splitlines()])
diff --git a/testsuite/driver/testutil.py b/testsuite/driver/testutil.py
index 2cfa8f12bd..2f037f0a49 100644
--- a/testsuite/driver/testutil.py
+++ b/testsuite/driver/testutil.py
@@ -1,5 +1,8 @@
 # -----------------------------------------------------------------------------
 # Utils
+
+import subprocess
+
 def version_to_ints(v):
     return [ int(x) for x in v.split('.') ]
 
@@ -18,3 +21,18 @@ def version_ge(x, y):
 def strip_quotes(s):
     # Don't wrap commands to subprocess.call/Popen in quotes.
     return s.strip('\'"')
+
+def getStdout(cmd_and_args):
+    # Can't use subprocess.check_output as it's not available in Python 2.6;
+    # It's also not quite the same as check_output, since we also verify that
+    # no stderr was produced
+    p = subprocess.Popen([strip_quotes(cmd_and_args[0])] + cmd_and_args[1:],
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE)
+    (stdout, stderr) = p.communicate()
+    r = p.wait()
+    if r != 0:
+        raise Exception("Command failed: " + str(cmd_and_args))
+    if stderr != '':
+        raise Exception("stderr from command: " + str(cmd_and_args))
+    return stdout