[lldb] Convert builders to classes so we can use inheritance to override platform specific methods (NFC)

[JDevlieghere]

Summary

Use inheritance and a factory method to hand out builders instead of importing the desired module. This has the advantage that the platform specific implementations can override methods as they desire.

This came up in D85539

[labath]

lldb/packages/Python/lldbsuite/builders/freebsd.py

5–11

How about we put this into the base class, and remove all the noop builders?

lldb/packages/Python/lldbsuite/builders/__init__.py

+def get_builder(platform):
+  if platform.startswith("darwin"):
+    from .darwin import BuilderDarwin
+    return BuilderDarwin()
+
+  if platform.startswith("freebsd"):
+    from .freebsd import BuilderFreeBSD
+    return BuilderFreeBSD()
+
+  if platform.startswith("linux"):
+    from .linux import BuilderLinux
+    return BuilderLinux()
+
+  if platform.startswith("netbsd"):
+    from .netbsd import BuilderNetBSD
+    return BuilderNetBSD()
+
+  if platform.startswith("openbsd"):
+    from .openbsd import BuilderOpenBSD
+    return BuilderOpenBSD()
+
+  if platform.startswith("win32"):
+    from .win32 import BuilderWindows
+    return BuilderWindows()
+
+  raise RuntimeError("No builder for platform {}".format(platform))

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

+"""
+If the build* function is passed the compiler argument, for example, 'llvm-gcc',
+it is passed as a make variable to the make command.  Otherwise, we check the
+LLDB_CC environment variable; if it is defined, it is passed as a make variable
+to the make command.
+
+If neither the compiler keyword argument nor the LLDB_CC environment variable is
+specified, no CC make variable is passed to the make command.  The Makefile gets
+to define the default CC being used.
+
+Same idea holds for LLDB_ARCH environment variable, which maps to the ARCH make
+variable.
+"""
+
+# System imports
+import os
+import platform
+import subprocess
+import sys
+
+# Our imports
+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 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:
+            make = "make"
+
+        # Construct the base make invocation.
+        lldb_test = os.environ["LLDB_TEST"]
+        if not (lldb_test and configuration.test_build_dir and test_subdir
+                and test_name and (not os.path.isabs(test_subdir))):
+            raise Exception("Could not derive test directories")
+        build_dir = os.path.join(configuration.test_build_dir, test_subdir,
+                                 test_name)
+        src_dir = os.path.join(configuration.test_src_root, test_subdir)
+        # This is a bit of a hack to make inline testcases work.
+        makefile = os.path.join(src_dir, "Makefile")
+        if not os.path.isfile(makefile):
+            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.
+        """
+
+        # If d is None or an empty mapping, just return an empty string.
+        if not d:
+            return ""
+        pattern = '%s="%s"' if "win32" in sys.platform else "%s='%s'"
+
+        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)
+
+        cmdline = " ".join(
+            [setOrAppendVariable(k, v) for k, v in list(d.items())])
+
+        return cmdline
+
+    def runBuildCommands(self, commands, sender):
+        try:
+            lldbtest.system(commands, sender=sender)
+        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.
+        """
+        arch = architecture if architecture else None
+        if not arch and configuration.arch:
+            arch = configuration.arch
+
+        return ("ARCH=" + arch) if arch 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 ""
+
+    def getDsymutilSpec(self):
+        """
+        Helper function to return the key-value string to specify the dsymutil
+        used for the make system.
+        """
+        if configuration.dsymutil:
+            return "DSYMUTIL={}".format(configuration.dsymutil)
+        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 ""
+
+    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 ""
+
+    def buildDefault(self,
+                     sender=None,
+                     architecture=None,
+                     compiler=None,
+                     dictionary=None,
+                     testdir=None,
+                     testname=None):
+        """Build the binaries the default way."""
+        commands = []
+        commands.append(
+            self.getMake(testdir, testname) + [
+                "all",
+                self.getArchSpec(architecture),
+                self.getCCSpec(compiler),
+                self.getDsymutilSpec(),
+                self.getSDKRootSpec(),
+                self.getModuleCacheSpec(),
+                self.getCmdLine(dictionary)
+            ])
+
+        self.runBuildCommands(commands, sender=sender)
+
+        # True signifies that we can handle building default.
+        return True
+
+    def buildDwarf(self,
+                   sender=None,
+                   architecture=None,
+                   compiler=None,
+                   dictionary=None,
+                   testdir=None,
+                   testname=None):
+        """Build the binaries with dwarf debug info."""
+        commands = []
+        commands.append(
+            self.getMake(testdir, testname) + [
+                "MAKE_DSYM=NO",
+                self.getArchSpec(architecture),
+                self.getCCSpec(compiler),
+                self.getDsymutilSpec(),
+                self.getSDKRootSpec(),
+                self.getModuleCacheSpec(),
+                self.getCmdLine(dictionary)
+            ])
+
+        self.runBuildCommands(commands, sender=sender)
+        # True signifies that we can handle building dwarf.
+        return True
+
+    def buildDwo(self,
+                 sender=None,
+                 architecture=None,
+                 compiler=None,
+                 dictionary=None,
+                 testdir=None,
+                 testname=None):
+        """Build the binaries with dwarf debug info."""
+        commands = []
+        commands.append(
+            self.getMake(testdir, testname) + [
+                "MAKE_DSYM=NO", "MAKE_DWO=YES",
+                self.getArchSpec(architecture),
+                self.getCCSpec(compiler),
+                self.getDsymutilSpec(),
+                self.getSDKRootSpec(),
+                self.getModuleCacheSpec(),
+                self.getCmdLine(dictionary)
+            ])
+
+        self.runBuildCommands(commands, sender=sender)
+        # True signifies that we can handle building dwo.
+        return True
+
+    def buildGModules(self,
+                      sender=None,
+                      architecture=None,
+                      compiler=None,
+                      dictionary=None,
+                      testdir=None,
+                      testname=None):
+        """Build the binaries with dwarf debug info."""
+        commands = []
+        commands.append(
+            self.getMake(testdir, testname) + [
+                "MAKE_DSYM=NO", "MAKE_GMODULES=YES",
+                self.getArchSpec(architecture),
+                self.getCCSpec(compiler),
+                self.getDsymutilSpec(),
+                self.getSDKRootSpec(),
+                self.getModuleCacheSpec(),
+                self.getCmdLine(dictionary)
+            ])
+
+        self.runBuildCommands(commands, sender=sender)
+        # True signifies that we can handle building with gmodules.
+        return True
+
+    def cleanup(self, sender=None, dictionary=None):
+        """Perform a platform-specific cleanup after the test."""
+        return True

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

