bugfix: make compiler preferences slightly saner (#17590)
* bugfix: make compiler preferences slightly saner This fixes two issues with the way we currently select compilers. If multiple compilers have the same "id" (os/arch/compiler/version), we currently prefer them by picking this one with the most supported languages. This can have some surprising effects: * If you have no `gfortran` but you have `gfortran-8`, you can detect `clang` that has no configured C compiler -- just `f77` and `f90`. This happens frequently on macOS with homebrew. The bug is due to some kludginess about the way we detect mixed `clang`/`gfortran`. * We can prefer suffixed versions of compilers to non-suffixed versions, which means we may select `clang-gpu` over `clang` at LLNL. But, `clang-gpu` is not actually clang, and it can break builds. We should prefer `clang` if it's available. - [x] prefer compilers that have C compilers and prefer no name variation to variation. * tests: add test for which()
This commit is contained in:
Todd Gamblin
Gregory Becker
parent
665a47607e
commit
f528022a7d
5 changed files with 192 additions and 20 deletions
|
|
@ -28,7 +28,7 @@
|
|||
|
||||
|
||||
@llnl.util.lang.memoized
|
||||
def get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
|
||||
def _get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
|
||||
"""Invokes the compiler at a given path passing a single
|
||||
version argument and returns the output.
|
||||
|
||||
|
|
@ -42,6 +42,18 @@ def get_compiler_version_output(compiler_path, version_arg, ignore_errors=()):
|
|||
return output
|
||||
|
||||
|
||||
def get_compiler_version_output(compiler_path, *args, **kwargs):
|
||||
"""Wrapper for _get_compiler_version_output()."""
|
||||
# This ensures that we memoize compiler output by *absolute path*,
|
||||
# not just executable name. If we don't do this, and the path changes
|
||||
# (e.g., during testing), we can get incorrect results.
|
||||
if not os.path.isabs(compiler_path):
|
||||
compiler_path = spack.util.executable.which_string(
|
||||
compiler_path, required=True)
|
||||
|
||||
return _get_compiler_version_output(compiler_path, *args, **kwargs)
|
||||
|
||||
|
||||
def tokenize_flags(flags_str):
|
||||
"""Given a compiler flag specification as a string, this returns a list
|
||||
where the entries are the flags. For compiler options which set values
|
||||
|
|
|
|||
|
|
@ -650,23 +650,18 @@ def make_compiler_list(detected_versions):
|
|||
Returns:
|
||||
list of Compiler objects
|
||||
"""
|
||||
# We don't sort on the path of the compiler
|
||||
sort_fn = lambda x: (x.id, x.variation, x.language)
|
||||
compilers_s = sorted(detected_versions, key=sort_fn)
|
||||
group_fn = lambda x: (x.id, x.variation, x.language)
|
||||
sorted_compilers = sorted(detected_versions, key=group_fn)
|
||||
|
||||
# Gather items in a dictionary by the id, name variation and language
|
||||
compilers_d = {}
|
||||
for sort_key, group in itertools.groupby(compilers_s, key=sort_fn):
|
||||
for sort_key, group in itertools.groupby(sorted_compilers, key=group_fn):
|
||||
compiler_id, name_variation, language = sort_key
|
||||
by_compiler_id = compilers_d.setdefault(compiler_id, {})
|
||||
by_name_variation = by_compiler_id.setdefault(name_variation, {})
|
||||
by_name_variation[language] = next(x.path for x in group)
|
||||
|
||||
# For each unique compiler id select the name variation with most entries
|
||||
# i.e. the one that supports most languages
|
||||
compilers = []
|
||||
|
||||
def _default(cmp_id, paths):
|
||||
def _default_make_compilers(cmp_id, paths):
|
||||
operating_system, compiler_name, version = cmp_id
|
||||
compiler_cls = spack.compilers.class_for_compiler_name(compiler_name)
|
||||
spec = spack.spec.CompilerSpec(compiler_cls.name, version)
|
||||
|
|
@ -677,16 +672,38 @@ def _default(cmp_id, paths):
|
|||
)
|
||||
return [compiler]
|
||||
|
||||
for compiler_id, by_compiler_id in compilers_d.items():
|
||||
_, selected_name_variation = max(
|
||||
(len(by_compiler_id[variation]), variation)
|
||||
for variation in by_compiler_id
|
||||
)
|
||||
# For compilers with the same compiler id:
|
||||
#
|
||||
# - Prefer with C compiler to without
|
||||
# - Prefer with C++ compiler to without
|
||||
# - Prefer no variations to variations (e.g., clang to clang-gpu)
|
||||
#
|
||||
sort_fn = lambda variation: (
|
||||
'cc' not in by_compiler_id[variation], # None last
|
||||
'cxx' not in by_compiler_id[variation], # None last
|
||||
variation.prefix,
|
||||
variation.suffix,
|
||||
)
|
||||
|
||||
compilers = []
|
||||
for compiler_id, by_compiler_id in compilers_d.items():
|
||||
ordered = sorted(by_compiler_id, key=sort_fn)
|
||||
selected_variation = ordered[0]
|
||||
selected = by_compiler_id[selected_variation]
|
||||
|
||||
# fill any missing parts from subsequent entries
|
||||
for lang in ['cxx', 'f77', 'fc']:
|
||||
if lang not in selected:
|
||||
next_lang = next((
|
||||
by_compiler_id[v][lang] for v in ordered
|
||||
if lang in by_compiler_id[v]), None)
|
||||
if next_lang:
|
||||
selected[lang] = next_lang
|
||||
|
||||
# Add it to the list of compilers
|
||||
selected = by_compiler_id[selected_name_variation]
|
||||
operating_system, _, _ = compiler_id
|
||||
make_compilers = getattr(operating_system, 'make_compilers', _default)
|
||||
make_compilers = getattr(
|
||||
operating_system, 'make_compilers', _default_make_compilers)
|
||||
|
||||
compilers.extend(make_compilers(compiler_id, selected))
|
||||
|
||||
return compilers
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@
|
|||
#
|
||||
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
|
|
@ -14,7 +16,7 @@
|
|||
|
||||
|
||||
@pytest.fixture
|
||||
def no_compilers_yaml(mutable_config, monkeypatch):
|
||||
def no_compilers_yaml(mutable_config):
|
||||
"""Creates a temporary configuration without compilers.yaml"""
|
||||
|
||||
for scope, local_config in mutable_config.scopes.items():
|
||||
|
|
@ -130,3 +132,121 @@ def test_compiler_add(
|
|||
new_compiler = new_compilers - old_compilers
|
||||
assert any(c.version == spack.version.Version(mock_compiler_version)
|
||||
for c in new_compiler)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def clangdir(tmpdir):
|
||||
"""Create a directory with some dummy compiler scripts in it.
|
||||
|
||||
Scripts are:
|
||||
- clang
|
||||
- clang++
|
||||
- gcc
|
||||
- g++
|
||||
- gfortran-8
|
||||
|
||||
"""
|
||||
with tmpdir.as_cwd():
|
||||
with open('clang', 'w') as f:
|
||||
f.write("""\
|
||||
#!/bin/sh
|
||||
if [ "$1" = "--version" ]; then
|
||||
echo "clang version 11.0.0 (clang-1100.0.33.16)"
|
||||
echo "Target: x86_64-apple-darwin18.7.0"
|
||||
echo "Thread model: posix"
|
||||
echo "InstalledDir: /dummy"
|
||||
else
|
||||
echo "clang: error: no input files"
|
||||
exit 1
|
||||
fi
|
||||
""")
|
||||
shutil.copy('clang', 'clang++')
|
||||
|
||||
gcc_script = """\
|
||||
#!/bin/sh
|
||||
if [ "$1" = "-dumpversion" ]; then
|
||||
echo "8"
|
||||
elif [ "$1" = "-dumpfullversion" ]; then
|
||||
echo "8.4.0"
|
||||
elif [ "$1" = "--version" ]; then
|
||||
echo "{0} (GCC) 8.4.0 20120313 (Red Hat 8.4.0-1)"
|
||||
echo "Copyright (C) 2010 Free Software Foundation, Inc."
|
||||
else
|
||||
echo "{1}: fatal error: no input files"
|
||||
echo "compilation terminated."
|
||||
exit 1
|
||||
fi
|
||||
"""
|
||||
with open('gcc-8', 'w') as f:
|
||||
f.write(gcc_script.format('gcc', 'gcc-8'))
|
||||
with open('g++-8', 'w') as f:
|
||||
f.write(gcc_script.format('g++', 'g++-8'))
|
||||
with open('gfortran-8', 'w') as f:
|
||||
f.write(gcc_script.format('GNU Fortran', 'gfortran-8'))
|
||||
os.chmod('clang', 0o700)
|
||||
os.chmod('clang++', 0o700)
|
||||
os.chmod('gcc-8', 0o700)
|
||||
os.chmod('g++-8', 0o700)
|
||||
os.chmod('gfortran-8', 0o700)
|
||||
|
||||
yield tmpdir
|
||||
|
||||
|
||||
@pytest.mark.regression('17590')
|
||||
def test_compiler_find_mixed_suffixes(
|
||||
no_compilers_yaml, working_env, clangdir):
|
||||
"""Ensure that we'll mix compilers with different suffixes when necessary.
|
||||
"""
|
||||
os.environ['PATH'] = str(clangdir)
|
||||
output = compiler('find', '--scope=site')
|
||||
|
||||
assert 'clang@11.0.0' in output
|
||||
assert 'gcc@8.4.0' in output
|
||||
|
||||
config = spack.compilers.get_compiler_config('site', False)
|
||||
clang = next(c['compiler'] for c in config
|
||||
if c['compiler']['spec'] == 'clang@11.0.0')
|
||||
gcc = next(c['compiler'] for c in config
|
||||
if c['compiler']['spec'] == 'gcc@8.4.0')
|
||||
|
||||
gfortran_path = str(clangdir.join('gfortran-8'))
|
||||
|
||||
assert clang['paths'] == {
|
||||
'cc': str(clangdir.join('clang')),
|
||||
'cxx': str(clangdir.join('clang++')),
|
||||
# we only auto-detect mixed clang on macos
|
||||
'f77': gfortran_path if sys.platform == 'darwin' else None,
|
||||
'fc': gfortran_path if sys.platform == 'darwin' else None,
|
||||
}
|
||||
|
||||
assert gcc['paths'] == {
|
||||
'cc': str(clangdir.join('gcc-8')),
|
||||
'cxx': str(clangdir.join('g++-8')),
|
||||
'f77': gfortran_path,
|
||||
'fc': gfortran_path,
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.regression('17590')
|
||||
def test_compiler_find_prefer_no_suffix(
|
||||
no_compilers_yaml, working_env, clangdir):
|
||||
"""Ensure that we'll pick 'clang' over 'clang-gpu' when there is a choice.
|
||||
"""
|
||||
with clangdir.as_cwd():
|
||||
shutil.copy('clang', 'clang-gpu')
|
||||
shutil.copy('clang++', 'clang++-gpu')
|
||||
os.chmod('clang-gpu', 0o700)
|
||||
os.chmod('clang++-gpu', 0o700)
|
||||
|
||||
os.environ['PATH'] = str(clangdir)
|
||||
output = compiler('find', '--scope=site')
|
||||
|
||||
assert 'clang@11.0.0' in output
|
||||
assert 'gcc@8.4.0' in output
|
||||
|
||||
config = spack.compilers.get_compiler_config('site', False)
|
||||
clang = next(c['compiler'] for c in config
|
||||
if c['compiler']['spec'] == 'clang@11.0.0')
|
||||
|
||||
assert clang['paths']['cc'] == str(clangdir.join('clang'))
|
||||
assert clang['paths']['cxx'] == str(clangdir.join('clang++'))
|
||||
|
|
|
|||
|
|
@ -6,7 +6,10 @@
|
|||
import sys
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
import llnl.util.filesystem as fs
|
||||
|
||||
import spack
|
||||
import spack.util.executable as ex
|
||||
from spack.hooks.sbang import filter_shebangs_in_directory
|
||||
|
|
@ -35,3 +38,18 @@ def test_read_unicode(tmpdir, working_env):
|
|||
# read the unicode back in and see whether things work
|
||||
script = ex.Executable('./%s' % script_name)
|
||||
assert u'\xc3' == script(output=str).strip()
|
||||
|
||||
|
||||
def test_which(tmpdir):
|
||||
os.environ["PATH"] = str(tmpdir)
|
||||
assert ex.which("spack-test-exe") is None
|
||||
with pytest.raises(ex.CommandNotFoundError):
|
||||
ex.which("spack-test-exe", required=True)
|
||||
|
||||
with tmpdir.as_cwd():
|
||||
fs.touch("spack-test-exe")
|
||||
fs.set_executable('spack-test-exe')
|
||||
|
||||
exe = ex.which("spack-test-exe")
|
||||
assert exe is not None
|
||||
assert exe.path == str(tmpdir.join("spack-test-exe"))
|
||||
|
|
|
|||
|
|
@ -239,7 +239,8 @@ def which_string(*args, **kwargs):
|
|||
return exe
|
||||
|
||||
if required:
|
||||
tty.die("spack requires '%s'. Make sure it is in your path." % args[0])
|
||||
raise CommandNotFoundError(
|
||||
"spack requires '%s'. Make sure it is in your path." % args[0])
|
||||
|
||||
return None
|
||||
|
||||
|
|
@ -266,3 +267,7 @@ def which(*args, **kwargs):
|
|||
|
||||
class ProcessError(spack.error.SpackError):
|
||||
"""ProcessErrors are raised when Executables exit with an error code."""
|
||||
|
||||
|
||||
class CommandNotFoundError(spack.error.SpackError):
|
||||
"""Raised when ``which()`` can't find a required executable."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue