From a2b4146e104fc00442b847306df2240d2bb6c0b6 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 19 Dec 2016 09:09:53 -0800 Subject: Fixes for various hash issues (#2626) * Better output for disambiguate_specs() * Fix wrong exception name. * Fix satsifies(): concrete specs require matching by hash. - Fixes uninstall by hash and other places where we need to match a specific spec. - Fix an error in provider_index (satisfies() call was backwards) - Fix an error in satisfies_dependencies(): checks were too shallow. * Fix default args in Spec.tree() * Move installed_dependents() to DB to avoid unknown package error. * Make `spack find` and `sapck.store.db.query()` faster for hashes. * Add a test to ensure satisfies() respects concrete Specs' hashes. --- lib/spack/spack/cmd/__init__.py | 4 +++- lib/spack/spack/cmd/common/arguments.py | 18 ++++++++++++----- lib/spack/spack/cmd/dependents.py | 16 +++++++++------ lib/spack/spack/cmd/find.py | 3 +++ lib/spack/spack/cmd/uninstall.py | 8 ++++++-- lib/spack/spack/database.py | 21 ++++++++++++++++++++ lib/spack/spack/package.py | 20 +------------------ lib/spack/spack/provider_index.py | 3 ++- lib/spack/spack/spec.py | 35 ++++++++++----------------------- lib/spack/spack/test/spec_semantics.py | 15 ++++++++++++++ 10 files changed, 84 insertions(+), 59 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index 8000c9f1f2..e712ba8e1d 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -144,7 +144,9 @@ def disambiguate_spec(spec): elif len(matching_specs) > 1: args = ["%s matches multiple packages." % spec, "Matching packages:"] - args += [" " + str(s) for s in matching_specs] + color = sys.stdout.isatty() + args += [colorize(" @K{%s} " % s.dag_hash(7), color=color) + + s.format('$_$@$%@$=', color=color) 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 1839132218..53e75a4f2c 100644 --- a/lib/spack/spack/cmd/common/arguments.py +++ b/lib/spack/spack/cmd/common/arguments.py @@ -25,6 +25,7 @@ import argparse +import spack.cmd import spack.store import spack.modules from spack.util.pattern import Args @@ -59,11 +60,18 @@ class ConstraintAction(argparse.Action): namespace.specs = self._specs def _specs(self, **kwargs): - specs = [s for s in spack.store.db.query(**kwargs)] - values = ' '.join(self.values) - if values: - specs = [x for x in specs if x.satisfies(values, strict=True)] - return specs + qspecs = spack.cmd.parse_specs(self.values) + + # return everything for an empty query. + if not qspecs: + return spack.store.db.query() + + # Return only matching stuff otherwise. + specs = set() + for spec in qspecs: + for s in spack.store.db.query(spec, **kwargs): + specs.add(s) + return sorted(specs) _arguments['constraint'] = Args( diff --git a/lib/spack/spack/cmd/dependents.py b/lib/spack/spack/cmd/dependents.py index 7729105e62..dc2ee658ac 100644 --- a/lib/spack/spack/cmd/dependents.py +++ b/lib/spack/spack/cmd/dependents.py @@ -27,6 +27,7 @@ import argparse import llnl.util.tty as tty import spack +import spack.store import spack.cmd description = "Show installed packages that depend on another." @@ -39,11 +40,14 @@ def setup_parser(subparser): def dependents(parser, args): - specs = spack.cmd.parse_specs(args.spec, concretize=True) + specs = spack.cmd.parse_specs(args.spec) if len(specs) != 1: tty.die("spack dependents takes only one spec.") - - fmt = '$_$@$%@$+$=$#' - deps = [d.format(fmt, color=True) - for d in specs[0].package.installed_dependents] - tty.msg("Dependents of %s" % specs[0].format(fmt, color=True), *deps) + spec = spack.cmd.disambiguate_spec(specs[0]) + + tty.msg("Dependents of %s" % spec.format('$_$@$%@$#', color=True)) + deps = spack.store.db.installed_dependents(spec) + if deps: + spack.cmd.display_specs(deps) + else: + print "No dependents" diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py index 9f1759afc0..ab0b29c30e 100644 --- a/lib/spack/spack/cmd/find.py +++ b/lib/spack/spack/cmd/find.py @@ -114,14 +114,17 @@ def query_arguments(args): def find(parser, args): q_args = query_arguments(args) query_specs = args.specs(**q_args) + # Exit early if no package matches the constraint if not query_specs and args.constraint: msg = "No package matches the query: {0}".format(args.constraint) tty.msg(msg) return + # Display the result if sys.stdout.isatty(): tty.msg("%d installed packages." % len(query_specs)) + display_specs(query_specs, mode=args.mode, long=args.long, diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py index 2f7e15ba55..0fc22ce538 100644 --- a/lib/spack/spack/cmd/uninstall.py +++ b/lib/spack/spack/cmd/uninstall.py @@ -124,7 +124,8 @@ def installed_dependents(specs): """ dependents = {} for item in specs: - lst = [x for x in item.package.installed_dependents if x not in specs] + lst = [x for x in spack.store.db.installed_dependents(item) + if x not in specs] if lst: lst = list(set(lst)) dependents[item] = lst @@ -152,7 +153,7 @@ def do_uninstall(specs, force): # 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): - return len(pkg.installed_dependents) + return len(spack.store.db.installed_dependents(pkg.spec)) packages.sort(key=num_installed_deps) for item in packages: @@ -163,11 +164,14 @@ def get_uninstall_list(args): specs = [any] if args.packages: specs = spack.cmd.parse_specs(args.packages) + # Gets the list of installed specs that match the ones give via cli # takes care of '-a' is given in the cli uninstall_list = concretize_specs(specs, args.all, args.force) + # Takes care of '-d' 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: diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index a01795ca0f..3a0c028d5b 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -604,6 +604,15 @@ class Database(object): with self.write_transaction(): return self._remove(spec) + @_autospec + def installed_dependents(self, spec): + """List the installed specs that depend on this one.""" + dependents = set() + for spec in self.query(spec): + for dependent in spec.traverse(direction='parents', root=False): + dependents.add(dependent) + return dependents + @_autospec def installed_extensions_for(self, extendee_spec): """ @@ -655,6 +664,18 @@ class Database(object): """ with self.read_transaction(): + # Just look up concrete specs with hashes; no fancy search. + if (isinstance(query_spec, spack.spec.Spec) and + query_spec._concrete): + + hash_key = query_spec.dag_hash() + if hash_key in self._data: + return [self._data[hash_key].spec] + else: + return [] + + # Abstract specs require more work -- currently we test + # against everything. results = [] for key, rec in self._data.items(): if installed is not any and rec.installed != installed: diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 3502eb2931..0c924cfcbc 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -855,24 +855,6 @@ class PackageBase(object): def installed(self): return os.path.isdir(self.prefix) - @property - def installed_dependents(self): - """Return a list of the specs of all installed packages that depend - on this one. - - TODO: move this method to database.py? - """ - dependents = [] - for spec in spack.store.db.query(): - if self.name == spec.name: - continue - # XXX(deptype): Should build dependencies not count here? - # for dep in spec.traverse(deptype=('run')): - for dep in spec.traverse(deptype=spack.alldeps): - if self.spec == dep: - dependents.append(spec) - return dependents - @property def prefix_lock(self): """Prefix lock is a byte range lock on the nth byte of a file. @@ -1528,7 +1510,7 @@ class PackageBase(object): raise InstallError(str(self.spec) + " is not installed.") if not force: - dependents = self.installed_dependents + dependents = spack.store.db.installed_dependents(self.spec) if dependents: raise PackageStillNeededError(self.spec, dependents) diff --git a/lib/spack/spack/provider_index.py b/lib/spack/spack/provider_index.py index 6bcf98009e..3911947e4a 100644 --- a/lib/spack/spack/provider_index.py +++ b/lib/spack/spack/provider_index.py @@ -100,7 +100,8 @@ class ProviderIndex(object): for provided_spec, provider_spec in pkg.provided.iteritems(): # We want satisfaction other than flags provider_spec.compiler_flags = spec.compiler_flags.copy() - if provider_spec.satisfies(spec, deps=False): + + if spec.satisfies(provider_spec, deps=False): provided_name = provided_spec.name provider_map = self.providers.setdefault(provided_name, {}) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 238eb07e1f..a8377e324f 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -818,7 +818,7 @@ class Spec(object): dep = self._dependencies.get(name) if dep is not None: return dep - raise InvalidDependencyException( + raise InvalidDependencyError( self.name + " does not depend on " + comma_or(name)) def _find_deps(self, where, deptype): @@ -1395,26 +1395,6 @@ class Spec(object): dependent._add_dependency(concrete, deptypes, dep_spec.default_deptypes) - def _replace_node(self, replacement): - """Replace this spec with another. - - Connects all dependents of this spec to its replacement, and - disconnects this spec from any dependencies it has. New spec - will have any dependencies the replacement had, and may need - to be normalized. - - """ - for name, dep_spec in self._dependents.items(): - dependent = dep_spec.spec - deptypes = dep_spec.deptypes - del dependent._dependencies[self.name] - dependent._add_dependency( - replacement, deptypes, dep_spec.default_deptypes) - - for name, dep_spec in self._dependencies.items(): - del dep_spec.spec.dependents[self.name] - del self._dependencies[dep.name] - def _expand_virtual_packages(self): """Find virtual packages in this spec, replace them with providers, and normalize again to include the provider's (potentially virtual) @@ -2043,6 +2023,10 @@ class Spec(object): """ other = self._autospec(other) + # The only way to satisfy a concrete spec is to match its hash exactly. + if other._concrete: + return self._concrete and self.dag_hash() == other.dag_hash() + # A concrete provider can satisfy a virtual dependency. if not self.virtual and other.virtual: pkg = spack.repo.get(self.fullname) @@ -2113,8 +2097,9 @@ class Spec(object): if other._dependencies and not self._dependencies: return False - if not all(dep in self._dependencies - for dep in other._dependencies): + alldeps = set(d.name for d in self.traverse(root=False)) + if not all(dep.name in alldeps + for dep in other.traverse(root=False)): return False elif not self._dependencies or not other._dependencies: @@ -2620,9 +2605,9 @@ class Spec(object): with indentation.""" color = kwargs.pop('color', False) depth = kwargs.pop('depth', False) - hashes = kwargs.pop('hashes', True) + hashes = kwargs.pop('hashes', False) hlen = kwargs.pop('hashlen', None) - install_status = kwargs.pop('install_status', True) + install_status = kwargs.pop('install_status', False) cover = kwargs.pop('cover', 'nodes') indent = kwargs.pop('indent', 0) fmt = kwargs.pop('format', '$_$@$%@+$+$=') diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index c165934948..16d6121dea 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -312,6 +312,21 @@ class SpecSematicsTest(MockPackagesTest): Spec('netlib-lapack ^netlib-blas').satisfies( 'netlib-lapack ^netlib-blas')) + def test_satisfies_same_spec_with_different_hash(self): + """Ensure that concrete specs are matched *exactly* by hash.""" + s1 = Spec('mpileaks').concretized() + s2 = s1.copy() + + self.assertTrue(s1.satisfies(s2)) + self.assertTrue(s2.satisfies(s1)) + + # Simulate specs that were installed before and after a change to + # Spack's hashing algorithm. This just reverses s2's hash. + s2._hash = s1.dag_hash()[-1::-1] + + self.assertFalse(s1.satisfies(s2)) + self.assertFalse(s2.satisfies(s1)) + # ======================================================================== # Indexing specs # ======================================================================== -- cgit v1.2.3-60-g2f50