From b1868f35ec915f8538b77a2528594619fc53f302 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Wed, 11 Sep 2019 10:51:44 -0700 Subject: [PATCH] module parsing: make heuristic to get paths from modules more robust (#12693) * module parsing: make heuristic to get paths from modules more robust * refactor module parsing --- lib/spack/spack/test/module_parsing.py | 36 ++++++++--- lib/spack/spack/util/module_cmd.py | 87 ++++++++++++++++++-------- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/lib/spack/spack/test/module_parsing.py b/lib/spack/spack/test/module_parsing.py index 0594724eaf..95a65ab33f 100644 --- a/lib/spack/spack/test/module_parsing.py +++ b/lib/spack/spack/test/module_parsing.py @@ -10,7 +10,7 @@ from spack.util.module_cmd import ( module, get_path_from_module, - get_path_arg_from_module_line, + get_path_args_from_module_line, get_path_from_module_contents ) @@ -89,14 +89,23 @@ def test_get_path_from_module_contents(): whatis("Version: 3.9.2") whatis("Category: Tools") whatis("URL: https://cmake.org/") -prepend_path("PATH","/path/to/cmake-3.9.2/bin") +prepend_path("LD_LIBRARY_PATH","/bad/path") +prepend_path("PATH","/path/to/cmake-3.9.2/bin:/other/bad/path") prepend_path("MANPATH","/path/to/cmake/cmake-3.9.2/share/man") +prepend_path("LD_LIBRARY_PATH","/path/to/cmake-3.9.2/lib64") """ module_show_lines = module_show_output.split('\n') + + # PATH and LD_LIBRARY_PATH outvote MANPATH and the other PATH and + # LD_LIBRARY_PATH entries assert (get_path_from_module_contents(module_show_lines, 'cmake-3.9.2') == '/path/to/cmake-3.9.2') +def test_get_path_from_empty_module(): + assert get_path_from_module_contents('', 'test') is None + + def test_pkg_dir_from_module_name(): module_show_lines = ['setenv FOO_BAR_DIR /path/to/foo-bar'] @@ -108,16 +117,25 @@ def test_pkg_dir_from_module_name(): def test_get_argument_from_module_line(): - lines = ['prepend-path LD_LIBRARY_PATH /lib/path', - 'prepend-path LD_LIBRARY_PATH /lib/path', - "prepend_path('PATH' , '/lib/path')", - 'prepend_path( "PATH" , "/lib/path" )', - 'prepend_path("PATH",' + "'/lib/path')"] + simple_lines = ['prepend-path LD_LIBRARY_PATH /lib/path', + 'prepend-path LD_LIBRARY_PATH /lib/path', + "prepend_path('PATH' , '/lib/path')", + 'prepend_path( "PATH" , "/lib/path" )', + 'prepend_path("PATH",' + "'/lib/path')"] + + complex_lines = ['prepend-path LD_LIBRARY_PATH /lib/path:/pkg/path', + 'prepend-path LD_LIBRARY_PATH /lib/path:/pkg/path', + "prepend_path('PATH' , '/lib/path:/pkg/path')", + 'prepend_path( "PATH" , "/lib/path:/pkg/path" )', + 'prepend_path("PATH",' + "'/lib/path:/pkg/path')"] bad_lines = ['prepend_path(PATH,/lib/path)', 'prepend-path (LD_LIBRARY_PATH) /lib/path'] - assert all(get_path_arg_from_module_line(l) == '/lib/path' for l in lines) + assert all(get_path_args_from_module_line(l) == ['/lib/path'] + for l in simple_lines) + assert all(get_path_args_from_module_line(l) == ['/lib/path', '/pkg/path'] + for l in complex_lines) for bl in bad_lines: with pytest.raises(ValueError): - get_path_arg_from_module_line(bl) + get_path_args_from_module_line(bl) diff --git a/lib/spack/spack/util/module_cmd.py b/lib/spack/spack/util/module_cmd.py index 581fae5540..75f61a6466 100644 --- a/lib/spack/spack/util/module_cmd.py +++ b/lib/spack/spack/util/module_cmd.py @@ -77,7 +77,7 @@ def load_module(mod): module('load', mod) -def get_path_arg_from_module_line(line): +def get_path_args_from_module_line(line): if '(' in line and ')' in line: # Determine which lua quote symbol is being used for the argument comma_index = line.index(',') @@ -92,7 +92,9 @@ def get_path_arg_from_module_line(line): path_arg = words_and_symbols[-2] else: path_arg = line.split()[2] - return path_arg + + paths = path_arg.split(':') + return paths def get_path_from_module(mod): @@ -119,37 +121,68 @@ def get_path_from_module_contents(text, module_name): pkg_var_prefix = components[-2] tty.debug("Package directory variable prefix: " + pkg_var_prefix) - # If it sets the LD_LIBRARY_PATH or CRAY_LD_LIBRARY_PATH, use that + path_occurrences = {} + + def strip_path(path, endings): + for ending in endings: + if path.endswith(ending): + return path[:-len(ending)] + if path.endswith(ending + '/'): + return path[:-(len(ending) + 1)] + return path + + def match_pattern_and_strip(line, pattern, strip=[]): + if re.search(pattern, line): + paths = get_path_args_from_module_line(line) + for path in paths: + path = strip_path(path, strip) + path_occurrences[path] = path_occurrences.get(path, 0) + 1 + + def match_flag_and_strip(line, flag, strip=[]): + flag_idx = line.find(flag) + if flag_idx >= 0: + end = line.find(' ', flag_idx) + if end >= 0: + path = line[flag_idx + len(flag):end] + else: + path = line[flag_idx + len(flag):] + path = strip_path(path, strip) + path_occurrences[path] = path_occurrences.get(path, 0) + 1 + + lib_endings = ['/lib64', '/lib'] + bin_endings = ['/bin'] + man_endings = ['/share/man', '/man'] + for line in text: + # Check entries of LD_LIBRARY_PATH and CRAY_LD_LIBRARY_PATH pattern = r'\W(CRAY_)?LD_LIBRARY_PATH' - if re.search(pattern, line): - path = get_path_arg_from_module_line(line) - return path[:path.find('/lib')] + match_pattern_and_strip(line, pattern, lib_endings) - # If it lists its package directory, return that - for line in text: + # Check {name}_DIR entries pattern = r'\W{0}_DIR'.format(pkg_var_prefix) - if re.search(pattern, line): - return get_path_arg_from_module_line(line) + match_pattern_and_strip(line, pattern) - # If it lists a -rpath instruction, use that - for line in text: - rpath = line.find('-rpath/') - if rpath >= 0: - return line[rpath + 6:line.find('/lib')] + # Check {name}_ROOT entries + pattern = r'\W{0}_ROOT'.format(pkg_var_prefix) + match_pattern_and_strip(line, pattern) - # If it lists a -L instruction, use that - for line in text: - lib_paths = line.find('-L/') - if lib_paths >= 0: - return line[lib_paths + 2:line.find('/lib')] - - # If it sets the PATH, use it - for line in text: + # Check entries that update the PATH variable pattern = r'\WPATH' - if re.search(pattern, line): - path = get_path_arg_from_module_line(line) - return path[:path.find('/bin')] + match_pattern_and_strip(line, pattern, bin_endings) - # Unable to find module path + # Check entries that update the MANPATH variable + pattern = r'MANPATH' + match_pattern_and_strip(line, pattern, man_endings) + + # Check entries that add a `-rpath` flag to a variable + match_flag_and_strip(line, '-rpath', lib_endings) + + # Check entries that add a `-L` flag to a variable + match_flag_and_strip(line, '-L', lib_endings) + + # Whichever path appeared most in the module, we assume is the correct path + if len(path_occurrences) > 0: + return max(path_occurrences.items(), key=lambda x: x[1])[0] + + # Unable to find path in module return None