Module cmd fix (#3250)

* Parse modules in a way that works for both lmod and tcl

* added test and made method more robust

* refactoring for pythonic clarity

* Improved detection of 'module' shell function + refactored module utilities into spack.util.module_cmd

* Improved regex to reject nested parentheses we are not prepared to handle

* make tests backwards compatible with python 2.6

* Improved regex to account for sh being aliased to bash and used in bash module definition on some systems

* Improve test compatibility with lmod

* Added error for None module_cmd

* Add test for get_module_cmd_from_which()

Add test for get_module_cmd_from_which().
Add -c argument to Popen call to typeset -f module in get_module_cmd_from_bash().

* Increased detection options

Included BASH_FUNC_module() variable outside of typeset as a detection option
This should work on bash even in restricted_shell mode
Kept the typeset detection as an option in case the module function is not exported in bash

Also added try statements to tests, with environment recreation in finally blocks.

* More tests added; some hackiness

* increased test coverage for util/module_cmd

7 files changed, 346 insertions, 70 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index e216d4aa7c..ac44e57c09 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -67,8 +67,8 @@ import spack
 import spack.store
 from spack.environment import EnvironmentModifications, validate
 from spack.util.environment import *
-from spack.util.executable import Executable, which
-
+from spack.util.executable import Executable
+from spack.util.module_cmd import load_module, get_path_from_module
 #
 # This can be set by the user to globally disable parallel builds.
 #
@@ -120,67 +120,6 @@ class MakeExecutable(Executable):
         return super(MakeExecutable, self).__call__(*args, **kwargs)
 
 
-def load_module(mod):
-    """Takes a module name and removes modules until it is possible to
-    load that module. It then loads the provided module. Depends on the
-    modulecmd implementation of modules used in cray and lmod.
-    """
-    # Create an executable of the module command that will output python code
-    modulecmd = which('modulecmd')
-    modulecmd.add_default_arg('python')
-
-    # Read the module and remove any conflicting modules
-    # We do this without checking that they are already installed
-    # for ease of programming because unloading a module that is not
-    # loaded does nothing.
-    text = modulecmd('show', mod, output=str, error=str).split()
-    for i, word in enumerate(text):
-        if word == 'conflict':
-            exec(compile(modulecmd('unload', text[i + 1], output=str,
-                                   error=str), '<string>', 'exec'))
-    # Load the module now that there are no conflicts
-    load = modulecmd('load', mod, output=str, error=str)
-    exec(compile(load, '<string>', 'exec'))
-
-
-def get_path_from_module(mod):
-    """Inspects a TCL module for entries that indicate the absolute path
-    at which the library supported by said module can be found.
-    """
-    # Create a modulecmd executable
-    modulecmd = which('modulecmd')
-    modulecmd.add_default_arg('python')
-
-    # Read the module
-    text = modulecmd('show', mod, output=str, error=str).split('\n')
-    # If it lists its package directory, return that
-    for line in text:
-        if line.find(mod.upper() + '_DIR') >= 0:
-            words = line.split()
-            return words[2]
-
-    # 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')]
-
-    # If it lists a -L instruction, use that
-    for line in text:
-        L = line.find('-L/')
-        if L >= 0:
-            return line[L + 2:line.find('/lib')]
-
-    # 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:
-            words = line.split()
-            path = words[2]
-            return path[:path.find('/lib')]
-    # Unable to find module path
-    return None
-
-
 def set_compiler_environment_variables(pkg, env):
     assert(pkg.spec.concrete)
     compiler = pkg.compiler
diff --git a/lib/spack/spack/operating_systems/cnl.py b/lib/spack/spack/operating_systems/cnl.py
index b5c759bbcb..95f23a076e 100644
--- a/lib/spack/spack/operating_systems/cnl.py
+++ b/lib/spack/spack/operating_systems/cnl.py
@@ -25,10 +25,10 @@
 import re
 
 from spack.architecture import OperatingSystem
-from spack.util.executable import *
 import spack.spec
 from spack.util.multiproc import parmap
 import spack.compilers
+from spack.util.module_cmd import get_module_cmd
 
 
 class Cnl(OperatingSystem):
@@ -63,8 +63,7 @@ class Cnl(OperatingSystem):
             if not cmp_cls.PrgEnv_compiler:
                 tty.die('Must supply PrgEnv_compiler with PrgEnv')
 
-            modulecmd = which('modulecmd')
-            modulecmd.add_default_arg('python')
+            modulecmd = get_module_cmd()
 
             output = modulecmd(
                 'avail', cmp_cls.PrgEnv_compiler, output=str, error=str)
diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py
index 8b1fe08154..f9c4ced97e 100644
--- a/lib/spack/spack/package_prefs.py
+++ b/lib/spack/spack/package_prefs.py
@@ -200,7 +200,7 @@ def spec_externals(spec):
     """Return a list of external specs (w/external directory path filled in),
        one for each known external installation."""
     # break circular import.
-    from spack.build_environment import get_path_from_module  # noqa: F401
+    from spack.util.module_cmd import get_path_from_module # NOQA: ignore=F401
 
     allpkgs = get_packages_config()
     name = spec.name
diff --git a/lib/spack/spack/platforms/cray.py b/lib/spack/spack/platforms/cray.py
index 1cd08e5565..76f7e59674 100644
--- a/lib/spack/spack/platforms/cray.py
+++ b/lib/spack/spack/platforms/cray.py
@@ -31,6 +31,7 @@ from spack.architecture import Platform, Target, NoPlatformError
 from spack.operating_systems.linux_distro import LinuxDistro
 from spack.operating_systems.cnl import Cnl
 from llnl.util.filesystem import join_path
+from spack.util.module_cmd import get_module_cmd
 
 
 def _get_modules_in_modulecmd_output(output):
@@ -142,8 +143,7 @@ class Cray(Platform):
     def _avail_targets(self):
         '''Return a list of available CrayPE CPU targets.'''
         if getattr(self, '_craype_targets', None) is None:
-            module = which('modulecmd', required=True)
-            module.add_default_arg('python')
+            module = get_module_cmd()
             output = module('avail', '-t', 'craype-', output=str, error=str)
             craype_modules = _get_modules_in_modulecmd_output(output)
             self._craype_targets = targets = []
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index eed93eccb7..602f2fd878 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -120,7 +120,7 @@ import spack.util.spack_yaml as syaml
 from llnl.util.filesystem import find_headers, find_libraries, is_exe
 from llnl.util.lang import *
 from llnl.util.tty.color import *
-from spack.build_environment import get_path_from_module, load_module
+from spack.util.module_cmd import get_path_from_module, load_module
 from spack.error import SpecError, UnsatisfiableSpecError
 from spack.provider_index import ProviderIndex
 from spack.util.crypto import prefix_bits
diff --git a/lib/spack/spack/test/module_parsing.py b/lib/spack/spack/test/module_parsing.py
new file mode 100644
index 0000000000..6ddb9d2dbf
--- /dev/null
+++ b/lib/spack/spack/test/module_parsing.py
@@ -0,0 +1,143 @@
+##############################################################################
+# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/llnl/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+import pytest
+import subprocess
+import os
+from spack.util.module_cmd import *
+
+typeset_func = subprocess.Popen('module avail',
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE,
+                                shell=True)
+typeset_func.wait()
+typeset = typeset_func.stderr.read()
+MODULE_NOT_DEFINED = b'not found' in typeset
+
+
+@pytest.fixture
+def save_env():
+    old_PATH = os.environ.get('PATH', None)
+    old_bash_func = os.environ.get('BASH_FUNC_module()', None)
+
+    yield
+
+    if old_PATH:
+        os.environ['PATH'] = old_PATH
+    if old_bash_func:
+        os.environ['BASH_FUNC_module()'] = old_bash_func
+
+
+def test_get_path_from_module(save_env):
+    lines = ['prepend-path LD_LIBRARY_PATH /path/to/lib',
+             'setenv MOD_DIR /path/to',
+             'setenv LDFLAGS -Wl,-rpath/path/to/lib',
+             'setenv LDFLAGS -L/path/to/lib']
+
+    for line in lines:
+        module_func = '() { eval `echo ' + line + ' bash filler`\n}'
+        os.environ['BASH_FUNC_module()'] = module_func
+        path = get_path_from_module('mod')
+
+        assert path == '/path/to'
+
+    os.environ['BASH_FUNC_module()'] = '() { eval $(echo fill bash $*)\n}'
+    path = get_path_from_module('mod')
+
+    assert path is None
+
+
+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')"]
+
+    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)
+    for bl in bad_lines:
+        with pytest.raises(ValueError):
+            get_argument_from_module_line(bl)
+
+
+@pytest.mark.skipif(MODULE_NOT_DEFINED, reason='Depends on defined module fn')
+def test_get_module_cmd_from_bash_using_modules():
+    module_list_proc = subprocess.Popen(['module list'],
+                                        stdout=subprocess.PIPE,
+                                        stderr=subprocess.STDOUT,
+                                        executable='/bin/bash',
+                                        shell=True)
+    module_list_proc.wait()
+    module_list = module_list_proc.stdout.read()
+
+    module_cmd = get_module_cmd_from_bash()
+    module_cmd_list = module_cmd('list', output=str, error=str)
+
+    # Lmod command reprints some env variables on every invocation.
+    # Test containment to avoid false failures on lmod systems.
+    assert module_list in module_cmd_list
+
+
+def test_get_module_cmd_from_bash_ticks(save_env):
+    os.environ['BASH_FUNC_module()'] = '() { eval `echo bash $*`\n}'
+
+    module_cmd = get_module_cmd()
+    module_cmd_list = module_cmd('list', output=str, error=str)
+
+    assert module_cmd_list == 'python list\n'
+
+
+def test_get_module_cmd_from_bash_parens(save_env):
+    os.environ['BASH_FUNC_module()'] = '() { eval $(echo fill bash $*)\n}'
+
+    module_cmd = get_module_cmd()
+    module_cmd_list = module_cmd('list', output=str, error=str)
+
+    assert module_cmd_list == 'fill python list\n'
+
+
+def test_get_module_cmd_fails(save_env):
+    os.environ.pop('BASH_FUNC_module()')
+    os.environ.pop('PATH')
+    with pytest.raises(ModuleError):
+        module_cmd = get_module_cmd(b'--norc')
+        module_cmd()  # Here to avoid Flake F841 on previous line
+
+
+def test_get_module_cmd_from_which(tmpdir, save_env):
+    f = tmpdir.mkdir('bin').join('modulecmd')
+    f.write('#!/bin/bash\n'
+            'echo $*')
+    f.chmod(0o770)
+
+    os.environ['PATH'] = str(tmpdir.join('bin')) + ':' + os.environ['PATH']
+    os.environ.pop('BASH_FUNC_module()')
+
+    module_cmd = get_module_cmd(b'--norc')
+    module_cmd_list = module_cmd('list', output=str, error=str)
+
+    assert module_cmd_list == 'python list\n'
diff --git a/lib/spack/spack/util/module_cmd.py b/lib/spack/spack/util/module_cmd.py
new file mode 100644
index 0000000000..bdd8463757
--- /dev/null
+++ b/lib/spack/spack/util/module_cmd.py
@@ -0,0 +1,195 @@
+##############################################################################
+# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/llnl/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+"""
+This module contains routines related to the module command for accessing and
+parsing environment modules.
+"""
+import subprocess
+import re
+import os
+import llnl.util.tty as tty
+from spack.util.executable import which
+
+
+def get_module_cmd(bashopts=''):
+    try:
+        return get_module_cmd_from_bash(bashopts)
+    except ModuleError:
+        # Don't catch the exception this time; we have no other way to do it.
+        tty.warn("Could not detect module function from bash."
+                 " Trying to detect modulecmd from `which`")
+        try:
+            return get_module_cmd_from_which()
+        except ModuleError:
+            raise ModuleError('Spack requires modulecmd or a defined module'
+                              ' fucntion. Make sure modulecmd is in your path'
+                              ' or the function "module" is defined in your'
+                              ' bash environment.')
+
+
+def get_module_cmd_from_which():
+    module_cmd = which('modulecmd')
+    if not module_cmd:
+        raise ModuleError('`which` did not find any modulecmd executable')
+    module_cmd.add_default_arg('python')
+
+    # Check that the executable works
+    module_cmd('list', output=str, error=str, fail_on_error=False)
+    if module_cmd.returncode != 0:
+        raise ModuleError('get_module_cmd cannot determine the module command')
+
+    return module_cmd
+
+
+def get_module_cmd_from_bash(bashopts=''):
+    # Find how the module function is defined in the environment
+    module_func = os.environ.get('BASH_FUNC_module()', None)
+    if module_func:
+        module_func = os.path.expandvars(module_func)
+    else:
+        module_func_proc = subprocess.Popen(['{0} typeset -f module | '
+                                             'envsubst'.format(bashopts)],
+                                            stdout=subprocess.PIPE,
+                                            stderr=subprocess.STDOUT,
+                                            executable='/bin/bash',
+                                            shell=True)
+        module_func_proc.wait()
+        module_func = module_func_proc.stdout.read()
+
+    # Find the portion of the module function that is evaluated
+    try:
+        find_exec = re.search(r'.*`(.*(:? bash | sh ).*)`.*', module_func)
+        exec_line = find_exec.group(1)
+    except:
+        try:
+            # This will fail with nested parentheses. TODO: expand regex.
+            find_exec = re.search(r'.*\(([^()]*(:? bash | sh )[^()]*)\).*',
+                                  module_func)
+            exec_line = find_exec.group(1)
+        except:
+            raise ModuleError('get_module_cmd cannot '
+                              'determine the module command from bash')
+
+    # Create an executable
+    args = exec_line.split()
+    module_cmd = which(args[0])
+    if module_cmd:
+        for arg in args[1:]:
+            if arg == 'bash':
+                module_cmd.add_default_arg('python')
+                break
+            else:
+                module_cmd.add_default_arg(arg)
+    else:
+        raise ModuleError('Could not create executable based on module'
+                          ' function.')
+
+    # Check that the executable works
+    module_cmd('list', output=str, error=str, fail_on_error=False)
+    if module_cmd.returncode != 0:
+        raise ModuleError('get_module_cmd cannot determine the module command'
+                          'from bash.')
+
+    return module_cmd
+
+
+def load_module(mod):
+    """Takes a module name and removes modules until it is possible to
+    load that module. It then loads the provided module. Depends on the
+    modulecmd implementation of modules used in cray and lmod.
+    """
+    # Create an executable of the module command that will output python code
+    modulecmd = get_module_cmd()
+
+    # Read the module and remove any conflicting modules
+    # We do this without checking that they are already installed
+    # for ease of programming because unloading a module that is not
+    # loaded does nothing.
+    text = modulecmd('show', mod, output=str, error=str).split()
+    for i, word in enumerate(text):
+        if word == 'conflict':
+            exec(compile(modulecmd('unload', text[i + 1], output=str,
+                                   error=str), '<string>', 'exec'))
+    # Load the module now that there are no conflicts
+    load = modulecmd('load', mod, output=str, error=str)
+    exec(compile(load, '<string>', 'exec'))
+
+
+def get_argument_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(',')
+        cline = line[comma_index:]
+        try:
+            quote_index = min(cline.find(q) for q in ['"', "'"] if q in cline)
+            lua_quote = cline[quote_index]
+        except ValueError:
+            # 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]
+    else:
+        return line.split()[2]
+
+
+def get_path_from_module(mod):
+    """Inspects a TCL module for entries that indicate the absolute path
+    at which the library supported by said module can be found.
+    """
+    # Create a modulecmd executable
+    modulecmd = get_module_cmd()
+
+    # Read the module
+    text = modulecmd('show', mod, output=str, error=str).split('\n')
+
+    # 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)
+            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)
+
+    # 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')]
+
+    # If it lists a -L instruction, use that
+    for line in text:
+        L = line.find('-L/')
+        if L >= 0:
+            return line[L + 2:line.find('/lib')]
+
+    # Unable to find module path
+    return None
+
+
+class ModuleError(Exception):
+    """Raised the the module_cmd utility to indicate errors."""