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)

6 files changed, 25 insertions, 37 deletions

diff --git a/var/spack/repos/builtin/packages/gptune/package.py b/var/spack/repos/builtin/packages/gptune/package.py
index 2affba20ef..c0c321c9a4 100644
--- a/var/spack/repos/builtin/packages/gptune/package.py
+++ b/var/spack/repos/builtin/packages/gptune/package.py
@@ -52,6 +52,7 @@ class Gptune(CMakePackage):
     depends_on("py-pyaml", type=("build", "run"))
     depends_on("py-statsmodels@0.13.0:", type=("build", "run"))
     depends_on("py-mpi4py@3.0.3:", type=("build", "run"))
+    depends_on("python", type=("build", "run"))
     depends_on("pygmo", type=("build", "run"))
     depends_on("openturns", type=("build", "run"))
     depends_on("py-pymoo", type=("build", "run"), when="@3.0.0:")
diff --git a/var/spack/repos/builtin/packages/llvm/package.py b/var/spack/repos/builtin/packages/llvm/package.py
index b7a78c6fdd..7e110a248e 100644
--- a/var/spack/repos/builtin/packages/llvm/package.py
+++ b/var/spack/repos/builtin/packages/llvm/package.py
@@ -752,14 +752,6 @@ class Llvm(CMakePackage, CudaPackage):
                     os.symlink(bin, sym)
             env.prepend_path("PATH", self.stage.path)
 
-    def setup_run_environment(self, env):
-        if "+clang" in self.spec:
-            env.set("CC", join_path(self.spec.prefix.bin, "clang"))
-            env.set("CXX", join_path(self.spec.prefix.bin, "clang++"))
-        if "+flang" in self.spec:
-            env.set("FC", join_path(self.spec.prefix.bin, "flang"))
-            env.set("F77", join_path(self.spec.prefix.bin, "flang"))
-
     root_cmakelists_dir = "llvm"
 
     def cmake_args(self):
diff --git a/var/spack/repos/builtin/packages/perl/package.py b/var/spack/repos/builtin/packages/perl/package.py
index bffaf875a2..7a4e15c1f7 100644
--- a/var/spack/repos/builtin/packages/perl/package.py
+++ b/var/spack/repos/builtin/packages/perl/package.py
@@ -401,14 +401,13 @@ class Perl(Package):  # Perl doesn't use Autotools, it should subclass Package
                 maker()
                 maker("install")
 
-    def _setup_dependent_env(self, env, dependent_spec, deptype):
+    def _setup_dependent_env(self, env, dependent_spec):
         """Set PATH and PERL5LIB to include the extension and
         any other perl extensions it depends on,
         assuming they were installed with INSTALL_BASE defined."""
         perl_lib_dirs = []
-        for d in dependent_spec.traverse(deptype=deptype):
-            if d.package.extends(self.spec):
-                perl_lib_dirs.append(d.prefix.lib.perl5)
+        if dependent_spec.package.extends(self.spec):
+            perl_lib_dirs.append(dependent_spec.prefix.lib.perl5)
         if perl_lib_dirs:
             perl_lib_path = ":".join(perl_lib_dirs)
             env.prepend_path("PERL5LIB", perl_lib_path)
@@ -416,10 +415,10 @@ class Perl(Package):  # Perl doesn't use Autotools, it should subclass Package
             env.append_path("PATH", self.prefix.bin)
 
     def setup_dependent_build_environment(self, env, dependent_spec):
-        self._setup_dependent_env(env, dependent_spec, deptype=("build", "run", "test"))
+        self._setup_dependent_env(env, dependent_spec)
 
     def setup_dependent_run_environment(self, env, dependent_spec):