+from .builder import Builder
+
+
+class BuilderDarwin(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None,
+                  testname=None):
+        """Build the binaries with dsym debug info."""
+        commands = []
+        commands.append(
+            self.getMake(testdir, testname) + [
+                "MAKE_DSYM=YES",
+                self.getArchSpec(architecture),
+                self.getCCSpec(compiler),
+                self.getDsymutilSpec(),
+                self.getSDKRootSpec(),
+                self.getModuleCacheSpec(), "all",
+                self.getCmdLine(dictionary)
+            ])
+
+        self.runBuildCommands(commands, sender=sender)
+
+        # True signifies that we can handle building dsym.
+        return True

lldb/packages/Python/lldbsuite/builders/freebsd.py

+from builder import *
+
+
+class BuilderFreeBSD(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None):
+        return False

lldb/packages/Python/lldbsuite/builders/linux.py

+from builder import *
+
+
+class BuilderLinux(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None):
+        return False

lldb/packages/Python/lldbsuite/builders/netbsd.py

+from builder import *
+
+
+class BuilderNetBSD(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None):
+        return False

lldb/packages/Python/lldbsuite/builders/openbsd.py

+from builder_base import *
+
+
+class BuilderNetBSD(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None):
+        return False

lldb/packages/Python/lldbsuite/builders/win32.py

+from builder_base import *
+
+
+class BuilderWindows(Builder):
+    def buildDsym(self,
+                  sender=None,
+                  architecture=None,
+                  compiler=None,
+                  dictionary=None,
+                  testdir=None):
+        return False

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

 # LLDB modules
 import lldb
 from . import configuration
 from . import decorators
 from . import lldbplatformutil
 from . import lldbtest_config
 from . import lldbutil
 from . import test_categories
