From 72b36ac14446782f47519db8ee5cff6631b2b4c9 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Thu, 19 Oct 2023 20:44:05 +0200 Subject: Improve setup build / run / test environment (#35737) This adds a `SetupContext` class which is responsible for setting package.py module globals, and computing the changes to environment variables for the build, test or run context. The class uses `effective_deptypes` which takes a list of specs (e.g. single item of a spec to build, or a list of environment roots) and a context (build, run, test), and outputs a flat list of specs that affect the environment together with a flag in what way they do so. This list is topologically ordered from root to leaf, so that one can be assured that dependents override variables set by dependencies, not the other way around. This is used to replace the logic in `modifications_from_dependencies`, which has several issues: missing calls to `setup_run_environment`, and the order in which operations are applied. Further, it should improve performance a bit in certain cases, since `effective_deptypes` run in O(v + e) time, whereas `spack env activate` currently can take up to O(v^2 + e) time due to loops over roots. Each edge in the DAG is visited once by calling `effective_deptypes` with `env.concrete_roots()`. By marking and propagating flags through the DAG, this commit also fixes a bug where Spack wouldn't call `setup_run_environment` for runtime dependencies of link dependencies. And this PR ensures that Spack correctly sets up the runtime environment of direct build dependencies. Regarding test dependencies: in a build context they are are build-time test deps, whereas in a test context they are install-time test deps. Since there are no means to distinguish the build/install type test deps, they're both. Further changes: - all `package.py` module globals are guaranteed to be set before any of the `setup_(dependent)_(run|build)_env` functions is called - traversal order during setup: first the group of externals, then the group of non-externals, with specs in each group traversed topological (dependencies are setup before dependents) - modules: only ever call `setup_dependent_run_environment` of *direct* link/run type deps - the marker in `set_module_variables_for_package` is dropped, since we should call the method once per spec. This allows us to set only a cheap subset of globals on the module: for example it's not necessary to compute the expensive `cmake_args` and w/e if the spec under consideration is not the root node to be built. - `spack load`'s `--only` is deprecated (it has no effect now), and `spack load x` now means: do everything that's required for `x` to work at runtime, which requires runtime deps to be setup -- just like `spack env activate`. - `spack load` no longer loads build deps (of build deps) ... - `spack env activate` on partially installed or broken environments: this is all or nothing now. If some spec errors during setup of its runtime env, you'll only get the unconditional variables + a warning that says the runtime changes for specs couldn't be applied. - Remove traversal in upward direction from `setup_dependent_*` in packages. Upward traversal may iterate to specs that aren't children of the roots (e.g. zlib / python have hundreds of dependents, only a small fraction is reachable from the roots. Packages should only modify the direct dependent they receive as an argument) --- share/spack/qa/setup-env-test.fish | 7 +++---- share/spack/qa/setup-env-test.sh | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-) (limited to 'share') diff --git a/share/spack/qa/setup-env-test.fish b/share/spack/qa/setup-env-test.fish index 86563b4b08..6474917b70 100755 --- a/share/spack/qa/setup-env-test.fish +++ b/share/spack/qa/setup-env-test.fish @@ -335,15 +335,14 @@ set _b_bin $_b_loc"/bin" set _a_loc (spack -m location -i shell-a) set _a_bin $_a_loc"/bin" -spt_contains "set -gx PATH $_b_bin" spack -m load --only package --fish shell-b +spt_contains "set -gx PATH $_b_bin" spack -m load --fish shell-b spt_succeeds spack -m load shell-b set LIST_CONTENT (spack -m load shell-b; spack load --list) spt_contains "shell-b@" echo $LIST_CONTENT spt_does_not_contain "shell-a@" echo $LIST_CONTENT # test a variable MacOS clears and one it doesn't for recursive loads -spt_contains "set -gx PATH $_a_bin:$_b_bin" spack -m load --fish shell-a -spt_succeeds spack -m load --only dependencies shell-a -spt_succeeds spack -m load --only package shell-a + +spt_succeeds spack -m load shell-a spt_fails spack -m load d spt_contains "usage: spack load " spack -m load -h spt_contains "usage: spack load " spack -m load -h d diff --git a/share/spack/qa/setup-env-test.sh b/share/spack/qa/setup-env-test.sh index 58feca69ea..4172a40155 100755 --- a/share/spack/qa/setup-env-test.sh +++ b/share/spack/qa/setup-env-test.sh @@ -104,7 +104,7 @@ contains "usage: spack module " spack -m module --help contains "usage: spack module " spack -m module title 'Testing `spack load`' -contains "export PATH=$(spack -m location -i shell-b)/bin" spack -m load --only package --sh shell-b +contains "export PATH=$(spack -m location -i shell-b)/bin" spack -m load --sh shell-b succeeds spack -m load shell-b LIST_CONTENT=`spack -m load shell-b; spack load --list` contains "shell-b@" echo $LIST_CONTENT @@ -113,8 +113,7 @@ fails spack -m load -l # test a variable MacOS clears and one it doesn't for recursive loads contains "export PATH=$(spack -m location -i shell-a)/bin" spack -m load --sh shell-a contains "export PATH=$(spack -m location -i shell-b)/bin" spack -m load --sh shell-b -succeeds spack -m load --only dependencies shell-a -succeeds spack -m load --only package shell-a +succeeds spack -m load shell-a fails spack -m load d contains "usage: spack load " spack -m load -h contains "usage: spack load " spack -m load -h d -- cgit v1.2.3-70-g09d2