From a2b4146e104fc00442b847306df2240d2bb6c0b6 Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Mon, 19 Dec 2016 09:09:53 -0800
Subject: [PATCH] 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(-)
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 @@ def __call__(self, parser, namespace, values, option_string=None):
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 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 @@ def remove(self, spec):
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 @@ def query(self, query_spec=any, known=any, installed=True, explicit=any):
"""
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 @@ def provides(self, vpkg_name):
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 @@ def do_uninstall(self, force=False):
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 @@ def update(self, spec):
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 @@ def get_dependency(self, name):
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 @@ def _replace_with(self, concrete):
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 @@ def satisfies(self, other, deps=True, strict=False):
"""
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 @@ def satisfies_dependencies(self, other, strict=False):
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 @@ def tree(self, **kwargs):
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 @@ def test_satisfies_virtual_dep_with_virtual_constraint(self):
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
# ========================================================================
--
GitLab