+from lldbsuite.builders import get_builder
 from lldbsuite.support import encoded_file
 from lldbsuite.support import funcutils
 
 # See also dotest.parseOptionsAndInitTestdirs(), where the environment variables
 # LLDB_COMMAND_TRACE is set from '-t' option.
 
 # By default, traceAlways is False.
 if "LLDB_COMMAND_TRACE" in os.environ and os.environ[
def getsource_if_available(obj):
     import inspect
     try:
         return inspect.getsource(obj)
     except:
         return repr(obj)
 
 
 def builder_module():
-    if sys.platform.startswith("freebsd"):
-        return __import__("builder_freebsd")
-    if sys.platform.startswith("openbsd"):
-        return __import__("builder_openbsd")
-    if sys.platform.startswith("netbsd"):
-        return __import__("builder_netbsd")
-    if sys.platform.startswith("linux"):
-        # sys.platform with Python-3.x returns 'linux', but with
-        # Python-2.x it returns 'linux2'.
-        return __import__("builder_linux")
-    return __import__("builder_" + sys.platform)
+    return get_builder(sys.platform)
 
 
 class Base(unittest2.TestCase):
     """
     Abstract base for performing lldb (see TestBase) or other generic tests (see
     BenchBase for one example).  lldbtest.Base works with the test driver to
     accomplish things.
 
Important things for test class writers:
           test cases to send a command to the command interpreter and to perform
           string/pattern matching on the output of such command execution.  The
           expect method also provides a mode to peform string/pattern matching
           without running a command.
 
         - The build methods buildDefault, buildDsym, and buildDwarf are used to
           build the binaries used during a particular test scenario.  A plugin
           should be provided for the sys.platform running the test suite.  The
-          Mac OS X implementation is located in plugins/darwin.py.
+          Mac OS X implementation is located in builders/darwin.py.
     """
 
     # Subclasses can set this to true (if they don't depend on debug info) to avoid running the
     # test multiple times with various debug info types.
     NO_DEBUG_INFO_TESTCASE = False
 
     # Maximum allowed attempts when launching the inferior process.
     # Can be overridden by the LLDB_MAX_LAUNCH_COUNT environment variable.

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

else
   ARCH = x86_64
 endif
 endif
 
 #----------------------------------------------------------------------
 # CC defaults to clang.
 #
 # If you change the defaults of CC, be sure to also change it in the file
-# test/plugins/builder_base.py, which provides a Python way to return the
+# test/builders/builder_base.py, which provides a Python way to return the
 # value of the make variable CC -- getCompiler().
 #
 # See also these functions:
 #   o cxx_compiler
 #   o cxx_linker
 #----------------------------------------------------------------------
 ifeq "$(CC)" ""
 $(error "C compiler is not specified. Please run tests through lldb-dotest or lit")

lldb/packages/Python/lldbsuite/test/plugins/builder_base.py

-"""
-If the build* function is passed the compiler argument, for example, 'llvm-gcc',
-it is passed as a make variable to the make command.  Otherwise, we check the
-LLDB_CC environment variable; if it is defined, it is passed as a make variable
-to the make command.
-
-If neither the compiler keyword argument nor the LLDB_CC environment variable is
-specified, no CC make variable is passed to the make command.  The Makefile gets
-to define the default CC being used.
-
-Same idea holds for LLDB_ARCH environment variable, which maps to the ARCH make
-variable.
-"""
-
-# System imports
-import os
-import platform
-import subprocess
-import sys
-
-# Our imports
-import lldbsuite.test.lldbtest as lldbtest
-import lldbsuite.test.lldbutil as lldbutil
-from lldbsuite.test import configuration
-from lldbsuite.test_event import build_exception
-
-
-def getArchitecture():
-    """Returns the architecture in effect the test suite is running with."""
-    return configuration.arch if configuration.arch else ""
-
-
-def getCompiler():
-    """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 getMake(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:
-        make = "make"
-
-    # Construct the base make invocation.
-    lldb_test = os.environ["LLDB_TEST"]
-    if not (lldb_test and configuration.test_build_dir and test_subdir and
-            test_name and (not os.path.isabs(test_subdir))):
-        raise Exception("Could not derive test directories")
-    build_dir = os.path.join(configuration.test_build_dir, test_subdir, test_name)
-    src_dir = os.path.join(configuration.test_src_root, test_subdir)
-    # This is a bit of a hack to make inline testcases work.
-    makefile = os.path.join(src_dir, "Makefile")
-    if not os.path.isfile(makefile):
-        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 getArchSpec(architecture):
-    """
-    Helper function to return the key-value string to specify the architecture
-    used for the make system.
-    """
-    arch = architecture if architecture else None
-    if not arch and configuration.arch:
-        arch = configuration.arch
-
-    return ("ARCH=" + arch) if arch else ""
-
-
-def getCCSpec(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 ""
-
-def getDsymutilSpec():
-    """
-    Helper function to return the key-value string to specify the dsymutil
-    used for the make system.
-    """
-    if configuration.dsymutil:
-        return "DSYMUTIL={}".format(configuration.dsymutil)
-    return ""
-
-def getSDKRootSpec():
-    """
-    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 ""
-
-def getModuleCacheSpec():
-    """
-    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 ""
-
-def getCmdLine(d):
-    """
-    Helper function to return a properly formatted command line argument(s)
-    string used for the make system.
-    """
-
-    # If d is None or an empty mapping, just return an empty string.
-    if not d:
-        return ""
-    pattern = '%s="%s"' if "win32" in sys.platform else "%s='%s'"
-
-    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)
-    cmdline = " ".join([setOrAppendVariable(k, v) for k, v in list(d.items())])
-
-    return cmdline
-
-
-def runBuildCommands(commands, sender):
-    try:
-        lldbtest.system(commands, sender=sender)
-    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 buildDefault(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None,
-        testname=None):
-    """Build the binaries the default way."""
-    commands = []
-    commands.append(getMake(testdir, testname) +
-                    ["all",
-                     getArchSpec(architecture),
-                     getCCSpec(compiler),
-                     getDsymutilSpec(),
-                     getSDKRootSpec(),
-                     getModuleCacheSpec(),
-                     getCmdLine(dictionary)])
-
-    runBuildCommands(commands, sender=sender)
-
-    # True signifies that we can handle building default.
-    return True
-
-
-def buildDwarf(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None,
-        testname=None):
-    """Build the binaries with dwarf debug info."""
-    commands = []
-    commands.append(getMake(testdir, testname) +
-                    ["MAKE_DSYM=NO",
-                     getArchSpec(architecture),
-                     getCCSpec(compiler),
-                     getDsymutilSpec(),
-                     getSDKRootSpec(),
-                     getModuleCacheSpec(),
-                     getCmdLine(dictionary)])
-
-    runBuildCommands(commands, sender=sender)
-    # True signifies that we can handle building dwarf.
-    return True
-
-
-def buildDwo(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None,
-        testname=None):
-    """Build the binaries with dwarf debug info."""
-    commands = []
-    commands.append(getMake(testdir, testname) +
-                    ["MAKE_DSYM=NO",
-                     "MAKE_DWO=YES",
-                     getArchSpec(architecture),
-                     getCCSpec(compiler),
-                     getDsymutilSpec(),
-                     getSDKRootSpec(),
-                     getModuleCacheSpec(),
-                     getCmdLine(dictionary)])
-
-    runBuildCommands(commands, sender=sender)
-    # True signifies that we can handle building dwo.
-    return True
-
-
-def buildGModules(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None,
-        testname=None):
-    """Build the binaries with dwarf debug info."""
-    commands = []
-    commands.append(getMake(testdir, testname) +
-                    ["MAKE_DSYM=NO",
-                     "MAKE_GMODULES=YES",
-                     getArchSpec(architecture),
-                     getCCSpec(compiler),
-                     getDsymutilSpec(),
-                     getSDKRootSpec(),
-                     getModuleCacheSpec(),
-                     getCmdLine(dictionary)])
-
-    runBuildCommands(commands, sender=sender)
-    # True signifies that we can handle building with gmodules.
-    return True
-
-
-def cleanup(sender=None, dictionary=None):
-    """Perform a platform-specific cleanup after the test."""
-    return True

lldb/packages/Python/lldbsuite/test/plugins/builder_darwin.py

-
-import lldbsuite.test.lldbtest as lldbtest
-
-from builder_base import *
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None,
-        testname=None):
-    """Build the binaries with dsym debug info."""
-    commands = []
-    commands.append(getMake(testdir, testname) +
-                    ["MAKE_DSYM=YES",
-                     getArchSpec(architecture),
-                     getCCSpec(compiler),
-                     getDsymutilSpec(),
-                     getSDKRootSpec(),
-                     getModuleCacheSpec(),
-                     "all",
-                     getCmdLine(dictionary)])
-
-    runBuildCommands(commands, sender=sender)
-
-    # True signifies that we can handle building dsym.
-    return True

lldb/packages/Python/lldbsuite/test/plugins/builder_freebsd.py

-from builder_base import *
-
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None):
-    return False

lldb/packages/Python/lldbsuite/test/plugins/builder_linux.py

-from builder_base import *
-
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None):
-    return False

lldb/packages/Python/lldbsuite/test/plugins/builder_netbsd.py

-from builder_base import *
-
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None):
-    return False

lldb/packages/Python/lldbsuite/test/plugins/builder_openbsd.py

-from builder_base import *
-
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None):
-    return False

lldb/packages/Python/lldbsuite/test/plugins/builder_win32.py

-from builder_base import *
-
-
-def buildDsym(
-        sender=None,
-        architecture=None,
-        compiler=None,
-        dictionary=None,
-        testdir=None):
-    return False