zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

set_packge_py_globals: only set pure build related globals on the root in build context (#42215)

Browse Source 
Previously `std_args` was called on non-roots in a build context, which is redundant, and also leads to issues when `std_args` expects build deps of the `pkg` to be installed.
This commit is contained in:
Harmen Stoppels 2024-01-24 10:14:13 +01:00 committed by GitHub
parent 61421b3a67
commit 28b7f72b96
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 63 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
lib/spack/spack/build_environment.py
View File

@ -555,58 +555,55 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD):
"""
module = ModuleChangePropagator(pkg)
m = module
if context == Context.BUILD:
jobs = determine_number_of_jobs(parallel=pkg.parallel)
m.make_jobs = jobs
module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
module.std_meson_args = spack.build_systems.meson.MesonBuilder.std_args(pkg)
module.std_pip_args = spack.build_systems.python.PythonPipBuilder.std_args(pkg)
# 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, supports_jobserver=False)
# 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":
m.nmake = Executable("nmake")
m.msbuild = Executable("msbuild")
# analog to configure for win32
m.cscript = Executable("cscript")
jobs = determine_number_of_jobs(parallel=pkg.parallel)
module.make_jobs = jobs
# Find the configure script in the archive path
# Don't use which for this; we want to find it in the current dir.
m.configure = Executable("./configure")
# TODO: make these build deps that can be installed if not found.
module.make = MakeExecutable("make", jobs)
module.gmake = MakeExecutable("gmake", jobs)
module.ninja = MakeExecutable("ninja", jobs, supports_jobserver=False)
# 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")
# analog to configure for win32
module.cscript = Executable("cscript")
# Standard CMake arguments
m.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
m.std_meson_args = spack.build_systems.meson.MesonBuilder.std_args(pkg)
m.std_pip_args = spack.build_systems.python.PythonPipBuilder.std_args(pkg)
# Find the configure script in the archive path
# Don't use which for this; we want to find it in the current dir.
module.configure = Executable("./configure")
# Put spack compiler paths in module scope. (Some packages use it
# in setup_run_environment etc, so don't put it context == build)
link_dir = spack.paths.build_env_path
m.spack_cc = os.path.join(link_dir, pkg.compiler.link_paths["cc"])
m.spack_cxx = os.path.join(link_dir, pkg.compiler.link_paths["cxx"])
m.spack_f77 = os.path.join(link_dir, pkg.compiler.link_paths["f77"])
m.spack_fc = os.path.join(link_dir, pkg.compiler.link_paths["fc"])
module.spack_cc = os.path.join(link_dir, pkg.compiler.link_paths["cc"])
module.spack_cxx = os.path.join(link_dir, pkg.compiler.link_paths["cxx"])
module.spack_f77 = os.path.join(link_dir, pkg.compiler.link_paths["f77"])
module.spack_fc = os.path.join(link_dir, pkg.compiler.link_paths["fc"])
# Useful directories within the prefix are encapsulated in
# a Prefix object.
m.prefix = pkg.prefix
module.prefix = pkg.prefix
# Platform-specific library suffix.
m.dso_suffix = dso_suffix
module.dso_suffix = dso_suffix
def static_to_shared_library(static_lib, shared_lib=None, **kwargs):
compiler_path = kwargs.get("compiler", m.spack_cc)
compiler_path = kwargs.get("compiler", module.spack_cc)
compiler = Executable(compiler_path)
return _static_to_shared_library(
pkg.spec.architecture, compiler, static_lib, shared_lib, **kwargs
)
m.static_to_shared_library = static_to_shared_library
module.static_to_shared_library = static_to_shared_library
module.propagate_changes_to_mro()
@ -975,8 +972,8 @@ def __init__(self, *specs: spack.spec.Spec, context: Context) -> None:
self.should_set_package_py_globals = (
self.should_setup_dependent_build_env | self.should_setup_run_env | UseMode.ROOT
)
# In a build context, the root and direct build deps need build-specific globals set.
self.needs_build_context = UseMode.ROOT | UseMode.BUILDTIME_DIRECT
# In a build context, the root needs build-specific globals set.
self.needs_build_context = UseMode.ROOT
def set_all_package_py_globals(self):
"""Set the globals in modules of package.py files."""

4
lib/spack/spack/build_systems/meson.py
View File

@ -149,7 +149,7 @@ def std_args(pkg):
else:
default_library = "shared"
args = [
return [
"-Dprefix={0}".format(pkg.prefix),
# If we do not specify libdir explicitly, Meson chooses something
# like lib/x86_64-linux-gnu, which causes problems when trying to
@ -163,8 +163,6 @@ def std_args(pkg):
"-Dwrap_mode=nodownload",
]
return args
@property
def build_dirname(self):
"""Returns the directory name to use when building the package."""

34
lib/spack/spack/test/build_environment.py
View File

@ -457,10 +457,10 @@ def test_parallel_false_is_not_propagating(default_mock_concretization):
# b (parallel =True)
s = default_mock_concretization("a foobar=bar")
spack.build_environment.set_package_py_globals(s.package)
spack.build_environment.set_package_py_globals(s.package, context=Context.BUILD)
assert s["a"].package.module.make_jobs == 1
spack.build_environment.set_package_py_globals(s["b"].package)
spack.build_environment.set_package_py_globals(s["b"].package, context=Context.BUILD)
assert s["b"].package.module.make_jobs == spack.build_environment.determine_number_of_jobs(
parallel=s["b"].package.parallel
)
@ -685,3 +685,33 @@ def test_clear_compiler_related_runtime_variables_of_build_deps(default_mock_con
assert "FC" not in result
assert "F77" not in result
assert result["ANOTHER_VAR"] == "this-should-be-present"
@pytest.mark.parametrize("context", [Context.BUILD, Context.RUN])
def test_build_system_globals_only_set_on_root_during_build(default_mock_concretization, context):
"""Test whether when setting up a build environment, the build related globals are set only
in the top level spec.
TODO: Since module instances are globals themselves, and Spack defines properties on them, they
persist across tests. In principle this is not terrible, cause the variables are mostly static.
But obviously it can lead to very hard to find bugs... We should get rid of those globals and
define them instead as a property on the package instance.
"""
root = spack.spec.Spec("mpileaks").concretized()
build_variables = ("std_cmake_args", "std_meson_args", "std_pip_args")
# See todo above, we clear out any properties that may have been set by the previous test.
# Commenting this loop will make the test fail. I'm leaving it here as a reminder that those
# globals were always a bad idea, and we should pass them to the package instance.
for spec in root.traverse():
for variable in build_variables:
spec.package.module.__dict__.pop(variable, None)
spack.build_environment.SetupContext(root, context=context).set_all_package_py_globals()
# Excpect the globals to be set at the root in a build context only.
should_be_set = lambda depth: context == Context.BUILD and depth == 0
for depth, spec in root.traverse(depth=True, root=True):
for variable in build_variables:
assert hasattr(spec.package.module, variable) == should_be_set(depth)