diff options
31 files changed, 302 insertions, 34 deletions
diff --git a/.travis.yml b/.travis.yml index d64a33174a..18dc499ecc 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 d3a716f552..b333c7e0c2 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1139,7 +1139,15 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): 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 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): 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 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): 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 cac0711b48..8915b813cb 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 0000000000..a580687aa0 --- /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 0000000000..bbd931b84c --- /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 0000000000..e73101f01a --- /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 0000000000..77ea4d0972 --- /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 0000000000..f24ab957cb --- /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 0000000000..76e4478aae --- /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 0000000000..942f8ab96e --- /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 0000000000..22b42f3f83 --- /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 0000000000..c9b5d4d920 --- /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 0000000000..5924d7d702 --- /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 0000000000..96d7eff3af --- /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 0000000000..836ad0edb6 --- /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 0000000000..ea315731eb --- /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 0000000000..586aea18dc --- /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 0000000000..50e58f24cc --- /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 0000000000..e3af305dbe --- /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 0000000000..c9ce4e61a6 --- /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 0000000000..8a0d0f7c1d --- /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 0000000000..f002938b10 --- /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 0000000000..dd59a9932f --- /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 0000000000..19ef7e3509 --- /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 0000000000..8a0d0f7c1d --- /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 0000000000..6a3cd0b0ff --- /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 0000000000..bf9e14ed0b --- /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 0000000000..0a85aa7af8 --- /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 0000000000..12efb5839a --- /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 0000000000..2164bda6d9 --- /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 0000000000..73aafbf5bd --- /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 |