From dd27662b401e2852f22eafecdd4532155deca3d6 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Tue, 4 Sep 2018 13:37:19 -0500 Subject: [PATCH] Enable testing in parallel when using CMake. (#8484) * Add 'extra_env' argument to Executable.__call__: this will be added to the environment but does not affect whether the current environment is reused. If 'env' is not set, then the current environment is copied and the variables from 'extra_env' are added to it. * MakeExecutable can take a 'jobs_env' parameter that specifies the name of an environment variable used to set the level of parallelism. This is added to 'extra_env' (so does not affect whether the current environment is reused). * CMake-based Spack packages set 'jobs_env' when executing the 'test' target for make and ninja (which does not use -j) --- lib/spack/spack/build_environment.py | 29 +++++++++++++++++-------- lib/spack/spack/build_systems/cmake.py | 6 +++-- lib/spack/spack/package.py | 8 +++---- lib/spack/spack/test/make_executable.py | 7 ++++++ lib/spack/spack/util/executable.py | 9 ++++++++ 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 92e86018bfd..41b760c21fe 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -110,11 +110,14 @@ class MakeExecutable(Executable): - """Special callable executable object for make so the user can - specify parallel or not on a per-invocation basis. Using - 'parallel' as a kwarg will override whatever the package's - global setting is, so you can either default to true or false - and override particular calls. + """Special callable executable object for make so the user can specify + parallelism options on a per-invocation basis. Specifying + 'parallel' to the call will override whatever the package's + global setting is, so you can either default to true or false and + override particular calls. Specifying 'jobs_env' to a particular + call will name an environment variable which will be set to the + parallelism level (without affecting the normal invocation with + -j). Note that if the SPACK_NO_PARALLEL_MAKE env var is set it overrides everything. @@ -125,12 +128,20 @@ def __init__(self, name, jobs): self.jobs = jobs def __call__(self, *args, **kwargs): + """parallel, and jobs_env from kwargs are swallowed and used here; + remaining arguments are passed through to the superclass. + """ + disable = env_flag(SPACK_NO_PARALLEL_MAKE) - parallel = not disable and kwargs.get('parallel', self.jobs > 1) + parallel = (not disable) and kwargs.pop('parallel', self.jobs > 1) if parallel: - jobs = "-j%d" % self.jobs - args = (jobs,) + args + args = ('-j{0}'.format(self.jobs),) + args + jobs_env = kwargs.pop('jobs_env', None) + if jobs_env: + # Caller wants us to set an environment variable to + # control the parallelism. + kwargs['extra_env'] = {jobs_env: str(self.jobs)} return super(MakeExecutable, self).__call__(*args, **kwargs) @@ -388,7 +399,7 @@ def set_module_variables_for_package(pkg, module): m.meson = Executable('meson') m.cmake = Executable('cmake') - m.ctest = Executable('ctest') + m.ctest = MakeExecutable('ctest', jobs) # Standard CMake arguments m.std_cmake_args = spack.build_systems.cmake.CMakePackage._std_args(pkg) diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py index 1592ae4be52..6da901e473a 100644 --- a/lib/spack/spack/build_systems/cmake.py +++ b/lib/spack/spack/build_systems/cmake.py @@ -255,10 +255,12 @@ def check(self): """ with working_dir(self.build_directory): if self.generator == 'Unix Makefiles': - self._if_make_target_execute('test') + self._if_make_target_execute('test', + jobs_env='CTEST_PARALLEL_LEVEL') self._if_make_target_execute('check') elif self.generator == 'Ninja': - self._if_ninja_target_execute('test') + self._if_ninja_target_execute('test', + jobs_env='CTEST_PARALLEL_LEVEL') self._if_ninja_target_execute('check') # Check that self.prefix is there after installation diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 5f29746f6e2..a614e108e4c 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1191,7 +1191,7 @@ def _has_make_target(self, target): return True - def _if_make_target_execute(self, target): + def _if_make_target_execute(self, target, *args, **kwargs): """Runs ``make target`` if 'target' is a valid target in the Makefile. Parameters: @@ -1199,7 +1199,7 @@ def _if_make_target_execute(self, target): """ if self._has_make_target(target): # Execute target - inspect.getmodule(self).make(target) + inspect.getmodule(self).make(target, *args, **kwargs) def _has_ninja_target(self, target): """Checks to see if 'target' is a valid target in a Ninja build script. @@ -1231,7 +1231,7 @@ def _has_ninja_target(self, target): return True - def _if_ninja_target_execute(self, target): + def _if_ninja_target_execute(self, target, *args, **kwargs): """Runs ``ninja target`` if 'target' is a valid target in the Ninja build script. @@ -1240,7 +1240,7 @@ def _if_ninja_target_execute(self, target): """ if self._has_ninja_target(target): # Execute target - inspect.getmodule(self).ninja(target) + inspect.getmodule(self).ninja(target, *args, **kwargs) def _get_needed_resources(self): resources = [] diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py index e6593b8ffa1..16947861ab5 100644 --- a/lib/spack/spack/test/make_executable.py +++ b/lib/spack/spack/test/make_executable.py @@ -122,3 +122,10 @@ def test_make_parallel_precedence(self): output=str).strip(), '-j8 install') del os.environ['SPACK_NO_PARALLEL_MAKE'] + + def test_make_jobs_env(self): + make = MakeExecutable('make', 8) + dump_env = {} + self.assertEqual(make(output=str, jobs_env='MAKE_PARALLELISM', + _dump_env=dump_env).strip(), '-j8') + self.assertEqual(dump_env['MAKE_PARALLELISM'], '8') diff --git a/lib/spack/spack/util/executable.py b/lib/spack/spack/util/executable.py index e80848b8824..1817d0a77b4 100644 --- a/lib/spack/spack/util/executable.py +++ b/lib/spack/spack/util/executable.py @@ -93,7 +93,11 @@ def __call__(self, *args, **kwargs): *args (str): Command-line arguments to the executable to run Keyword Arguments: + _dump_env (dict): Dict to be set to the environment actually + used (envisaged for testing purposes only) env (dict): The environment to run the executable with + extra_env (dict): Extra items to add to the environment + (neither requires nor precludes env) fail_on_error (bool): Raise an exception if the subprocess returns an error. Default is True. The return code is available as ``exe.returncode`` @@ -115,6 +119,7 @@ def __call__(self, *args, **kwargs): for ``input`` By default, the subprocess inherits the parent's file descriptors. + """ # Environment env_arg = kwargs.get('env', None) @@ -124,6 +129,10 @@ def __call__(self, *args, **kwargs): else: env = self.default_env.copy() env.update(env_arg) + env.update(kwargs.get('extra_env', {})) + if '_dump_env' in kwargs: + kwargs['_dump_env'].clear() + kwargs['_dump_env'].update(env) fail_on_error = kwargs.pop('fail_on_error', True) ignore_errors = kwargs.pop('ignore_errors', ())