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.
This commit is contained in:
Todd Gamblin
GitHub
parent
08d323b1f8
commit
a2b4146e10
10 changed files with 83 additions and 58 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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.")
|
||||
spec = spack.cmd.disambiguate_spec(specs[0])
|
||||
|
||||
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)
|
||||
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"
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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, {})
|
||||
|
|
|
|||
|
|
@ -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', '$_$@$%@+$+$=')
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# ========================================================================
|
||||
|
|
|
|||
Loading…
Reference in a new issue