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

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, '<string>', '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