Improve setup build / run / test environment

[haampie]

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)

[haampie]

Notice that this exposes some weird things, like pkg depends on gettext as a link dep, and gettext has +tar as a default, making it depend on tar as (build,run), tar depends on compression methods gzip/pigz/xz/.. as run deps, so those compression packages are marked RUNTIME_EXECUTABLE.

gettext is a bit weird.

[haampie]

libxc failing is irrelevant, the download is returning something incomplete.

[alalazo]

I started reviewing the PR and have a few initial comments. I think most package changes are worth being extracted and been given a fast track to merge. Let me know what you think on that.

[alalazo]

👍

[alalazo]

Do we need to bump version here?

[alalazo]

@haampie I think this ☝️ needs to be:

Suggested change

	if args.things_to_load != "package,dependencies":
	tty.warn(
	"The `--only` flag in spack load is deprecated and will be removed in Spack v0.21.0"
	)
	if args.things_to_load != "package,dependencies":
	tty.warn(
	"The `--only` flag in spack load is deprecated and will be removed in Spack v0.22.0"
	)

[haampie]

@spackbot rebuild everything

[spackbot-app]

I'm sorry, gitlab does not have your latest revision yet, I can't run that pipeline for you right now.

One likely possibility is that your PR pipeline has been temporarily deferred, in which case, it is awaiting a develop pipeline, and will be run when that finishes.

Please check the gitlab commit status message to see if more information is available.

Details

pr head: f59f9a1, gitlab commit parents: ['f384177', '337c028']

[trws]

Does this mean we get failures if run is removed here?

[haampie]

Yeah, this is used in spack build-env <spec>. If runtime deps of the root are not installed, Spack errors.

If I remember correctly this mirrors the installer: spack install --only=package <spec> would also error if the root has missing run deps.

I guess that we require pure run deps to be installed s.t. the dependents can in principle register absolute paths to their executables. An example of this is tar: it has a runtime dep on zstd, but also works without setting PATH=<zstd prefix>/bin, because the absolute path to the run dependency registered. (In principle this still doesn't require zstd to be installed first... but if zstd is installed first, you could search for the executable in zstd's bin dir, I can imagine that configure scripts do this)

[balay]

This PR breaks petsc build with oneapi compilers

Executing: /software/intel/oneapi/mpi/2021.10.0/bin/mpiicc -E  -I/tmp/petsc-gmbc5xcy/config.setCompilers  /tmp/petsc-gmbc5xcy/config.setCompilers/conftest.c
Possible ERROR while running preprocessor:exit code 0
stdout:
stdout:
# 1 "/tmp/petsc-gmbc5xcy/config.setCompilers/conftest.c"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
<snip>
# 1156 "/usr/include/stdlib.h" 2 3 4
# 4 "/tmp/petsc-gmbc5xcy/config.setCompilers/conftest.c" 2

stderr:
icx: warning: -Wl,-rpath,/software/intel/oneapi/mkl/2023.2.0/lib/intel64: 'linker' input unused [-Wunused-command-line-argument]
Source:
#include "confdefs.h"
#include "conffix.h"
#include <stdlib.h>

Somehow this PR change is triggering this STDERR message? [this breaks this simple configure test - resulting in build failure]

ref: spack install petsc %oneapi@2023.2.0

[balay]

icx: warning: -Wl,-rpath,/software/intel/oneapi/mkl/2023.2.0/lib/intel64: 'linker' input unused [-Wunused-command-line-argument]

Checking how to deal with this issue.

[balay]

For now working around this issue by adding -Wno-unused-command-line-argument to compilers.yaml

- compiler:
    spec: oneapi@=2023.2.0
    paths:
      cc: /software/intel/oneapi/compiler/2023.2.0/linux/bin/icx
      cxx: /software/intel/oneapi/compiler/2023.2.0/linux/bin/icpx
      f77: /software/intel/oneapi/compiler/2023.2.0/linux/bin/ifx
      fc: /software/intel/oneapi/compiler/2023.2.0/linux/bin/ifx
    #flags: {}
    flags:
      cflags: -Wno-unused-command-line-argument
      cxxflags: -Wno-unused-command-line-argument
    operating_system: fedora39
    target: x86_64
    modules: []
    environment: {}
    extra_rpaths: []

[balay]

For now working around this issue by adding -Wno-unused-command-line-argument to compilers.yaml

this didn't work. It turned out to be a "wack-a-mole" thingy - with different errors coming up with usage of different flags :(

[haampie]

@balay looks like intel packages are running

    def setup_run_environment(self, env):
        """Adds environment variables to the generated module file.

        These environment variables come from running:

        .. code-block:: console

           $ source {prefix}/{component}/{version}/env/vars.sh
        """
        # Only if environment modifications are desired (default is +envmods)
        if "~envmods" not in self.spec:
            env.extend(
                EnvironmentModifications.from_sourcing_file(
                    join_path(self.component_prefix, "env", "vars.sh"), *self.env_script_args
                )
            )

and we call setup_run_environment in more cases than before when setting up the build environment.

Can you check if #40916 fixes your issue?

Otherwise can you check the difference between spack build-env ... before / after this PR?

[balay]

Can you check if #40916 fixes your issue?

@haampie this MR doesn't work for me.

I'm not sure why my earlier debugging of this issue pointed to this PR [reverting this PR change worked for me at that time].

Now (with my simpler build command - without xsdk) I think the trigger is the presence of the following new values in env:

__INTEL_POST_CFLAGS=-Wl,-rpath,/opt/intel/oneapi/mkl/2022.2.0/lib/intel64
__INTEL_POST_FFLAGS=-Wl,-rpath,/opt/intel/oneapi/mkl/2022.2.0/lib/intel64

And the commit (from "git bisect") that makes the difference is: e5f3ffc

And my current work-around is at: 62915c2

Wrt e5f3ffc - attaching build-env - before and after this commit.

xsdk@pj02:~/spack$ git checkout -q e5f3ffc04fb7a1fea237226210b0cafc9246d0f2~1
xsdk@pj02:~/spack$ ./bin/spack build-env petsc~fortran~metis~hypre~hdf5 > build-env-prev.txt
xsdk@pj02:~/spack$ git checkout -q e5f3ffc04fb7a1fea237226210b0cafc9246d0f2
xsdk@pj02:~/spack$ ./bin/spack build-env petsc~fortran~metis~hypre~hdf5 > build-env-after.txt
xsdk@pj02:~/spack$

build-env-prev.txt
build-env-after.txt