Add better generator support to CMakePackage (#4988)

* Add better generator support to CMakePackage

* List valid CMake generators on error

4 files changed, 71 insertions, 31 deletions

diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py
index db3240e8a5..f643320b10 100644
--- a/lib/spack/spack/build_systems/cmake.py
+++ b/lib/spack/spack/build_systems/cmake.py
@@ -30,12 +30,15 @@ import platform
 import spack.build_environment
 from llnl.util.filesystem import working_dir, join_path
 from spack.directives import depends_on, variant
-from spack.package import PackageBase, run_after
+from spack.package import PackageBase, InstallError, run_after
 
 
 class CMakePackage(PackageBase):
     """Specialized class for packages built using CMake
 
+    For more information on the CMake build system, see:
+    https://cmake.org/cmake/help/latest/
+
     This class provides three phases that can be overridden:
 
         1. :py:meth:`~.CMakePackage.cmake`
@@ -69,6 +72,16 @@ class CMakePackage(PackageBase):
 
     build_time_test_callbacks = ['check']
 
+    #: The build system generator to use.
+    #:
+    #: See ``cmake --help`` for a list of valid generators.
+    #: Currently, "Unix Makefiles" and "Ninja" are the only generators
+    #: that Spack supports. Defaults to "Unix Makefiles".
+    #:
+    #: See https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html
+    #: for more information.
+    generator = 'Unix Makefiles'
+
     # https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html
     variant('build_type', default='RelWithDebInfo',
             description='The build type to build',
@@ -101,13 +114,30 @@ class CMakePackage(PackageBase):
     def _std_args(pkg):
         """Computes the standard cmake arguments for a generic package"""
         try:
+            generator = pkg.generator
+        except AttributeError:
+            generator = 'Unix Makefiles'
+
+        # Make sure a valid generator was chosen
+        valid_generators = ['Unix Makefiles', 'Ninja']
+        if generator not in valid_generators:
+            msg  = "Invalid CMake generator: '{0}'\n".format(generator)
+            msg += "CMakePackage currently supports the following "
+            msg += "generators: '{0}'".format("', '".join(valid_generators))
+            raise InstallError(msg)
+
+        try:
             build_type = pkg.spec.variants['build_type'].value
         except KeyError:
             build_type = 'RelWithDebInfo'
 
-        args = ['-DCMAKE_INSTALL_PREFIX:PATH={0}'.format(pkg.prefix),
-                '-DCMAKE_BUILD_TYPE:STRING={0}'.format(build_type),
-                '-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON']
+        args = [
+            '-G', generator,
+            '-DCMAKE_INSTALL_PREFIX:PATH={0}'.format(pkg.prefix),
+            '-DCMAKE_BUILD_TYPE:STRING={0}'.format(build_type),
+            '-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON'
+        ]
+
         if platform.mac_ver()[0]:
             args.append('-DCMAKE_FIND_FRAMEWORK:STRING=LAST')
 
@@ -158,12 +188,18 @@ class CMakePackage(PackageBase):
     def build(self, spec, prefix):
         """Make the build targets"""
         with working_dir(self.build_directory):
-            inspect.getmodule(self).make(*self.build_targets)
+            if self.generator == 'Unix Makefiles':
+                inspect.getmodule(self).make(*self.build_targets)
+            elif self.generator == 'Ninja':
+                inspect.getmodule(self).ninja(*self.build_targets)
 
     def install(self, spec, prefix):
         """Make the install targets"""
         with working_dir(self.build_directory):
-            inspect.getmodule(self).make(*self.install_targets)
+            if self.generator == 'Unix Makefiles':
+                inspect.getmodule(self).make(*self.install_targets)
+            elif self.generator == 'Ninja':
+                inspect.getmodule(self).ninja(*self.install_targets)
 
     run_after('build')(PackageBase._run_default_build_time_test_callbacks)
 
@@ -172,7 +208,10 @@ class CMakePackage(PackageBase):
         and runs it if found.
         """
         with working_dir(self.build_directory):
-            self._if_make_target_execute('test')
+            if self.generator == 'Unix Makefiles':
+                self._if_make_target_execute('test')
+            elif self.generator == 'Ninja':
+                self._if_ninja_target_execute('test')
 
     # Check that self.prefix is there after installation
     run_after('install')(PackageBase.sanity_check_prefix)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 8c849573a7..cbf7d92ea6 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1091,12 +1091,30 @@ class PackageBase(with_metaclass(PackageMeta, object)):
             matches = [line for line in f.readlines() if regex.match(line)]
 
         if not matches:
-            tty.msg('Target \'' + target + ':\' not found in Makefile')
+            tty.msg("Target '" + target + ":' not found in Makefile")
             return
 
         # Execute target
         inspect.getmodule(self).make(target)
 
+    def _if_ninja_target_execute(self, target):
+        # Check if we have a ninja build script
+        if not os.path.exists('build.ninja'):
+            tty.msg('No ninja build script found in the build directory')
+            return
+
+        # Check if 'target' is in the ninja build script
+        regex = re.compile('^build ' + target + ':')
+        with open('build.ninja', 'r') as f:
+            matches = [line for line in f.readlines() if regex.match(line)]
+
+        if not matches:
+            tty.msg("Target 'build " + target + ":' not found in build.ninja")
+            return
+
+        # Execute target
+        inspect.getmodule(self).ninja(target)
+
     def _get_needed_resources(self):
         resources = []
         # Select the resources that are needed for this build
diff --git a/var/spack/repos/builtin/packages/archer/package.py b/var/spack/repos/builtin/packages/archer/package.py
index 0883191277..f5e4fbf8c1 100644
--- a/var/spack/repos/builtin/packages/archer/package.py
+++ b/var/spack/repos/builtin/packages/archer/package.py
@@ -36,22 +36,14 @@ class Archer(CMakePackage):
 
     depends_on('cmake@3.4.3:', type='build')
     depends_on('llvm')
-    depends_on('ninja', type='build')
+    depends_on('ninja@1.5:', type='build')
     depends_on('llvm-openmp-ompt')
 
+    generator = 'Ninja'
+
     def cmake_args(self):
         return [
-            '-G', 'Ninja',
             '-DCMAKE_C_COMPILER=clang',
             '-DCMAKE_CXX_COMPILER=clang++',
             '-DOMP_PREFIX:PATH=%s' % self.spec['llvm-openmp-ompt'].prefix,
         ]
-
-    # TODO: Add better ninja support to CMakePackage
-    def build(self, spec, prefix):
-        with working_dir(self.build_directory):
-            ninja()
-
-    def install(self, spec, prefix):
-        with working_dir(self.build_directory):
-            ninja('install')
diff --git a/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py b/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
index 8039ec1717..edbd9f04e1 100644
--- a/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
+++ b/var/spack/repos/builtin/packages/llvm-openmp-ompt/package.py
@@ -44,24 +44,15 @@ class LlvmOpenmpOmpt(CMakePackage):
 
     depends_on('cmake@2.8:', type='build')
     depends_on('llvm')
-    depends_on('ninja', type='build')
+    depends_on('ninja@1.5:', type='build')
+
+    generator = 'Ninja'
 
     def cmake_args(self):
         return [
-            '-G', 'Ninja',
             '-DCMAKE_C_COMPILER=clang',
             '-DCMAKE_CXX_COMPILER=clang++',
-            '-DCMAKE_BUILD_TYPE=Release',
             '-DLIBOMP_OMPT_SUPPORT=on',
             '-DLIBOMP_OMPT_BLAME=on',
             '-DLIBOMP_OMPT_TRACE=on'
         ]
-
-    # TODO: Add better ninja support to CMakePackage
-    def build(self, spec, prefix):
-        with working_dir(self.build_directory):
-            ninja()
-
-    def install(self, spec, prefix):
-        with working_dir(self.build_directory):
-            ninja('install')