-        self._setup_dependent_env(env, dependent_spec, deptype=("run",))
+        self._setup_dependent_env(env, dependent_spec)
 
     def setup_dependent_package(self, module, dependent_spec):
         """Called before perl modules' install() methods.
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 59306d8f23..83dbfb382f 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -1244,12 +1244,11 @@ print(json.dumps(config))
         """Set PYTHONPATH to include the site-packages directory for the
         extension and any other python extensions it depends on.
         """
-        for d in dependent_spec.traverse(deptype=("run"), root=True):
-            if d.package.extends(self.spec):
-                # Packages may be installed in platform-specific or platform-independent
-                # site-packages directories
-                for directory in {self.platlib, self.purelib}:
-                    env.prepend_path("PYTHONPATH", os.path.join(d.prefix, directory))
+        if dependent_spec.package.extends(self.spec):
+            # Packages may be installed in platform-specific or platform-independent
+            # site-packages directories
+            for directory in {self.platlib, self.purelib}:
+                env.prepend_path("PYTHONPATH", os.path.join(dependent_spec.prefix, directory))
 
     def setup_dependent_package(self, module, dependent_spec):
         """Called before python modules' install() methods."""
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 60cbc76cd8..694c2551e1 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -83,9 +83,8 @@ class Ruby(AutotoolsPackage, NMakePackage):
         return url.format(version.up_to(2), version)
 
     def setup_dependent_run_environment(self, env, dependent_spec):
-        for d in dependent_spec.traverse(deptype=("run"), root=True):
-            if d.package.extends(self.spec):
-                env.prepend_path("GEM_PATH", d.prefix)
+        if dependent_spec.package.extends(self.spec):
+            env.prepend_path("GEM_PATH", dependent_spec.prefix)
 
     def setup_dependent_package(self, module, dependent_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
diff --git a/var/spack/repos/builtin/packages/tcl/package.py b/var/spack/repos/builtin/packages/tcl/package.py
index 33bff65e25..c0082dc52c 100644
--- a/var/spack/repos/builtin/packages/tcl/package.py
+++ b/var/spack/repos/builtin/packages/tcl/package.py
@@ -151,13 +151,12 @@ class Tcl(AutotoolsPackage, SourceforgePackage):
         # https://core.tcl-lang.org/tk/tktview/447bd3e4abe17452d19a80e6840dcc8a2603fcbc
         env.prepend_path("TCLLIBPATH", self.spec["tcl"].libs.directories[0], separator=" ")
 
-        for d in dependent_spec.traverse(deptype=("build", "run", "test")):
-            if d.package.extends(self.spec):
-                # Tcl libraries may be installed in lib or lib64, see #19546
-                for lib in ["lib", "lib64"]:
-                    tcllibpath = join_path(d.prefix, lib)
-                    if os.path.exists(tcllibpath):
-                        env.prepend_path("TCLLIBPATH", tcllibpath, separator=" ")
+        if dependent_spec.package.extends(self.spec):
+            # Tcl libraries may be installed in lib or lib64, see #19546
+            for lib in ["lib", "lib64"]:
+                tcllibpath = join_path(dependent_spec.prefix, lib)
+                if os.path.exists(tcllibpath):
+                    env.prepend_path("TCLLIBPATH", tcllibpath, separator=" ")
 
     def setup_dependent_run_environment(self, env, dependent_spec):
         """Set TCLLIBPATH to include the tcl-shipped directory for
@@ -167,10 +166,9 @@ class Tcl(AutotoolsPackage, SourceforgePackage):
 
         * https://wiki.tcl-lang.org/page/TCLLIBPATH
         """
-        for d in dependent_spec.traverse(deptype=("build", "run", "test")):
-            if d.package.extends(self.spec):
-                # Tcl libraries may be installed in lib or lib64, see #19546
-                for lib in ["lib", "lib64"]:
-                    tcllibpath = join_path(d.prefix, lib)
-                    if os.path.exists(tcllibpath):
-                        env.prepend_path("TCLLIBPATH", tcllibpath, separator=" ")
+        if dependent_spec.package.extends(self.spec):
+            # Tcl libraries may be installed in lib or lib64, see #19546
+            for lib in ["lib", "lib64"]:
+                tcllibpath = join_path(dependent_spec.prefix, lib)
+                if os.path.exists(tcllibpath):
+                    env.prepend_path("TCLLIBPATH", tcllibpath, separator=" ")