Move build_llvm into LLVMBuildConfig

This makes it possible to know what build tool to use in dry run mode
when the build directory has not been configured yet.

Change-Id: Ib620167c62f4c290621ace582d6ad893ad475bc3

4 files changed, 97 insertions, 64 deletions

diff --git a/modules/llvm.py b/modules/llvm.py
index 953b513..512821c 100644
--- a/modules/llvm.py
+++ b/modules/llvm.py
@@ -300,12 +300,24 @@ class LLVMBuildConfig(object):
     run a build command. The directory must already exist, but it may be empty.
     """
 
-    def __init__(self, sourceConfig, buildDir=None):
-        """Create an LLVMBuildConfig."""
+    def __init__(self, sourceConfig, buildDir, commandConsumer):
+        """
+        Create an LLVMBuildConfig.
+
+        The 'sourceConfig' may be None if the 'buildDir' has already been
+        configured and you don't intend to call the 'cmake' method.
+
+        The 'commandConsumer' should have a 'consume' method taking two
+        parameters: the command to be consumed (in the form of a list) and the
+        directory where the command should be run. Any exceptions that may be
+        raised by that method should be handled by the calling code.
+        """
         self.sourceConfig = sourceConfig
         self.buildDir = buildDir
+        self.commandConsumer = commandConsumer
+        self.buildTool = None
 
-    def cmake(self, commandConsumer, cmakeFlags, generator):
+    def cmake(self, cmakeFlags, generator):
         """
         Generate the CMake command needed for configuring the build directory
         with the given flags and generator, and pass it to the 'commandConsumer'.
@@ -317,15 +329,42 @@ class LLVMBuildConfig(object):
         up trying to build subprojects that were removed, or not build
         subprojects that were added).
 
-        The 'commandConsumer' should have a 'consume' method taking two
-        parameters: the command to be consumed (in the form of a list) and the
-        directory where the command should be run. Any exceptions that may be
-        raised by that method should be handled by the calling code.
         """
         cmakeSubprojFlags = self.__get_subproj_flags()
         command = ["cmake", "-G", generator] + cmakeSubprojFlags + \
             cmakeFlags + [self.sourceConfig.get_path()]
-        commandConsumer.consume(command, self.buildDir)
+        self.commandConsumer.consume(command, self.buildDir)
+
+        self.buildTool = self.__get_build_tool_from(generator)
+
+    def build(self, flags=[]):
+        """
+        Generate a build command and pass it to the 'commandConsumer'.
+
+        The build command that is generated will depend on how the build
+        directory was configured. If it has been configured through this object,
+        then whatever generator was passed in at that time determines the build
+        tool that will be used. Otherwise, if the build directory contains a
+        'build.ninja', a 'ninja' command will be generated. Otherwise, if it
+        contains a 'Makefile', a 'make' command will be generated. Otherwise, a
+        RuntimeError will be thrown.
+
+        The 'flags' should be a list of flags to be passed to the build command,
+        e.g. ["-j8", "llvm-check-codegen-aarch64"].
+        """
+        if self.buildTool is None:
+            if os.path.exists(os.path.join(self.buildDir, 'build.ninja')):
+                self.buildTool = "ninja"
+            elif os.path.exists(os.path.join(self.buildDir, 'Makefile')):
+                self.buildTool = "make"
+            else:
+                raise RuntimeError(
+                    "Couldn't identify build system to use for {}. "
+                    "It does not contain 'build.ninja' or 'Makefile'. "
+                    "Are you sure it was configured?".format(self.buildDir))
+
+        command = [self.buildTool] + flags
+        self.commandConsumer.consume(command, self.buildDir)
 
     def __get_subproj_flags(self):
         """
@@ -349,42 +388,14 @@ class LLVMBuildConfig(object):
 
         return cmakeSubprojFlags
 
+    def __get_build_tool_from(self, generator):
+        if generator == "Ninja":
+            return "ninja"
 
