summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/spack/spack/build_environment.py57
-rw-r--r--lib/spack/spack/test/make_executable.py38
-rw-r--r--var/spack/repos/builtin/packages/cmake/package.py17
-rw-r--r--var/spack/repos/builtin/packages/ninja-fortran/package.py9
-rw-r--r--var/spack/repos/builtin/packages/ninja/package.py1
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"),
)