From 52307309410bf2f076df78f40ca3aa762b8897fc Mon Sep 17 00:00:00 2001
From: Peter Scheibel <scheibel1@llnl.gov>
Date: Tue, 11 May 2021 14:30:57 -0700
Subject: Environments: add run deps to shell modifications (#23485)

When adding an Environment to a user's shell, Spack was only adding
root specs. This now includes run dependencies of root specs.
---
 lib/spack/spack/environment.py                     | 41 +++++++++++++++-------
 lib/spack/spack/test/cmd/env.py                    | 13 +++++++
 .../packages/depends-on-run-env/package.py         | 22 ++++++++++++
 .../packages/modifies-run-env/package.py           | 21 +++++++++++
 4 files changed, 85 insertions(+), 12 deletions(-)
 create mode 100644 var/spack/repos/builtin.mock/packages/depends-on-run-env/package.py
 create mode 100644 var/spack/repos/builtin.mock/packages/modifies-run-env/package.py

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 46541fb32a..14e349ff11 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1276,19 +1276,36 @@ class Environment(object):
     def _env_modifications_for_default_view(self, reverse=False):
         all_mods = spack.util.environment.EnvironmentModifications()
 
+        visited = set()
+
         errors = []
-        for _, spec in self.concretized_specs():
-            if spec in self.default_view and spec.package.installed:
-                try:
-                    mods = uenv.environment_modifications_for_spec(
-                        spec, self.default_view)
-                except Exception as e:
-                    msg = ("couldn't get environment settings for %s"
-                           % spec.format("{name}@{version} /{hash:7}"))
-                    errors.append((msg, str(e)))
-                    continue
-
-                all_mods.extend(mods.reversed() if reverse else mods)
+        for _, root_spec in self.concretized_specs():
+            if root_spec in self.default_view and root_spec.package.installed:
+                for spec in root_spec.traverse(deptype='run', root=True):
+                    if spec.name in visited:
+                        # It is expected that only one instance of the package
+                        # can be added to the environment - do not attempt to
+                        # add multiple.
+                        tty.debug(
+                            "Not adding {0} to shell modifications: "
+                            "this package has already been added".format(
+                                spec.format("{name}/{hash:7}")
+                            )
+                        )
+                        continue
+                    else:
+                        visited.add(spec.name)
+
+                    try:
+                        mods = uenv.environment_modifications_for_spec(
+                            spec, self.default_view)
+                    except Exception as e:
+                        msg = ("couldn't get environment settings for %s"
+                               % spec.format("{name}@{version} /{hash:7}"))
+                        errors.append((msg, str(e)))
+                        continue
+
+                    all_mods.extend(mods.reversed() if reverse else mods)
 
         return all_mods, errors
 
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 5ff0f0de9f..9358d0a3a9 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -200,6 +200,19 @@ def test_env_modifications_error_on_activate(
     assert "Warning: couldn't get environment settings" in err
 
 
+def test_activate_adds_transitive_run_deps_to_path(
+        install_mockery, mock_fetch, monkeypatch):
+    env('create', 'test')
+    install = SpackCommand('install')
+
+    e = ev.read('test')
+    with e:
+        install('depends-on-run-env')
+
+    cmds = spack.environment.activate(e)
+    assert 'DEPENDENCY_ENV_VAR=1' in cmds
+
+
 def test_env_install_same_spec_twice(install_mockery, mock_fetch):
     env('create', 'test')
 
diff --git a/var/spack/repos/builtin.mock/packages/depends-on-run-env/package.py b/var/spack/repos/builtin.mock/packages/depends-on-run-env/package.py
new file mode 100644
index 0000000000..c85a249e9a
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/depends-on-run-env/package.py
@@ -0,0 +1,22 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack import *
+
+
+class DependsOnRunEnv(Package):
+    """This package has a runtime dependency on another package which needs
+       to perform shell modifications to run.
+    """
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/a-1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    depends_on('modifies-run-env', type=('run',))
+
+    def install(self, spec, prefix):
+        mkdirp(prefix.bin)
diff --git a/var/spack/repos/builtin.mock/packages/modifies-run-env/package.py b/var/spack/repos/builtin.mock/packages/modifies-run-env/package.py
new file mode 100644
index 0000000000..401750875c
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/modifies-run-env/package.py
@@ -0,0 +1,21 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack import *
+
+
+class ModifiesRunEnv(Package):
+    """Dependency package which needs to make shell modifications to run"""
+
+    homepage = "http://www.example.com"
+    url      = "http://www.example.com/a-1.0.tar.gz"
+
+    version('1.0', '0123456789abcdef0123456789abcdef')
+
+    def setup_run_environment(self, env):
+        env.set('DEPENDENCY_ENV_VAR', '1')
+
+    def install(self, spec, prefix):
+        mkdirp(prefix.bin)
-- 
cgit v1.2.3-70-g09d2