-def build_llvm(commandConsumer, buildDir, flags=[]):
-    """
-    Generate a build command and pass it to the 'commandConsumer'.
-
-    The build command that is generated will depend on what 'buildDir' contains.
-    If it contains a 'build.ninja', a 'ninja' command will be generated.
-    Otherwise, if it contains a 'Makefile', a 'make' command will be generated.
-    Otherwise, a RuntimeError will be thrown.
-
-    The 'commandConsumer' should have a 'consume' method taking two parameters:
-    the command to be consumed (in the form of a list) and the directory where
-    the command should be run. Any exceptions that may be raised by that method
-    should be handled by the calling code.
+        if generator == "Unix Makefiles":
+            return "make"
 
-    The 'buildDir' needs to be the path to an already-configured build
-    directory.
-
-    The 'flags' should be a list of flags to be passed to the build command,
-    e.g. ["-j8", "llvm-check-codegen-aarch64"].
-    """
-    useNinja = os.path.exists(os.path.join(buildDir, 'build.ninja'))
-    useMake = os.path.exists(os.path.join(buildDir, 'Makefile'))
-
-    if useNinja:
-        buildTool = "ninja"
-    elif useMake:
-        buildTool = "make"
-    else:
-        raise RuntimeError(
-            "Couldn't identify build system to use for {}. "
-            "It does not contain 'build.ninja' or 'Makefile'. "
-            "Are you sure it was configured?".format(buildDir))
-
-    command = [buildTool] + flags
-    commandConsumer.consume(command, buildDir)
+        return None
 
 
 def setup_test_suite(commandConsumer, sandbox, lnt):
diff --git a/scripts/llvm.py b/scripts/llvm.py
index 3a4954c..7c3fb85 100644
--- a/scripts/llvm.py
+++ b/scripts/llvm.py
@@ -4,7 +4,6 @@ import os
 from sys import argv
 from sys import exit
 
-from modules.llvm import build_llvm
 from modules.llvm import LLVMBuildConfig
 from modules.llvm import LLVMSubproject
 from modules.llvm import LLVMSourceConfig
@@ -101,11 +100,6 @@ def configure_build(args):
     llvm_worktree_root = args.sources
     sourceConfig = LLVMSourceConfig(proj, llvm_worktree_root, args.dry)
 
-    buildConfig = LLVMBuildConfig(sourceConfig, args.build)
-
-    if args.defs:
-        args.defs = ["-D{}".format(v) for v in args.defs]
-
     if args.dry:
         consumer = CommandPrinter()
     else:
@@ -113,8 +107,13 @@ def configure_build(args):
             os.makedirs(args.build)
         consumer = CommandRunner()
 
+    buildConfig = LLVMBuildConfig(sourceConfig, args.build, consumer)
+
+    if args.defs:
+        args.defs = ["-D{}".format(v) for v in args.defs]
+
     try:
-        buildConfig.cmake(consumer, args.defs, args.generator)
+        buildConfig.cmake(args.defs, args.generator)
     except RuntimeError as exc:
         die("Failed to configure {} because:\n{}".format(args.build, str(exc)))
 
@@ -129,7 +128,7 @@ def run_build(args):
         consumer = CommandRunner()
 
     try:
-        build_llvm(consumer, args.build, args.flags)
+        LLVMBuildConfig(None, args.build, consumer).build(args.flags)
     except RuntimeError as exc:
         die("Failed to build {} because:\n{}".format(args.build, str(exc)))
 
diff --git a/tests/unittests/testbuildllvm.py b/tests/unittests/testbuildllvm.py
index 5a31eb3..a1f9282 100644
--- a/tests/unittests/testbuildllvm.py
+++ b/tests/unittests/testbuildllvm.py
@@ -1,4 +1,4 @@
-from modules.llvm import build_llvm
+from modules.llvm import LLVMBuildConfig
 
 import os
 
@@ -27,8 +27,9 @@ class TestBuildLLVM(TestCase):
     def test_invalid_build_dir(self):
         self.buildDir = create_empty_dir()
 
