diff options
| -rw-r--r-- | lib/spack/spack/cmd/__init__.py | 46 | ||||
| -rw-r--r-- | lib/spack/spack/extensions.py | 87 | ||||
| -rw-r--r-- | lib/spack/spack/main.py | 10 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/init_py_functions.py | 29 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd_extensions.py | 245 |
5 files changed, 332 insertions, 85 deletions
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 83e12004a1..594b5e3a8e 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -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 ######################################## diff --git a/lib/spack/spack/extensions.py b/lib/spack/spack/extensions.py index b8fde7d7fa..4358dcd52f 100644 --- a/lib/spack/spack/extensions.py +++ b/lib/spack/spack/extensions.py @@ -11,10 +11,17 @@ import sys 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 load_command_extension(command, path): 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)) diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index f6da7b0fb0..3677764cb6 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -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() diff --git a/lib/spack/spack/test/cmd/init_py_functions.py b/lib/spack/spack/test/cmd/init_py_functions.py new file mode 100644 index 0000000000..2e63106283 --- /dev/null +++ b/lib/spack/spack/test/cmd/init_py_functions.py @@ -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")) diff --git a/lib/spack/spack/test/cmd_extensions.py b/lib/spack/spack/test/cmd_extensions.py index 486ac925cf..cb16609278 100644 --- a/lib/spack/spack/test/cmd_extensions.py +++ b/lib/spack/spack/test/cmd_extensions.py @@ -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) -@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(""" + 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(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') + yield extension - 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_cmd(hello_world_extension): + """Create and return an invokable "hello-world" extension command.""" + yield spack.main.SpackCommand('hello-world') -@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_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) |