External packages are now registered in the DB (#1167)

* treats correctly a change from `explicit=False` to `explicit=True` in an external package DB entry.
* added unit tests
* fixed issues raised by @tgamblin . In particular the PR is no more hash-changing for packages that are not external.
* added a test to check correctness of a spec/yaml round-trip for things that involve an external
* Don't find external module path at each step of concretization
    * it's not necessary.. The paths are retrieved at the end of concretizaion
* Don't find replacements for external packages.
* Test root of the DAG if external
    * No reason not to test if the root of the DAG is external when external
packages are now first class citizens!
* Create `external` property for Spec (for external_path and external_module)
* Allow users to specify external package paths relative to spack
    * Canonicalize external package paths so that users may specify their
locations relative to spack's directory.
* Update tests to use new external_path and external properly.
* skip license hooks on external
This commit is contained in:
Massimiliano Culpo 2017-04-23 03:06:27 +02:00 committed by Todd Gamblin
parent 4e17ae911b
commit 3b52d0a883
15 changed files with 230 additions and 57 deletions

View file

@ -465,7 +465,10 @@ def parent_class_modules(cls):
def load_external_modules(pkg):
""" traverse the spec list and find any specs that have external modules.
"""Traverse the spec list associated with a package
and find any specs that have external modules.
:param pkg: package under consideration
"""
for dep in list(pkg.spec.traverse()):
if dep.external_module:

View file

@ -99,7 +99,7 @@ def _valid_virtuals_and_externals(self, spec):
# Use a sort key to order the results
return sorted(usable, key=lambda spec: (
not (spec.external or spec.external_module), # prefer externals
not spec.external, # prefer externals
pref_key(spec), # respect prefs
spec.name, # group by name
reverse_order(spec.versions), # latest version

View file

@ -67,9 +67,9 @@
_db_dirname = '.spack-db'
# DB version. This is stuck in the DB file to track changes in format.
_db_version = Version('0.9.2')
_db_version = Version('0.9.3')
# Default timeout for spack database locks is 5 min.
# Timeout for spack database locks in seconds
_db_lock_timeout = 60
# Types of dependencies tracked by the database
@ -409,24 +409,40 @@ def _read_suppress_error():
self._data = {}
transaction = WriteTransaction(
self.lock, _read_suppress_error, self._write, _db_lock_timeout)
self.lock, _read_suppress_error, self._write, _db_lock_timeout
)
with transaction:
if self._error:
tty.warn(
"Spack database was corrupt. Will rebuild. Error was:",
str(self._error))
str(self._error)
)
self._error = None
# Read first the `spec.yaml` files in the prefixes. They should be
# considered authoritative with respect to DB reindexing, as
# entries in the DB may be corrupted in a way that still makes
# them readable. If we considered DB entries authoritative
# instead, we would perpetuate errors over a reindex.
old_data = self._data
try:
# Initialize data in the reconstructed DB
self._data = {}
# Ask the directory layout to traverse the filesystem.
# Start inspecting the installed prefixes
processed_specs = set()
for spec in directory_layout.all_specs():
# Try to recover explicit value from old DB, but
# default it to False if DB was corrupt.
explicit = False
# default it to True if DB was corrupt. This is
# just to be conservative in case a command like
# "autoremove" is run by the user after a reindex.
tty.debug(
'RECONSTRUCTING FROM SPEC.YAML: {0}'.format(spec)
)
explicit = True
if old_data is not None:
old_info = old_data.get(spec.dag_hash())
if old_info is not None:
@ -434,6 +450,42 @@ def _read_suppress_error():
self._add(spec, directory_layout, explicit=explicit)
processed_specs.add(spec)
for key, entry in old_data.items():
# We already took care of this spec using
# `spec.yaml` from its prefix.
if entry.spec in processed_specs:
msg = 'SKIPPING RECONSTRUCTION FROM OLD DB: {0}'
msg += ' [already reconstructed from spec.yaml]'
tty.debug(msg.format(entry.spec))
continue
# If we arrived here it very likely means that
# we have external specs that are not dependencies
# of other specs. This may be the case for externally
# installed compilers or externally installed
# applications.
tty.debug(
'RECONSTRUCTING FROM OLD DB: {0}'.format(entry.spec)
)
try:
layout = spack.store.layout
if entry.spec.external:
layout = None
kwargs = {
'spec': entry.spec,
'directory_layout': layout,
'explicit': entry.explicit
}
self._add(**kwargs)
processed_specs.add(entry.spec)
except Exception as e:
# Something went wrong, so the spec was not restored
# from old data
tty.debug(e.message)
pass
self._check_ref_counts()
except:
@ -542,7 +594,7 @@ def _add(self, spec, directory_layout=None, explicit=False):
key = spec.dag_hash()
if key not in self._data:
installed = False
installed = bool(spec.external)
path = None
if not spec.external and directory_layout:
path = directory_layout.path_for_spec(spec)

View file

@ -193,7 +193,7 @@ def relative_path_for_spec(self, spec):
_check_concrete(spec)
if spec.external:
return spec.external
return spec.external_path
path = spec.format(self.path_scheme)
return path

View file

@ -31,7 +31,7 @@
def pre_install(pkg):
"""This hook handles global license setup for licensed software."""
if pkg.license_required:
if pkg.license_required and not pkg.spec.external:
set_up_license(pkg)
@ -145,7 +145,7 @@ def write_license_file(pkg, license_path):
def post_install(pkg):
"""This hook symlinks local licenses to the global license for
licensed software."""
if pkg.license_required:
if pkg.license_required and not pkg.spec.external:
symlink_license(pkg)

View file

@ -104,8 +104,12 @@ def filter_shebangs_in_directory(directory, filenames=None):
def post_install(pkg):
"""This hook edits scripts so that they call /bin/bash
$spack_prefix/bin/sbang instead of something longer than the
shebang limit."""
$spack_prefix/bin/sbang instead of something longer than the
shebang limit.
"""
if pkg.spec.external:
tty.debug('SKIP: shebang filtering [external package]')
return
for directory, _, filenames in os.walk(pkg.prefix):
filter_shebangs_in_directory(directory, filenames)

View file

@ -1081,6 +1081,51 @@ def _stage_and_write_lock(self):
with spack.store.db.prefix_write_lock(self.spec):
yield
def _process_external_package(self, explicit):
"""Helper function to process external packages. It runs post install
hooks and registers the package in the DB.
:param bool explicit: True if the package was requested explicitly by
the user, False if it was pulled in as a dependency of an explicit
package.
"""
if self.spec.external_module:
message = '{s.name}@{s.version} : has external module in {module}'
tty.msg(message.format(s=self, module=self.spec.external_module))
message = '{s.name}@{s.version} : is actually installed in {path}'
tty.msg(message.format(s=self, path=self.spec.external_path))
else:
message = '{s.name}@{s.version} : externally installed in {path}'
tty.msg(message.format(s=self, path=self.spec.external_path))
try:
# Check if the package was already registered in the DB
# If this is the case, then just exit
rec = spack.store.db.get_record(self.spec)
message = '{s.name}@{s.version} : already registered in DB'
tty.msg(message.format(s=self))
# Update the value of rec.explicit if it is necessary
self._update_explicit_entry_in_db(rec, explicit)
except KeyError:
# If not register it and generate the module file
# For external packages we just need to run
# post-install hooks to generate module files
message = '{s.name}@{s.version} : generating module file'
tty.msg(message.format(s=self))
spack.hooks.post_install(self)
# Add to the DB
message = '{s.name}@{s.version} : registering into DB'
tty.msg(message.format(s=self))
spack.store.db.add(self.spec, None, explicit=explicit)
def _update_explicit_entry_in_db(self, rec, explicit):
if explicit and not rec.explicit:
with spack.store.db.write_transaction():
rec = spack.store.db.get_record(self.spec)
rec.explicit = True
message = '{s.name}@{s.version} : marking the package explicit'
tty.msg(message.format(s=self))
def do_install(self,
keep_prefix=False,
keep_stage=False,
@ -1121,11 +1166,10 @@ def do_install(self,
raise ValueError("Can only install concrete packages: %s."
% self.spec.name)
# No installation needed if package is external
# For external packages the workflow is simplified, and basically
# consists in module file generation and registration in the DB
if self.spec.external:
tty.msg("%s is externally installed in %s" %
(self.name, self.spec.external))
return
return self._process_external_package(explicit)
# Ensure package is not already installed
layout = spack.store.layout
@ -1135,14 +1179,10 @@ def do_install(self,
tty.msg(
"Continuing from partial install of %s" % self.name)
elif layout.check_installed(self.spec):
tty.msg(
"%s is already installed in %s" % (self.name, self.prefix))
msg = '{0.name} is already installed in {0.prefix}'
tty.msg(msg.format(self))
rec = spack.store.db.get_record(self.spec)
if (not rec.explicit) and explicit:
with spack.store.db.write_transaction():
rec = spack.store.db.get_record(self.spec)
rec.explicit = True
return
return self._update_explicit_entry_in_db(rec, explicit)
# Dirty argument takes precedence over dirty config setting.
if dirty is None:
@ -1150,10 +1190,9 @@ def do_install(self,
self._do_install_pop_kwargs(kwargs)
tty.msg("Installing %s" % self.name)
# First, install dependencies recursively.
if install_deps:
tty.debug('Installing {0} dependencies'.format(self.name))
for dep in self.spec.dependencies():
dep.package.do_install(
keep_prefix=keep_prefix,
@ -1168,6 +1207,8 @@ def do_install(self,
**kwargs
)
tty.msg('Installing %s' % self.name)
# Set run_tests flag before starting build.
self.run_tests = run_tests
@ -1265,7 +1306,7 @@ def build_process(input_stream):
# Fork a child to do the actual installation
spack.build_environment.fork(self, build_process, dirty=dirty)
# If we installed then we should keep the prefix
keep_prefix = True if self.last_phase is None else keep_prefix
keep_prefix = self.last_phase is None or keep_prefix
# note: PARENT of the build process adds the new package to
# the database, so that we don't need to re-read from file.
spack.store.db.add(
@ -1490,12 +1531,19 @@ def uninstall_by_spec(spec, force=False):
spack.hooks.pre_uninstall(pkg)
# Uninstalling in Spack only requires removing the prefix.
spack.store.layout.remove_install_directory(spec)
if not spec.external:
msg = 'Deleting package prefix [{0}]'
tty.debug(msg.format(spec.short_spec))
spack.store.layout.remove_install_directory(spec)
# Delete DB entry
msg = 'Deleting DB entry [{0}]'
tty.debug(msg.format(spec.short_spec))
spack.store.db.remove(spec)
tty.msg("Successfully uninstalled %s" % spec.short_spec)
# TODO: refactor hooks to use specs, not packages.
if pkg is not None:
spack.hooks.post_uninstall(pkg)
# TODO: refactor hooks to use specs, not packages.
if pkg is not None:
spack.hooks.post_uninstall(pkg)
tty.msg("Successfully uninstalled %s" % spec.short_spec)

View file

@ -29,6 +29,7 @@
import spack
import spack.error
from spack.util.path import canonicalize_path
from spack.version import *
@ -199,7 +200,7 @@ def spec_externals(spec):
"""Return a list of external specs (w/external directory path filled in),
one for each known external installation."""
# break circular import.
from spack.build_environment import get_path_from_module
from spack.build_environment import get_path_from_module # noqa: F401
allpkgs = get_packages_config()
name = spec.name
@ -215,7 +216,8 @@ def spec_externals(spec):
# skip entries without paths (avoid creating extra Specs)
continue
external_spec = spack.spec.Spec(external_spec, external=path)
external_spec = spack.spec.Spec(external_spec,
external_path=canonicalize_path(path))
if external_spec.satisfies(spec):
external_specs.append(external_spec)
@ -223,10 +225,8 @@ def spec_externals(spec):
if not module:
continue
path = get_path_from_module(module)
external_spec = spack.spec.Spec(
external_spec, external=path, external_module=module)
external_spec, external_module=module)
if external_spec.satisfies(spec):
external_specs.append(external_spec)

View file

@ -954,7 +954,7 @@ def __init__(self, spec_like, *dep_like, **kwargs):
self._concrete = kwargs.get('concrete', False)
# Allow a spec to be constructed with an external path.
self.external = kwargs.get('external', None)
self.external_path = kwargs.get('external_path', None)
self.external_module = kwargs.get('external_module', None)
# This allows users to construct a spec DAG with literals.
@ -976,6 +976,10 @@ def __init__(self, spec_like, *dep_like, **kwargs):
self._add_dependency(spec, deptypes)
deptypes = ()
@property
def external(self):
return bool(self.external_path) or bool(self.external_module)
def get_dependency(self, name):
dep = self._dependencies.get(name)
if dep is not None:
@ -1349,6 +1353,12 @@ def to_node_dict(self):
if params:
d['parameters'] = params
if self.external:
d['external'] = {
'path': self.external_path,
'module': bool(self.external_module)
}
# TODO: restore build dependencies here once we have less picky
# TODO: concretization.
deps = self.dependencies_dict(deptype=('link', 'run'))
@ -1412,6 +1422,22 @@ def from_node_dict(node):
for name in FlagMap.valid_compiler_flags():
spec.compiler_flags[name] = []
if 'external' in node:
spec.external_path = None
spec.external_module = None
# This conditional is needed because sometimes this function is
# called with a node already constructed that contains a 'versions'
# and 'external' field. Related to virtual packages provider
# indexes.
if node['external']:
spec.external_path = node['external']['path']
spec.external_module = node['external']['module']
if spec.external_module is False:
spec.external_module = None
else:
spec.external_path = None
spec.external_module = None
# Don't read dependencies here; from_node_dict() is used by
# from_yaml() to read the root *and* each dependency spec.
@ -1578,6 +1604,8 @@ def _expand_virtual_packages(self):
done = True
for spec in list(self.traverse()):
replacement = None
if spec.external:
continue
if spec.virtual:
replacement = self._find_provider(spec, self_index)
if replacement:
@ -1614,7 +1642,7 @@ def _expand_virtual_packages(self):
continue
# If replacement is external then trim the dependencies
if replacement.external or replacement.external_module:
if replacement.external:
if (spec._dependencies):
changed = True
spec._dependencies = DependencyMap()
@ -1632,7 +1660,8 @@ def feq(cfield, sfield):
feq(replacement.architecture, spec.architecture) and
feq(replacement._dependencies, spec._dependencies) and
feq(replacement.variants, spec.variants) and
feq(replacement.external, spec.external) and
feq(replacement.external_path,
spec.external_path) and
feq(replacement.external_module,
spec.external_module)):
continue
@ -1691,14 +1720,14 @@ def concretize(self):
if s.namespace is None:
s.namespace = spack.repo.repo_for_pkg(s.name).namespace
for s in self.traverse(root=False):
for s in self.traverse():
if s.external_module:
compiler = spack.compilers.compiler_for_spec(
s.compiler, s.architecture)
for mod in compiler.modules:
load_module(mod)
s.external = get_path_from_module(s.external_module)
s.external_path = get_path_from_module(s.external_module)
# Mark everything in the spec as concrete, as well.
self._mark_concrete()
@ -1919,7 +1948,7 @@ def _normalize_helper(self, visited, spec_deps, provider_index):
# if we descend into a virtual spec, there's nothing more
# to normalize. Concretize will finish resolving it later.
if self.virtual or self.external or self.external_module:
if self.virtual or self.external:
return False
# Combine constraints from package deps with constraints from
@ -2325,7 +2354,7 @@ def _dup(self, other, deps=True, cleardeps=True):
self.variants != other.variants and
self._normal != other._normal and
self.concrete != other.concrete and
self.external != other.external and
self.external_path != other.external_path and
self.external_module != other.external_module and
self.compiler_flags != other.compiler_flags)
@ -2341,7 +2370,7 @@ def _dup(self, other, deps=True, cleardeps=True):
self.compiler_flags = other.compiler_flags.copy()
self.variants = other.variants.copy()
self.variants.spec = self
self.external = other.external
self.external_path = other.external_path
self.external_module = other.external_module
self.namespace = other.namespace
@ -2988,7 +3017,7 @@ def spec(self, name):
spec.variants = VariantMap(spec)
spec.architecture = None
spec.compiler = None
spec.external = None
spec.external_path = None
spec.external_module = None
spec.compiler_flags = FlagMap(spec)
spec._dependents = DependencyMap()

View file

@ -53,7 +53,7 @@ def test_uninstall(database):
uninstall(parser, args)
all_specs = spack.store.layout.all_specs()
assert len(all_specs) == 7
assert len(all_specs) == 8
# query specs with multiple configurations
mpileaks_specs = [s for s in all_specs if s.satisfies('mpileaks')]
callpath_specs = [s for s in all_specs if s.satisfies('callpath')]

View file

@ -293,7 +293,7 @@ def test_compiler_inheritance(self):
def test_external_package(self):
spec = Spec('externaltool%gcc')
spec.concretize()
assert spec['externaltool'].external == '/path/to/external_tool'
assert spec['externaltool'].external_path == '/path/to/external_tool'
assert 'externalprereq' not in spec
assert spec['externaltool'].compiler.satisfies('gcc')
@ -322,8 +322,8 @@ def test_nobuild_package(self):
def test_external_and_virtual(self):
spec = Spec('externaltest')
spec.concretize()
assert spec['externaltool'].external == '/path/to/external_tool'
assert spec['stuff'].external == '/path/to/external_virtual_gcc'
assert spec['externaltool'].external_path == '/path/to/external_tool'
assert spec['stuff'].external_path == '/path/to/external_virtual_gcc'
assert spec['externaltool'].compiler.satisfies('gcc')
assert spec['stuff'].compiler.satisfies('gcc')

View file

@ -170,4 +170,4 @@ def test_external_mpi(self):
# ensure that once config is in place, external is used
spec = Spec('mpi')
spec.concretize()
assert spec['mpich'].external == '/dummy/path'
assert spec['mpich'].external_path == '/dummy/path'

View file

@ -252,6 +252,7 @@ def _refresh():
_install('mpileaks ^mpich')
_install('mpileaks ^mpich2')
_install('mpileaks ^zmpi')
_install('externaltest')
t = Database(
real=real,
@ -265,6 +266,7 @@ def _refresh():
t.install('mpileaks ^mpich')
t.install('mpileaks ^mpich2')
t.install('mpileaks ^zmpi')
t.install('externaltest')
yield t

View file

@ -86,11 +86,17 @@ def _check_merkleiness():
def _check_db_sanity(install_db):
"""Utiilty function to check db against install layout."""
expected = sorted(spack.store.layout.all_specs())
pkg_in_layout = sorted(spack.store.layout.all_specs())
actual = sorted(install_db.query())
assert len(expected) == len(actual)
for e, a in zip(expected, actual):
externals = sorted([x for x in actual if x.external])
nexpected = len(pkg_in_layout) + len(externals)
assert nexpected == len(actual)
non_external_in_db = sorted([x for x in actual if not x.external])
for e, a in zip(pkg_in_layout, non_external_in_db):
assert e == a
_check_merkleiness()
@ -127,7 +133,7 @@ def test_005_db_exists(database):
def test_010_all_install_sanity(database):
"""Ensure that the install layout reflects what we think it does."""
all_specs = spack.store.layout.all_specs()
assert len(all_specs) == 13
assert len(all_specs) == 14
# Query specs with multiple configurations
mpileaks_specs = [s for s in all_specs if s.satisfies('mpileaks')]
@ -207,7 +213,7 @@ def test_050_basic_query(database):
"""Ensure querying database is consistent with what is installed."""
install_db = database.mock.db
# query everything
assert len(spack.store.db.query()) == 13
assert len(spack.store.db.query()) == 16
# query specs with multiple configurations
mpileaks_specs = install_db.query('mpileaks')
@ -365,3 +371,22 @@ def fail_while_writing():
# reload DB and make sure cmake was not written.
with install_db.read_transaction():
assert install_db.query('cmake', installed=any) == []
def test_external_entries_in_db(database):
install_db = database.mock.db
rec = install_db.get_record('mpileaks ^zmpi')
assert rec.spec.external_path is None
assert rec.spec.external_module is None
rec = install_db.get_record('externaltool')
assert rec.spec.external_path == '/path/to/external_tool'
assert rec.spec.external_module is None
assert rec.explicit is False
rec.spec.package.do_install(fake=True, explicit=True)
rec = install_db.get_record('externaltool')
assert rec.spec.external_path == '/path/to/external_tool'
assert rec.spec.external_module is None
assert rec.explicit is True

View file

@ -52,6 +52,16 @@ def test_normal_spec(builtin_mock):
check_yaml_round_trip(spec)
def test_external_spec(config, builtin_mock):
spec = Spec('externaltool')
spec.concretize()
check_yaml_round_trip(spec)
spec = Spec('externaltest')
spec.concretize()
check_yaml_round_trip(spec)
def test_ambiguous_version_spec(builtin_mock):
spec = Spec('mpileaks@1.0:5.0,6.1,7.3+debug~opt')
spec.normalize()