[Lldb-commits] [lldb] 551d118 - [lldb/test] Remove quote/unquote steps from the make invocations
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 20 02:35:44 PDT 2021
Author: Pavel Labath
Date: 2021-10-20T11:35:28+02:00
New Revision: 551d118805c808936956e464dc21e05acb478f78
URL: https://github.com/llvm/llvm-project/commit/551d118805c808936956e464dc21e05acb478f78
DIFF: https://github.com/llvm/llvm-project/commit/551d118805c808936956e464dc21e05acb478f78.diff
LOG: [lldb/test] Remove quote/unquote steps from the make invocations
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.
Differential Revision: https://reviews.llvm.org/D111990
Added:
Modified:
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/builders/darwin.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index b49c6324c8ca..38e64220ed63 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -2,6 +2,7 @@
import platform
import subprocess
import sys
+import itertools
import lldbsuite.test.lldbtest as lldbtest
import lldbsuite.test.lldbutil as lldbutil
@@ -25,11 +26,11 @@ 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.
@@ -59,29 +60,27 @@ def getMake(self, test_subdir, test_name):
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
@@ -93,7 +92,7 @@ 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):
"""
@@ -104,9 +103,8 @@ def getCCSpec(self, compiler):
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):
"""
@@ -114,8 +112,8 @@ def getSDKRootSpec(self):
used for the make system.
"""
if configuration.sdkroot:
- return "SDKROOT={}".format(configuration.sdkroot)
- return ""
+ return ["SDKROOT={}".format(configuration.sdkroot)]
+ return []
def getModuleCacheSpec(self):
"""
@@ -123,9 +121,9 @@ def getModuleCacheSpec(self):
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:
@@ -138,28 +136,23 @@ def _getDebugInfoArgs(self, debug_info):
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)
- ])
-
- self.runBuildCommands(commands, sender=sender)
+ 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([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
diff --git a/lldb/packages/Python/lldbsuite/test/builders/darwin.py b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
index 9fbd34482422..7718fd4d91f4 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/darwin.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/darwin.py
@@ -80,16 +80,14 @@ def getExtraMakeArgs(self):
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])
@@ -101,7 +99,7 @@ def getArchCFlags(self, arch):
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":
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index c2d4f47d0a86..169f43f59c2f 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -494,24 +494,16 @@ def system(commands, **kwargs):
'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()
@@ -1473,8 +1465,8 @@ def build(
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")
# ==================================================
@@ -1673,7 +1665,7 @@ def getBuildFlags(
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)
More information about the lldb-commits
mailing list