[lldb/test] Remove quote/unquote steps from the make invocations

[labath]

Summary

None of the commands we run really rely on shell features. Running them
with shell=False, simplifies the code as there is no need for elaborate
quoting.

Depends on D111989.

[JDevlieghere]

LGTM. Thanks for cleaning that up.

lldb/packages/Python/lldbsuite/test/builders/builder.py

 import os
 import platform
 import subprocess
 import sys
+import itertools
 
 import lldbsuite.test.lldbtest as lldbtest
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test import configuration
 from lldbsuite.test_event import build_exception
 
 
 class Builder:
     def getArchitecture(self):
         """Returns the architecture in effect the test suite is running with."""
         return configuration.arch if configuration.arch else ""
 
     def getCompiler(self):
         """Returns the compiler in effect the test suite is running with."""
         compiler = configuration.compiler if configuration.compiler else "clang"
         compiler = lldbutil.which(compiler)
         return os.path.abspath(compiler)
 
     def getExtraMakeArgs(self):
         """
         Helper function to return extra argumentsfor the make system. This
         method is meant to be overridden by platform specific builders.
         """
-        return ""
+        return []
 
     def getArchCFlags(self, architecture):
         """Returns the ARCH_CFLAGS for the make system."""
-        return ""
+        return []
 
     def getMake(self, test_subdir, test_name):
         """Returns the invocation for GNU make.
         The first argument is a tuple of the relative path to the testcase
         and its filename stem."""
         if platform.system() == "FreeBSD" or platform.system() == "NetBSD":
             make = "gmake"
         else:
def getMake(self, test_subdir, test_name):
             makefile = os.path.join(build_dir, "Makefile")
         return [
             make, "VPATH=" + src_dir, "-C", build_dir, "-I", src_dir, "-I",
             os.path.join(lldb_test, "make"), "-f", makefile
         ]
 
     def getCmdLine(self, d):
         """
-        Helper function to return a properly formatted command line argument(s)
-        string used for the make system.
+        Helper function to return a command line argument string used for the
+        make system.
         """
 
-        # If d is None or an empty mapping, just return an empty string.
+        # If d is None or an empty mapping, just return an empty list.
         if not d:
-            return ""
-        pattern = '%s="%s"' if "win32" in sys.platform else "%s='%s'"
+            return []
 
         def setOrAppendVariable(k, v):
             append_vars = ["CFLAGS", "CFLAGS_EXTRAS", "LD_EXTRAS"]
             if k in append_vars and k in os.environ:
                 v = os.environ[k] + " " + v
-            return pattern % (k, v)
+            return '%s=%s' % (k, v)
 
-        cmdline = " ".join(
-            [setOrAppendVariable(k, v) for k, v in list(d.items())])
+        cmdline = [setOrAppendVariable(k, v) for k, v in list(d.items())]
 
         return cmdline
 
-    def runBuildCommands(self, commands, sender):
+    def runBuildCommands(self, commands):
         try:
-            lldbtest.system(commands, sender=sender)
+            lldbtest.system(commands)
         except subprocess.CalledProcessError as called_process_error:
             # Convert to a build-specific error.
             # We don't do that in lldbtest.system() since that
             # is more general purpose.
             raise build_exception.BuildError(called_process_error)
 
     def getArchSpec(self, architecture):
         """
         Helper function to return the key-value string to specify the architecture
         used for the make system.
         """
-        return ("ARCH=" + architecture) if architecture else ""
+        return ["ARCH=" + architecture] if architecture else []
 
     def getCCSpec(self, compiler):
         """
         Helper function to return the key-value string to specify the compiler
         used for the make system.
         """
         cc = compiler if compiler else None
         if not cc and configuration.compiler:
             cc = configuration.compiler
         if cc:
-            return "CC=\"%s\"" % cc
-        else:
-            return ""
+            return ["CC=\"%s\"" % cc]
+        return []
 
     def getSDKRootSpec(self):
         """
         Helper function to return the key-value string to specify the SDK root
         used for the make system.
         """
         if configuration.sdkroot:
-            return "SDKROOT={}".format(configuration.sdkroot)
-        return ""
+            return ["SDKROOT={}".format(configuration.sdkroot)]
+        return []
 
     def getModuleCacheSpec(self):
         """
         Helper function to return the key-value string to specify the clang
         module cache used for the make system.
         """
         if configuration.clang_module_cache_dir:
-            return "CLANG_MODULE_CACHE_DIR={}".format(
-                configuration.clang_module_cache_dir)
-        return ""
+            return ["CLANG_MODULE_CACHE_DIR={}".format(
+                configuration.clang_module_cache_dir)]
+        return []
 
     def _getDebugInfoArgs(self, debug_info):
         if debug_info is None:
             return []
         if debug_info == "dwarf":
             return ["MAKE_DSYM=NO"]
         if debug_info == "dwo":
             return ["MAKE_DSYM=NO", "MAKE_DWO=YES"]
         if debug_info == "gmodules":
             return ["MAKE_DSYM=NO", "MAKE_GMODULES=YES"]
         return None
 
