From 26a55ff749b0a245ee08a29fe0867b5ba161a170 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 29 Oct 2018 02:40:23 -0700 Subject: env: move `env uninstall` into `spack uninstall` - uninstall now: - restricts its spec search to the current environment - removes uninstalled specs from the current environment - reports envs that still need specs you're trying to uninstall - removed spack env uninstall command - updated tests --- lib/spack/spack/cmd/__init__.py | 12 ++- lib/spack/spack/cmd/common/arguments.py | 2 +- lib/spack/spack/cmd/env.py | 21 +---- lib/spack/spack/cmd/uninstall.py | 133 +++++++++++++++++++++++++------- lib/spack/spack/environment.py | 61 ++++++++++----- lib/spack/spack/test/cmd/env.py | 34 +++++++- 6 files changed, 191 insertions(+), 72 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 39a5bcd234..e207617009 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -7,6 +7,7 @@ from __future__ import print_function import os import re +import sys import llnl.util.tty as tty from llnl.util.lang import attr_setdefault, index_by @@ -207,6 +208,7 @@ def display_specs(specs, args=None, **kwargs): namespace (bool): Print namespaces along with names show_flags (bool): Show compiler flags with specs variants (bool): Show variants with specs + indent (int): indent each line this much """ def get_arg(name, default=None): @@ -225,6 +227,9 @@ def display_specs(specs, args=None, **kwargs): full_compiler = get_arg('show_full_compiler', False) variants = get_arg('variants', False) + indent = get_arg('indent', 0) + ispace = indent * ' ' + hlen = 7 if get_arg('very_long', False): hashes = True @@ -255,6 +260,7 @@ def display_specs(specs, args=None, **kwargs): # If they don't have a compiler / architecture attached to them, # then skip the header if architecture is not None or compiler is not None: + sys.stdout.write(ispace) tty.hline(colorize(header), char='-') specs = index[(architecture, compiler)] @@ -269,7 +275,7 @@ def display_specs(specs, args=None, **kwargs): for abbrv, spec in zip(abbreviated, specs): prefix = gray_hash(spec, hlen) if hashes else '' - print(prefix + (format % (abbrv, spec.prefix))) + print(ispace + prefix + (format % (abbrv, spec.prefix))) elif mode == 'deps': for spec in specs: @@ -290,13 +296,13 @@ def display_specs(specs, args=None, **kwargs): return string - colify(fmt(s) for s in specs) + colify((fmt(s) for s in specs), indent=indent) # 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.cformat(format_string) + '\n') + print(ispace + hsh + spec.cformat(format_string)) else: raise ValueError( diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py index 6b20fbc014..671dde4fa0 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -56,7 +56,7 @@ class ConstraintAction(argparse.Action): # only its installed packages. env = spack.environment.active if env: - kwargs['hashes'] = set(env.specs_by_hash.keys()) + kwargs['hashes'] = set(env.all_hashes()) # return everything for an empty query. if not qspecs: diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 776f6f0544..2d5f95f640 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -34,7 +34,6 @@ subcommands = [ ['list', 'ls'], ['status', 'st'], 'loads', - 'uninstall', ] @@ -234,7 +233,7 @@ def env_remove(args): for env_name in args.env: env = ev.disambiguate(env_name) - if ev.active and ev.active.path == env.path: + if env.active: tty.die("Environment %s can't be removed while activated.") env.destroy() @@ -250,7 +249,7 @@ def env_list_setup_parser(subparser): def env_list(args): - names = ev.list_environments() + names = ev.all_environment_names() color_names = [] for name in names: @@ -268,21 +267,6 @@ def env_list(args): colify(color_names, indent=4) -# REMOVE -# env uninstall -# -def env_uninstall_setup_parser(subparser): - """uninstall packages from an environment""" - subparser.add_argument( - 'env', nargs='?', help='uninstall all packages in this environment') - spack.cmd.uninstall.add_common_arguments(subparser) - - -def env_uninstall(args): - env = ev.get_env(args, 'env uninstall') - env.uninstall(args) - - # # env status # @@ -308,6 +292,7 @@ def env_status(args): hashlen=None if args.very_long else 7, install_status=True) + # # env loads # diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py index 8fea7c0ac2..6425e88d9e 100644 --- a/lib/spack/spack/cmd/uninstall.py +++ b/lib/spack/spack/cmd/uninstall.py @@ -8,12 +8,14 @@ from __future__ import print_function import argparse import spack.cmd +import spack.environment as ev import spack.package import spack.cmd.common.arguments as arguments import spack.repo import spack.store from llnl.util import tty +from llnl.util.tty.colify import colify description = "remove installed packages" section = "build" @@ -28,14 +30,16 @@ error_message = """You can either: display_args = { 'long': True, 'show_flags': True, - 'variants': True + 'variants': True, + 'indent': 4, } def add_common_arguments(subparser): subparser.add_argument( '-f', '--force', action='store_true', dest='force', - help="remove regardless of whether other packages depend on this one") + help="remove regardless of whether other packages or environments " + "depend on this one") arguments.add_common_arguments( subparser, ['recurse_dependents', 'yes_to_all']) @@ -44,10 +48,12 @@ def setup_parser(subparser): add_common_arguments(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 `uninstall --all libelf`," - " ALL versions of `libelf` are uninstalled. if no spec is " - "supplied all installed software will be uninstalled.") + help="USE CAREFULLY. Remove ALL installed packages that match each " + "supplied spec. i.e., if you `uninstall --all libelf`," + " ALL versions of `libelf` are uninstalled. If no spec is " + "supplied, all installed packages will be uninstalled. " + "If used in an environment, all packages in the environment " + "will be uninstalled.") subparser.add_argument( 'packages', @@ -66,11 +72,13 @@ def find_matching_specs(specs, allow_multiple_matches=False, force=False): Return: list of specs """ + hashes = ev.active.all_hashes() if ev.active else None + # List of specs that match expressions given via command line specs_from_cli = [] has_errors = False for spec in specs: - matching = spack.store.db.query(spec) + matching = spack.store.db.query(spec, hashes=hashes) # For each spec provided, make sure it refers to only one package. # Fail and ask user to be unambiguous if it doesn't if not allow_multiple_matches and len(matching) > 1: @@ -82,11 +90,14 @@ def find_matching_specs(specs, allow_multiple_matches=False, force=False): # No installed package matches the query if len(matching) == 0 and spec is not any: - tty.error('{0} does not match any installed packages.'.format( - spec)) - has_errors = True + if ev.active: + pkg_type = "packages in environment '%s'" % ev.active.name + else: + pkg_type = 'installed packages' + tty.die('{0} does not match any {1}.'.format(spec, pkg_type)) specs_from_cli.extend(matching) + if has_errors: tty.die(error_message) @@ -94,14 +105,14 @@ def find_matching_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 + """Map each spec to a list of its installed dependents. Args: - specs: list of specs to be checked for dependents + specs (list): list of Specs Returns: - dictionary of installed dependents + (dict): mapping from spec to lists of Environments + """ dependents = {} for item in specs: @@ -114,6 +125,28 @@ def installed_dependents(specs): return dependents +def dependent_environments(specs): + """Map each spec to environments that depend on it. + + This excludes the active environment, because we allow uninstalling + from the active environment. + + Args: + specs (list): list of Specs + Returns: + (dict): mapping from spec to lists of Environments + + """ + dependents = {} + for env in ev.all_environments(): + if not env.active: + hashes = set([s.dag_hash() for s in env.all_specs()]) + for spec in specs: + if spec.dag_hash() in hashes: + dependents.setdefault(spec, []).append(env) + return dependents + + def do_uninstall(specs, force): """ Uninstalls all the specs in a list. @@ -132,6 +165,12 @@ def do_uninstall(specs, force): # want to uninstall. spack.package.Package.uninstall_by_spec(item, force=True) + if ev.active: + try: + ev.active.remove(item, force=True) + except ev.EnvError: + pass # ignore errors from specs that are not roots + # Sort packages to be uninstalled by the number of installed dependents # This ensures we do things in the right order def num_installed_deps(pkg): @@ -143,6 +182,10 @@ def do_uninstall(specs, force): for item in packages: item.do_uninstall(force=force) + # write any changes made to the active environment + if ev.active: + ev.active.write() + def get_uninstall_list(args, specs): # Gets the list of installed specs that match the ones give via cli @@ -150,25 +193,54 @@ def get_uninstall_list(args, specs): uninstall_list = find_matching_specs(specs, args.all, args.force) # Takes care of '-R' - dependent_list = installed_dependents(uninstall_list) - - # 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.cformat("$_$@$%@$/")) - print('') - print('The following packages depend on it:') - spack.cmd.display_specs(lst, **display_args) - print('') - has_error = True + spec_dependents = installed_dependents(uninstall_list) + spec_envs = dependent_environments(uninstall_list) + + # Process spec_dependents and update uninstall_list + has_error = not args.force and ( + (spec_dependents and not args.dependents) or + spec_envs) + + # say why each problem spec is needed + if has_error: + specs = set(list(spec_dependents.keys()) + list(spec_envs.keys())) + for i, spec in enumerate(sorted(specs)): + # space out blocks of reasons + if i > 0: + print() + + tty.info("Will not uninstall %s" % spec.cformat("$_$@$%@$/"), + format='*r') + + dependents = spec_dependents.get(spec) + if dependents: + print('The following packages depend on it:') + spack.cmd.display_specs(dependents, **display_args) + + envs = spec_envs.get(spec) + if envs: + print('It is used by the following environments:') + colify([e.name for e in envs], indent=4) + + msgs = [] + if spec_dependents: + msgs.append( + 'use `spack uninstall --dependents` to uninstall dependents ' + 'as well.') + if spec_envs: + msgs.append( + 'use `spack env remove` to remove environments, or ' + '`spack remove` to remove specs from environments.') + if ev.active: + msgs.append('consider using `spack remove` to remove the spec ' + 'from this environment') + print() + tty.die('There are still dependents.', *msgs) + elif args.dependents: - for key, lst in dependent_list.items(): + for spec, lst in spec_dependents.items(): uninstall_list.extend(lst) uninstall_list = list(set(uninstall_list)) - if has_error: - tty.die('Use `spack uninstall --dependents` ' - 'to uninstall these dependencies as well.') return uninstall_list @@ -196,5 +268,6 @@ def uninstall(parser, args): tty.die('uninstall requires at least one package argument.', ' Use `spack uninstall --all` to uninstall ALL packages.') + # [any] here handles the --all case by forcing all specs to be returned uninstall_specs( args, spack.cmd.parse_specs(args.packages) if args.packages else [any]) diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 87f4d31026..c221aab855 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -242,7 +242,7 @@ def config_dict(yaml_data): return yaml_data[key] -def list_environments(): +def all_environment_names(): """List the names of environments that currently exist.""" # just return empty if the env path does not exist. A read-only # operation like list should not try to create a directory. @@ -258,6 +258,12 @@ def list_environments(): return names +def all_environments(): + """Generator for all named Environments.""" + for name in all_environment_names(): + yield read(name) + + def validate(data, filename=None): global _validator if _validator is None: @@ -362,6 +368,11 @@ class Environment(object): else: return self.path + @property + def active(self): + """True if this environment is currently active.""" + return active and self.path == active.path + @property def manifest_path(self): """Path to spack.yaml file in this environment.""" @@ -480,8 +491,7 @@ class Environment(object): specs_hashes = zip( self.concretized_user_specs, self.concretized_order) matches = [ - s for s, h in specs_hashes - if s.satisfies(query_spec) or query_spec.dag_hash() == h] + s for s, h in specs_hashes if query_spec.dag_hash() == h] if not matches: raise EnvError("Not found: {0}".format(query_spec)) @@ -615,12 +625,6 @@ class Environment(object): os.remove(build_log_link) os.symlink(spec.package.build_log_path, build_log_link) - def uninstall(self, args): - """Uninstall all the specs in an Environment.""" - specs = self._get_environment_specs(recurse_dependencies=True) - args.all = False - spack.cmd.uninstall.uninstall_specs(args, specs) - def status(self, stream, **kwargs): """List the specs in an environment.""" tty.msg('In environment %s' % self.name) @@ -635,10 +639,10 @@ class Environment(object): current = [(s, c) for s, c in concretized if s in self.user_specs] def write_kind(s): - color.cwrite('@c{%s}\n' % str(s), stream) + color.cwrite('@c{%s}\n' % color.cescape(s), stream) def write_user_spec(s, c): - color.cwrite('@%s{----} %s\n' % (c, str(s)), stream) + color.cwrite('@%s{----} %s\n' % (c, color.cescape(s)), stream) if added: write_kind('added:') @@ -661,23 +665,44 @@ class Environment(object): write_user_spec(s, 'r') stream.write(c.tree(**kwargs)) + def all_specs_by_hash(self): + """Map of hashes to spec for all specs in this environment.""" + hashes = {} + for h in self.concretized_order: + specs = self.specs_by_hash[h].traverse(deptype=('link', 'run')) + for spec in specs: + hashes[spec.dag_hash()] = spec + return hashes + + def all_specs(self): + """Return all specs, even those a user spec would shadow.""" + return sorted(self.all_specs_by_hash().values()) + + def all_hashes(self): + """Return all specs, even those a user spec would shadow.""" + return list(self.all_specs_by_hash().keys()) + def _get_environment_specs(self, recurse_dependencies=True): """Returns the specs of all the packages in an environment. + If these specs appear under different user_specs, only one copy - is added to the list returned.""" + is added to the list returned. + """ package_to_spec = {} spec_list = list() for spec_hash in self.concretized_order: spec = self.specs_by_hash[spec_hash] - specs = spec.traverse(deptype=('link', 'run')) \ - if recurse_dependencies else (spec,) + specs = (spec.traverse(deptype=('link', 'run')) + if recurse_dependencies else (spec,)) + for dep in specs: - if dep.name in package_to_spec: - tty.warn("{0} takes priority over {1}" - .format(package_to_spec[dep.name].format(), - dep.format())) + prior = package_to_spec.get(dep.name) + if prior and prior != dep: + tty.debug("{0} takes priority over {1}" + .format(package_to_spec[dep.name].format(), + dep.format())) else: package_to_spec[dep.name] = dep spec_list.append(dep) diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index b49bcbcb9b..2828f5806c 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -27,6 +27,7 @@ add = SpackCommand('add') remove = SpackCommand('remove') concretize = SpackCommand('concretize') stage = SpackCommand('stage') +uninstall = SpackCommand('uninstall') def test_add(): @@ -541,9 +542,38 @@ def test_env_commands_die_with_no_env_arg(): # these have an optional env arg and raise errors via tty.die with pytest.raises(spack.main.SpackCommandError): env('loads') - with pytest.raises(spack.main.SpackCommandError): - env('uninstall') # This should NOT raise an error with no environment # it just tells the user there isn't an environment env('status') + + +def test_env_blocks_uninstall(mock_stage, mock_fetch, install_mockery): + env('create', 'test') + with ev.read('test'): + add('mpileaks') + install('--fake') + + out = uninstall('mpileaks', fail_on_error=False) + assert uninstall.returncode == 1 + assert 'used by the following environments' in out + + +def test_uninstall_removes_from_env(mock_stage, mock_fetch, install_mockery): + env('create', 'test') + with ev.read('test'): + add('mpileaks') + add('libelf') + install('--fake') + + test = ev.read('test') + assert any(s.name == 'mpileaks' for s in test.specs_by_hash.values()) + assert any(s.name == 'libelf' for s in test.specs_by_hash.values()) + + with ev.read('test'): + uninstall('-ya') + + test = ev.read('test') + assert not test.specs_by_hash + assert not test.concretized_order + assert not test.user_specs -- cgit v1.2.3-60-g2f50