diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 4a8487daab..06ac65f552 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -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: diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 2a5ce65fa4..0b29d2874e 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -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 diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py index 3cb8ced342..33eb7bea4c 100644 --- a/lib/spack/spack/database.py +++ b/lib/spack/spack/database.py @@ -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) diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index 9d09875484..e7c9d6c590 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -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 diff --git a/lib/spack/spack/hooks/licensing.py b/lib/spack/spack/hooks/licensing.py index a99099749c..9a244dbdef 100644 --- a/lib/spack/spack/hooks/licensing.py +++ b/lib/spack/spack/hooks/licensing.py @@ -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) diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index 6f9736a018..fb824470e2 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/lib/spack/spack/hooks/sbang.py @@ -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) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 0402926590..bc479c9cf5 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -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) diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py index f9dac2bef0..8b1fe08154 100644 --- a/lib/spack/spack/package_prefs.py +++ b/lib/spack/spack/package_prefs.py @@ -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) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index b4170a0f7a..3d067e083f 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -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() diff --git a/lib/spack/spack/test/cmd/uninstall.py b/lib/spack/spack/test/cmd/uninstall.py index bfbb9b8148..5765f4613d 100644 --- a/lib/spack/spack/test/cmd/uninstall.py +++ b/lib/spack/spack/test/cmd/uninstall.py @@ -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')] diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 3b383584ce..2063088184 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -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') diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index bf915064b2..0b9f7a0046 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -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' diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 796d95a0cf..120425794f 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -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 diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 0d7999cd36..a4b35e1df7 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -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 diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index 0bcd2de3cf..adf262a60e 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -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()