From b71d430af6025cb46ac574e91e516bc5c4caf048 Mon Sep 17 00:00:00 2001 From: alalazo Date: Tue, 28 Jun 2016 17:59:34 +0200 Subject: [PATCH 01/11] module : can regenerate single module files, homogenized cli options spack module : - refresh accepts a constraint - find and refresh share common cli options - ask for confirmation before refreshing - deleting the module file tree is now optional --- lib/spack/spack/cmd/module.py | 136 ++++++++++++++++++++++------------ lib/spack/spack/modules.py | 8 +- share/spack/csh/spack.csh | 8 +- share/spack/setup-env.sh | 8 +- 4 files changed, 101 insertions(+), 59 deletions(-) diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 5292d42225..c71e615d1c 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -32,73 +32,115 @@ from spack.modules import module_types from spack.util.string import * -description = "Manipulate modules and dotkits." +from spack.cmd.uninstall import ask_for_confirmation + +description = "Manipulate module files" + + +def _add_common_arguments(subparser): + type_help = 'Type of module files' + subparser.add_argument('--module-type', help=type_help, required=True, choices=module_types) + constraint_help = 'Optional constraint to select a subset of installed packages' + subparser.add_argument('constraint', nargs='*', help=constraint_help) def setup_parser(subparser): sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command') + # spack module refresh + refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.') + refresh_parser.add_argument('--delete-tree', help='Delete the module file tree before refresh', action='store_true') + _add_common_arguments(refresh_parser) - sp.add_parser('refresh', help='Regenerate all module files.') - + # spack module find find_parser = sp.add_parser('find', help='Find module files for packages.') - find_parser.add_argument('module_type', - help="Type of module to find file for. [" + - '|'.join(module_types) + "]") - find_parser.add_argument('spec', - nargs='+', - help='spec to find a module file for.') + _add_common_arguments(find_parser) -def module_find(mtype, spec_array): - """Look at all installed packages and see if the spec provided - matches any. If it does, check whether there is a module file - of type there, and print out the name that the user - should type to use that package's module. +class MultipleMatches(Exception): + pass + + +class NoMatch(Exception): + pass + + +def module_find(mtype, specs, args): + """ + Look at all installed packages and see if the spec provided + matches any. If it does, check whether there is a module file + of type there, and print out the name that the user + should type to use that package's module. """ - if mtype not in module_types: - tty.die("Invalid module type: '%s'. Options are %s" % - (mtype, comma_or(module_types))) - - specs = spack.cmd.parse_specs(spec_array) - if len(specs) > 1: - tty.die("You can only pass one spec.") - spec = specs[0] - - specs = spack.installed_db.query(spec) if len(specs) == 0: - tty.die("No installed packages match spec %s" % spec) + raise NoMatch() if len(specs) > 1: - tty.error("Multiple matches for spec %s. Choose one:" % spec) - for s in specs: - sys.stderr.write(s.tree(color=True)) - sys.exit(1) + raise MultipleMatches() - mt = module_types[mtype] - mod = mt(specs[0]) + mod = module_types[mtype](specs.pop()) if not os.path.isfile(mod.file_name): tty.die("No %s module is installed for %s" % (mtype, spec)) print(mod.use_name) -def module_refresh(): - """Regenerate all module files for installed packages known to - spack (some packages may no longer exist).""" - specs = [s for s in spack.installed_db.query(installed=True, known=True)] +def module_refresh(name, specs, args): + """ + Regenerate all module files for installed packages known to + spack (some packages may no longer exist). + """ + # Prompt a message to the user about what is going to change + if not specs: + tty.msg('No package matches your query') + return - for name, cls in module_types.items(): - tty.msg("Regenerating %s module files." % name) - if os.path.isdir(cls.path): - shutil.rmtree(cls.path, ignore_errors=False) - mkdirp(cls.path) - for spec in specs: - cls(spec).write() + tty.msg('You are about to regenerate the {name} module files for the following specs:'.format(name=name)) + for s in specs: + print(s.format(color=True)) + ask_for_confirmation('Do you want to proceed ? ') + + cls = module_types[name] + tty.msg('Regenerating {name} module files'.format(name=name)) + if os.path.isdir(cls.path) and args.delete_tree: + shutil.rmtree(cls.path, ignore_errors=False) + mkdirp(cls.path) + for spec in specs: + cls(spec).write() + +# Qualifiers to be used when querying the db for specs +constraint_qualifiers = { + 'refresh': { + 'installed': True, + 'known': True + }, + 'find': { + } +} + +# Dictionary of callbacks based on the value of module_command +callbacks = { + 'refresh': module_refresh, + 'find': module_find +} def module(parser, args): - if args.module_command == 'refresh': - module_refresh() - - elif args.module_command == 'find': - module_find(args.module_type, args.spec) + module_type = args.module_type + # Query specs from command line + qualifiers = constraint_qualifiers[args.module_command] + specs = [s for s in spack.installed_db.query(**qualifiers)] + constraint = ' '.join(args.constraint) + if constraint: + specs = [x for x in specs if x.satisfies(constraint, strict=True)] + # Call the appropriate function + try: + callbacks[args.module_command](module_type, specs, args) + except MultipleMatches: + message = 'the constraint \'{query}\' matches multiple packages, and this is not allowed in this context' + tty.error(message.format(query=constraint)) + for s in specs: + sys.stderr.write(s.format(color=True) + '\n') + raise SystemExit(1) + except NoMatch: + message = 'the constraint \'{query}\' match no package, and this is not allowed in this context' + tty.die(message.format(query=constraint)) diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py index ce46047fa3..068179c0ce 100644 --- a/lib/spack/spack/modules.py +++ b/lib/spack/spack/modules.py @@ -454,7 +454,7 @@ def remove(self): class Dotkit(EnvModule): name = 'dotkit' - + path = join_path(spack.share_path, 'dotkit') environment_modifications_formats = { PrependPath: 'dk_alter {name} {value}\n', SetEnv: 'dk_setenv {name} {value}\n' @@ -466,7 +466,7 @@ class Dotkit(EnvModule): @property def file_name(self): - return join_path(spack.share_path, "dotkit", self.spec.architecture, + return join_path(self.path, self.spec.architecture, '%s.dk' % self.use_name) @property @@ -494,7 +494,7 @@ def prerequisite(self, spec): class TclModule(EnvModule): name = 'tcl' - + path = join_path(spack.share_path, "modules") environment_modifications_formats = { PrependPath: 'prepend-path --delim "{delim}" {name} \"{value}\"\n', AppendPath: 'append-path --delim "{delim}" {name} \"{value}\"\n', @@ -514,7 +514,7 @@ class TclModule(EnvModule): @property def file_name(self): - return join_path(spack.share_path, "modules", self.spec.architecture, self.use_name) + return join_path(self.path, self.spec.architecture, self.use_name) @property def header(self): diff --git a/share/spack/csh/spack.csh b/share/spack/csh/spack.csh index d64ce8935b..5acd190449 100644 --- a/share/spack/csh/spack.csh +++ b/share/spack/csh/spack.csh @@ -74,25 +74,25 @@ case unload: # tool's commands to add/remove the result from the environment. switch ($_sp_subcommand) case "use": - set _sp_full_spec = ( "`\spack $_sp_flags module find dotkit $_sp_spec`" ) + set _sp_full_spec = ( "`\spack $_sp_flags module find --module-type dotkit $_sp_spec`" ) if ( $? == 0 ) then use $_sp_module_args $_sp_full_spec endif breaksw case "unuse": - set _sp_full_spec = ( "`\spack $_sp_flags module find dotkit $_sp_spec`" ) + set _sp_full_spec = ( "`\spack $_sp_flags module find --module-type dotkit $_sp_spec`" ) if ( $? == 0 ) then unuse $_sp_module_args $_sp_full_spec endif breaksw case "load": - set _sp_full_spec = ( "`\spack $_sp_flags module find tcl $_sp_spec`" ) + set _sp_full_spec = ( "`\spack $_sp_flags module find --module-type tcl $_sp_spec`" ) if ( $? == 0 ) then module load $_sp_module_args $_sp_full_spec endif breaksw case "unload": - set _sp_full_spec = ( "`\spack $_sp_flags module find tcl $_sp_spec`" ) + set _sp_full_spec = ( "`\spack $_sp_flags module find --module-type tcl $_sp_spec`" ) if ( $? == 0 ) then module unload $_sp_module_args $_sp_full_spec endif diff --git a/share/spack/setup-env.sh b/share/spack/setup-env.sh index 8aa259cf15..3975672e7b 100755 --- a/share/spack/setup-env.sh +++ b/share/spack/setup-env.sh @@ -105,19 +105,19 @@ function spack { # If spack module command comes back with an error, do nothing. case $_sp_subcommand in "use") - if _sp_full_spec=$(command spack $_sp_flags module find dotkit $_sp_spec); then + if _sp_full_spec=$(command spack $_sp_flags module find --module-type dotkit $_sp_spec); then use $_sp_module_args $_sp_full_spec fi ;; "unuse") - if _sp_full_spec=$(command spack $_sp_flags module find dotkit $_sp_spec); then + if _sp_full_spec=$(command spack $_sp_flags module find --module-type dotkit $_sp_spec); then unuse $_sp_module_args $_sp_full_spec fi ;; "load") - if _sp_full_spec=$(command spack $_sp_flags module find tcl $_sp_spec); then + if _sp_full_spec=$(command spack $_sp_flags module find --module-type tcl $_sp_spec); then module load $_sp_module_args $_sp_full_spec fi ;; "unload") - if _sp_full_spec=$(command spack $_sp_flags module find tcl $_sp_spec); then + if _sp_full_spec=$(command spack $_sp_flags module find --module-type tcl $_sp_spec); then module unload $_sp_module_args $_sp_full_spec fi ;; esac From 1b7eedbb7df7c6dee9ab84217e75dc8ec54dcee1 Mon Sep 17 00:00:00 2001 From: alalazo Date: Thu, 30 Jun 2016 12:56:47 +0200 Subject: [PATCH 02/11] modules.yaml : added hash_length as a new keyword config : - added `hash_length` under the modules section EnvModules : - take into consideration hash_length when constructing `file_name` - added logic to warn and skip module file writing in case of file name clash --- lib/spack/spack/config.py | 5 +++++ lib/spack/spack/modules.py | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 84179e1469..3a66e9f2a6 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -328,6 +328,11 @@ 'anyOf': [ { 'properties': { + 'hash_length': { + 'type': 'integer', + 'minimum': 0, + 'default': 7 + }, 'whitelist': {'$ref': '#/definitions/array_of_strings'}, 'blacklist': {'$ref': '#/definitions/array_of_strings'}, 'naming_scheme': { diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py index 068179c0ce..82016feb84 100644 --- a/lib/spack/spack/modules.py +++ b/lib/spack/spack/modules.py @@ -188,6 +188,7 @@ def parse_config_options(module_generator): ##### # Automatic loading loads + module_file_actions['hash_length'] = module_configuration.get('hash_length', 7) module_file_actions['autoload'] = dependencies( module_generator.spec, module_file_actions.get('autoload', 'none')) # Prerequisites @@ -295,7 +296,9 @@ def use_name(self): if constraint in self.spec: suffixes.append(suffix) # Always append the hash to make the module file unique - suffixes.append(self.spec.dag_hash()) + hash_length = configuration.pop('hash_length', 7) + if hash_length != 0: + suffixes.append(self.spec.dag_hash(length=hash_length)) name = '-'.join(suffixes) return name @@ -338,7 +341,7 @@ def blacklisted(self): return False - def write(self): + def write(self, overwrite=False): """ Writes out a module file for this object. @@ -399,6 +402,15 @@ def write(self): for line in self.module_specific_content(module_configuration): module_file_content += line + # Print a warning in case I am accidentally overwriting + # a module file that is already there (name clash) + if not overwrite and os.path.exists(self.file_name): + message = 'Module file already exists : skipping creation\n' + message += 'file : {0.file_name}\n' + message += 'spec : {0.spec}' + tty.warn(message.format(self)) + return + # Dump to file with open(self.file_name, 'w') as f: f.write(module_file_content) From ba87937fffc5843b5f32246571b79d499dafeba3 Mon Sep 17 00:00:00 2001 From: alalazo Date: Thu, 30 Jun 2016 13:44:49 +0200 Subject: [PATCH 03/11] module : added detection of file name clashes --- lib/spack/spack/cmd/module.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index c71e615d1c..4f6b4764b6 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -25,6 +25,7 @@ import os import shutil import sys +import collections import llnl.util.tty as tty import spack.cmd @@ -100,12 +101,32 @@ def module_refresh(name, specs, args): ask_for_confirmation('Do you want to proceed ? ') cls = module_types[name] + + # Detect name clashes + writers = [cls(spec) for spec in specs] + file2writer = collections.defaultdict(list) + for item in writers: + file2writer[item.file_name].append(item) + + if len(file2writer) != len(writers): + message = 'Name clashes detected in module files:\n' + for filename, writer_list in file2writer.items(): + if len(writer_list) > 1: + message += 'file : {0}\n'.format(filename) + for x in writer_list: + message += 'spec : {0}\n'.format(x.spec.format(color=True)) + message += '\n' + tty.error(message) + tty.error('Operation aborted') + raise SystemExit(1) + + # Proceed regenerating module files tty.msg('Regenerating {name} module files'.format(name=name)) if os.path.isdir(cls.path) and args.delete_tree: shutil.rmtree(cls.path, ignore_errors=False) mkdirp(cls.path) - for spec in specs: - cls(spec).write() + for x in writers: + x.write(overwrite=True) # Qualifiers to be used when querying the db for specs constraint_qualifiers = { From a770151359a9c573bfd967f79eab26670d499af0 Mon Sep 17 00:00:00 2001 From: alalazo Date: Thu, 30 Jun 2016 16:49:24 +0200 Subject: [PATCH 04/11] module : minor improvement to output formatting --- bin/spack | 2 +- lib/spack/spack/cmd/module.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/spack b/bin/spack index e9307d1485..9b1276a866 100755 --- a/bin/spack +++ b/bin/spack @@ -77,7 +77,7 @@ import llnl.util.tty as tty from llnl.util.tty.color import * import spack from spack.error import SpackError -import argparse +from external import argparse # Command parsing parser = argparse.ArgumentParser( diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 4f6b4764b6..4f0593e45e 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -95,9 +95,10 @@ def module_refresh(name, specs, args): tty.msg('No package matches your query') return - tty.msg('You are about to regenerate the {name} module files for the following specs:'.format(name=name)) + tty.msg('You are about to regenerate the {name} module files for the following specs:\n'.format(name=name)) for s in specs: print(s.format(color=True)) + print('') ask_for_confirmation('Do you want to proceed ? ') cls = module_types[name] @@ -112,10 +113,9 @@ def module_refresh(name, specs, args): message = 'Name clashes detected in module files:\n' for filename, writer_list in file2writer.items(): if len(writer_list) > 1: - message += 'file : {0}\n'.format(filename) + message += '\nfile : {0}\n'.format(filename) for x in writer_list: message += 'spec : {0}\n'.format(x.spec.format(color=True)) - message += '\n' tty.error(message) tty.error('Operation aborted') raise SystemExit(1) From 6793a54748246efcda302b1fba24239ca5e19c2d Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 30 Jun 2016 23:18:32 +0200 Subject: [PATCH 05/11] --prefix : defaults to empty string --- lib/spack/spack/cmd/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 916bb65646..0d980bc918 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -65,7 +65,7 @@ def setup_parser(subparser): help='Generate shell script (instead of input for module command)') find_parser.add_argument( - '-p', '--prefix', dest='prefix', + '-p', '--prefix', dest='prefix', default='', help='Prepend to module names when issuing module load commands') _add_common_arguments(find_parser) From fe4ef286f2887c27b7fe18a594a11eb4647c5cf3 Mon Sep 17 00:00:00 2001 From: alalazo Date: Fri, 1 Jul 2016 12:38:04 +0200 Subject: [PATCH 06/11] module : added the command 'load-list' --- lib/spack/spack/cmd/module.py | 119 ++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 57 deletions(-) diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 0d980bc918..5628ab17a5 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -23,6 +23,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## from __future__ import print_function + import os import shutil import sys @@ -41,7 +42,7 @@ def _add_common_arguments(subparser): type_help = 'Type of module files' - subparser.add_argument('--module-type', help=type_help, required=True, choices=module_types) + subparser.add_argument('--module-type', help=type_help, default='tcl', choices=module_types) constraint_help = 'Optional constraint to select a subset of installed packages' subparser.add_argument('constraint', nargs='*', help=constraint_help) @@ -55,19 +56,26 @@ def setup_parser(subparser): # spack module find find_parser = sp.add_parser('find', help='Find module files for packages.') - find_parser.add_argument( + _add_common_arguments(find_parser) + + # spack module env + loadlist_parser = sp.add_parser( + 'load-list', + help='Prompt the list of modules associated with packages that satisfy a contraint' + ) + loadlist_parser.add_argument( '-r', '--dependencies', action='store_true', dest='recurse_dependencies', help='Recursively traverse dependencies for modules to load.') - find_parser.add_argument( + loadlist_parser.add_argument( '-s', '--shell', action='store_true', dest='shell', help='Generate shell script (instead of input for module command)') - find_parser.add_argument( + loadlist_parser.add_argument( '-p', '--prefix', dest='prefix', default='', help='Prepend to module names when issuing module load commands') - _add_common_arguments(find_parser) + _add_common_arguments(loadlist_parser) class MultipleMatches(Exception): @@ -78,7 +86,45 @@ class NoMatch(Exception): pass -def module_find(mtype, specs, args): +def load_list(mtype, specs, args): + + # Get a comprehensive list of specs + if args.recurse_dependencies: + specs_from_user_constraint = specs[:] + specs = [] + # FIXME : during module file creation nodes seem to be visited multiple + # FIXME : times even if cover='nodes' is given. This work around permits + # FIXME : to get a unique list of spec anyhow. Do we miss a merge + # FIXME : step among nodes that refer to the same package? + # FIXME : (same problem as in spack/modules.py) + seen = set() + seen_add = seen.add + for spec in specs_from_user_constraint: + specs.extend( + [item for item in spec.traverse(order='post', cover='nodes') if not (item in seen or seen_add(item))] + ) + + module_cls = module_types[mtype] + modules = [(spec, module_cls(spec).use_name) for spec in specs if os.path.exists(module_cls(spec).file_name)] + + module_commands = { + 'tcl': 'module load ', + 'dotkit': 'dotkit use ' + } + + d = { + 'command': '' if not args.shell else module_commands[mtype], + 'prefix': args.prefix + } + + prompt_template = '{comment}{command}{prefix}{name}' + for spec, mod in modules: + d['comment'] = '' if not args.shell else '# {0}\n'.format(spec.format()) + d['name'] = mod + print( prompt_template.format(**d)) + + +def find(mtype, specs, args): """ Look at all installed packages and see if the spec provided matches any. If it does, check whether there is a module file @@ -92,56 +138,12 @@ def module_find(mtype, specs, args): raise MultipleMatches() spec = specs.pop() - if not args.recurse_dependencies: - mod = module_types[mtype](spec) - if not os.path.isfile(mod.file_name): - tty.die("No %s module is installed for %s" % (mtype, spec)) + mod = module_types[mtype](spec) + if not os.path.isfile(mod.file_name): + tty.die("No %s module is installed for %s" % (mtype, spec)) + print(mod.use_name) - print(mod.use_name) - else: - - def _find_modules(spec, modules_list): - """Finds all modules and sub-modules for a spec""" - if str(spec.version) == 'system': - # No Spack module for system-installed packages - return - - if args.recurse_dependencies: - for dep in spec.dependencies.values(): - _find_modules(dep, modules_list) - - mod = module_types[mtype](spec) - if not os.path.isfile(mod.file_name): - tty.die("No %s module is installed for %s" % (mtype, spec)) - modules_list.append((spec, mod)) - # -------------------------------------- - - modules = set() # Modules we will load - seen = set() - - # ----------- Chase down modules for it and all its dependencies - modules_dups = list() - _find_modules(spec, modules_dups) - - # Remove duplicates while keeping order - modules_unique = list() - for spec, mod in modules_dups: - if mod.use_name not in seen: - modules_unique.append((spec,mod)) - seen.add(mod.use_name) - - # Output... - if args.shell: - module_cmd = {'tcl': 'module load', 'dotkit': 'dotkit use'}[mtype] - for spec, mod in modules_unique: - if args.shell: - print('# %s' % spec.format()) - print('%s %s%s' % (module_cmd, args.prefix, mod.use_name)) - else: - print(mod.use_name) - - -def module_refresh(name, specs, args): +def refresh(name, specs, args): """ Regenerate all module files for installed packages known to spack (some packages may no longer exist). @@ -191,13 +193,16 @@ def module_refresh(name, specs, args): 'known': True }, 'find': { + }, + 'load-list':{ } } # Dictionary of callbacks based on the value of module_command callbacks = { - 'refresh': module_refresh, - 'find': module_find + 'refresh': refresh, + 'find': find, + 'load-list': load_list } From f0f7b23c8a6804da6d215e15712c99a6d72e782a Mon Sep 17 00:00:00 2001 From: alalazo Date: Fri, 1 Jul 2016 14:27:55 +0200 Subject: [PATCH 07/11] module : added rm subcommand, encapsulated logic for constraints in argarse.Action subclass --- lib/spack/spack/cmd/module.py | 127 +++++++++++++++++++++++----------- 1 file changed, 88 insertions(+), 39 deletions(-) diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 5628ab17a5..6f9ede21ac 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -28,6 +28,7 @@ import shutil import sys import collections +import argparse import llnl.util.tty as tty import spack.cmd @@ -40,33 +41,76 @@ description = "Manipulate module files" +# Qualifiers to be used when querying the db for specs +constraint_qualifiers = { + 'refresh': { + 'installed': True, + 'known': True + }, + 'find': { + }, + 'load-list':{ + }, + 'rm': { + } +} + + +class ConstraintAction(argparse.Action): + qualifiers = {} + + def __call__(self, parser, namespace, values, option_string=None): + # Query specs from command line + d = self.qualifiers.get(namespace.subparser_name, {}) + specs = [s for s in spack.installed_db.query(**d)] + values = ' '.join(values) + if values: + specs = [x for x in specs if x.satisfies(values, strict=True)] + namespace.specs = specs + +# TODO : this needs better wrapping to be extracted +ConstraintAction.qualifiers.update(constraint_qualifiers) + + def _add_common_arguments(subparser): type_help = 'Type of module files' - subparser.add_argument('--module-type', help=type_help, default='tcl', choices=module_types) + subparser.add_argument('-m', '--module-type', help=type_help, default='tcl', choices=module_types) constraint_help = 'Optional constraint to select a subset of installed packages' - subparser.add_argument('constraint', nargs='*', help=constraint_help) + subparser.add_argument('constraint', nargs='*', help=constraint_help, action=ConstraintAction) def setup_parser(subparser): - sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command') + sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subparser_name') # spack module refresh refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.') refresh_parser.add_argument('--delete-tree', help='Delete the module file tree before refresh', action='store_true') _add_common_arguments(refresh_parser) + refresh_parser.add_argument( + '-y', '--yes-to-all', action='store_true', dest='yes_to_all', + help='Assume "yes" is the answer to every confirmation asked to the user.' + ) # spack module find find_parser = sp.add_parser('find', help='Find module files for packages.') _add_common_arguments(find_parser) - # spack module env + # spack module rm + rm_parser = sp.add_parser('rm', help='Find module files for packages.') + _add_common_arguments(rm_parser) + rm_parser.add_argument( + '-y', '--yes-to-all', action='store_true', dest='yes_to_all', + help='Assume "yes" is the answer to every confirmation asked to the user.' + ) + + # spack module load-list loadlist_parser = sp.add_parser( 'load-list', help='Prompt the list of modules associated with packages that satisfy a contraint' ) loadlist_parser.add_argument( - '-r', '--dependencies', action='store_true', + '-d', '--dependencies', action='store_true', dest='recurse_dependencies', - help='Recursively traverse dependencies for modules to load.') + help='Recursively traverse spec dependencies') loadlist_parser.add_argument( '-s', '--shell', action='store_true', dest='shell', @@ -87,7 +131,6 @@ class NoMatch(Exception): def load_list(mtype, specs, args): - # Get a comprehensive list of specs if args.recurse_dependencies: specs_from_user_constraint = specs[:] @@ -121,7 +164,7 @@ def load_list(mtype, specs, args): for spec, mod in modules: d['comment'] = '' if not args.shell else '# {0}\n'.format(spec.format()) d['name'] = mod - print( prompt_template.format(**d)) + print(prompt_template.format(**d)) def find(mtype, specs, args): @@ -143,7 +186,29 @@ def find(mtype, specs, args): tty.die("No %s module is installed for %s" % (mtype, spec)) print(mod.use_name) -def refresh(name, specs, args): + +def rm(mtype, specs, args): + module_cls = module_types[mtype] + modules = [module_cls(spec) for spec in specs if os.path.exists(module_cls(spec).file_name)] + + if not modules: + tty.msg('No module file matches your query') + return + + # Ask for confirmation + if not args.yes_to_all: + tty.msg('You are about to remove the following module files:\n') + for s in modules: + print(s.file_name) + print('') + ask_for_confirmation('Do you want to proceed ? ') + + # Remove the module files + for s in modules: + s.remove() + + +def refresh(mtype, specs, args): """ Regenerate all module files for installed packages known to spack (some packages may no longer exist). @@ -153,13 +218,14 @@ def refresh(name, specs, args): tty.msg('No package matches your query') return - tty.msg('You are about to regenerate the {name} module files for the following specs:\n'.format(name=name)) - for s in specs: - print(s.format(color=True)) - print('') - ask_for_confirmation('Do you want to proceed ? ') + if not args.yes_to_all: + tty.msg('You are about to regenerate the {name} module files for the following specs:\n'.format(name=mtype)) + for s in specs: + print(s.format(color=True)) + print('') + ask_for_confirmation('Do you want to proceed ? ') - cls = module_types[name] + cls = module_types[mtype] # Detect name clashes writers = [cls(spec) for spec in specs] @@ -179,48 +245,31 @@ def refresh(name, specs, args): raise SystemExit(1) # Proceed regenerating module files - tty.msg('Regenerating {name} module files'.format(name=name)) + tty.msg('Regenerating {name} module files'.format(name=mtype)) if os.path.isdir(cls.path) and args.delete_tree: shutil.rmtree(cls.path, ignore_errors=False) mkdirp(cls.path) for x in writers: x.write(overwrite=True) -# Qualifiers to be used when querying the db for specs -constraint_qualifiers = { - 'refresh': { - 'installed': True, - 'known': True - }, - 'find': { - }, - 'load-list':{ - } -} - -# Dictionary of callbacks based on the value of module_command +# Dictionary of callbacks based on the value of subparser_name callbacks = { 'refresh': refresh, 'find': find, - 'load-list': load_list + 'load-list': load_list, + 'rm': rm } def module(parser, args): module_type = args.module_type - # Query specs from command line - qualifiers = constraint_qualifiers[args.module_command] - specs = [s for s in spack.installed_db.query(**qualifiers)] - constraint = ' '.join(args.constraint) - if constraint: - specs = [x for x in specs if x.satisfies(constraint, strict=True)] - # Call the appropriate function + constraint = args.constraint try: - callbacks[args.module_command](module_type, specs, args) + callbacks[args.subparser_name](module_type, args.specs, args) except MultipleMatches: message = 'the constraint \'{query}\' matches multiple packages, and this is not allowed in this context' tty.error(message.format(query=constraint)) - for s in specs: + for s in args.specs: sys.stderr.write(s.format(color=True) + '\n') raise SystemExit(1) except NoMatch: From 0e171127b99e06d05631fe6391cb20b813bc1747 Mon Sep 17 00:00:00 2001 From: alalazo Date: Fri, 1 Jul 2016 14:30:01 +0200 Subject: [PATCH 08/11] module : reverted typo introduced in a previous commit --- bin/spack | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/spack b/bin/spack index 9b1276a866..e9307d1485 100755 --- a/bin/spack +++ b/bin/spack @@ -77,7 +77,7 @@ import llnl.util.tty as tty from llnl.util.tty.color import * import spack from spack.error import SpackError -from external import argparse +import argparse # Command parsing parser = argparse.ArgumentParser( From d10fceaacc7a1f8349f249c8700381f858b359b1 Mon Sep 17 00:00:00 2001 From: alalazo Date: Fri, 1 Jul 2016 23:06:07 +0200 Subject: [PATCH 09/11] spack commands : refactoring of cli arguments and common utiities. Implemented suggestions on `spack module loads` - Common cli arguments now are in their own module - Moved utilities that can be reused by different commands into spack.cmd.__init__.py - Modifications to `spack module loads` --- lib/spack/spack/cmd/__init__.py | 101 ++++++++++++++++- lib/spack/spack/cmd/common/__init__.py | 24 ++++ lib/spack/spack/cmd/common/arguments.py | 88 +++++++++++++++ lib/spack/spack/cmd/find.py | 85 +------------- lib/spack/spack/cmd/module.py | 142 +++++++++--------------- lib/spack/spack/cmd/uninstall.py | 20 +--- lib/spack/spack/test/cmd/find.py | 6 +- lib/spack/spack/util/pattern.py | 6 + 8 files changed, 272 insertions(+), 200 deletions(-) create mode 100644 lib/spack/spack/cmd/common/__init__.py create mode 100644 lib/spack/spack/cmd/common/arguments.py diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 672999159c..02f6f5b98a 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -27,11 +27,12 @@ import sys import llnl.util.tty as tty -from llnl.util.lang import attr_setdefault - import spack -import spack.spec import spack.config +import spack.spec +from llnl.util.lang import * +from llnl.util.tty.colify import * +from llnl.util.tty.color import * # # Settings for commands that modify configuration @@ -145,3 +146,97 @@ def disambiguate_spec(spec): tty.die(*args) return matching_specs[0] + + +def ask_for_confirmation(message): + while True: + tty.msg(message + '[y/n]') + choice = raw_input().lower() + if choice == 'y': + break + elif choice == 'n': + raise SystemExit('Operation aborted') + tty.warn('Please reply either "y" or "n"') + + +def gray_hash(spec, length): + return colorize('@K{%s}' % spec.dag_hash(length)) + + +def display_specs(specs, **kwargs): + mode = kwargs.get('mode', 'short') + hashes = kwargs.get('long', False) + namespace = kwargs.get('namespace', False) + flags = kwargs.get('show_flags', False) + variants = kwargs.get('variants', False) + + hlen = 7 + if kwargs.get('very_long', False): + hashes = True + hlen = None + + nfmt = '.' if namespace else '_' + ffmt = '$%+' if flags else '' + vfmt = '$+' if variants else '' + format_string = '$%s$@%s%s' % (nfmt, ffmt, vfmt) + + # Make a dict with specs keyed by architecture and compiler. + index = index_by(specs, ('architecture', 'compiler')) + + # Traverse the index and print out each package + for i, (architecture, compiler) in enumerate(sorted(index)): + if i > 0: + print + + header = "%s{%s} / %s{%s}" % (spack.spec.architecture_color, + architecture, spack.spec.compiler_color, + compiler) + tty.hline(colorize(header), char='-') + + specs = index[(architecture, compiler)] + specs.sort() + + abbreviated = [s.format(format_string, color=True) for s in specs] + if mode == 'paths': + # Print one spec per line along with prefix path + width = max(len(s) for s in abbreviated) + width += 2 + format = " %%-%ds%%s" % width + + for abbrv, spec in zip(abbreviated, specs): + if hashes: + print(gray_hash(spec, hlen), ) + print(format % (abbrv, spec.prefix)) + + elif mode == 'deps': + for spec in specs: + print(spec.tree( + format=format_string, + color=True, + indent=4, + prefix=(lambda s: gray_hash(s, hlen)) if hashes else None)) + + elif mode == 'short': + # Print columns of output if not printing flags + if not flags: + + def fmt(s): + string = "" + if hashes: + string += gray_hash(s, hlen) + ' ' + string += s.format('$-%s$@%s' % (nfmt, vfmt), color=True) + + return string + + colify(fmt(s) for s in specs) + # Print one entry per line if including flags + else: + for spec in specs: + # Print the hash if necessary + hsh = gray_hash(spec, hlen) + ' ' if hashes else '' + print(hsh + spec.format(format_string, color=True) + '\n') + + else: + raise ValueError( + "Invalid mode for display_specs: %s. Must be one of (paths," + "deps, short)." % mode) # NOQA: ignore=E501 diff --git a/lib/spack/spack/cmd/common/__init__.py b/lib/spack/spack/cmd/common/__init__.py new file mode 100644 index 0000000000..ed1ec23bca --- /dev/null +++ b/lib/spack/spack/cmd/common/__init__.py @@ -0,0 +1,24 @@ +############################################################################## +# 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 +############################################################################## diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py new file mode 100644 index 0000000000..a50bac5ac5 --- /dev/null +++ b/lib/spack/spack/cmd/common/arguments.py @@ -0,0 +1,88 @@ +############################################################################## +# 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 argparse + +import spack.modules +from spack.util.pattern import Bunch +__all__ = ['add_common_arguments'] + +_arguments = {} + + +def add_common_arguments(parser, list_of_arguments): + for argument in list_of_arguments: + if argument not in _arguments: + message = 'Trying to add the non existing argument "{0}" to a command' + raise KeyError(message.format(argument)) + x = _arguments[argument] + parser.add_argument(*x.flags, **x.kwargs) + + +class ConstraintAction(argparse.Action): + """Constructs a list of specs based on a constraint given on the command line + + An instance of this class is supposed to be used as an argument action in a parser. + + It will read a constraint and will attach a list of matching specs to the namespace + """ + qualifiers = {} + + def __call__(self, parser, namespace, values, option_string=None): + # Query specs from command line + d = self.qualifiers.get(namespace.subparser_name, {}) + specs = [s for s in spack.installed_db.query(**d)] + values = ' '.join(values) + if values: + specs = [x for x in specs if x.satisfies(values, strict=True)] + namespace.specs = specs + +_arguments['constraint'] = Bunch(flags=('constraint',), + kwargs={ + 'nargs': '*', + 'help': 'Optional constraint to select a subset of installed packages', + 'action': ConstraintAction + }) + +_arguments['module_type'] = Bunch(flags=('-m', '--module-type'), + kwargs={ + 'help': 'Type of module files', + 'default': 'tcl', + 'choices': spack.modules.module_types + }) + +_arguments['yes_to_all'] = Bunch(flags=('-y', '--yes-to-all'), + kwargs={ + 'action': 'store_true', + 'dest': 'yes_to_all', + 'help': 'Assume "yes" is the answer to every confirmation asked to the user.' + }) + +_arguments['recurse_dependencies'] = Bunch(flags=('-r', '--dependencies'), + kwargs={ + 'action': 'store_true', + 'dest': 'recurse_dependencies', + 'help': 'Recursively traverse spec dependencies' + }) diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py index 3ec671f93f..d3ea38c573 100644 --- a/lib/spack/spack/cmd/find.py +++ b/lib/spack/spack/cmd/find.py @@ -31,7 +31,7 @@ from llnl.util.lang import * from llnl.util.tty.colify import * from llnl.util.tty.color import * -from llnl.util.lang import * +from spack.cmd import display_specs description = "Find installed spack packages" @@ -104,89 +104,6 @@ def setup_parser(subparser): help='optional specs to filter results') -def gray_hash(spec, length): - return colorize('@K{%s}' % spec.dag_hash(length)) - - -def display_specs(specs, **kwargs): - mode = kwargs.get('mode', 'short') - hashes = kwargs.get('long', False) - namespace = kwargs.get('namespace', False) - flags = kwargs.get('show_flags', False) - variants = kwargs.get('variants', False) - - hlen = 7 - if kwargs.get('very_long', False): - hashes = True - hlen = None - - nfmt = '.' if namespace else '_' - ffmt = '$%+' if flags else '' - vfmt = '$+' if variants else '' - format_string = '$%s$@%s%s' % (nfmt, ffmt, vfmt) - - # Make a dict with specs keyed by architecture and compiler. - index = index_by(specs, ('architecture', 'compiler')) - - # Traverse the index and print out each package - for i, (architecture, compiler) in enumerate(sorted(index)): - if i > 0: - print - - header = "%s{%s} / %s{%s}" % (spack.spec.architecture_color, - architecture, spack.spec.compiler_color, - compiler) - tty.hline(colorize(header), char='-') - - specs = index[(architecture, compiler)] - specs.sort() - - abbreviated = [s.format(format_string, color=True) for s in specs] - if mode == 'paths': - # Print one spec per line along with prefix path - width = max(len(s) for s in abbreviated) - width += 2 - format = " %%-%ds%%s" % width - - for abbrv, spec in zip(abbreviated, specs): - if hashes: - print(gray_hash(spec, hlen), ) - print(format % (abbrv, spec.prefix)) - - elif mode == 'deps': - for spec in specs: - print(spec.tree( - format=format_string, - color=True, - indent=4, - prefix=(lambda s: gray_hash(s, hlen)) if hashes else None)) - - elif mode == 'short': - # Print columns of output if not printing flags - if not flags: - - def fmt(s): - string = "" - if hashes: - string += gray_hash(s, hlen) + ' ' - string += s.format('$-%s$@%s' % (nfmt, vfmt), color=True) - - return string - - colify(fmt(s) for s in specs) - # Print one entry per line if including flags - else: - for spec in specs: - # Print the hash if necessary - hsh = gray_hash(spec, hlen) + ' ' if hashes else '' - print(hsh + spec.format(format_string, color=True) + '\n') - - else: - raise ValueError( - "Invalid mode for display_specs: %s. Must be one of (paths," - "deps, short)." % mode) # NOQA: ignore=E501 - - def query_arguments(args): # Check arguments if args.explicit and args.implicit: diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 6f9ede21ac..2c47e9fcb6 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -24,102 +24,59 @@ ############################################################################## from __future__ import print_function +import collections import os import shutil import sys -import collections -import argparse import llnl.util.tty as tty import spack.cmd +import spack.cmd.common.arguments as arguments from llnl.util.filesystem import mkdirp from spack.modules import module_types -from spack.util.string import * - -from spack.cmd.uninstall import ask_for_confirmation +#from spack.util.string import * description = "Manipulate module files" - -# Qualifiers to be used when querying the db for specs -constraint_qualifiers = { - 'refresh': { - 'installed': True, - 'known': True - }, - 'find': { - }, - 'load-list':{ - }, - 'rm': { - } -} +callbacks = {} -class ConstraintAction(argparse.Action): - qualifiers = {} - - def __call__(self, parser, namespace, values, option_string=None): - # Query specs from command line - d = self.qualifiers.get(namespace.subparser_name, {}) - specs = [s for s in spack.installed_db.query(**d)] - values = ' '.join(values) - if values: - specs = [x for x in specs if x.satisfies(values, strict=True)] - namespace.specs = specs - -# TODO : this needs better wrapping to be extracted -ConstraintAction.qualifiers.update(constraint_qualifiers) - - -def _add_common_arguments(subparser): - type_help = 'Type of module files' - subparser.add_argument('-m', '--module-type', help=type_help, default='tcl', choices=module_types) - constraint_help = 'Optional constraint to select a subset of installed packages' - subparser.add_argument('constraint', nargs='*', help=constraint_help, action=ConstraintAction) +def subcommand(subparser_name): + """Registers a function in the callbacks dictionary""" + def decorator(callback): + callbacks[subparser_name] = callback + return callback + return decorator def setup_parser(subparser): sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subparser_name') # spack module refresh - refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.') + refresh_parser = sp.add_parser('refresh', help='Regenerate module files') refresh_parser.add_argument('--delete-tree', help='Delete the module file tree before refresh', action='store_true') - _add_common_arguments(refresh_parser) - refresh_parser.add_argument( - '-y', '--yes-to-all', action='store_true', dest='yes_to_all', - help='Assume "yes" is the answer to every confirmation asked to the user.' - ) + arguments.add_common_arguments(refresh_parser, ['constraint', 'module_type', 'yes_to_all']) # spack module find - find_parser = sp.add_parser('find', help='Find module files for packages.') - _add_common_arguments(find_parser) + find_parser = sp.add_parser('find', help='Find module files for packages') + arguments.add_common_arguments(find_parser, ['constraint', 'module_type']) # spack module rm - rm_parser = sp.add_parser('rm', help='Find module files for packages.') - _add_common_arguments(rm_parser) - rm_parser.add_argument( - '-y', '--yes-to-all', action='store_true', dest='yes_to_all', - help='Assume "yes" is the answer to every confirmation asked to the user.' + rm_parser = sp.add_parser('rm', help='Remove module files') + arguments.add_common_arguments(rm_parser, ['constraint', 'module_type', 'yes_to_all']) + + # spack module loads + loads_parser = sp.add_parser( + 'loads', + help='Prompt the list of modules associated with packages that satisfy a constraint' ) + loads_parser.add_argument( + '--input-only', action='store_false', dest='shell', + help='Generate input for module command (instead of a shell script)') - # spack module load-list - loadlist_parser = sp.add_parser( - 'load-list', - help='Prompt the list of modules associated with packages that satisfy a contraint' - ) - loadlist_parser.add_argument( - '-d', '--dependencies', action='store_true', - dest='recurse_dependencies', - help='Recursively traverse spec dependencies') - - loadlist_parser.add_argument( - '-s', '--shell', action='store_true', dest='shell', - help='Generate shell script (instead of input for module command)') - - loadlist_parser.add_argument( + loads_parser.add_argument( '-p', '--prefix', dest='prefix', default='', help='Prepend to module names when issuing module load commands') - _add_common_arguments(loadlist_parser) + arguments.add_common_arguments(loads_parser, ['constraint', 'module_type', 'recurse_dependencies']) class MultipleMatches(Exception): @@ -129,8 +86,8 @@ class MultipleMatches(Exception): class NoMatch(Exception): pass - -def load_list(mtype, specs, args): +@subcommand('loads') +def loads(mtype, specs, args): # Get a comprehensive list of specs if args.recurse_dependencies: specs_from_user_constraint = specs[:] @@ -166,7 +123,7 @@ def load_list(mtype, specs, args): d['name'] = mod print(prompt_template.format(**d)) - +@subcommand('find') def find(mtype, specs, args): """ Look at all installed packages and see if the spec provided @@ -187,9 +144,11 @@ def find(mtype, specs, args): print(mod.use_name) +@subcommand('rm') def rm(mtype, specs, args): module_cls = module_types[mtype] - modules = [module_cls(spec) for spec in specs if os.path.exists(module_cls(spec).file_name)] + specs_with_modules = [spec for spec in specs if os.path.exists(module_cls(spec).file_name)] + modules = [module_cls(spec) for spec in specs_with_modules] if not modules: tty.msg('No module file matches your query') @@ -197,21 +156,20 @@ def rm(mtype, specs, args): # Ask for confirmation if not args.yes_to_all: - tty.msg('You are about to remove the following module files:\n') - for s in modules: - print(s.file_name) + tty.msg('You are about to remove {0} module files the following specs:\n'.format(mtype)) + spack.cmd.display_specs(specs_with_modules, long=True) print('') - ask_for_confirmation('Do you want to proceed ? ') + spack.cmd.ask_for_confirmation('Do you want to proceed ? ') # Remove the module files for s in modules: s.remove() +@subcommand('refresh') def refresh(mtype, specs, args): - """ - Regenerate all module files for installed packages known to - spack (some packages may no longer exist). + """Regenerate module files for installed packages + """ # Prompt a message to the user about what is going to change if not specs: @@ -219,11 +177,10 @@ def refresh(mtype, specs, args): return if not args.yes_to_all: - tty.msg('You are about to regenerate the {name} module files for the following specs:\n'.format(name=mtype)) - for s in specs: - print(s.format(color=True)) + tty.msg('You are about to regenerate {name} module files for the following specs:\n'.format(name=mtype)) + spack.cmd.display_specs(specs, long=True) print('') - ask_for_confirmation('Do you want to proceed ? ') + spack.cmd.ask_for_confirmation('Do you want to proceed ? ') cls = module_types[mtype] @@ -252,16 +209,17 @@ def refresh(mtype, specs, args): for x in writers: x.write(overwrite=True) -# Dictionary of callbacks based on the value of subparser_name -callbacks = { - 'refresh': refresh, - 'find': find, - 'load-list': load_list, - 'rm': rm -} - def module(parser, args): + # Qualifiers to be used when querying the db for specs + constraint_qualifiers = { + 'refresh': { + 'installed': True, + 'known': True + }, + } + arguments.ConstraintAction.qualifiers.update(constraint_qualifiers) + module_type = args.module_type constraint = args.constraint try: diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py index a6f08d09ed..92de33873b 100644 --- a/lib/spack/spack/cmd/uninstall.py +++ b/lib/spack/spack/cmd/uninstall.py @@ -30,7 +30,6 @@ import spack import spack.cmd import spack.repository -from spack.cmd.find import display_specs description = "Remove an installed package" @@ -47,17 +46,6 @@ } -def ask_for_confirmation(message): - while True: - tty.msg(message + '[y/n]') - choice = raw_input().lower() - if choice == 'y': - break - elif choice == 'n': - raise SystemExit('Operation aborted') - tty.warn('Please reply either "y" or "n"') - - def setup_parser(subparser): subparser.add_argument( '-f', '--force', action='store_true', dest='force', @@ -99,7 +87,7 @@ def concretize_specs(specs, allow_multiple_matches=False, force=False): if not allow_multiple_matches and len(matching) > 1: tty.error("%s matches multiple packages:" % spec) print() - display_specs(matching, **display_args) + spack.cmd.display_specs(matching, **display_args) print() has_errors = True @@ -179,7 +167,7 @@ def uninstall(parser, args): tty.error("Will not uninstall %s" % spec.format("$_$@$%@$#", color=True)) print('') print("The following packages depend on it:") - display_specs(lst, **display_args) + spack.cmd.display_specs(lst, **display_args) print('') has_error = True elif args.dependents: @@ -193,9 +181,9 @@ def uninstall(parser, args): if not args.yes_to_all: tty.msg("The following packages will be uninstalled : ") print('') - display_specs(uninstall_list, **display_args) + spack.cmd.display_specs(uninstall_list, **display_args) print('') - ask_for_confirmation('Do you want to proceed ? ') + spack.cmd.ask_for_confirmation('Do you want to proceed ? ') # Uninstall everything on the list do_uninstall(uninstall_list, args.force) diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py index 371e9650e0..fa82db7733 100644 --- a/lib/spack/spack/test/cmd/find.py +++ b/lib/spack/spack/test/cmd/find.py @@ -27,11 +27,7 @@ import spack.cmd.find import unittest - -class Bunch(object): - - def __init__(self, **kwargs): - self.__dict__.update(kwargs) +from spack.util.pattern import Bunch class FindTest(unittest.TestCase): diff --git a/lib/spack/spack/util/pattern.py b/lib/spack/spack/util/pattern.py index 6d4bcb1039..6af39c87d8 100644 --- a/lib/spack/spack/util/pattern.py +++ b/lib/spack/spack/util/pattern.py @@ -114,3 +114,9 @@ def getter(*args, **kwargs): return wrapper_class return cls_decorator + + +class Bunch(object): + """Carries a bunch of named attributes (from Alex Martelli bunch)""" + def __init__(self, **kwargs): + self.__dict__.update(kwargs) From 3100c5948a7278bd1429c316a1c78264b5fafcb3 Mon Sep 17 00:00:00 2001 From: alalazo Date: Sat, 2 Jul 2016 12:14:30 +0200 Subject: [PATCH 10/11] module : added unit tests --- lib/spack/spack/cmd/module.py | 28 ++++++---- lib/spack/spack/test/__init__.py | 2 +- lib/spack/spack/test/cmd/module.py | 82 ++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 lib/spack/spack/test/cmd/module.py diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 2c47e9fcb6..374e71a4d8 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -32,12 +32,16 @@ import llnl.util.tty as tty import spack.cmd import spack.cmd.common.arguments as arguments -from llnl.util.filesystem import mkdirp +import llnl.util.filesystem as filesystem from spack.modules import module_types -#from spack.util.string import * description = "Manipulate module files" +# Dictionary that will be populated with the list of sub-commands +# Each sub-command must be callable and accept 3 arguments : +# - mtype : the type of the module file +# - specs : the list of specs to be processed +# - args : namespace containing the parsed command line arguments callbacks = {} @@ -51,6 +55,7 @@ def decorator(callback): def setup_parser(subparser): sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subparser_name') + # spack module refresh refresh_parser = sp.add_parser('refresh', help='Regenerate module files') refresh_parser.add_argument('--delete-tree', help='Delete the module file tree before refresh', action='store_true') @@ -71,11 +76,12 @@ def setup_parser(subparser): ) loads_parser.add_argument( '--input-only', action='store_false', dest='shell', - help='Generate input for module command (instead of a shell script)') - + help='Generate input for module command (instead of a shell script)' + ) loads_parser.add_argument( '-p', '--prefix', dest='prefix', default='', - help='Prepend to module names when issuing module load commands') + help='Prepend to module names when issuing module load commands' + ) arguments.add_common_arguments(loads_parser, ['constraint', 'module_type', 'recurse_dependencies']) @@ -86,8 +92,10 @@ class MultipleMatches(Exception): class NoMatch(Exception): pass + @subcommand('loads') def loads(mtype, specs, args): + """Prompt the list of modules associated with a list of specs""" # Get a comprehensive list of specs if args.recurse_dependencies: specs_from_user_constraint = specs[:] @@ -123,6 +131,7 @@ def loads(mtype, specs, args): d['name'] = mod print(prompt_template.format(**d)) + @subcommand('find') def find(mtype, specs, args): """ @@ -146,13 +155,14 @@ def find(mtype, specs, args): @subcommand('rm') def rm(mtype, specs, args): + """Deletes module files associated with items in specs""" module_cls = module_types[mtype] specs_with_modules = [spec for spec in specs if os.path.exists(module_cls(spec).file_name)] modules = [module_cls(spec) for spec in specs_with_modules] if not modules: tty.msg('No module file matches your query') - return + raise SystemExit(1) # Ask for confirmation if not args.yes_to_all: @@ -168,9 +178,7 @@ def rm(mtype, specs, args): @subcommand('refresh') def refresh(mtype, specs, args): - """Regenerate module files for installed packages - - """ + """Regenerate module files for item in specs""" # Prompt a message to the user about what is going to change if not specs: tty.msg('No package matches your query') @@ -205,7 +213,7 @@ def refresh(mtype, specs, args): tty.msg('Regenerating {name} module files'.format(name=mtype)) if os.path.isdir(cls.path) and args.delete_tree: shutil.rmtree(cls.path, ignore_errors=False) - mkdirp(cls.path) + filesystem.mkdirp(cls.path) for x in writers: x.write(overwrite=True) diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py index fb91f24721..dfbd73b91f 100644 --- a/lib/spack/spack/test/__init__.py +++ b/lib/spack/spack/test/__init__.py @@ -40,7 +40,7 @@ 'cc', 'link_tree', 'spec_yaml', 'optional_deps', 'make_executable', 'configure_guess', 'lock', 'database', 'namespace_trie', 'yaml', 'sbang', 'environment', 'cmd.find', - 'cmd.uninstall', 'cmd.test_install', 'cmd.test_compiler_cmd'] + 'cmd.uninstall', 'cmd.test_install', 'cmd.test_compiler_cmd', 'cmd.module'] def list_tests(): diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py new file mode 100644 index 0000000000..52628f5b3b --- /dev/null +++ b/lib/spack/spack/test/cmd/module.py @@ -0,0 +1,82 @@ +############################################################################## +# 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 argparse +import os.path + +import spack.cmd.module as module +import spack.modules as modules +import spack.test.mock_database + + +class TestModule(spack.test.mock_database.MockDatabase): + def _get_module_files(self, args): + return [ + modules.module_types[args.module_type](spec).file_name for spec in args.specs + ] + + def test_module_common_operations(self): + parser = argparse.ArgumentParser() + module.setup_parser(parser) + # Try to remove a non existing module [tcl] + args = parser.parse_args(['rm', 'doesnotexist']) + self.assertRaises(SystemExit, module.module, parser, args) + # Remove existing modules [tcl] + args = parser.parse_args(['rm', '-y', 'mpileaks']) + module_files = self._get_module_files(args) + for item in module_files: + self.assertTrue(os.path.exists(item)) + module.module(parser, args) + for item in module_files: + self.assertFalse(os.path.exists(item)) + # Add them back [tcl] + args = parser.parse_args(['refresh', '-y', 'mpileaks']) + module.module(parser, args) + for item in module_files: + self.assertTrue(os.path.exists(item)) + # TODO : test the --delete-tree option + # TODO : this requires having a separate directory for test modules + # Try to find a module with multiple matches + args = parser.parse_args(['find', 'mpileaks']) + self.assertRaises(SystemExit, module.module, parser, args) + # Try to find a module with no matches + args = parser.parse_args(['find', 'doesnotexist']) + self.assertRaises(SystemExit, module.module, parser, args) + # Try to find a module + args = parser.parse_args(['find', 'libelf']) + module.module(parser, args) + # Remove existing modules [dotkit] + args = parser.parse_args(['rm', '-y', '-m', 'dotkit', 'mpileaks']) + module_files = self._get_module_files(args) + for item in module_files: + self.assertTrue(os.path.exists(item)) + module.module(parser, args) + for item in module_files: + self.assertFalse(os.path.exists(item)) + # Add them back [dotkit] + args = parser.parse_args(['refresh', '-y', '-m', 'dotkit', 'mpileaks']) + module.module(parser, args) + for item in module_files: + self.assertTrue(os.path.exists(item)) + # TODO : add tests for loads and find to check the prompt format From 6d988dde8dcc2d666631151e5dab429811ffed37 Mon Sep 17 00:00:00 2001 From: alalazo Date: Sat, 2 Jul 2016 12:54:36 +0200 Subject: [PATCH 11/11] qa : fixed flake8 issues --- lib/spack/spack/cmd/__init__.py | 19 +++---- lib/spack/spack/cmd/common/arguments.py | 64 ++++++++++++---------- lib/spack/spack/cmd/module.py | 46 ++++++++++------ lib/spack/spack/cmd/uninstall.py | 42 +++++++++------ lib/spack/spack/modules.py | 4 +- lib/spack/spack/test/__init__.py | 6 ++- lib/spack/spack/test/cmd/module.py | 3 +- lib/spack/spack/util/pattern.py | 72 +++++++++++++++---------- 8 files changed, 153 insertions(+), 103 deletions(-) diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 02f6f5b98a..230115df50 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -37,7 +37,8 @@ # # Settings for commands that modify configuration # -# Commands that modify confguration By default modify the *highest* priority scope. +# Commands that modify confguration By default modify the *highest* +# priority scope. default_modify_scope = spack.config.highest_precedence_scope().name # Commands that list confguration list *all* scopes by default. default_list_scope = None @@ -49,7 +50,7 @@ ignore_files = r'^\.|^__init__.py$|^#' SETUP_PARSER = "setup_parser" -DESCRIPTION = "description" +DESCRIPTION = "description" command_path = os.path.join(spack.lib_path, "spack", "cmd") @@ -72,7 +73,7 @@ def get_module(name): module_name, fromlist=[name, SETUP_PARSER, DESCRIPTION], level=0) - attr_setdefault(module, SETUP_PARSER, lambda *args: None) # null-op + attr_setdefault(module, SETUP_PARSER, lambda *args: None) # null-op attr_setdefault(module, DESCRIPTION, "") fn_name = get_cmd_function_name(name) @@ -102,17 +103,17 @@ def parse_specs(args, **kwargs): specs = spack.spec.parse(args) for spec in specs: if concretize: - spec.concretize() # implies normalize + spec.concretize() # implies normalize elif normalize: spec.normalize() return specs - except spack.parse.ParseError, e: + except spack.parse.ParseError as e: tty.error(e.message, e.string, e.pos * " " + "^") sys.exit(1) - except spack.spec.SpecError, e: + except spack.spec.SpecError as e: tty.error(e.message) sys.exit(1) @@ -128,7 +129,7 @@ def elide_list(line_list, max_num=10): [1, 2, 3, '...', 6] """ if len(line_list) > max_num: - return line_list[:max_num-1] + ['...'] + line_list[-1:] + return line_list[:max_num - 1] + ['...'] + line_list[-1:] else: return line_list @@ -139,8 +140,8 @@ def disambiguate_spec(spec): tty.die("Spec '%s' matches no installed packages." % spec) elif len(matching_specs) > 1: - args = ["%s matches multiple packages." % spec, - "Matching packages:"] + args = ["%s matches multiple packages." % spec, + "Matching packages:"] args += [" " + str(s) for s in matching_specs] args += ["Use a more specific spec."] tty.die(*args) diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py index a50bac5ac5..af04170824 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -35,7 +35,7 @@ def add_common_arguments(parser, list_of_arguments): for argument in list_of_arguments: if argument not in _arguments: - message = 'Trying to add the non existing argument "{0}" to a command' + message = 'Trying to add the non existing argument "{0}" to a command' # NOQA: ignore=E501 raise KeyError(message.format(argument)) x = _arguments[argument] parser.add_argument(*x.flags, **x.kwargs) @@ -44,9 +44,9 @@ def add_common_arguments(parser, list_of_arguments): class ConstraintAction(argparse.Action): """Constructs a list of specs based on a constraint given on the command line - An instance of this class is supposed to be used as an argument action in a parser. - - It will read a constraint and will attach a list of matching specs to the namespace + An instance of this class is supposed to be used as an argument action + in a parser. It will read a constraint and will attach a list of matching + specs to the namespace """ qualifiers = {} @@ -59,30 +59,38 @@ def __call__(self, parser, namespace, values, option_string=None): specs = [x for x in specs if x.satisfies(values, strict=True)] namespace.specs = specs -_arguments['constraint'] = Bunch(flags=('constraint',), - kwargs={ - 'nargs': '*', - 'help': 'Optional constraint to select a subset of installed packages', - 'action': ConstraintAction - }) +parms = Bunch( + flags=('constraint',), + kwargs={ + 'nargs': '*', + 'help': 'Constraint to select a subset of installed packages', + 'action': ConstraintAction + }) +_arguments['constraint'] = parms -_arguments['module_type'] = Bunch(flags=('-m', '--module-type'), - kwargs={ - 'help': 'Type of module files', - 'default': 'tcl', - 'choices': spack.modules.module_types - }) +parms = Bunch( + flags=('-m', '--module-type'), + kwargs={ + 'help': 'Type of module files', + 'default': 'tcl', + 'choices': spack.modules.module_types + }) +_arguments['module_type'] = parms -_arguments['yes_to_all'] = Bunch(flags=('-y', '--yes-to-all'), - kwargs={ - 'action': 'store_true', - 'dest': 'yes_to_all', - 'help': 'Assume "yes" is the answer to every confirmation asked to the user.' - }) +parms = Bunch( + flags=('-y', '--yes-to-all'), + kwargs={ + 'action': 'store_true', + 'dest': 'yes_to_all', + 'help': 'Assume "yes" is the answer to every confirmation asked to the user.' # NOQA: ignore=E501 + }) +_arguments['yes_to_all'] = parms -_arguments['recurse_dependencies'] = Bunch(flags=('-r', '--dependencies'), - kwargs={ - 'action': 'store_true', - 'dest': 'recurse_dependencies', - 'help': 'Recursively traverse spec dependencies' - }) +parms = Bunch( + flags=('-r', '--dependencies'), + kwargs={ + 'action': 'store_true', + 'dest': 'recurse_dependencies', + 'help': 'Recursively traverse spec dependencies' + }) +_arguments['recurse_dependencies'] = parms diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py index 374e71a4d8..a10e36e077 100644 --- a/lib/spack/spack/cmd/module.py +++ b/lib/spack/spack/cmd/module.py @@ -58,8 +58,14 @@ def setup_parser(subparser): # spack module refresh refresh_parser = sp.add_parser('refresh', help='Regenerate module files') - refresh_parser.add_argument('--delete-tree', help='Delete the module file tree before refresh', action='store_true') - arguments.add_common_arguments(refresh_parser, ['constraint', 'module_type', 'yes_to_all']) + refresh_parser.add_argument( + '--delete-tree', + help='Delete the module file tree before refresh', + action='store_true' + ) + arguments.add_common_arguments( + refresh_parser, ['constraint', 'module_type', 'yes_to_all'] + ) # spack module find find_parser = sp.add_parser('find', help='Find module files for packages') @@ -67,12 +73,14 @@ def setup_parser(subparser): # spack module rm rm_parser = sp.add_parser('rm', help='Remove module files') - arguments.add_common_arguments(rm_parser, ['constraint', 'module_type', 'yes_to_all']) + arguments.add_common_arguments( + rm_parser, ['constraint', 'module_type', 'yes_to_all'] + ) # spack module loads loads_parser = sp.add_parser( 'loads', - help='Prompt the list of modules associated with packages that satisfy a constraint' + help='Prompt the list of modules associated with a constraint' ) loads_parser.add_argument( '--input-only', action='store_false', dest='shell', @@ -82,7 +90,9 @@ def setup_parser(subparser): '-p', '--prefix', dest='prefix', default='', help='Prepend to module names when issuing module load commands' ) - arguments.add_common_arguments(loads_parser, ['constraint', 'module_type', 'recurse_dependencies']) + arguments.add_common_arguments( + loads_parser, ['constraint', 'module_type', 'recurse_dependencies'] + ) class MultipleMatches(Exception): @@ -100,20 +110,20 @@ def loads(mtype, specs, args): if args.recurse_dependencies: specs_from_user_constraint = specs[:] specs = [] - # FIXME : during module file creation nodes seem to be visited multiple - # FIXME : times even if cover='nodes' is given. This work around permits - # FIXME : to get a unique list of spec anyhow. Do we miss a merge - # FIXME : step among nodes that refer to the same package? + # FIXME : during module file creation nodes seem to be visited + # FIXME : multiple times even if cover='nodes' is given. This + # FIXME : work around permits to get a unique list of spec anyhow. # FIXME : (same problem as in spack/modules.py) seen = set() seen_add = seen.add for spec in specs_from_user_constraint: specs.extend( - [item for item in spec.traverse(order='post', cover='nodes') if not (item in seen or seen_add(item))] + [item for item in spec.traverse(order='post', cover='nodes') if not (item in seen or seen_add(item))] # NOQA: ignore=E501 ) module_cls = module_types[mtype] - modules = [(spec, module_cls(spec).use_name) for spec in specs if os.path.exists(module_cls(spec).file_name)] + modules = [(spec, module_cls(spec).use_name) + for spec in specs if os.path.exists(module_cls(spec).file_name)] module_commands = { 'tcl': 'module load ', @@ -127,7 +137,8 @@ def loads(mtype, specs, args): prompt_template = '{comment}{command}{prefix}{name}' for spec, mod in modules: - d['comment'] = '' if not args.shell else '# {0}\n'.format(spec.format()) + d['comment'] = '' if not args.shell else '# {0}\n'.format( + spec.format()) d['name'] = mod print(prompt_template.format(**d)) @@ -157,7 +168,8 @@ def find(mtype, specs, args): def rm(mtype, specs, args): """Deletes module files associated with items in specs""" module_cls = module_types[mtype] - specs_with_modules = [spec for spec in specs if os.path.exists(module_cls(spec).file_name)] + specs_with_modules = [ + spec for spec in specs if os.path.exists(module_cls(spec).file_name)] modules = [module_cls(spec) for spec in specs_with_modules] if not modules: @@ -166,7 +178,7 @@ def rm(mtype, specs, args): # Ask for confirmation if not args.yes_to_all: - tty.msg('You are about to remove {0} module files the following specs:\n'.format(mtype)) + tty.msg('You are about to remove {0} module files the following specs:\n'.format(mtype)) # NOQA: ignore=E501 spack.cmd.display_specs(specs_with_modules, long=True) print('') spack.cmd.ask_for_confirmation('Do you want to proceed ? ') @@ -185,7 +197,7 @@ def refresh(mtype, specs, args): return if not args.yes_to_all: - tty.msg('You are about to regenerate {name} module files for the following specs:\n'.format(name=mtype)) + tty.msg('You are about to regenerate {name} module files for the following specs:\n'.format(name=mtype)) # NOQA: ignore=E501 spack.cmd.display_specs(specs, long=True) print('') spack.cmd.ask_for_confirmation('Do you want to proceed ? ') @@ -233,11 +245,11 @@ def module(parser, args): try: callbacks[args.subparser_name](module_type, args.specs, args) except MultipleMatches: - message = 'the constraint \'{query}\' matches multiple packages, and this is not allowed in this context' + message = 'the constraint \'{query}\' matches multiple packages, and this is not allowed in this context' # NOQA: ignore=E501 tty.error(message.format(query=constraint)) for s in args.specs: sys.stderr.write(s.format(color=True) + '\n') raise SystemExit(1) except NoMatch: - message = 'the constraint \'{query}\' match no package, and this is not allowed in this context' + message = 'the constraint \'{query}\' match no package, and this is not allowed in this context' # NOQA: ignore=E501 tty.die(message.format(query=constraint)) diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py index 92de33873b..a17b7c685c 100644 --- a/lib/spack/spack/cmd/uninstall.py +++ b/lib/spack/spack/cmd/uninstall.py @@ -42,7 +42,7 @@ display_args = { 'long': True, 'show_flags': True, - 'variants':True + 'variants': True } @@ -53,32 +53,37 @@ def setup_parser(subparser): subparser.add_argument( '-a', '--all', action='store_true', dest='all', help="USE CAREFULLY. Remove ALL installed packages that match each " + - "supplied spec. i.e., if you say uninstall libelf, ALL versions of " + - "libelf are uninstalled. This is both useful and dangerous, like rm -r.") + "supplied spec. i.e., if you say uninstall libelf, ALL versions of " + # NOQA: ignore=E501 + "libelf are uninstalled. This is both useful and dangerous, like rm -r.") # NOQA: ignore=E501 subparser.add_argument( '-d', '--dependents', action='store_true', dest='dependents', - help='Also uninstall any packages that depend on the ones given via command line.' + help='Also uninstall any packages that depend on the ones given via command line.' # NOQA: ignore=E501 ) subparser.add_argument( '-y', '--yes-to-all', action='store_true', dest='yes_to_all', - help='Assume "yes" is the answer to every confirmation asked to the user.' + help='Assume "yes" is the answer to every confirmation asked to the user.' # NOQA: ignore=E501 ) - subparser.add_argument('packages', nargs=argparse.REMAINDER, help="specs of packages to uninstall") + subparser.add_argument( + 'packages', + nargs=argparse.REMAINDER, + help="specs of packages to uninstall" + ) def concretize_specs(specs, allow_multiple_matches=False, force=False): - """ - Returns a list of specs matching the non necessarily concretized specs given from cli + """Returns a list of specs matching the non necessarily + concretized specs given from cli Args: specs: list of specs to be matched against installed packages - allow_multiple_matches : boolean (if True multiple matches for each item in specs are admitted) + allow_multiple_matches : if True multiple matches are admitted Return: list of specs """ - specs_from_cli = [] # List of specs that match expressions given via command line + # List of specs that match expressions given via command line + specs_from_cli = [] has_errors = False for spec in specs: matching = spack.installed_db.query(spec) @@ -104,8 +109,8 @@ def concretize_specs(specs, allow_multiple_matches=False, force=False): def installed_dependents(specs): - """ - Returns a dictionary that maps a spec with a list of its installed dependents + """Returns a dictionary that maps a spec with a list of its + installed dependents Args: specs: list of specs to be checked for dependents @@ -135,7 +140,7 @@ def do_uninstall(specs, force): try: # should work if package is known to spack packages.append(item.package) - except spack.repository.UnknownPackageError as e: + except spack.repository.UnknownPackageError: # The package.py file has gone away -- but still # want to uninstall. spack.Package(item).do_uninstall(force=True) @@ -157,14 +162,17 @@ def uninstall(parser, args): with spack.installed_db.write_transaction(): specs = spack.cmd.parse_specs(args.packages) # Gets the list of installed specs that match the ones give via cli - uninstall_list = concretize_specs(specs, args.all, args.force) # takes care of '-a' is given in the cli - dependent_list = installed_dependents(uninstall_list) # takes care of '-d' + # takes care of '-a' is given in the cli + uninstall_list = concretize_specs(specs, args.all, args.force) + dependent_list = installed_dependents( + uninstall_list) # takes care of '-d' # Process dependent_list and update uninstall_list has_error = False if dependent_list and not args.dependents and not args.force: for spec, lst in dependent_list.items(): - tty.error("Will not uninstall %s" % spec.format("$_$@$%@$#", color=True)) + tty.error("Will not uninstall %s" % + spec.format("$_$@$%@$#", color=True)) print('') print("The following packages depend on it:") spack.cmd.display_specs(lst, **display_args) @@ -176,7 +184,7 @@ def uninstall(parser, args): uninstall_list = list(set(uninstall_list)) if has_error: - tty.die('You can use spack uninstall --dependents to uninstall these dependencies as well') + tty.die('You can use spack uninstall --dependents to uninstall these dependencies as well') # NOQA: ignore=E501 if not args.yes_to_all: tty.msg("The following packages will be uninstalled : ") diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py index 82016feb84..e648c6140f 100644 --- a/lib/spack/spack/modules.py +++ b/lib/spack/spack/modules.py @@ -188,7 +188,8 @@ def parse_config_options(module_generator): ##### # Automatic loading loads - module_file_actions['hash_length'] = module_configuration.get('hash_length', 7) + module_file_actions['hash_length'] = module_configuration.get( + 'hash_length', 7) module_file_actions['autoload'] = dependencies( module_generator.spec, module_file_actions.get('autoload', 'none')) # Prerequisites @@ -238,6 +239,7 @@ class EnvModule(object): formats = {} class __metaclass__(type): + def __init__(cls, name, bases, dict): type.__init__(cls, name, bases, dict) if cls.name != 'env_module' and cls.name in CONFIGURATION[ diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py index dfbd73b91f..bf46e011b1 100644 --- a/lib/spack/spack/test/__init__.py +++ b/lib/spack/spack/test/__init__.py @@ -32,7 +32,8 @@ from spack.test.tally_plugin import Tally """Names of tests to be included in Spack's test suite""" -test_names = ['architecture', 'versions', 'url_parse', 'url_substitution', 'packages', 'stage', +test_names = ['architecture', 'versions', 'url_parse', 'url_substitution', + 'packages', 'stage', 'spec_syntax', 'spec_semantics', 'spec_dag', 'concretize', 'multimethod', 'install', 'package_sanity', 'config', 'directory_layout', 'pattern', 'python_version', 'git_fetch', @@ -40,7 +41,8 @@ 'cc', 'link_tree', 'spec_yaml', 'optional_deps', 'make_executable', 'configure_guess', 'lock', 'database', 'namespace_trie', 'yaml', 'sbang', 'environment', 'cmd.find', - 'cmd.uninstall', 'cmd.test_install', 'cmd.test_compiler_cmd', 'cmd.module'] + 'cmd.uninstall', 'cmd.test_install', + 'cmd.test_compiler_cmd', 'cmd.module'] def list_tests(): diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index 52628f5b3b..36a4a73fe6 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/lib/spack/spack/test/cmd/module.py @@ -31,9 +31,10 @@ class TestModule(spack.test.mock_database.MockDatabase): + def _get_module_files(self, args): return [ - modules.module_types[args.module_type](spec).file_name for spec in args.specs + modules.module_types[args.module_type](spec).file_name for spec in args.specs # NOQA: ignore=E501 ] def test_module_common_operations(self): diff --git a/lib/spack/spack/util/pattern.py b/lib/spack/spack/util/pattern.py index 6af39c87d8..bc5e9d2ffe 100644 --- a/lib/spack/spack/util/pattern.py +++ b/lib/spack/spack/util/pattern.py @@ -28,42 +28,50 @@ def composite(interface=None, method_list=None, container=list): - """ - Returns a class decorator that patches a class adding all the methods it needs to be a composite for a given - interface. + """Returns a class decorator that patches a class adding all the methods + it needs to be a composite for a given interface. - :param interface: class exposing the interface to which the composite object must conform. Only non-private and - non-special methods will be taken into account + :param interface: class exposing the interface to which the composite + object must conform. Only non-private and non-special methods will be + taken into account :param method_list: names of methods that should be part of the composite - :param container: container for the composite object (default = list). Must fulfill the MutableSequence contract. - The composite class will expose the container API to manage object composition + :param container: container for the composite object (default = list). + Must fulfill the MutableSequence contract. The composite class will expose + the container API to manage object composition :return: class decorator """ - # Check if container fulfills the MutableSequence contract and raise an exception if it doesn't - # The patched class returned by the decorator will inherit from the container class to expose the - # interface needed to manage objects composition + # Check if container fulfills the MutableSequence contract and raise an + # exception if it doesn't. The patched class returned by the decorator will + # inherit from the container class to expose the interface needed to manage + # objects composition if not issubclass(container, collections.MutableSequence): raise TypeError("Container must fulfill the MutableSequence contract") - # Check if at least one of the 'interface' or the 'method_list' arguments are defined + # Check if at least one of the 'interface' or the 'method_list' arguments + # are defined if interface is None and method_list is None: - raise TypeError("Either 'interface' or 'method_list' must be defined on a call to composite") + raise TypeError("Either 'interface' or 'method_list' must be defined on a call to composite") # NOQA : ignore=E501 def cls_decorator(cls): - # Retrieve the base class of the composite. Inspect its methods and decide which ones will be overridden + # Retrieve the base class of the composite. Inspect its methods and + # decide which ones will be overridden def no_special_no_private(x): return inspect.ismethod(x) and not x.__name__.startswith('_') - # Patch the behavior of each of the methods in the previous list. This is done associating an instance of the - # descriptor below to any method that needs to be patched. + # Patch the behavior of each of the methods in the previous list. + # This is done associating an instance of the descriptor below to + # any method that needs to be patched. class IterateOver(object): + """Decorator used to patch methods in a composite. + + It iterates over all the items in the instance containing the + associated attribute and calls for each of them an attribute + with the same name """ - Decorator used to patch methods in a composite. It iterates over all the items in the instance containing the - associated attribute and calls for each of them an attribute with the same name - """ + def __init__(self, name, func=None): self.name = name self.func = func @@ -72,8 +80,9 @@ def __get__(self, instance, owner): def getter(*args, **kwargs): for item in instance: getattr(item, self.name)(*args, **kwargs) - # If we are using this descriptor to wrap a method from an interface, then we must conditionally - # use the `functools.wraps` decorator to set the appropriate fields. + # If we are using this descriptor to wrap a method from an + # interface, then we must conditionally use the + # `functools.wraps` decorator to set the appropriate fields if self.func is not None: getter = functools.wraps(self.func)(getter) return getter @@ -81,7 +90,8 @@ def getter(*args, **kwargs): dictionary_for_type_call = {} # Construct a dictionary with the methods explicitly passed as name if method_list is not None: - # python@2.7: method_list_dict = {name: IterateOver(name) for name in method_list} + # python@2.7: method_list_dict = {name: IterateOver(name) for name + # in method_list} method_list_dict = {} for name in method_list: method_list_dict[name] = IterateOver(name) @@ -89,28 +99,33 @@ def getter(*args, **kwargs): # Construct a dictionary with the methods inspected from the interface if interface is not None: ########## - # python@2.7: interface_methods = {name: method for name, method in inspect.getmembers(interface, predicate=no_special_no_private)} + # python@2.7: interface_methods = {name: method for name, method in + # inspect.getmembers(interface, predicate=no_special_no_private)} interface_methods = {} - for name, method in inspect.getmembers(interface, predicate=no_special_no_private): + for name, method in inspect.getmembers(interface, predicate=no_special_no_private): # NOQA: ignore=E501 interface_methods[name] = method ########## - # python@2.7: interface_methods_dict = {name: IterateOver(name, method) for name, method in interface_methods.iteritems()} + # python@2.7: interface_methods_dict = {name: IterateOver(name, + # method) for name, method in interface_methods.iteritems()} interface_methods_dict = {} for name, method in interface_methods.iteritems(): interface_methods_dict[name] = IterateOver(name, method) ########## dictionary_for_type_call.update(interface_methods_dict) - # Get the methods that are defined in the scope of the composite class and override any previous definition + # Get the methods that are defined in the scope of the composite + # class and override any previous definition ########## - # python@2.7: cls_method = {name: method for name, method in inspect.getmembers(cls, predicate=inspect.ismethod)} + # python@2.7: cls_method = {name: method for name, method in + # inspect.getmembers(cls, predicate=inspect.ismethod)} cls_method = {} - for name, method in inspect.getmembers(cls, predicate=inspect.ismethod): + for name, method in inspect.getmembers(cls, predicate=inspect.ismethod): # NOQA: ignore=E501 cls_method[name] = method ########## dictionary_for_type_call.update(cls_method) # Generate the new class on the fly and return it # FIXME : inherit from interface if we start to use ABC classes? - wrapper_class = type(cls.__name__, (cls, container), dictionary_for_type_call) + wrapper_class = type(cls.__name__, (cls, container), + dictionary_for_type_call) return wrapper_class return cls_decorator @@ -118,5 +133,6 @@ def getter(*args, **kwargs): class Bunch(object): """Carries a bunch of named attributes (from Alex Martelli bunch)""" + def __init__(self, **kwargs): self.__dict__.update(kwargs)