From 38ac78489aa688327412c81ecc9799576ba98e7e Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Thu, 28 May 2020 17:11:28 -0700 Subject: bugfix: use flags when computing implicit rpaths (#16634) * make verbose_flag a property * tests --- lib/spack/spack/compiler.py | 27 +++++++++---- lib/spack/spack/compilers/arm.py | 4 +- lib/spack/spack/compilers/cce.py | 4 +- lib/spack/spack/compilers/clang.py | 4 +- lib/spack/spack/compilers/fj.py | 4 +- lib/spack/spack/compilers/gcc.py | 4 +- lib/spack/spack/compilers/intel.py | 4 +- lib/spack/spack/compilers/pgi.py | 4 +- lib/spack/spack/compilers/xl.py | 4 +- lib/spack/spack/test/compilers.py | 79 +++++++++++++++++++++++++++++++++++++- 10 files changed, 113 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py index 94dbb190d2..5aa156dfc9 100644 --- a/lib/spack/spack/compiler.py +++ b/lib/spack/spack/compiler.py @@ -297,8 +297,10 @@ class Compiler(object): if self.enable_implicit_rpaths is False: return [] + # Put CXX first since it has the most linking issues + # And because it has flags that affect linking exe_paths = [ - x for x in [self.cc, self.cxx, self.fc, self.f77] if x] + x for x in [self.cxx, self.cc, self.fc, self.f77] if x] link_dirs = self._get_compiler_link_paths(exe_paths) all_required_libs = ( @@ -313,16 +315,24 @@ class Compiler(object): # By default every compiler returns the empty list return [] - @classmethod - def _get_compiler_link_paths(cls, paths): + def _get_compiler_link_paths(self, paths): first_compiler = next((c for c in paths if c), None) if not first_compiler: return [] - if not cls.verbose_flag(): + if not self.verbose_flag: # In this case there is no mechanism to learn what link directories # are used by the compiler return [] + # What flag types apply to first_compiler, in what order + flags = ['cppflags', 'ldflags'] + if first_compiler == self.cc: + flags = ['cflags'] + flags + elif first_compiler == self.cxx: + flags = ['cxxflags'] + flags + else: + flags.append('fflags') + try: tmpdir = tempfile.mkdtemp(prefix='spack-implicit-link-info') fout = os.path.join(tmpdir, 'output') @@ -333,7 +343,10 @@ class Compiler(object): 'int main(int argc, char* argv[]) { ' '(void)argc; (void)argv; return 0; }\n') compiler_exe = spack.util.executable.Executable(first_compiler) - output = str(compiler_exe(cls.verbose_flag(), fin, '-o', fout, + for flag_type in flags: + for flag in self.flags.get(flag_type, []): + compiler_exe.add_default_arg(flag) + output = str(compiler_exe(self.verbose_flag, fin, '-o', fout, output=str, error=str)) # str for py2 return _parse_non_system_link_dirs(output) @@ -344,8 +357,8 @@ class Compiler(object): finally: shutil.rmtree(tmpdir, ignore_errors=True) - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): """ This property should be overridden in the compiler subclass if a verbose flag is available. diff --git a/lib/spack/spack/compilers/arm.py b/lib/spack/spack/compilers/arm.py index 2f6e195006..90514b4df8 100644 --- a/lib/spack/spack/compilers/arm.py +++ b/lib/spack/spack/compilers/arm.py @@ -51,8 +51,8 @@ class Arm(spack.compiler.Compiler): temp = match.group(1) + "." + match.group(2) return temp - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/cce.py b/lib/spack/spack/compilers/cce.py index 0d30a69d3e..55fbe72556 100644 --- a/lib/spack/spack/compilers/cce.py +++ b/lib/spack/spack/compilers/cce.py @@ -40,8 +40,8 @@ class Cce(Compiler): version_regex = r'[Vv]ersion.*?(\d+(\.\d+)+)' - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py index b14eaa1278..9aa713e1a5 100644 --- a/lib/spack/spack/compilers/clang.py +++ b/lib/spack/spack/compilers/clang.py @@ -81,8 +81,8 @@ class Clang(Compiler): ver_string = str(self.version) return ver_string.endswith('-apple') - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/fj.py b/lib/spack/spack/compilers/fj.py index 54d9308c4a..255fa35672 100644 --- a/lib/spack/spack/compilers/fj.py +++ b/lib/spack/spack/compilers/fj.py @@ -30,8 +30,8 @@ class Fj(spack.compiler.Compiler): required_libs = ['libfj90i', 'libfj90f', 'libfjsrcinfo'] - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/gcc.py b/lib/spack/spack/compilers/gcc.py index fc9074a67a..5fd4677b3b 100644 --- a/lib/spack/spack/compilers/gcc.py +++ b/lib/spack/spack/compilers/gcc.py @@ -38,8 +38,8 @@ class Gcc(Compiler): PrgEnv = 'PrgEnv-gnu' PrgEnv_compiler = 'gcc' - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/intel.py b/lib/spack/spack/compilers/intel.py index c7d853e3fa..6d0aa09b87 100644 --- a/lib/spack/spack/compilers/intel.py +++ b/lib/spack/spack/compilers/intel.py @@ -32,8 +32,8 @@ class Intel(Compiler): version_argument = '--version' version_regex = r'\((?:IFORT|ICC)\) ([^ ]+)' - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" required_libs = ['libirc', 'libifcore', 'libifcoremt', 'libirng'] diff --git a/lib/spack/spack/compilers/pgi.py b/lib/spack/spack/compilers/pgi.py index 13d8f69ec5..ab866f1f2c 100644 --- a/lib/spack/spack/compilers/pgi.py +++ b/lib/spack/spack/compilers/pgi.py @@ -33,8 +33,8 @@ class Pgi(Compiler): ignore_version_errors = [2] # `pgcc -V` on PowerPC annoyingly returns 2 version_regex = r'pg[^ ]* ([0-9.]+)-[0-9]+ (LLVM )?[^ ]+ target on ' - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-v" @property diff --git a/lib/spack/spack/compilers/xl.py b/lib/spack/spack/compilers/xl.py index 46a5002e25..af84d5edb9 100644 --- a/lib/spack/spack/compilers/xl.py +++ b/lib/spack/spack/compilers/xl.py @@ -29,8 +29,8 @@ class Xl(Compiler): version_argument = '-qversion' version_regex = r'([0-9]?[0-9]\.[0-9])' - @classmethod - def verbose_flag(cls): + @property + def verbose_flag(self): return "-V" @property diff --git a/lib/spack/spack/test/compilers.py b/lib/spack/spack/test/compilers.py index 586bb215cf..b38f19dcbc 100644 --- a/lib/spack/spack/test/compilers.py +++ b/lib/spack/spack/test/compilers.py @@ -145,8 +145,8 @@ default_compiler_entry = { 'paths': { 'cc': 'cc-path', 'cxx': 'cxx-path', - 'fc': None, - 'f77': None + 'fc': 'fc-path', + 'f77': 'f77-path' }, 'flags': {}, 'modules': None @@ -165,6 +165,8 @@ class MockCompiler(Compiler): default_compiler_entry['paths']['fc'], default_compiler_entry['paths']['f77']]) + _get_compiler_link_paths = Compiler._get_compiler_link_paths + @property def name(self): return "mockcompiler" @@ -173,6 +175,12 @@ class MockCompiler(Compiler): def version(self): return "1.0.0" + _verbose_flag = "--verbose" + + @property + def verbose_flag(self): + return self._verbose_flag + required_libs = ['libgfortran'] @@ -192,6 +200,73 @@ def test_implicit_rpaths(dirs_with_libfiles, monkeypatch): assert set(retrieved_rpaths) == expected_rpaths +no_flag_dirs = ['/path/to/first/lib', '/path/to/second/lib64'] +no_flag_output = 'ld -L%s -L%s' % tuple(no_flag_dirs) + +flag_dirs = ['/path/to/first/with/flag/lib', '/path/to/second/lib64'] +flag_output = 'ld -L%s -L%s' % tuple(flag_dirs) + + +def call_compiler(exe, *args, **kwargs): + # This method can replace Executable.__call__ to emulate a compiler that + # changes libraries depending on a flag. + if '--correct-flag' in exe.exe: + return flag_output + return no_flag_output + + +@pytest.mark.parametrize('exe,flagname', [ + ('cxx', ''), + ('cxx', 'cxxflags'), + ('cxx', 'cppflags'), + ('cxx', 'ldflags'), + ('cc', ''), + ('cc', 'cflags'), + ('cc', 'cppflags'), + ('fc', ''), + ('fc', 'fflags'), + ('f77', 'fflags'), + ('f77', 'cppflags'), +]) +def test_get_compiler_link_paths(monkeypatch, exe, flagname): + # create fake compiler that emits mock verbose output + compiler = MockCompiler() + monkeypatch.setattr( + spack.util.executable.Executable, '__call__', call_compiler) + + # Grab executable path to test + paths = [getattr(compiler, exe)] + + # Test without flags + dirs = compiler._get_compiler_link_paths(paths) + assert dirs == no_flag_dirs + + if flagname: + # set flags and test + setattr(compiler, 'flags', {flagname: ['--correct-flag']}) + dirs = compiler._get_compiler_link_paths(paths) + assert dirs == flag_dirs + + +def test_get_compiler_link_paths_no_path(): + compiler = MockCompiler() + compiler.cc = None + compiler.cxx = None + compiler.f77 = None + compiler.fc = None + + dirs = compiler._get_compiler_link_paths([compiler.cxx]) + assert dirs == [] + + +def test_get_compiler_link_paths_no_verbose_flag(): + compiler = MockCompiler() + compiler._verbose_flag = None + + dirs = compiler._get_compiler_link_paths([compiler.cxx]) + assert dirs == [] + + # Get the desired flag from the specified compiler spec. def flag_value(flag, spec): compiler = None -- cgit v1.2.3-60-g2f50