bugfix: ensure proper dependency handling for package-only installs (#15197)

The distributed build PR (#13100) -- did not check the install status of dependencies when using the `--only package` option so would refuse to install a package with the claim that it had uninstalled dependencies whether that was the case or not.

- [x] add install status checks for the `--only package` case.
- [x] add initial set of tests

lib/spack/spack/installer.py

@@ -651,6 +651,66 @@ def _add_bootstrap_compilers(self, pkg):
             if package_id(comp_pkg) not in self.build_tasks:
                 self._push_task(comp_pkg, is_compiler, 0, 0, STATUS_ADDED)
 
+    def _check_db(self, spec):
+        """Determine if the spec is flagged as installed in the database
+
+        Args:
+            spec (Spec): spec whose database install status is being checked
+
+        Return:
+            (rec, installed_in_db) tuple where rec is the database record, or
+                None, if there is no matching spec, and installed_in_db is
+                ``True`` if the spec is considered installed and ``False``
+                otherwise
+        """
+        try:
+            rec = spack.store.db.get_record(spec)
+            installed_in_db = rec.installed if rec else False
+        except KeyError:
+            # KeyError is raised if there is no matching spec in the database
+            # (versus no matching specs that are installed).
+            rec = None
+            installed_in_db = False
+        return rec, installed_in_db
+
+    def _check_deps_status(self):
+        """Check the install status of the explicit spec's dependencies"""
+
+        err = 'Cannot proceed with {0}: {1}'
+        for dep in self.spec.traverse(order='post', root=False):
+            dep_pkg = dep.package
+            dep_id = package_id(dep_pkg)
+
+            # Check for failure since a prefix lock is not required
+            if spack.store.db.prefix_failed(dep):
+                action = "'spack install' the dependency"
+                msg = '{0} is marked as an install failure: {1}' \
+                    .format(dep_id, action)
+                raise InstallError(err.format(self.pkg_id, msg))
+
+            # Attempt to get a write lock to ensure another process does not
+            # uninstall the dependency while the requested spec is being
+            # installed
+            ltype, lock = self._ensure_locked('write', dep_pkg)
+            if lock is None:
+                msg = '{0} is write locked by another process'.format(dep_id)
+                raise InstallError(err.format(self.pkg_id, msg))
+
+            # Flag external and upstream packages as being installed
+            if dep_pkg.spec.external or dep_pkg.installed_upstream:
+                form = 'external' if dep_pkg.spec.external else 'upstream'
+                tty.debug('Flagging {0} {1} as installed'.format(form, dep_id))
+                self.installed.add(dep_id)
+                continue
+
+            # Check the database to see if the dependency has been installed
+            # and flag as such if appropriate
+            rec, installed_in_db = self._check_db(dep)
+            if installed_in_db:
+                tty.debug('Flagging {0} as installed per the database'
+                          .format(dep_id))
+                self.installed.add(dep_id)
+
     def _prepare_for_install(self, task, keep_prefix, keep_stage,
                              restage=False):
         """
@@ -680,14 +740,7 @@ def _prepare_for_install(self, task, keep_prefix, keep_stage,
             return
 
         # Determine if the spec is flagged as installed in the database
-        try:
+        rec, installed_in_db = self._check_db(task.pkg.spec)
-            rec = spack.store.db.get_record(task.pkg.spec)
-            installed_in_db = rec.installed if rec else False
-        except KeyError:
-            # KeyError is raised if there is no matching spec in the database
-            # (versus no matching specs that are installed).
-            rec = None
-            installed_in_db = False
 
         # Make sure the installation directory is in the desired state
         # for uninstalled specs.
@@ -929,6 +982,11 @@ def _init_queue(self, install_deps, install_package):
             # Be sure to clear any previous failure
             spack.store.db.clear_failure(self.pkg.spec, force=True)
 
+            # If not installing dependencies, then determine their
+            # installation status before proceeding
+            if not install_deps:
+                self._check_deps_status()
+
             # Now add the package itself, if appropriate
             self._push_task(self.pkg, False, 0, 0, STATUS_ADDED)
 
@@ -1022,7 +1080,8 @@ def build_process():
                                 configure_args = getattr(pkg, attr)()
                                 configure_args = ' '.join(configure_args)
 
-                                with open(pkg.configure_args_path, 'w') as args_file:
+                                with open(pkg.configure_args_path, 'w') as \
+                                        args_file:
                                     args_file.write(configure_args)
 
                                 break

lib/spack/spack/test/cmd/install.py

@@ -632,6 +632,30 @@ def test_install_only_dependencies(tmpdir, mock_fetch, install_mockery):
     assert not os.path.exists(root.prefix)
 
 
+def test_install_only_package(tmpdir, mock_fetch, install_mockery, capfd):
+    msg = ''
+    with capfd.disabled():
+        try:
+            install('--only', 'package', 'dependent-install')
+        except spack.installer.InstallError as e:
+            msg = str(e)
+
+    assert 'Cannot proceed with dependent-install'  in msg
+    assert '1 uninstalled dependency' in msg
+
+
+def test_install_deps_then_package(tmpdir, mock_fetch, install_mockery):
+    dep = Spec('dependency-install').concretized()
+    root = Spec('dependent-install').concretized()
+
+    install('--only', 'dependencies', 'dependent-install')
+    assert os.path.exists(dep.prefix)
+    assert not os.path.exists(root.prefix)
+
+    install('--only', 'package', 'dependent-install')
+    assert os.path.exists(root.prefix)
+
+
 @pytest.mark.regression('12002')
 def test_install_only_dependencies_in_env(tmpdir, mock_fetch, install_mockery,
                                           mutable_mock_env_path):

lib/spack/spack/test/installer.py

@@ -27,6 +27,12 @@ def _none(*args, **kwargs):
     return None
 
 
+def _not_locked(installer, lock_type, pkg):
+    """Generic monkeypatch function for _ensure_locked to return no lock"""
+    tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name))
+    return lock_type, None
+
+
 def _true(*args, **kwargs):
     """Generic monkeypatch function that always returns True."""
     return True
@@ -285,6 +291,57 @@ def test_dump_packages_deps(install_mockery, tmpdir):
         inst.dump_packages(spec, '.')
 
 
+@pytest.mark.tld
+def test_check_deps_status_errs(install_mockery, monkeypatch):
+    """Test to cover _check_deps_status failures."""
+    spec, installer = create_installer('a')
+
+    # Make sure the package is identified as failed
+    orig_fn = spack.database.Database.prefix_failed
+    monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true)
+
+    with pytest.raises(inst.InstallError, matches='install failure'):
+        installer._check_deps_status()
+
+    monkeypatch.setattr(spack.database.Database, 'prefix_failed', orig_fn)
+
+    # Ensure do not acquire the lock
+    monkeypatch.setattr(inst.PackageInstaller, '_ensure_locked', _not_locked)
+
+    with pytest.raises(inst.InstallError, matches='write locked by another'):
+        installer._check_deps_status()
+
+
+@pytest.mark.tld
+def test_check_deps_status_external(install_mockery, monkeypatch):
+    """Test to cover _check_deps_status for external."""
+    spec, installer = create_installer('a')
+
+    deps = spec.dependencies()
+    assert len(deps) > 0
+    dep_id = 'b'
+
+    # Ensure the known dependent is installed if flagged as external
+    monkeypatch.setattr(spack.spec.Spec, 'external', True)
+    installer._check_deps_status()
+    assert dep_id in installer.installed
+
+
+@pytest.mark.tld
+def test_check_deps_status_upstream(install_mockery, monkeypatch):
+    """Test to cover _check_deps_status for upstream."""
+    spec, installer = create_installer('a')
+
+    deps = spec.dependencies()
+    assert len(deps) > 0
+    dep_id = 'b'
+
+    # Ensure the known dependent, b, is installed if flagged as upstream
+    monkeypatch.setattr(spack.package.PackageBase, 'installed_upstream', True)
+    installer._check_deps_status()
+    assert dep_id in installer.installed
+
+
 def test_add_bootstrap_compilers(install_mockery, monkeypatch):
     """Test to cover _add_bootstrap_compilers."""
     def _pkgs(pkg):
@@ -458,10 +515,6 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd):
     def _requeued(installer, task):
         tty.msg('requeued {0}' .format(task.pkg.spec.name))
 
-    def _not_locked(installer, lock_type, pkg):
-        tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name))
-        return lock_type, None
-
     spec, installer = create_installer('b')
 
     # Ensure never acquire a lock
@@ -485,10 +538,6 @@ def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd):
     def _install(installer, task, **kwargs):
         tty.msg('{0} installing'.format(task.pkg.spec.name))
 
-    def _not_locked(installer, lock_type, pkg):
-        tty.msg('{0} locked {1}' .format(lock_type, pkg.spec.name))
-        return lock_type, None
-
     def _prep(installer, task, keep_prefix, keep_stage, restage):
         installer.installed.add('b')
         tty.msg('{0} is installed' .format(task.pkg.spec.name))