build_environment: verify compiler executables exist are are accessible (#17260)

* build_environment: verify compiler executables exist and are accessible

* fix existing tests

* test compiler executable verification

4 files changed, 77 insertions, 20 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 8a063377a7..1b8aae31dc 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -198,6 +198,9 @@ def set_compiler_environment_variables(pkg, env):
     compiler = pkg.compiler
     spec = pkg.spec
 
+    # Make sure the executables for this compiler exist
+    compiler.verify_executables()
+
     # Set compiler variables used by CMake and autotools
     assert all(key in compiler.link_paths for key in (
         'cc', 'cxx', 'f77', 'fc'))
diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py
index f8c5ac65ad..5dd3d60778 100644
--- a/lib/spack/spack/compiler.py
+++ b/lib/spack/spack/compiler.py
@@ -27,12 +27,6 @@ from spack.util.environment import filter_system_paths
 __all__ = ['Compiler']
 
 
-def _verify_executables(*paths):
-    for path in paths:
-        if not os.path.isfile(path) and os.access(path, os.X_OK):
-            raise CompilerAccessError(path)
-
-
 @llnl.util.lang.memoized
 def get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
     """Invokes the compiler at a given path passing a single
@@ -271,20 +265,16 @@ class Compiler(object):
         self.extra_rpaths = extra_rpaths
         self.enable_implicit_rpaths = enable_implicit_rpaths
 
-        def check(exe):
-            if exe is None:
-                return None
-            _verify_executables(exe)
-            return exe
-
-        self.cc  = check(paths[0])
-        self.cxx = check(paths[1])
+        self.cc  = paths[0]
+        self.cxx = paths[1]
+        self.f77 = None
+        self.fc = None
         if len(paths) > 2:
-            self.f77 = check(paths[2])
+            self.f77 = paths[2]
             if len(paths) == 3:
                 self.fc = self.f77
             else:
-                self.fc  = check(paths[3])
+                self.fc  = paths[3]
 
         self.environment = environment
         self.extra_rpaths = extra_rpaths or []
@@ -298,6 +288,21 @@ class Compiler(object):
             if value is not None:
                 self.flags[flag] = tokenize_flags(value)
 
+    def verify_executables(self):
+        """Raise an error if any of the compiler executables is not valid.
+
+        This method confirms that for all of the compilers (cc, cxx, f77, fc)
+        that have paths, those paths exist and are executable by the current
+        user.
+        Raises a CompilerAccessError if any of the non-null paths for the
+        compiler are not accessible.
+        """
+        missing = [cmp for cmp in (self.cc, self.cxx, self.f77, self.fc)
+                   if cmp and not (os.path.isfile(cmp) and
+                                   os.access(cmp, os.X_OK))]
+        if missing:
+            raise CompilerAccessError(self, missing)
+
     @property
     def version(self):
         return self.spec.version
@@ -575,10 +580,10 @@ class Compiler(object):
 
 
 class CompilerAccessError(spack.error.SpackError):
-
-    def __init__(self, path):
-        super(CompilerAccessError, self).__init__(
-            "'%s' is not a valid compiler." % path)
+    def __init__(self, compiler, paths):
+        msg = "Compiler '%s' has executables that are missing" % compiler.spec
+        msg += " or are not executable: %s" % paths
+        super(CompilerAccessError, self).__init__(msg)
 
 
 class InvalidCompilerError(spack.error.SpackError):
diff --git a/lib/spack/spack/test/compilers/basics.py b/lib/spack/spack/test/compilers/basics.py
index 9ccf44f14f..9840b9f9af 100644
--- a/lib/spack/spack/test/compilers/basics.py
+++ b/lib/spack/spack/test/compilers/basics.py
@@ -823,3 +823,33 @@ def test_xcode_not_available(
     pkg = MockPackage()
     with pytest.raises(OSError):
         compiler.setup_custom_environment(pkg, env)
+
+
+@pytest.mark.enable_compiler_verification
+def test_compiler_executable_verification_raises(tmpdir):
+    compiler = MockCompiler()
+    compiler.cc = '/this/path/does/not/exist'
+
+    with pytest.raises(spack.compiler.CompilerAccessError):
+        compiler.verify_executables()
+
+
+@pytest.mark.enable_compiler_verification
+def test_compiler_executable_verification_success(tmpdir):
+    def prepare_executable(name):
+        real = str(tmpdir.join('cc').ensure())
+        fs.set_executable(real)
+        setattr(compiler, name, real)
+
+    # setup mock compiler with real paths
+    compiler = MockCompiler()
+    for name in ('cc', 'cxx', 'f77', 'fc'):
+        prepare_executable(name)
+
+    # testing that this doesn't raise an error because the paths exist and
+    # are executable
+    compiler.verify_executables()
+
+    # Test that null entries don't fail
+    compiler.cc = None
+    compiler.verify_executables()
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index 9a292821f5..98f82ca027 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -70,6 +70,25 @@ def clean_user_environment():
         ev.activate(active)
 
 
+#
+# Disable checks on compiler executable existence
+#
+@pytest.fixture(scope='function', autouse=True)
+def mock_compiler_executable_verification(request, monkeypatch):
+    """Mock the compiler executable verification to allow missing executables.
+
+    This fixture can be disabled for tests of the compiler verification
+    functionality by::
+
+        @pytest.mark.enable_compiler_verification
+
+    If a test is marked in that way this is a no-op."""
+    if 'enable_compiler_verification' not in request.keywords:
+        monkeypatch.setattr(spack.compiler.Compiler,
+                            'verify_executables',
+                            lambda x: None)
+
+
 # Hooks to add command line options or set other custom behaviors.
 # They must be placed here to be found by pytest. See:
 #