Raise InstallError if any explicit specs is skipped (#21980)

4 files changed, 32 insertions, 11 deletions

diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index a9c0d7e85c..69474df4a5 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -1609,15 +1609,22 @@ class PackageInstaller(object):
         self._cleanup_all_tasks()
 
         # Ensure we properly report if one or more explicit specs failed
-        if exists_errors or failed_explicits:
+        # or were not installed when should have been.
+        missing = [request.pkg_id for request in self.build_requests if
+                   request.install_args.get('install_package') and
+                   request.pkg_id not in self.installed]
+        if exists_errors or failed_explicits or missing:
             for pkg_id, err in exists_errors:
                 tty.error('{0}: {1}'.format(pkg_id, err))
 
             for pkg_id, err in failed_explicits:
                 tty.error('{0}: {1}'.format(pkg_id, err))
 
+            for pkg_id in missing:
+                tty.error('{0}: Package was not installed'.format(pkg_id))
+
             raise InstallError('Installation request failed.  Refer to '
-                               'recent errors for specific package(s).')
+                               'reported errors for failing package(s).')
 
 
 def build_process(pkg, kwargs):
diff --git a/lib/spack/spack/test/cmd/dev_build.py b/lib/spack/spack/test/cmd/dev_build.py
index 273b311e9c..03f4ea5d12 100644
--- a/lib/spack/spack/test/cmd/dev_build.py
+++ b/lib/spack/spack/test/cmd/dev_build.py
@@ -89,7 +89,7 @@ def test_dev_build_until_last_phase(tmpdir, mock_packages, install_mockery):
     assert os.path.exists(str(tmpdir))
 
 
-def test_dev_build_before_until(tmpdir, mock_packages, install_mockery):
+def test_dev_build_before_until(tmpdir, mock_packages, install_mockery, capsys):
     spec = spack.spec.Spec('dev-build-test-install@0.0.0 dev_path=%s' % tmpdir)
     spec.concretize()
 
@@ -103,13 +103,18 @@ def test_dev_build_before_until(tmpdir, mock_packages, install_mockery):
 
         bad_phase = 'phase_that_does_not_exist'
         not_allowed = 'is not a valid phase'
-        out = dev_build('-u', bad_phase, 'dev-build-test-install@0.0.0')
+        not_installed = 'was not installed'
+        out = dev_build('-u', bad_phase, 'dev-build-test-install@0.0.0',
+                        fail_on_error=False)
         assert bad_phase in out
         assert not_allowed in out
+        assert not_installed in out
 
-        out = dev_build('-b', bad_phase, 'dev-build-test-install@0.0.0')
+        out = dev_build('-b', bad_phase, 'dev-build-test-install@0.0.0',
+                        fail_on_error=False)
         assert bad_phase in out
         assert not_allowed in out
+        assert not_installed in out
 
 
 def print_spack_cc(*args):
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index dfc48ece07..f770bc4184 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -629,10 +629,11 @@ def test_cache_only_fails(tmpdir, mock_fetch, install_mockery, capfd):
     # libelf from cache fails to install, which automatically removes the
     # the libdwarf build task
     with capfd.disabled():
-        out = install('--cache-only', 'libdwarf')
+        out = install('--cache-only', 'libdwarf', fail_on_error=False)
 
     assert 'Failed to install libelf' in out
     assert 'Skipping build of libdwarf' in out
+    assert 'was not installed' in out
 
     # Check that failure prefix locks are still cached
     failure_lock_prefixes = ','.join(spack.store.db._prefix_failures.keys())
diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py
index fb00081931..3d1b72e559 100644
--- a/lib/spack/spack/test/installer.py
+++ b/lib/spack/spack/test/installer.py
@@ -879,7 +879,8 @@ def test_install_failed(install_mockery, monkeypatch, capsys):
     # Make sure the package is identified as failed
     monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true)
 
-    installer.install()
+    with pytest.raises(inst.InstallError, match='request failed'):
+        installer.install()
 
     out = str(capsys.readouterr())
     assert installer.build_requests[0].pkg_id in out
@@ -894,7 +895,8 @@ def test_install_failed_not_fast(install_mockery, monkeypatch, capsys):
     # Make sure the package is identified as failed
     monkeypatch.setattr(spack.database.Database, 'prefix_failed', _true)
 
-    installer.install()
+    with pytest.raises(inst.InstallError, match='request failed'):
+        installer.install()
 
     out = str(capsys.readouterr())
     assert 'failed to install' in out
@@ -1046,7 +1048,9 @@ def test_install_lock_failures(install_mockery, monkeypatch, capfd):
     # Ensure don't continually requeue the task
     monkeypatch.setattr(inst.PackageInstaller, '_requeue_task', _requeued)
 
-    installer.install()
+    with pytest.raises(inst.InstallError, match='request failed'):
+        installer.install()
+
     out = capfd.readouterr()[0]
     expected = ['write locked', 'read locked', 'requeued']
     for exp, ln in zip(expected, out.split('\n')):
@@ -1077,7 +1081,9 @@ def test_install_lock_installed_requeue(install_mockery, monkeypatch, capfd):
     # Ensure don't continually requeue the task
     monkeypatch.setattr(inst.PackageInstaller, '_requeue_task', _requeued)
 
-    installer.install()
+    with pytest.raises(inst.InstallError, match='request failed'):
+        installer.install()
+
     assert b_pkg_id not in installer.installed
 
     out = capfd.readouterr()[0]
@@ -1113,7 +1119,9 @@ def test_install_read_locked_requeue(install_mockery, monkeypatch, capfd):
     const_arg = installer_args(['b'], {})
     installer = create_installer(const_arg)
 
-    installer.install()
+    with pytest.raises(inst.InstallError, match='request failed'):
+        installer.install()
+
     assert 'b' not in installer.installed
 
     out = capfd.readouterr()[0]