[lldb/test] Print build commands in trace mode

Running tests with -t prints all lldb commands being run. It makes sense
to print all the build commands as well.

Differential Revision: https://reviews.llvm.org/D112212

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

@@ -82,15 +82,6 @@ class Builder:
 
         return cmdline
 
-    def runBuildCommands(self, commands):
-        try:
-            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
@@ -140,11 +131,11 @@ class Builder:
             return ["MAKE_DSYM=NO", "MAKE_GMODULES=YES"]
         return None
 
-    def build(self, debug_info, architecture=None, compiler=None,
+    def getBuildCommand(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
+            return None
 
         command_parts = [
             self.getMake(testdir, testname), debug_info_args, ["all"],
@@ -154,8 +145,7 @@ class Builder:
             self.getCmdLine(dictionary)]
         command = list(itertools.chain(*command_parts))
 
-        self.runBuildCommands([command])
-        return True
+        return command
 
     def cleanup(self, dictionary=None):
         """Perform a platform-specific cleanup after the test."""

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

@@ -45,6 +45,7 @@ import os.path
 import re
 import shutil
 import signal
+import shlex
 from subprocess import *
 import sys
 import time
@@ -68,6 +69,7 @@ from . import test_categories
 from lldbsuite.support import encoded_file
 from lldbsuite.support import funcutils
 from lldbsuite.test.builders import get_builder
+from lldbsuite.test_event import build_exception
 
 # See also dotest.parseOptionsAndInitTestdirs(), where the environment variables
 # LLDB_COMMAND_TRACE is set from '-t' option.
@@ -470,61 +472,6 @@ class _RemoteProcess(_BaseProcess):
     def terminate(self):
         lldb.remote_platform.Kill(self._pid)
 
-# From 2.7's subprocess.check_output() convenience function.
-# Return a tuple (stdoutdata, stderrdata).
-
-
-def system(commands, **kwargs):
-    r"""Run an os command with arguments and return its output as a byte string.
-
-    If the exit code was non-zero it raises a CalledProcessError.  The
-    CalledProcessError object will have the return code in the returncode
-    attribute and output in the output attribute.
-
-    The arguments are the same as for the Popen constructor.  Example:
-
-    >>> check_output(["ls", "-l", "/dev/null"])
-    'crw-rw-rw- 1 root root 1, 3 Oct 18  2007 /dev/null\n'
-
-    The stdout argument is not allowed as it is used internally.
-    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'
-    """
-
-    output = ""
-    error = ""
-    for shellCommand in commands:
-        if 'stdout' in kwargs:
-            raise ValueError(
-                'stdout argument not allowed, it will be overridden.')
-        process = Popen(
-            shellCommand,
-            stdout=PIPE,
-            stderr=STDOUT,
-            **kwargs)
-        pid = process.pid
-        this_output, this_error = process.communicate()
-        retcode = process.poll()
-
-        if retcode:
-            cmd = kwargs.get("args")
-            if cmd is None:
-                cmd = shellCommand
-            cpe = CalledProcessError(retcode, cmd)
-            # Ensure caller can access the stdout/stderr.
-            cpe.lldb_extensions = {
-                "combined_output": this_output,
-                "command": shellCommand
-            }
-            raise cpe
-        output = output + this_output.decode("utf-8", errors='ignore')
-    return output
-
-
 def getsource_if_available(obj):
     """
     Return the text of the source code for an object if available.  Otherwise,
@@ -1335,11 +1282,10 @@ class Base(unittest2.TestCase):
             Supports: llvm, clang.
         """
         compiler = self.getCompilerBinary()
-        version_output = system([[compiler, "--version"]])
-        for line in version_output.split(os.linesep):
-            m = re.search('version ([0-9.]+)', line)
-            if m:
-                return m.group(1)
+        version_output = check_output([compiler, "--version"], errors="replace")
+        m = re.search('version ([0-9.]+)', version_output)
+        if m:
+            return m.group(1)
         return 'unknown'
 
     def getDwarfVersion(self):
@@ -1469,10 +1415,22 @@ class Base(unittest2.TestCase):
         testname = self.getBuildDirBasename()
 
         module = builder_module()
-        if not module.build(debug_info, architecture, compiler, dictionary,
-                testdir, testname):
+        command = builder_module().getBuildCommand(debug_info, architecture,
+                compiler, dictionary, testdir, testname)
+        if command is None:
             raise Exception("Don't know how to build binary")
 
+        self.runBuildCommand(command)
+
+    def runBuildCommand(self, command):
+        self.trace(shlex.join(command))
+        try:
+            output = check_output(command, stderr=STDOUT, errors="replace")
+        except CalledProcessError as cpe:
+            raise build_exception.BuildError(cpe)
+        self.trace(output)
+
+
     # ==================================================
     # Build methods supported through a plugin interface
     # ==================================================
@@ -1619,7 +1577,7 @@ class Base(unittest2.TestCase):
         if not yaml2obj_bin:
             self.assertTrue(False, "No valid yaml2obj executable specified")
         command = [yaml2obj_bin, "-o=%s" % obj_path, yaml_path]
-        system([command])
+        self.runBuildCommand(command)
 
     def getBuildFlags(
             self,

lldb/packages/Python/lldbsuite/test_event/build_exception.py

@@ -1,10 +1,11 @@
+import shlex
+
 class BuildError(Exception):
 
     def __init__(self, called_process_error):
         super(BuildError, self).__init__("Error when building test subject")
-        self.command = called_process_error.lldb_extensions.get(
-            "command", "<command unavailable>")
-        self.build_error = called_process_error.lldb_extensions["combined_output"]
+        self.command = shlex.join(called_process_error.cmd)
+        self.build_error = called_process_error.output
 
     def __str__(self):
         return self.format_build_error(self.command, self.build_error)
@@ -12,4 +13,4 @@ class BuildError(Exception):
     @staticmethod
     def format_build_error(command, command_output):
         return "Error when building test subject.\n\nBuild Command:\n{}\n\nBuild Command Output:\n{}".format(
-            command, command_output.decode("utf-8", errors='ignore'))
+            command, command_output)

lldb/test/API/test_utils/base/Makefile

@@ -0,0 +1 @@
+include Makefile.rules

lldb/test/API/test_utils/base/TestBaseTest.py

@@ -0,0 +1,35 @@
+"""
+Test TestBase test functions.
+"""
+
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test_event import build_exception
+import six
+
+class TestBuildMethod(Base):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        super().setUp()
+        self._traces = []
+        self.traceAlways = True
+
+    # override the parent trace method
+    def trace(self, *args, **kwargs):
+        io = six.StringIO()
+        print(*args, file=io, **kwargs)
+        self._traces.append(io.getvalue())
+
+    def test_build_fails_helpfully(self):
+        try:
+            self.build(dictionary={"CXX_SOURCES": "nonexisting-file.cpp"})
+        except build_exception.BuildError as e:
+            self.assertIn("nonexisting-file.cpp", str(e))
+        else:
+            self.fail("BuildError not raised!")
+
+    def test_build_logs_traces(self):
+        self.build(dictionary={"CXX_SOURCES": "return0.cpp"})
+        self.assertIn("CXX_SOURCES", self._traces[0])
+        self.assertIn("return0.o", self._traces[1])

lldb/test/API/test_utils/base/return0.cpp

@@ -0,0 +1 @@
+int main() { return 0; }