fix: don't call setup_environment when not needed (#3060)

* Don't call setup_environment when not needed. fixes #3059

* setup_environment and modules: added unit tests

5 files changed, 22 insertions, 2 deletions

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 885f2197ef..a6ffded935 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -389,7 +389,6 @@ class EnvModule(object):
             for mod in modules:
                 set_module_variables_for_package(package, mod)
             set_module_variables_for_package(package, package.module)
-            package.setup_environment(spack_env, env)
             package.setup_dependent_package(self.pkg.module, self.spec)
             package.setup_dependent_environment(spack_env, env, self.spec)
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index e5ea8c56ad..361691379e 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1452,7 +1452,7 @@ class PackageBase(object):
         This is useful if there are some common steps to installing
         all extensions for a certain package.
         """
-        self.setup_environment(spack_env, run_env)
+        pass
 
     def setup_dependent_package(self, module, dependent_spec):
         """Set up Python module-scope variables for dependent packages.
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index 4f35df1982..bb1b0006f8 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -357,6 +357,21 @@ class TestTcl(object):
         generator = tcl_factory(spec)
         assert 'bar' in generator.use_name
 
+    def test_setup_environment(self, tcl_factory):
+        spec = spack.spec.Spec('mpileaks')
+        spec.concretize()
+        content = get_modulefile_content(tcl_factory, spec)
+        assert len([x for x in content if 'setenv FOOBAR' in x]) == 1
+        assert len(
+            [x for x in content if 'setenv FOOBAR "mpileaks"' in x]
+        ) == 1
+
+        content = get_modulefile_content(tcl_factory, spec['callpath'])
+        assert len([x for x in content if 'setenv FOOBAR' in x]) == 1
+        assert len(
+            [x for x in content if 'setenv FOOBAR "callpath"' in x]
+        ) == 1
+
 
 @pytest.mark.usefixtures('config', 'builtin_mock', 'stringio_open')
 class TestLmod(object):
diff --git a/var/spack/repos/builtin.mock/packages/callpath/package.py b/var/spack/repos/builtin.mock/packages/callpath/package.py
index 56b969df98..c00f408866 100644
--- a/var/spack/repos/builtin.mock/packages/callpath/package.py
+++ b/var/spack/repos/builtin.mock/packages/callpath/package.py
@@ -40,3 +40,6 @@ class Callpath(Package):
         configure("--prefix=%s" % prefix)
         make()
         make("install")
+
+    def setup_environment(self, senv, renv):
+        renv.set('FOOBAR', self.name)
diff --git a/var/spack/repos/builtin.mock/packages/mpileaks/package.py b/var/spack/repos/builtin.mock/packages/mpileaks/package.py
index 10fbf3845e..749fcc601a 100644
--- a/var/spack/repos/builtin.mock/packages/mpileaks/package.py
+++ b/var/spack/repos/builtin.mock/packages/mpileaks/package.py
@@ -44,3 +44,6 @@ class Mpileaks(Package):
 
     def install(self, spec, prefix):
         pass
+
+    def setup_environment(self, senv, renv):
+        renv.set('FOOBAR', self.name)