zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Spack command extensions: error-handling (#13635)

Browse Source 
Generally speaking, errors that are encountered when attempting to load
command extensions now terminate the running Spack instance.

* Added new exceptions `spack.cmd.PythonNameError` and
  `spack.cmd.CommandNameError`.
* New functions `spack.cmd.require_python_name(pname)` and
  `spack.cmd.require_cmd_name(cname)` check that `pname` and `cname`
  respectively meet requirements, throwing the appropriate error if not.
* `spack.cmd.get_module()` uses `require_cmd_name()` and passes through
  exceptions from module load attempts.
* `spack.cmd.get_command()` uses `require_cmd_name()` and invokes
  `get_module()` with the correct command-name form rather than the
  previous (incorrect) Python name.
* Added New exceptions `spack.extensions.CommandNotFoundError` and
  `spack.extensions.ExtensionNamingError`.
* `_extension_regexp` has a new leading underscore to indicate expected
  privacy.
* `spack.extensions.extension_name()` raises an `ExtensionNamingError`
  rather than using `tty.warn()`.
* `spack.extensions.load_command_extension()` checks command source
  existence early and bails out if missing. Also, exceptions raised by
  `load_module_from_file()` are passed through.
* `spack.extensions.get_module()` raises `CommandNotFoundError` as
  appropriate.
* Spack `main()` allows `parser.add_command()` exceptions to cause
  program end.

Tests:

* More common boilerplate has been pulled out into fixtures including
  `sys.modules` dictionary cleanup and resource-managed creation of a
  simple command extension with specified contents in the source file
  for a single named command.
* "Hello, World!" test now uses a command named `hello-world` instead of
  `hello` in order to verify correct handling of commands with hyphens.
* New tests for:
  * Missing (or misnamed) command.
  * Badly-named extension.
  * Verification that errors encountered during import of a command are
    propagated upward.

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
This commit is contained in:
Chris Green
2020-04-29 20:18:48 -05:00
committed by GitHub
parent d12e588c60
commit 9698907e74
5 changed files with 333 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
lib/spack/spack/cmd/__init__.py
View File

@@ -43,11 +43,28 @@ def python_name(cmd_name):
return cmd_name.replace("-", "_")
def require_python_name(pname):
"""Require that the provided name is a valid python name (per
python_name()). Useful for checking parameters for function
prerequisites."""
if python_name(pname) != pname:
raise PythonNameError(pname)
def cmd_name(python_name):
"""Convert module name (with ``_``) to command name (with ``-``)."""
return python_name.replace('_', '-')
def require_cmd_name(cname):
"""Require that the provided name is a valid command name (per
cmd_name()). Useful for checking parameters for function
prerequisites.
"""
if cmd_name(cname) != cname:
raise CommandNameError(cname)
#: global, cached list of all commands -- access through all_commands()
_all_commands = None
@@ -91,6 +108,7 @@ def get_module(cmd_name):
cmd_name (str): name of the command for which to get a module
(contains ``-``, not ``_``).
"""
require_cmd_name(cmd_name)
pname = python_name(cmd_name)
try:
@@ -102,8 +120,6 @@ def get_module(cmd_name):
tty.debug('Imported {0} from built-in commands'.format(pname))
except ImportError:
module = spack.extensions.get_module(cmd_name)
if not module:
raise
attr_setdefault(module, SETUP_PARSER, lambda *args: None) # null-op
attr_setdefault(module, DESCRIPTION, "")
@@ -116,14 +132,16 @@ def get_module(cmd_name):
def get_command(cmd_name):
"""Imports the command's function from a module and returns it.
"""Imports the command function associated with cmd_name.
The function's name is derived from cmd_name using python_name().
Args:
cmd_name (str): name of the command for which to get a module
(contains ``-``, not ``_``).
cmd_name (str): name of the command (contains ``-``, not ``_``).
"""
require_cmd_name(cmd_name)
pname = python_name(cmd_name)
return getattr(get_module(pname), pname)
return getattr(get_module(cmd_name), pname)
def parse_specs(args, **kwargs):
@@ -419,6 +437,22 @@ def spack_is_git_repo():
return os.path.isdir('.git')
class PythonNameError(spack.error.SpackError):
"""Exception class thrown for impermissible python names"""
def __init__(self, name):
self.name = name
super(PythonNameError, self).__init__(
'{0} is not a permissible Python name.'.format(name))
class CommandNameError(spack.error.SpackError):
"""Exception class thrown for impermissible command names"""
def __init__(self, name):
self.name = name
super(CommandNameError, self).__init__(
'{0} is not a permissible Spack command name.'.format(name))
########################################
# argparse types for argument validation
########################################

87
lib/spack/spack/extensions.py
View File

@@ -11,10 +11,17 @@
import types
import llnl.util.lang
import llnl.util.tty as tty
import spack.config
import spack.error
extension_regexp = re.compile(r'spack-([\w]*)')
_extension_regexp = re.compile(r'spack-(\w[-\w]*)$')
# TODO: For consistency we should use spack.cmd.python_name(), but
# currently this would create a circular relationship between
# spack.cmd and spack.extensions.
def _python_name(cmd_name):
return cmd_name.replace('-', '_')
def extension_name(path):
@@ -24,15 +31,16 @@ def extension_name(path):
path (str): path where the extension resides
Returns:
The extension name or None if path doesn't match the format
for Spack's extension.
The extension name.
Raises:
ExtensionNamingError: if path does not match the expected format
for a Spack command extension.
"""
regexp_match = re.search(extension_regexp, os.path.basename(path))
regexp_match = re.search(_extension_regexp,
os.path.basename(os.path.normpath(path)))
if not regexp_match:
msg = "[FOLDER NAMING]"
msg += " {0} doesn't match the format for Spack's extensions"
tty.warn(msg.format(path))
return None
raise ExtensionNamingError(path)
return regexp_match.group(1)
@@ -40,23 +48,30 @@ def load_command_extension(command, path):
"""Loads a command extension from the path passed as argument.
Args:
command (str): name of the command
command (str): name of the command (contains ``-``, not ``_``).
path (str): base path of the command extension
Returns:
A valid module object if the command is found or None
A valid module if found and loadable; None if not found. Module
loading exceptions are passed through.
"""
extension = extension_name(path)
if not extension:
return None
extension = _python_name(extension_name(path))
# Compute the name of the module we search, exit early if already imported
cmd_package = '{0}.{1}.cmd'.format(__name__, extension)
python_name = command.replace('-', '_')
python_name = _python_name(command)
module_name = '{0}.{1}'.format(cmd_package, python_name)
if module_name in sys.modules:
return sys.modules[module_name]
# Compute the absolute path of the file to be loaded, along with the
# name of the python module where it will be stored
cmd_path = os.path.join(path, extension, 'cmd', python_name + '.py')
# Short circuit if the command source file does not exist
if not os.path.exists(cmd_path):
return None
def ensure_package_creation(name):
package_name = '{0}.{1}'.format(__name__, name)
if package_name in sys.modules:
@@ -82,17 +97,10 @@ def ensure_package_creation(name):
ensure_package_creation(extension)
ensure_package_creation(extension + '.cmd')
# Compute the absolute path of the file to be loaded, along with the
# name of the python module where it will be stored
cmd_path = os.path.join(path, extension, 'cmd', command + '.py')
try:
# TODO: Upon removal of support for Python 2.6 substitute the call
# TODO: below with importlib.import_module(module_name)
module = llnl.util.lang.load_module_from_file(module_name, cmd_path)
sys.modules[module_name] = module
except (ImportError, IOError):
module = None
# TODO: Upon removal of support for Python 2.6 substitute the call
# TODO: below with importlib.import_module(module_name)
module = llnl.util.lang.load_module_from_file(module_name, cmd_path)
sys.modules[module_name] = module
return module
@@ -103,9 +111,8 @@ def get_command_paths():
extension_paths = spack.config.get('config:extensions') or []
for path in extension_paths:
extension = extension_name(path)
if extension:
command_paths.append(os.path.join(path, extension, 'cmd'))
extension = _python_name(extension_name(path))
command_paths.append(os.path.join(path, extension, 'cmd'))
return command_paths
@@ -144,7 +151,7 @@ def get_module(cmd_name):
if module:
return module
else:
return None
raise CommandNotFoundError(cmd_name)
def get_template_dirs():
@@ -154,3 +161,23 @@ def get_template_dirs():
extension_dirs = spack.config.get('config:extensions') or []
extensions = [os.path.join(x, 'templates') for x in extension_dirs]
return extensions
class CommandNotFoundError(spack.error.SpackError):
"""Exception class thrown when a requested command is not recognized as
such.
"""
def __init__(self, cmd_name):
super(CommandNotFoundError, self).__init__(
'{0} is not a recognized Spack command or extension command;'
' check with `spack commands`.'.format(cmd_name))
class ExtensionNamingError(spack.error.SpackError):
"""Exception class thrown when a configured extension does not follow
the expected naming convention.
"""
def __init__(self, path):
super(ExtensionNamingError, self).__init__(
'{0} does not match the format for a Spack extension path.'
.format(path))

10
lib/spack/spack/main.py
View File

@@ -736,17 +736,11 @@ def main(argv=None):
# ensure options on spack command come before everything
setup_main_options(args)
# Try to load the particular command the caller asked for. If there
# is no module for it, just die.
# Try to load the particular command the caller asked for.
cmd_name = args.command[0]
cmd_name = aliases.get(cmd_name, cmd_name)
try:
command = parser.add_command(cmd_name)
except ImportError:
if spack.config.get('config:debug'):
raise
tty.die("Unknown command: %s" % args.command[0])
command = parser.add_command(cmd_name)
# Re-parse with the proper sub-parser added.
args, unknown = parser.parse_known_args()

29
lib/spack/spack/test/cmd/init_py_functions.py Normal file
View File

@@ -0,0 +1,29 @@
# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import pytest
from spack.cmd import require_python_name, python_name, PythonNameError, \
require_cmd_name, cmd_name, CommandNameError
def test_require_python_name():
"""Python module names should not contain dashes---ensure that
require_python_name() raises the appropriate exception if one is
detected.
"""
require_python_name("okey_dokey")
with pytest.raises(PythonNameError):
require_python_name("okey-dokey")
require_python_name(python_name("okey-dokey"))
def test_require_cmd_name():
"""By convention, Spack command names should contain dashes rather than
underscores---ensure that require_cmd_name() raises the appropriate
exception if underscores are detected.
"""
require_cmd_name("okey-dokey")
with pytest.raises(CommandNameError):
require_cmd_name("okey_dokey")
require_cmd_name(cmd_name("okey_dokey"))

247
lib/spack/spack/test/cmd_extensions.py
View File

@@ -5,24 +5,71 @@
import pytest
import contextlib
import os
import sys
import spack.cmd
import spack.config
import spack.extensions
import spack.main
@pytest.fixture()
def extension_root(tmpdir):
root = tmpdir.mkdir('spack-testcommand')
root.ensure('testcommand', 'cmd', dir=True)
return root
class Extension:
"""Helper class to simplify the creation of simple command extension
directory structures with a conventional format for testing.
"""
def __init__(self, name, root):
"""Create a command extension.
Args:
name (str): The name of the command extension.
root (path object): The temporary root for the command extension
(e.g. from tmpdir.mkdir()).
"""
self.name = name
self.pname = spack.cmd.python_name(name)
self.root = root
self.main = self.root.ensure(self.pname, dir=True)
self.cmd = self.main.ensure('cmd', dir=True)
def add_command(self, command_name, contents):
"""Add a command to this command extension.
Args:
command_name (str): The name of the command.
contents (str): the desired contents of the new command module
file."""
spack.cmd.require_cmd_name(command_name)
python_name = spack.cmd.python_name(command_name)
cmd = self.cmd.ensure(python_name + '.py')
cmd.write(contents)
@pytest.fixture()
def hello_world_cmd(extension_root):
"""Simple extension command with code contained in a single file."""
hello = extension_root.ensure('testcommand', 'cmd', 'hello.py')
hello.write("""
@pytest.fixture(scope='function')
def extension_creator(tmpdir, config):
"""Create a basic extension command directory structure"""
@contextlib.contextmanager
def _ce(extension_name='testcommand'):
root = tmpdir.mkdir('spack-' + extension_name)
extension = Extension(extension_name, root)
with spack.config.override('config:extensions',
[str(extension.root)]):
yield extension
list_of_modules = list(sys.modules.keys())
try:
yield _ce
finally:
to_be_deleted = [x for x in sys.modules if x not in list_of_modules]
for module_name in to_be_deleted:
del sys.modules[module_name]
@pytest.fixture(scope='function')
def hello_world_extension(extension_creator):
"""Create an extension with a hello-world command."""
with extension_creator() as extension:
extension.add_command('hello-world', """
description = "hello world extension command"
section = "test command"
level = "long"
@@ -31,27 +78,33 @@ def setup_parser(subparser):
pass
def hello(parser, args):
def hello_world(parser, args):
print('Hello world!')
""")
list_of_modules = list(sys.modules.keys())
with spack.config.override('config:extensions', [str(extension_root)]):
yield spack.main.SpackCommand('hello')
to_be_deleted = [x for x in sys.modules if x not in list_of_modules]
for module_name in to_be_deleted:
del sys.modules[module_name]
yield extension
@pytest.fixture()
def hello_world_with_module_in_root(extension_root):
"""Extension command with additional code in the root folder."""
extension_root.ensure('testcommand', '__init__.py')
command_root = extension_root.join('testcommand', 'cmd')
hello = command_root.ensure('hello.py')
hello.write("""
@pytest.fixture(scope='function')
def hello_world_cmd(hello_world_extension):
"""Create and return an invokable "hello-world" extension command."""
yield spack.main.SpackCommand('hello-world')
@pytest.fixture(scope='function')
def hello_world_with_module_in_root(extension_creator):
"""Create a "hello-world" extension command with additional code in the
root folder.
"""
@contextlib.contextmanager
def _hwwmir(extension_name=None):
with extension_creator(extension_name) \
if extension_name else \
extension_creator() as extension:
# Note that the namespace of the extension is derived from the
# fixture.
extension.add_command('hello', """
# Test an absolute import
from spack.extensions.testcommand.implementation import hello_world
from spack.extensions.{ext_pname}.implementation import hello_world
# Test a relative import
from ..implementation import hello_folks
@@ -79,33 +132,143 @@ def hello(parser, args):
hello_folks()
elif args.subcommand == 'global':
print(global_message)
""")
implementation = extension_root.ensure('testcommand', 'implementation.py')
implementation.write("""
""".format(ext_pname=extension.pname))
extension.main.ensure('__init__.py')
implementation \
= extension.main.ensure('implementation.py')
implementation.write("""
def hello_world():
print('Hello world!')
def hello_folks():
print('Hello folks!')
""")
list_of_modules = list(sys.modules.keys())
with spack.config.override('config:extensions', [str(extension_root)]):
yield spack.main.SpackCommand('hello')
yield spack.main.SpackCommand('hello')
to_be_deleted = [x for x in sys.modules if x not in list_of_modules]
for module_name in to_be_deleted:
del sys.modules[module_name]
yield _hwwmir
def test_simple_command_extension(hello_world_cmd):
"""Basic test of a functioning command."""
output = hello_world_cmd()
assert 'Hello world!' in output
def test_command_with_import(hello_world_with_module_in_root):
output = hello_world_with_module_in_root('world')
assert 'Hello world!' in output
output = hello_world_with_module_in_root('folks')
assert 'Hello folks!' in output
output = hello_world_with_module_in_root('global')
assert 'bar' in output
def test_multi_extension_search(hello_world_extension, extension_creator):
"""Ensure we can find an extension command even if it's not in the first
place we look.
"""
with extension_creator('testcommand2'):
assert ('Hello world') in spack.main.SpackCommand('hello-world')()
def test_duplicate_module_load(hello_world_cmd, capsys):
"""Ensure duplicate module load attempts are successful.
The command module will already have been loaded once by the
hello_world_cmd fixture.
"""
parser = spack.main.make_argument_parser()
args = []
hw_cmd = spack.cmd.get_command(hello_world_cmd.command_name)
hw_cmd(parser, args)
captured = capsys.readouterr()
assert captured == ('Hello world!\n', '')
@pytest.mark.parametrize('extension_name',
[None, 'hyphenated-extension'],
ids=['simple', 'hyphenated_extension_name'])
def test_command_with_import(extension_name, hello_world_with_module_in_root):
"""Ensure we can write a functioning command with multiple imported
subcommands, including where the extension name contains a hyphen.
"""
with hello_world_with_module_in_root(extension_name) as hello_world:
output = hello_world('world')
assert 'Hello world!' in output
output = hello_world('folks')
assert 'Hello folks!' in output
output = hello_world('global')
assert 'bar' in output
def test_missing_command():
"""Ensure that we raise the expected exception if the desired command is
not present.
"""
with pytest.raises(spack.extensions.CommandNotFoundError):
spack.cmd.get_module("no-such-command")
@pytest.mark.\
parametrize('extension_path,expected_exception',
[('/my/bad/extension',
spack.extensions.ExtensionNamingError),
('', spack.extensions.ExtensionNamingError),
('/my/bad/spack--extra-hyphen',
spack.extensions.ExtensionNamingError),
('/my/good/spack-extension',
spack.extensions.CommandNotFoundError),
('/my/still/good/spack-extension/',
spack.extensions.CommandNotFoundError),
('/my/spack-hyphenated-extension',
spack.extensions.CommandNotFoundError)],
ids=['no_stem', 'vacuous', 'leading_hyphen',
'basic_good', 'trailing_slash', 'hyphenated'])
def test_extension_naming(extension_path, expected_exception, config):
"""Ensure that we are correctly validating configured extension paths
for conformity with the rules: the basename should match
``spack-<name>``; <name> may have embedded hyphens but not begin with one.
"""
with spack.config.override('config:extensions', [extension_path]):
with pytest.raises(expected_exception):
spack.cmd.get_module("no-such-command")
def test_missing_command_function(extension_creator, capsys):
"""Ensure we die as expected if a command module does not have the
expected command function defined.
"""
with extension_creator() as extension:
extension.\
add_command('bad-cmd',
"""\ndescription = "Empty command implementation"\n""")
with pytest.raises(SystemExit):
spack.cmd.get_module('bad-cmd')
capture = capsys.readouterr()
assert "must define function 'bad_cmd'." in capture[1]
def test_get_command_paths(config):
"""Exercise the construction of extension command search paths."""
extensions = ('extension-1', 'extension-2')
ext_paths = []
expected_cmd_paths = []
for ext in extensions:
ext_path = os.path.join('my', 'path', 'to', 'spack-' + ext)
ext_paths.append(ext_path)
expected_cmd_paths.append(os.path.join(ext_path,
spack.cmd.python_name(ext),
'cmd'))
with spack.config.override('config:extensions', ext_paths):
assert spack.extensions.get_command_paths() == expected_cmd_paths
@pytest.mark.parametrize('command_name,contents,exception',
[('bad-cmd', 'from oopsie.daisy import bad\n',
ImportError),
('bad-cmd', """var = bad_function_call('blech')\n""",
NameError),
('bad-cmd', ')\n', SyntaxError)],
ids=['ImportError', 'NameError', 'SyntaxError'])
def test_failing_command(command_name, contents, exception, extension_creator):
"""Ensure that the configured command fails to import with the specified
error.
"""
with extension_creator() as extension:
extension.add_command(command_name, contents)
with pytest.raises(exception):
spack.extensions.get_module(command_name)