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

lib/spack/spack/modules.py

@@ -389,7 +389,6 @@ def write(self, overwrite=False):
             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)
 

lib/spack/spack/package.py

@@ -1452,7 +1452,7 @@ def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
         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.

lib/spack/spack/test/modules.py

@@ -357,6 +357,21 @@ def test_suffixes(self, tcl_factory):
         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):

var/spack/repos/builtin.mock/packages/callpath/package.py

@@ -40,3 +40,6 @@ def install(self, spec, prefix):
         configure("--prefix=%s" % prefix)
         make()
         make("install")
+
+    def setup_environment(self, senv, renv):
+        renv.set('FOOBAR', self.name)

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)