diff options
| -rw-r--r-- | lib/spack/spack/build_environment.py | 57 | ||||
| -rw-r--r-- | lib/spack/spack/test/make_executable.py | 38 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/cmake/package.py | 17 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/ninja-fortran/package.py | 9 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/ninja/package.py | 1 |
5 files changed, 80 insertions, 42 deletions
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 1496e9359f..c69dd566e6 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -121,18 +121,18 @@ else: stat_suffix = "lib" if sys.platform == "win32" else "a" -def should_set_parallel_jobs(jobserver_support=False): - """Returns true in general, except when: - - The env variable SPACK_NO_PARALLEL_MAKE=1 is set - - jobserver_support is enabled, and a jobserver was found. - """ - if ( - jobserver_support - and "MAKEFLAGS" in os.environ - and "--jobserver" in os.environ["MAKEFLAGS"] - ): - return False - return not env_flag(SPACK_NO_PARALLEL_MAKE) +def jobserver_enabled(): + """Returns true if a posix jobserver (make) is detected.""" + return "MAKEFLAGS" in os.environ and "--jobserver" in os.environ["MAKEFLAGS"] + + +def get_effective_jobs(jobs, parallel=True, supports_jobserver=False): + """Return the number of jobs, or None if supports_jobserver and a jobserver is detected.""" + if not parallel or jobs <= 1 or env_flag(SPACK_NO_PARALLEL_MAKE): + return 1 + if supports_jobserver and jobserver_enabled(): + return None + return jobs class MakeExecutable(Executable): @@ -147,26 +147,33 @@ class MakeExecutable(Executable): """ def __init__(self, name, jobs, **kwargs): + supports_jobserver = kwargs.pop("supports_jobserver", True) super(MakeExecutable, self).__init__(name, **kwargs) + self.supports_jobserver = supports_jobserver 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. """ - # TODO: figure out how to check if we are using a jobserver-supporting ninja, - # the two split ninja packages make this very difficult right now - parallel = should_set_parallel_jobs(jobserver_support=True) and kwargs.pop( - "parallel", self.jobs > 1 - ) + parallel = kwargs.pop("parallel", True) + jobs_env = kwargs.pop("jobs_env", None) + jobs_env_supports_jobserver = kwargs.pop("jobs_env_supports_jobserver", False) - if parallel: - 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)} + jobs = get_effective_jobs( + self.jobs, parallel=parallel, supports_jobserver=self.supports_jobserver + ) + if jobs is not None: + args = ("-j{0}".format(jobs),) + args + + if jobs_env: + # Caller wants us to set an environment variable to + # control the parallelism. + jobs_env_jobs = get_effective_jobs( + self.jobs, parallel=parallel, supports_jobserver=jobs_env_supports_jobserver + ) + if jobs_env_jobs is not None: + kwargs["extra_env"] = {jobs_env: str(jobs_env_jobs)} return super(MakeExecutable, self).__call__(*args, **kwargs) @@ -547,7 +554,7 @@ def _set_variables_for_single_module(pkg, module): # TODO: make these build deps that can be installed if not found. m.make = MakeExecutable("make", jobs) m.gmake = MakeExecutable("gmake", jobs) - m.ninja = MakeExecutable("ninja", jobs) + m.ninja = MakeExecutable("ninja", jobs, supports_jobserver=False) # easy shortcut to os.environ m.env = os.environ diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py index b7063e5e10..efaff04de5 100644 --- a/lib/spack/spack/test/make_executable.py +++ b/lib/spack/spack/test/make_executable.py @@ -53,24 +53,24 @@ class MakeExecutableTest(unittest.TestCase): def test_make_one_job(self): make = MakeExecutable("make", 1) - self.assertEqual(make(output=str).strip(), "") - self.assertEqual(make("install", output=str).strip(), "install") + self.assertEqual(make(output=str).strip(), "-j1") + self.assertEqual(make("install", output=str).strip(), "-j1 install") def test_make_parallel_false(self): make = MakeExecutable("make", 8) - self.assertEqual(make(parallel=False, output=str).strip(), "") - self.assertEqual(make("install", parallel=False, output=str).strip(), "install") + self.assertEqual(make(parallel=False, output=str).strip(), "-j1") + self.assertEqual(make("install", parallel=False, output=str).strip(), "-j1 install") def test_make_parallel_disabled(self): make = MakeExecutable("make", 8) os.environ["SPACK_NO_PARALLEL_MAKE"] = "true" - self.assertEqual(make(output=str).strip(), "") - self.assertEqual(make("install", output=str).strip(), "install") + self.assertEqual(make(output=str).strip(), "-j1") + self.assertEqual(make("install", output=str).strip(), "-j1 install") os.environ["SPACK_NO_PARALLEL_MAKE"] = "1" - self.assertEqual(make(output=str).strip(), "") - self.assertEqual(make("install", output=str).strip(), "install") + self.assertEqual(make(output=str).strip(), "-j1") + self.assertEqual(make("install", output=str).strip(), "-j1 install") # These don't disable (false and random string) os.environ["SPACK_NO_PARALLEL_MAKE"] = "false" @@ -88,12 +88,12 @@ class MakeExecutableTest(unittest.TestCase): # These should work os.environ["SPACK_NO_PARALLEL_MAKE"] = "true" - self.assertEqual(make(parallel=True, output=str).strip(), "") - self.assertEqual(make("install", parallel=True, output=str).strip(), "install") + self.assertEqual(make(parallel=True, output=str).strip(), "-j1") + self.assertEqual(make("install", parallel=True, output=str).strip(), "-j1 install") os.environ["SPACK_NO_PARALLEL_MAKE"] = "1" - self.assertEqual(make(parallel=True, output=str).strip(), "") - self.assertEqual(make("install", parallel=True, output=str).strip(), "install") + self.assertEqual(make(parallel=True, output=str).strip(), "-j1") + self.assertEqual(make("install", parallel=True, output=str).strip(), "-j1 install") # These don't disable (false and random string) os.environ["SPACK_NO_PARALLEL_MAKE"] = "false" @@ -113,3 +113,17 @@ class MakeExecutableTest(unittest.TestCase): make(output=str, jobs_env="MAKE_PARALLELISM", _dump_env=dump_env).strip(), "-j8" ) self.assertEqual(dump_env["MAKE_PARALLELISM"], "8") + + def test_make_jobserver(self): + make = MakeExecutable("make", 8) + os.environ["MAKEFLAGS"] = "--jobserver-auth=X,Y" + self.assertEqual(make(output=str).strip(), "") + self.assertEqual(make(parallel=False, output=str).strip(), "-j1") + del os.environ["MAKEFLAGS"] + + def test_make_jobserver_not_supported(self): + make = MakeExecutable("make", 8, supports_jobserver=False) + os.environ["MAKEFLAGS"] = "--jobserver-auth=X,Y" + # Currently fallback on default job count, Maybe it should force -j1 ? + self.assertEqual(make(output=str).strip(), "-j8") + del os.environ["MAKEFLAGS"] diff --git a/var/spack/repos/builtin/packages/cmake/package.py b/var/spack/repos/builtin/packages/cmake/package.py index e988af5ba2..0ce352b9ab 100644 --- a/var/spack/repos/builtin/packages/cmake/package.py +++ b/var/spack/repos/builtin/packages/cmake/package.py @@ -307,11 +307,20 @@ class Cmake(Package): args = [] self.generator = make + if self.spec.satisfies("platform=windows"): + args.append("-GNinja") + self.generator = ninja + if not sys.platform == "win32": args.append("--prefix={0}".format(self.prefix)) - if spack.build_environment.should_set_parallel_jobs(jobserver_support=True): - args.append("--parallel={0}".format(make_jobs)) + jobs = spack.build_environment.get_effective_jobs( + make_jobs, + parallel=self.parallel, + supports_jobserver=self.generator.supports_jobserver, + ) + if jobs is not None: + args.append("--parallel={0}".format(jobs)) if "+ownlibs" in spec: # Build and link to the CMake-provided third-party libraries @@ -338,9 +347,7 @@ class Cmake(Package): args.append("--") else: args.append("-DCMAKE_INSTALL_PREFIX=%s" % self.prefix) - if self.spec.satisfies("platform=windows"): - args.append("-GNinja") - self.generator = ninja + args.append("-DCMAKE_BUILD_TYPE={0}".format(self.spec.variants["build_type"].value)) # Install CMake correctly, even if `spack install` runs diff --git a/var/spack/repos/builtin/packages/ninja-fortran/package.py b/var/spack/repos/builtin/packages/ninja-fortran/package.py index 371b350add..4709f51694 100644 --- a/var/spack/repos/builtin/packages/ninja-fortran/package.py +++ b/var/spack/repos/builtin/packages/ninja-fortran/package.py @@ -86,3 +86,12 @@ class NinjaFortran(Package): # instead of 'ninja'. Install both for uniformity. with working_dir(prefix.bin): symlink("ninja", "ninja-build") + + def setup_dependent_package(self, module, dspec): + name = "ninja" + + module.ninja = MakeExecutable( + which_string(name, path=[self.spec.prefix.bin], required=True), + determine_number_of_jobs(parallel=self.parallel), + supports_jobserver=True, # This fork supports jobserver + ) diff --git a/var/spack/repos/builtin/packages/ninja/package.py b/var/spack/repos/builtin/packages/ninja/package.py index e71b648f26..a6ee815a2b 100644 --- a/var/spack/repos/builtin/packages/ninja/package.py +++ b/var/spack/repos/builtin/packages/ninja/package.py @@ -79,4 +79,5 @@ class Ninja(Package): module.ninja = MakeExecutable( which_string(name, path=[self.spec.prefix.bin], required=True), determine_number_of_jobs(parallel=self.parallel), + supports_jobserver=self.spec.version == ver("kitware"), ) |