-    def build(self, debug_info, sender=None, architecture=None, compiler=None,
+    def build(self, debug_info, architecture=None, compiler=None,
             dictionary=None, testdir=None, testname=None):
         debug_info_args = self._getDebugInfoArgs(debug_info)
         if debug_info_args is None:
             return False
 
-        commands = []
-        commands.append(
-            self.getMake(testdir, testname) + debug_info_args + [
-                "all",
-                self.getArchCFlags(architecture),
-                self.getArchSpec(architecture),
-                self.getCCSpec(compiler),
-                self.getExtraMakeArgs(),
-                self.getSDKRootSpec(),
-                self.getModuleCacheSpec(),
-                self.getCmdLine(dictionary)
-            ])
+        command_parts = [
+            self.getMake(testdir, testname), debug_info_args, ["all"],
+            self.getArchCFlags(architecture), self.getArchSpec(architecture),
+            self.getCCSpec(compiler), self.getExtraMakeArgs(),
+            self.getSDKRootSpec(), self.getModuleCacheSpec(),
+            self.getCmdLine(dictionary)]
+        command = list(itertools.chain(*command_parts))
 
-        self.runBuildCommands(commands, sender=sender)
+        self.runBuildCommands([command])
         return True
 
-    def cleanup(self, sender=None, dictionary=None):
+    def cleanup(self, dictionary=None):
         """Perform a platform-specific cleanup after the test."""
         return True

lldb/packages/Python/lldbsuite/test/builders/darwin.py

def getExtraMakeArgs(self):
               entitlements_file = 'entitlements.plist'
             entitlements = os.path.join(test_dir, 'make', entitlements_file)
             args['CODESIGN'] = 'codesign --entitlements {}'.format(
                 entitlements)
         else:
             args['CODESIGN'] = 'codesign'
 
         # Return extra args as a formatted string.
-        return ' '.join(
-            {'{}="{}"'.format(key, value)
-             for key, value in args.items()})
+        return ['{}={}'.format(key, value) for key, value in args.items()]
 
     def getArchCFlags(self, arch):
         """Returns the ARCH_CFLAGS for the make system."""
         # Get the triple components.
         vendor, os, version, env = get_triple()
         if vendor is None or os is None or version is None or env is None:
-            return ""
+            return []
 
         # Construct the triple from its components.
         triple = '-'.join([arch, vendor, os, version, env])
 
         # Construct min version argument
         version_min = ""
         if env == "simulator":
             version_min = "-m{}-simulator-version-min={}".format(os, version)
         elif os == "macosx":
             version_min = "-m{}-version-min={}".format(os, version)
 
-        return "ARCH_CFLAGS=\"-target {} {}\"".format(triple, version_min)
+        return ["ARCH_CFLAGS=-target {} {}".format(triple, version_min)]
 
     def _getDebugInfoArgs(self, debug_info):
         if debug_info == "dsym":
             return ["MAKE_DSYM=YES"]
         return super(BuilderDarwin, self)._getDebugInfoArgs(debug_info)

lldb/packages/Python/lldbsuite/test/lldbtest.py

def system(commands, **kwargs):
     To capture standard error in the result, use stderr=STDOUT.
 
     >>> check_output(["/bin/sh", "-c",
     ...               "ls -l non_existent_file ; exit 0"],
     ...              stderr=STDOUT)
     'ls: non_existent_file: No such file or directory\n'
     """
 
-    # Assign the sender object to variable 'test' and remove it from kwargs.
-    test = kwargs.pop('sender', None)
-
-    # [['make', 'clean', 'foo'], ['make', 'foo']] -> ['make clean foo', 'make foo']
-    commandList = [' '.join(x) for x in commands]
     output = ""
     error = ""
-    for shellCommand in commandList:
+    for shellCommand in commands:
         if 'stdout' in kwargs:
             raise ValueError(
                 'stdout argument not allowed, it will be overridden.')
-        if 'shell' in kwargs and kwargs['shell'] == False:
-            raise ValueError('shell=False not allowed')
         process = Popen(
             shellCommand,
             stdout=PIPE,
             stderr=STDOUT,
-            shell=True,
             **kwargs)
         pid = process.pid
         this_output, this_error = process.communicate()
         retcode = process.poll()
 
         if retcode:
             cmd = kwargs.get("args")
             if cmd is None:
def build(
             debug_info = self.getDebugInfo()
 
         dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
 
         testdir = self.mydir
         testname = self.getBuildDirBasename()
 
         module = builder_module()
-        if not module.build(debug_info, self, architecture, compiler,
-                                   dictionary, testdir, testname):
+        if not module.build(debug_info, architecture, compiler, dictionary,
+                testdir, testname):
             raise Exception("Don't know how to build binary")
 
     # ==================================================
     # Build methods supported through a plugin interface
     # ==================================================
 
     def getstdlibFlag(self):
         """ Returns the proper -stdlib flag, or empty if not required."""
def getBuildFlags(
 
         return {'CFLAGS_EXTRAS': cflags,
                 'LD_EXTRAS': ldflags,
                 }
 
     def cleanup(self, dictionary=None):
         """Platform specific way to do cleanup after build."""
         module = builder_module()
-        if not module.cleanup(self, dictionary):
+        if not module.cleanup(dictionary):
             raise Exception(
                 "Don't know how to do cleanup with dictionary: " +
                 dictionary)
 
     def invoke(self, obj, name, trace=False):
         """Use reflection to call a method dynamically with no argument."""
         trace = (True if traceAlways else trace)