From 4a7508c9dfb470ed323f5521184eb558f48e8693 Mon Sep 17 00:00:00 2001 From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com> Date: Tue, 15 Apr 2025 16:44:25 -0400 Subject: [PATCH] Update make/nmake invocations (mostly Windows) (#49022) The second change technically affects non-Windows, but the behavior should be exactly the same: * Packages no longer have access to `.msbuild` and `.nmake` automatically; they now get them via a dependency on `msvc`. * Update two CMake-based packages that call `make test` to instead call `ctest` (`netcdf-cxx4` and `pegtl`). CMake-based packages should do this because on Windows `make test` will not generally work, but `ctest` does. * Fix `openssl` "make test" on Windows (WRT prior point: not a CMake-based package). --- lib/spack/spack/build_environment.py | 8 +++----- lib/spack/spack/package.py | 1 + var/spack/repos/builtin/packages/bzip2/package.py | 1 - var/spack/repos/builtin/packages/msvc/package.py | 7 +++++++ var/spack/repos/builtin/packages/netcdf-cxx4/package.py | 2 +- var/spack/repos/builtin/packages/openssl/package.py | 6 ++++-- var/spack/repos/builtin/packages/pegtl/package.py | 2 +- 7 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index a8857aecea9..36f74349c96 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -574,12 +574,10 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD): module.make = DeprecatedExecutable(pkg.name, "make", "gmake") module.gmake = DeprecatedExecutable(pkg.name, "gmake", "gmake") module.ninja = DeprecatedExecutable(pkg.name, "ninja", "ninja") - # TODO: johnwparent: add package or builder support to define these build tools - # for now there is no entrypoint for builders to define these on their - # own + if sys.platform == "win32": - module.nmake = Executable("nmake") - module.msbuild = Executable("msbuild") + module.nmake = DeprecatedExecutable(pkg.name, "nmake", "msvc") + module.msbuild = DeprecatedExecutable(pkg.name, "msbuild", "msvc") # analog to configure for win32 module.cscript = Executable("cscript") diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index bf2056d826d..c3846e5349d 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -162,6 +162,7 @@ class tty: configure: Executable make_jobs: int make: MakeExecutable +nmake: Executable ninja: MakeExecutable python_include: str python_platlib: str diff --git a/var/spack/repos/builtin/packages/bzip2/package.py b/var/spack/repos/builtin/packages/bzip2/package.py index 5f0ad51cd8e..6ce731ed257 100644 --- a/var/spack/repos/builtin/packages/bzip2/package.py +++ b/var/spack/repos/builtin/packages/bzip2/package.py @@ -131,7 +131,6 @@ def install(self, spec, prefix): # Build the static library and everything else if self.spec.satisfies("platform=windows"): # Build step - nmake = Executable("nmake.exe") nmake("-f", "makefile.msc") # Install step mkdirp(self.prefix.include) diff --git a/var/spack/repos/builtin/packages/msvc/package.py b/var/spack/repos/builtin/packages/msvc/package.py index ddab1972b9f..7140b321698 100644 --- a/var/spack/repos/builtin/packages/msvc/package.py +++ b/var/spack/repos/builtin/packages/msvc/package.py @@ -90,6 +90,13 @@ def determine_variants(cls, exes, version_str): extras["compilers"]["fortran"] = fortran_compiler return spec, extras + def setup_dependent_package(self, module, dependent_spec): + """Populates dependent module with tooling available from VS""" + # We want these to resolve to the paths set by MSVC's VCVARs + # so no paths + module.nmake = Executable("nmake") + module.msbuild = Executable("msbuild") + def setup_dependent_build_environment(self, env, dependent_spec): self.init_msvc() # Set the build environment variables for spack. Just using diff --git a/var/spack/repos/builtin/packages/netcdf-cxx4/package.py b/var/spack/repos/builtin/packages/netcdf-cxx4/package.py index 71b87db3c8c..4727f237714 100644 --- a/var/spack/repos/builtin/packages/netcdf-cxx4/package.py +++ b/var/spack/repos/builtin/packages/netcdf-cxx4/package.py @@ -95,4 +95,4 @@ def cmake_args(self): def check(self): with working_dir(self.build_directory): - make("test", parallel=False) + ctest() diff --git a/var/spack/repos/builtin/packages/openssl/package.py b/var/spack/repos/builtin/packages/openssl/package.py index d100f97aaf0..5fcb45f1c68 100644 --- a/var/spack/repos/builtin/packages/openssl/package.py +++ b/var/spack/repos/builtin/packages/openssl/package.py @@ -189,18 +189,20 @@ def install(self, spec, prefix): if spec.satisfies("platform=windows"): host_make = nmake + make_args = {} else: host_make = make + make_args = {"parallel": False} host_make() if self.run_tests: - host_make("test", parallel=False) # 'VERBOSE=1' + host_make("test", **make_args) # 'VERBOSE=1' install_tgt = "install" if self.spec.satisfies("+docs") else "install_sw" # See https://github.com/openssl/openssl/issues/7466#issuecomment-432148137 - host_make(install_tgt, parallel=False) + host_make(install_tgt, **make_args) @run_after("install") def link_system_certs(self): diff --git a/var/spack/repos/builtin/packages/pegtl/package.py b/var/spack/repos/builtin/packages/pegtl/package.py index e7829b97764..77239fdac1e 100644 --- a/var/spack/repos/builtin/packages/pegtl/package.py +++ b/var/spack/repos/builtin/packages/pegtl/package.py @@ -49,4 +49,4 @@ def cmake_args(self): @on_package_attributes(run_tests=True) def check(self): with working_dir(self.build_directory): - make("test", parallel=False) + ctest()