fix compiler environment handling to reset environment after (#17204)

bugfix: fix compiler environment handling to reset environment after

lib/spack/spack/compiler.py

@@ -355,11 +355,13 @@ def _get_compiler_link_paths(self, paths):
             for flag_type in flags:
                 for flag in self.flags.get(flag_type, []):
                     compiler_exe.add_default_arg(flag)
+
+            output = ''
             with self._compiler_environment():
                 output = str(compiler_exe(
                     self.verbose_flag, fin, '-o', fout,
                     output=str, error=str))  # str for py2
-                return _parse_non_system_link_dirs(output)
+            return _parse_non_system_link_dirs(output)
         except spack.util.executable.ProcessError as pe:
             tty.debug('ProcessError: Command exited with non-zero status: ' +
                       pe.long_message)
@@ -549,24 +551,27 @@ def _compiler_environment(self):
         # store environment to replace later
         backup_env = os.environ.copy()
 
-        # load modules and set env variables
-        for module in self.modules:
-            # On cray, mic-knl module cannot be loaded without cce module
-            # See: https://github.com/spack/spack/issues/3153
-            if os.environ.get("CRAY_CPU_TARGET") == 'mic-knl':
-                spack.util.module_cmd.load_module('cce')
-            spack.util.module_cmd.load_module(module)
+        try:
+            # load modules and set env variables
+            for module in self.modules:
+                # On cray, mic-knl module cannot be loaded without cce module
+                # See: https://github.com/spack/spack/issues/3153
+                if os.environ.get("CRAY_CPU_TARGET") == 'mic-knl':
+                    spack.util.module_cmd.load_module('cce')
+                spack.util.module_cmd.load_module(module)
 
-        # apply other compiler environment changes
-        env = spack.util.environment.EnvironmentModifications()
-        env.extend(spack.schema.environment.parse(self.environment))
-        env.apply_modifications()
+            # apply other compiler environment changes
+            env = spack.util.environment.EnvironmentModifications()
+            env.extend(spack.schema.environment.parse(self.environment))
+            env.apply_modifications()
 
-        yield
-
-        # Restore environment
-        os.environ.clear()
-        os.environ.update(backup_env)
+            yield
+        except BaseException:
+            raise
+        finally:
+            # Restore environment regardless of whether inner code succeeded
+            os.environ.clear()
+            os.environ.update(backup_env)
 
 
 class CompilerAccessError(spack.error.SpackError):

lib/spack/spack/test/compilers/basics.py

@@ -18,6 +18,7 @@
 import spack.compilers as compilers
 
 from spack.compiler import Compiler
+from spack.util.executable import ProcessError
 
 
 @pytest.fixture()
@@ -653,3 +654,62 @@ def module(*args):
     compiler = compilers[0]
     version = compiler.get_real_version()
     assert version == test_version
+
+
+def test_compiler_get_real_version_fails(working_env, monkeypatch, tmpdir):
+    # Test variables
+    test_version = '2.2.2'
+
+    # Create compiler
+    gcc = str(tmpdir.join('gcc'))
+    with open(gcc, 'w') as f:
+        f.write("""#!/bin/bash
+if [[ $CMP_ON == "1" ]]; then
+    echo "$CMP_VER"
+fi
+""")
+    fs.set_executable(gcc)
+
+    # Add compiler to config
+    compiler_info = {
+        'spec': 'gcc@foo',
+        'paths': {
+            'cc': gcc,
+            'cxx': None,
+            'f77': None,
+            'fc': None,
+        },
+        'flags': {},
+        'operating_system': 'fake',
+        'target': 'fake',
+        'modules': ['turn_on'],
+        'environment': {
+            'set': {'CMP_VER': test_version},
+        },
+        'extra_rpaths': [],
+    }
+    compiler_dict = {'compiler': compiler_info}
+
+    # Set module load to turn compiler on
+    def module(*args):
+        if args[0] == 'show':
+            return ''
+        elif args[0] == 'load':
+            os.environ['SPACK_TEST_CMP_ON'] = "1"
+    monkeypatch.setattr(spack.util.module_cmd, 'module', module)
+
+    # Make compiler fail when getting implicit rpaths
+    def _call(*args, **kwargs):
+        raise ProcessError("Failed intentionally")
+    monkeypatch.setattr(spack.util.executable.Executable, '__call__', _call)
+
+    # Run and no change to environment
+    compilers = spack.compilers.get_compilers([compiler_dict])
+    assert len(compilers) == 1
+    compiler = compilers[0]
+    try:
+        _ = compiler.get_real_version()
+        assert False
+    except ProcessError:
+        # Confirm environment does not change after failed call
+        assert 'SPACK_TEST_CMP_ON' not in os.environ