From 22fbb3dba70283d9fdfe8e4e285815e4fafed7d8 Mon Sep 17 00:00:00 2001 From: scheibelp Date: Wed, 12 Sep 2018 18:15:31 -0700 Subject: Bug fix: module file path parsing (#9100) Improve Spack's parsing of module show to eliminate some false positives (e.g. accepting MODULEPATH when it is in fact looking for PATH). This makes the following changes: * Updates the pattern searching for several paths to avoid the case where they are prefixes of unwanted paths * Adds a warning message when an extracted path doesn't exist (which may help catch future module parsing bugs faster) * Adds a test with the content mentioned in #9083 --- lib/spack/spack/test/module_parsing.py | 35 ++++++++++++++++++++++++++++------ lib/spack/spack/util/module_cmd.py | 31 +++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/lib/spack/spack/test/module_parsing.py b/lib/spack/spack/test/module_parsing.py index 59ea6402d1..f798c272b8 100644 --- a/lib/spack/spack/test/module_parsing.py +++ b/lib/spack/spack/test/module_parsing.py @@ -25,10 +25,13 @@ import pytest import subprocess import os -from spack.util.module_cmd import get_path_from_module -from spack.util.module_cmd import get_argument_from_module_line -from spack.util.module_cmd import get_module_cmd_from_bash -from spack.util.module_cmd import get_module_cmd, ModuleError +from spack.util.module_cmd import ( + get_path_from_module, + get_path_from_module_contents, + get_path_arg_from_module_line, + get_module_cmd_from_bash, + get_module_cmd, + ModuleError) typeset_func = subprocess.Popen('module avail', @@ -73,6 +76,26 @@ def test_get_path_from_module(save_env): assert path is None +def test_get_path_from_module_contents(): + module_show_output = """ +os.environ["MODULEPATH"] = "/path/to/modules1:/path/to/modules2"; +---------------------------------------------------------------------------- + /root/cmake/3.9.2.lua: +---------------------------------------------------------------------------- +help([[CMake Version 3.9.2 +]]) +whatis("Name: CMake") +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("MANPATH","/path/to/cmake/cmake-3.9.2/share/man") +""" + module_show_lines = module_show_output.split('\n') + assert (get_path_from_module_contents(module_show_lines, 'cmake-3.9.2') == + '/path/to/cmake-3.9.2') + + def test_get_argument_from_module_line(): lines = ['prepend-path LD_LIBRARY_PATH /lib/path', 'prepend-path LD_LIBRARY_PATH /lib/path', @@ -83,10 +106,10 @@ def test_get_argument_from_module_line(): bad_lines = ['prepend_path(PATH,/lib/path)', 'prepend-path (LD_LIBRARY_PATH) /lib/path'] - assert all(get_argument_from_module_line(l) == '/lib/path' for l in lines) + assert all(get_path_arg_from_module_line(l) == '/lib/path' for l in lines) for bl in bad_lines: with pytest.raises(ValueError): - get_argument_from_module_line(bl) + get_path_arg_from_module_line(bl) @pytest.mark.skipif(MODULE_NOT_DEFINED, reason='Depends on defined module fn') diff --git a/lib/spack/spack/util/module_cmd.py b/lib/spack/spack/util/module_cmd.py index d8a9ce27d6..d3e258792a 100644 --- a/lib/spack/spack/util/module_cmd.py +++ b/lib/spack/spack/util/module_cmd.py @@ -148,7 +148,7 @@ def load_module(mod): exec(compile(load, '', 'exec')) -def get_argument_from_module_line(line): +def get_path_arg_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(',') @@ -160,9 +160,15 @@ def get_argument_from_module_line(line): # Change error text to describe what is going on. raise ValueError("No lua quote symbol found in lmod module line.") words_and_symbols = line.split(lua_quote) - return words_and_symbols[-2] + path_arg = words_and_symbols[-2] else: - return line.split()[2] + path_arg = line.split()[2] + + if not os.path.exists(path_arg): + tty.warn("Extracted path from module does not exist:" + "\n\tExtracted path: " + path_arg + + "\n\tFull line: " + line) + return path_arg def get_path_from_module(mod): @@ -175,16 +181,22 @@ def get_path_from_module(mod): # Read the module text = modulecmd('show', mod, output=str, error=str).split('\n') + return get_path_from_module_contents(text, mod) + + +def get_path_from_module_contents(text, module_name): # If it sets the LD_LIBRARY_PATH or CRAY_LD_LIBRARY_PATH, use that for line in text: - if line.find('LD_LIBRARY_PATH') >= 0: - path = get_argument_from_module_line(line) + pattern = r'\WLD_LIBRARY_PATH' + if re.search(pattern, line): + path = get_path_arg_from_module_line(line) return path[:path.find('/lib')] # If it lists its package directory, return that for line in text: - if line.find(mod.upper() + '_DIR') >= 0: - return get_argument_from_module_line(line) + pattern = r'\W{0}_DIR'.format(module_name.upper()) + if re.search(pattern, line): + return get_path_arg_from_module_line(line) # If it lists a -rpath instruction, use that for line in text: @@ -200,8 +212,9 @@ def get_path_from_module(mod): # If it sets the PATH, use it for line in text: - if line.find('PATH') >= 0: - path = get_argument_from_module_line(line) + pattern = r'\WPATH' + if re.search(pattern, line): + path = get_path_arg_from_module_line(line) return path[:path.find('/bin')] # Unable to find module path -- cgit v1.2.3-60-g2f50