From c39983eb1a81c7c9133565e0d37f6a120dff97c6 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 26 Jun 2020 16:12:22 -0500 Subject: 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 --- lib/spack/spack/build_environment.py | 3 +++ lib/spack/spack/compiler.py | 45 ++++++++++++++++++-------------- lib/spack/spack/test/compilers/basics.py | 30 +++++++++++++++++++++ lib/spack/spack/test/conftest.py | 19 ++++++++++++++ 4 files changed, 77 insertions(+), 20 deletions(-) (limited to 'lib') 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: # -- cgit v1.2.3-60-g2f50