diff --git a/.travis.yml b/.travis.yml
index d64a33174abe1e9998f781fa42e48707823255d8..18dc499ecc75883d356ce95062b641b5017499c3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -132,6 +132,7 @@ addons:
- graphviz
- gnupg2
- cmake
+ - ninja-build
- r-base
- r-base-core
- r-base-dev
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index d3a716f552cf1e4bfcfbd1e5f03f25917f0ebd19..b333c7e0c21d220306ea1aec3a98d6ac9e9dc0ef 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1139,7 +1139,15 @@ def do_fake_install(self):
packages_dir = spack.store.layout.build_packages_path(self.spec)
dump_packages(self.spec, packages_dir)
- def _if_make_target_execute(self, target):
+ def _has_make_target(self, target):
+ """Checks to see if 'target' is a valid target in a Makefile.
+
+ Parameters:
+ target (str): the target to check for
+
+ Returns:
+ bool: True if 'target' is found, else False
+ """
make = inspect.getmodule(self).make
# Check if we have a Makefile
@@ -1148,49 +1156,68 @@ def _if_make_target_execute(self, target):
break
else:
tty.msg('No Makefile found in the build directory')
- return
+ return False
- # Check if 'target' is a valid target
- #
- # -q, --question
- # ``Question mode''. Do not run any commands, or print anything;
- # just return an exit status that is zero if the specified
- # targets are already up to date, nonzero otherwise.
+ # Check if 'target' is a valid target.
#
- # https://www.gnu.org/software/make/manual/html_node/Options-Summary.html
+ # `make -n target` performs a "dry run". It prints the commands that
+ # would be run but doesn't actually run them. If the target does not
+ # exist, you will see one of the following error messages:
#
- # The exit status of make is always one of three values:
+ # GNU Make:
+ # make: *** No rule to make target `test'. Stop.
#
- # 0 The exit status is zero if make is successful.
- #
- # 2 The exit status is two if make encounters any errors.
- # It will print messages describing the particular errors.
- #
- # 1 The exit status is one if you use the '-q' flag and make
- # determines that some target is not already up to date.
- #
- # https://www.gnu.org/software/make/manual/html_node/Running.html
- #
- # NOTE: This only works for GNU Make, not NetBSD Make.
- make('-q', target, fail_on_error=False)
- if make.returncode == 2:
- tty.msg("Target '" + target + "' not found in " + makefile)
- return
+ # BSD Make:
+ # make: don't know how to make test. Stop
+ missing_target_msgs = [
+ "No rule to make target `{0}'. Stop.",
+ "don't know how to make {0}. Stop",
+ ]
+
+ kwargs = {
+ 'fail_on_error': False,
+ 'output': os.devnull,
+ 'error': str,
+ }
+
+ stderr = make('-n', target, **kwargs)
+
+ for missing_target_msg in missing_target_msgs:
+ if missing_target_msg.format(target) in stderr:
+ tty.msg("Target '" + target + "' not found in " + makefile)
+ return False
- # Execute target
- make(target)
+ return True
- def _if_ninja_target_execute(self, target):
+ def _if_make_target_execute(self, target):
+ """Runs ``make target`` if 'target' is a valid target in the Makefile.
+
+ Parameters:
+ target (str): the target to potentially execute
+ """
+ if self._has_make_target(target):
+ # Execute target
+ inspect.getmodule(self).make(target)
+
+ def _has_ninja_target(self, target):
+ """Checks to see if 'target' is a valid target in a Ninja build script.
+
+ Parameters:
+ target (str): the target to check for
+
+ Returns:
+ bool: True if 'target' is found, else False
+ """
ninja = inspect.getmodule(self).ninja
# 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
+ return False
# Get a list of all targets in the Ninja build script
# https://ninja-build.org/manual.html#_extra_tools
- all_targets = ninja('-t', 'targets', output=str).split('\n')
+ all_targets = ninja('-t', 'targets', 'all', output=str).split('\n')
# Check if 'target' is a valid target
matches = [line for line in all_targets
@@ -1198,10 +1225,20 @@ def _if_ninja_target_execute(self, target):
if not matches:
tty.msg("Target '" + target + "' not found in build.ninja")
- return
+ return False
+
+ return True
+
+ def _if_ninja_target_execute(self, target):
+ """Runs ``ninja target`` if 'target' is a valid target in the Ninja
+ build script.
- # Execute target
- ninja(target)
+ Parameters:
+ target (str): the target to potentially execute
+ """
+ if self._has_ninja_target(target):
+ # Execute target
+ inspect.getmodule(self).ninja(target)
def _get_needed_resources(self):
resources = []
diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py
index cac0711b48cb7150f62869a04e7a8ccf7f3d7f5d..8915b813cb872006c521094f9db37cbf8322cec5 100644
--- a/lib/spack/spack/test/build_systems.py
+++ b/lib/spack/spack/test/build_systems.py
@@ -22,11 +22,101 @@
# License along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
##############################################################################
+import glob
+import os
import pytest
import spack.repo
-from spack.build_environment import get_std_cmake_args
+from llnl.util.filesystem import working_dir
+from spack.build_environment import get_std_cmake_args, setup_package
from spack.spec import Spec
+from spack.util.executable import which
+
+
+DATA_PATH = os.path.join(spack.paths.test_path, 'data')
+
+
+@pytest.mark.parametrize(
+ 'directory',
+ glob.iglob(os.path.join(DATA_PATH, 'make', 'affirmative', '*'))
+)
+def test_affirmative_make_check(directory, config, mock_packages):
+ """Tests that Spack correctly detects targets in a Makefile."""
+
+ # Get a fake package
+ s = Spec('mpich')
+ s.concretize()
+ pkg = spack.repo.get(s)
+ setup_package(pkg, False)
+
+ with working_dir(directory):
+ assert pkg._has_make_target('check')
+
+ pkg._if_make_target_execute('check')
+
+
+@pytest.mark.parametrize(
+ 'directory',
+ glob.iglob(os.path.join(DATA_PATH, 'make', 'negative', '*'))
+)
+def test_negative_make_check(directory, config, mock_packages):
+ """Tests that Spack correctly ignores false positives in a Makefile."""
+
+ # Get a fake package
+ s = Spec('mpich')
+ s.concretize()
+ pkg = spack.repo.get(s)
+ setup_package(pkg, False)
+
+ with working_dir(directory):
+ assert not pkg._has_make_target('check')
+
+ pkg._if_make_target_execute('check')
+
+
+@pytest.mark.skipif(not which('ninja'), reason='ninja is not installed')
+@pytest.mark.parametrize(
+ 'directory',
+ glob.iglob(os.path.join(DATA_PATH, 'ninja', 'affirmative', '*'))
+)
+def test_affirmative_ninja_check(directory, config, mock_packages):
+ """Tests that Spack correctly detects targets in a Ninja build script."""
+
+ # Get a fake package
+ s = Spec('mpich')
+ s.concretize()
+ pkg = spack.repo.get(s)
+ setup_package(pkg, False)
+
+ with working_dir(directory):
+ assert pkg._has_ninja_target('check')
+
+ pkg._if_ninja_target_execute('check')
+
+ # Clean up Ninja files
+ for filename in glob.iglob('.ninja_*'):
+ os.remove(filename)
+
+
+@pytest.mark.skipif(not which('ninja'), reason='ninja is not installed')
+@pytest.mark.parametrize(
+ 'directory',
+ glob.iglob(os.path.join(DATA_PATH, 'ninja', 'negative', '*'))
+)
+def test_negative_ninja_check(directory, config, mock_packages):
+ """Tests that Spack correctly ignores false positives in a Ninja
+ build script."""
+
+ # Get a fake package
+ s = Spec('mpich')
+ s.concretize()
+ pkg = spack.repo.get(s)
+ setup_package(pkg, False)
+
+ with working_dir(directory):
+ assert not pkg._has_ninja_target('check')
+
+ pkg._if_ninja_target_execute('check')
def test_cmake_std_args(config, mock_packages):
diff --git a/lib/spack/spack/test/data/make/affirmative/capital_makefile/Makefile b/lib/spack/spack/test/data/make/affirmative/capital_makefile/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..a580687aa08153e0405916ba11f45caf5e3fe290
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/capital_makefile/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack checks for Makefile
+
+check:
diff --git a/lib/spack/spack/test/data/make/affirmative/check_test/Makefile b/lib/spack/spack/test/data/make/affirmative/check_test/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..bbd931b84cc8a8fa29e23b89aeece1594c11980e
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/check_test/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack detects target when it is the first of two targets
+
+check test:
diff --git a/lib/spack/spack/test/data/make/affirmative/expansion/Makefile b/lib/spack/spack/test/data/make/affirmative/expansion/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..e73101f01a3a3e29f54e4519dd63418ac9eab2d6
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/expansion/Makefile
@@ -0,0 +1,5 @@
+# Tests that Spack can handle variable expansion targets
+
+TARGETS = check
+
+$(TARGETS):
diff --git a/lib/spack/spack/test/data/make/affirmative/gnu_makefile/GNUmakefile b/lib/spack/spack/test/data/make/affirmative/gnu_makefile/GNUmakefile
new file mode 100644
index 0000000000000000000000000000000000000000..77ea4d09727e533eeb34e0549945c00d5ebc706c
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/gnu_makefile/GNUmakefile
@@ -0,0 +1,3 @@
+# Tests that Spack checks for GNUmakefile
+
+check:
diff --git a/lib/spack/spack/test/data/make/affirmative/include/Makefile b/lib/spack/spack/test/data/make/affirmative/include/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..f24ab957cb969d0aefaa170340bdba47264aabb0
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/include/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack detects targets in include files
+
+include make.mk
diff --git a/lib/spack/spack/test/data/make/affirmative/include/make.mk b/lib/spack/spack/test/data/make/affirmative/include/make.mk
new file mode 100644
index 0000000000000000000000000000000000000000..76e4478aae542c65c44a79d5078f7a8d226ab073
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/include/make.mk
@@ -0,0 +1 @@
+check:
diff --git a/lib/spack/spack/test/data/make/affirmative/lowercase_makefile/makefile b/lib/spack/spack/test/data/make/affirmative/lowercase_makefile/makefile
new file mode 100644
index 0000000000000000000000000000000000000000..942f8ab96e27875f00260a6d37ecf6be7872a6cb
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/lowercase_makefile/makefile
@@ -0,0 +1,3 @@
+# Tests that Spack checks for makefile
+
+check:
diff --git a/lib/spack/spack/test/data/make/affirmative/prerequisites/Makefile b/lib/spack/spack/test/data/make/affirmative/prerequisites/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..22b42f3f8362abe8d193ed61d2209f578bf49634
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/prerequisites/Makefile
@@ -0,0 +1,5 @@
+# Tests that Spack detects a target even if it is followed by prerequisites
+
+check: check-recursive
+
+check-recursive:
diff --git a/lib/spack/spack/test/data/make/affirmative/spaces/Makefile b/lib/spack/spack/test/data/make/affirmative/spaces/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..c9b5d4d920311c1ae11fe8d094a1ceda485b17af
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/spaces/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack allows spaces following the target name
+
+check :
diff --git a/lib/spack/spack/test/data/make/affirmative/test_check/Makefile b/lib/spack/spack/test/data/make/affirmative/test_check/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..5924d7d702a20f8982b67cbee0971c733f9e603a
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/test_check/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack detects target when it is the second of two targets
+
+test check:
diff --git a/lib/spack/spack/test/data/make/affirmative/three_targets/Makefile b/lib/spack/spack/test/data/make/affirmative/three_targets/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..96d7eff3afd7857a0fa93fb8387e7b97611f5268
--- /dev/null
+++ b/lib/spack/spack/test/data/make/affirmative/three_targets/Makefile
@@ -0,0 +1,3 @@
+# Tests that Spack detects a target if it is in the middle of a list
+
+foo check bar:
diff --git a/lib/spack/spack/test/data/make/negative/no_makefile/readme.txt b/lib/spack/spack/test/data/make/negative/no_makefile/readme.txt
new file mode 100644
index 0000000000000000000000000000000000000000..836ad0edb6d53df9ab25471dabb45bb655a24688
--- /dev/null
+++ b/lib/spack/spack/test/data/make/negative/no_makefile/readme.txt
@@ -0,0 +1,3 @@
+# Tests that Spack ignores directories without a Makefile
+
+check:
diff --git a/lib/spack/spack/test/data/make/negative/partial_match/Makefile b/lib/spack/spack/test/data/make/negative/partial_match/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..ea315731eba4ffa9a65b839ec3e3b44f80a3ceed
--- /dev/null
+++ b/lib/spack/spack/test/data/make/negative/partial_match/Makefile
@@ -0,0 +1,11 @@
+# Tests that Spack ignores targets that contain a partial match
+
+checkinstall:
+
+installcheck:
+
+foo-check-bar:
+
+foo_check_bar:
+
+foo/check/bar:
diff --git a/lib/spack/spack/test/data/make/negative/variable/Makefile b/lib/spack/spack/test/data/make/negative/variable/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..586aea18dcb49e72e7ec75edab4fc3bcf2280a1d
--- /dev/null
+++ b/lib/spack/spack/test/data/make/negative/variable/Makefile
@@ -0,0 +1,5 @@
+# Tests that Spack ignores variable definitions
+
+check = FOO
+
+check := BAR
diff --git a/lib/spack/spack/test/data/ninja/.gitignore b/lib/spack/spack/test/data/ninja/.gitignore
new file mode 100644
index 0000000000000000000000000000000000000000..50e58f24cc9b2f2df1930192503ead036e6fc764
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/.gitignore
@@ -0,0 +1,2 @@
+.ninja_deps
+.ninja_log
diff --git a/lib/spack/spack/test/data/ninja/affirmative/check_test/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/check_test/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..e3af305dbe689d2e901978d58116410764538fa5
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/check_test/build.ninja
@@ -0,0 +1,6 @@
+# Tests that Spack detects target when it is the first of two targets
+
+rule cc
+ command = true
+
+build check test: cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/include/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/include/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..c9ce4e61a6c143f40926b177065f3104ce694bd8
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/include/build.ninja
@@ -0,0 +1,3 @@
+# Tests that Spack can handle targets in include files
+
+include include.ninja
diff --git a/lib/spack/spack/test/data/ninja/affirmative/include/include.ninja b/lib/spack/spack/test/data/ninja/affirmative/include/include.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..8a0d0f7c1d3871e07eb58af755d976efbfd97069
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/include/include.ninja
@@ -0,0 +1,4 @@
+rule cc
+ command = true
+
+build check: cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/simple/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/simple/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..f002938b101c23fe33ad819c3c6389d317563fe0
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/simple/build.ninja
@@ -0,0 +1,6 @@
+# Tests that Spack can handle a simple Ninja build script
+
+rule cc
+ command = true
+
+build check: cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/spaces/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/spaces/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..dd59a9932f87621f30dfeaa847de8ce1dd63fd9b
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/spaces/build.ninja
@@ -0,0 +1,6 @@
+# Tests that Spack allows spaces following the target name
+
+rule cc
+ command = true
+
+build check : cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/subninja/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/subninja/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..19ef7e35093d8f6a8cae0540a0fe6822367d0f72
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/subninja/build.ninja
@@ -0,0 +1,3 @@
+# Tests that Spack can handle targets in subninja files
+
+subninja subninja.ninja
diff --git a/lib/spack/spack/test/data/ninja/affirmative/subninja/subninja.ninja b/lib/spack/spack/test/data/ninja/affirmative/subninja/subninja.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..8a0d0f7c1d3871e07eb58af755d976efbfd97069
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/subninja/subninja.ninja
@@ -0,0 +1,4 @@
+rule cc
+ command = true
+
+build check: cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/test_check/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/test_check/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..6a3cd0b0ff251698267eea67e3202c05c555e20f
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/test_check/build.ninja
@@ -0,0 +1,6 @@
+# Tests that Spack detects target when it is the second of two targets
+
+rule cc
+ command = true
+
+build test check: cc
diff --git a/lib/spack/spack/test/data/ninja/affirmative/three_targets/build.ninja b/lib/spack/spack/test/data/ninja/affirmative/three_targets/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..bf9e14ed0b6b0d66b9617ff5495791866a53240b
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/affirmative/three_targets/build.ninja
@@ -0,0 +1,6 @@
+# Tests that Spack detects a target if it is in the middle of a list
+
+rule cc
+ command = true
+
+build foo check bar: cc
diff --git a/lib/spack/spack/test/data/ninja/negative/no_ninja/readme.txt b/lib/spack/spack/test/data/ninja/negative/no_ninja/readme.txt
new file mode 100644
index 0000000000000000000000000000000000000000..0a85aa7af85b5b968b390af09e07ce03e14d1f24
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/negative/no_ninja/readme.txt
@@ -0,0 +1,8 @@
+# Tests that Spack ignores directories without a Ninja build script
+
+cflags = -Wall
+
+rule cc
+ command = gcc $cflags -c $in -o $out
+
+build check: cc foo.c
diff --git a/lib/spack/spack/test/data/ninja/negative/partial_match/build.ninja b/lib/spack/spack/test/data/ninja/negative/partial_match/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..12efb5839a37f9c695920dae179e26e91fb9eb85
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/negative/partial_match/build.ninja
@@ -0,0 +1,16 @@
+# Tests that Spack ignores targets that contain a partial match
+
+cflags = -Wall
+
+rule cc
+ command = gcc $cflags -c $in -o $out
+
+build installcheck: cc foo.c
+
+build checkinstall: cc foo.c
+
+build foo-check-bar: cc foo.c
+
+build foo_check_bar: cc foo.c
+
+build foo/check/bar: cc foo.c
diff --git a/lib/spack/spack/test/data/ninja/negative/rule/build.ninja b/lib/spack/spack/test/data/ninja/negative/rule/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..2164bda6d9ad8259f141415388dae1c1b6327e51
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/negative/rule/build.ninja
@@ -0,0 +1,8 @@
+# Tests that Spack ignores rule names
+
+cflags = -Wall
+
+rule check
+ command = gcc $cflags -c $in -o $out
+
+build foo: check foo.c
diff --git a/lib/spack/spack/test/data/ninja/negative/variable/build.ninja b/lib/spack/spack/test/data/ninja/negative/variable/build.ninja
new file mode 100644
index 0000000000000000000000000000000000000000..73aafbf5bd9f87c6c1cbee76af31746b56eb4047
--- /dev/null
+++ b/lib/spack/spack/test/data/ninja/negative/variable/build.ninja
@@ -0,0 +1,8 @@
+# Tests that Spack ignores variable definitions
+
+check = -Wall
+
+rule cc
+ command = gcc $check -c $in -o $out
+
+build foo: cc foo.c