+        buildConfig = LLVMBuildConfig(None, self.buildDir, None)
         with self.assertRaises(RuntimeError) as context:
-            build_llvm(None, self.buildDir)
+            buildConfig.build()
 
         self.assertRegex(
             str(context.exception),
@@ -39,7 +40,8 @@ class TestBuildLLVM(TestCase):
         self.buildDir = create_dir_with_empty_file("build.ninja")
         consumer = MagicMock()
 
-        build_llvm(consumer, self.buildDir)
+        buildConfig = LLVMBuildConfig(None, self.buildDir, consumer)
+        buildConfig.build()
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(command, ["ninja"])
@@ -49,7 +51,8 @@ class TestBuildLLVM(TestCase):
         self.buildDir = create_dir_with_empty_file("Makefile")
         consumer = MagicMock()
 
-        build_llvm(consumer, self.buildDir)
+        buildConfig = LLVMBuildConfig(None, self.buildDir, consumer)
+        buildConfig.build()
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(command, ["make"])
@@ -59,8 +62,25 @@ class TestBuildLLVM(TestCase):
         self.buildDir = create_dir_with_empty_file("build.ninja")
         consumer = MagicMock()
 
-        build_llvm(consumer, self.buildDir, ["-t", "targets"])
+        buildConfig = LLVMBuildConfig(None, self.buildDir, consumer)
+        buildConfig.build(["-t", "targets"])
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(command, ["ninja", "-t", "targets"])
         self.assertEqual(directory, self.buildDir)
+
+    def test_cmake_then_build(self):
+        """
+        Test that when using the same build config to both configure and build,
+        we use the build tool corresponding to the generator used for cmake
+        regardless of what the build directory already contains.
+        """
+        self.buildDir = create_dir_with_empty_file("Makefile")
+        sourceConfig = MagicMock()
+        consumer = MagicMock()
+
+        buildConfig = LLVMBuildConfig(sourceConfig, self.buildDir, consumer)
+        buildConfig.cmake([], "Ninja")
+        buildConfig.build()
+
+        consumer.consume.assert_called_with(["ninja"], self.buildDir)
diff --git a/tests/unittests/testllvmbuildconfig.py b/tests/unittests/testllvmbuildconfig.py
index 509e8a4..efc7bfa 100644
--- a/tests/unittests/testllvmbuildconfig.py
+++ b/tests/unittests/testllvmbuildconfig.py
@@ -25,10 +25,11 @@ class TestLLVMBuildConfig(TestCase):
 
     def test_configure_generator(self):
         """Test that we can use a custom generator for our CMake command."""
-        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath)
-
         consumer = MagicMock()
-        buildConfig.cmake(consumer, [], "Unix Makefiles")
+        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath,
+                                      consumer)
+
+        buildConfig.cmake([], "Unix Makefiles")
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(directory, self.buildPath)
@@ -41,12 +42,13 @@ class TestLLVMBuildConfig(TestCase):
 
     def test_configure_definitions(self):
         """Test that we can define custom CMake variables."""
-        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath)
+        consumer = MagicMock()
+        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath,
+                                      consumer)
         flags = ["-DCMAKE_BUILD_TYPE=Release",
                  "-DCMAKE_CXX_FLAGS=\"-Oz -g\""]
 
-        consumer = MagicMock()
-        buildConfig.cmake(consumer, flags, "Ninja")
+        buildConfig.cmake(flags, "Ninja")
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(directory, self.buildPath)
@@ -74,10 +76,11 @@ class TestLLVMBuildConfig(TestCase):
 
         self.sourceConfig.for_each_subproj.side_effect = for_each_subproj
 
-        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath)
-
         consumer = MagicMock()
-        buildConfig.cmake(consumer, [], "Ninja")
+        buildConfig = LLVMBuildConfig(self.sourceConfig, self.buildPath,
+                                      consumer)
+
+        buildConfig.cmake([], "Ninja")
         command, directory = consumer.consume.call_args[0]
 
         self.assertEqual(directory, self.buildPath)