Add --show-log-on-error option to `spack install`

- converted `log_path` and `env_path` to properties of PackageBase.

- InstallErrors in build_environment are now annotated with the package
  that caused them, in the 'pkg' attribute.

- Add `--show-log-on-error` option to `spack install` that catches
  InstallErrors and prints the log to stderr if it exists.

Note that adding a reference to the Pakcage allows a lot of stuff
currently handled by do_install() and build_environment to be handled
externally.

4 files changed, 59 insertions, 14 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 66b7da9e55..a456efb8ca 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -598,6 +598,10 @@ def fork(pkg, function, dirty):
             target=child_process, args=(child_pipe, input_stream))
         p.start()
 
+    except InstallError as e:
+        e.pkg = pkg
+        raise
+
     finally:
         # Close the input stream in the parent process
         if input_stream is not None:
@@ -606,6 +610,10 @@ def fork(pkg, function, dirty):
     child_result = parent_pipe.recv()
     p.join()
 
+    # let the caller know which package went wrong.
+    if isinstance(child_result, InstallError):
+        child_result.pkg = pkg
+
     # If the child process raised an error, print its output here rather
     # than waiting until the call to SpackError.die() in main(). This
     # allows exception handling output to be logged from within Spack.
@@ -676,10 +684,15 @@ def get_package_context(traceback, context=3):
 
 
 class InstallError(spack.error.SpackError):
-    """Raised by packages when a package fails to install"""
+    """Raised by packages when a package fails to install.
+
+    Any subclass of InstallError will be annotated by Spack wtih a
+    ``pkg`` attribute on failure, which the caller can use to get the
+    package for which the exception was raised.
+    """
 
 
-class ChildError(spack.error.SpackError):
+class ChildError(InstallError):
     """Special exception class for wrapping exceptions from child processes
        in Spack's build environment.
 
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index f9d60c8f32..d42d06d06b 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -27,6 +27,8 @@ import codecs
 import functools
 import os
 import platform
+import shutil
+import sys
 import time
 import xml.dom.minidom
 import xml.etree.ElementTree as ET
@@ -69,6 +71,9 @@ the dependencies"""
         '--restage', action='store_true',
         help="if a partial install is detected, delete prior state")
     subparser.add_argument(
+        '--show-log-on-error', action='store_true',
+        help="print full build log to stderr if build fails")
+    subparser.add_argument(
         '--source', action='store_true', dest='install_source',
         help="install source files in prefix")
     subparser.add_argument(
@@ -367,13 +372,26 @@ def install(parser, args, **kwargs):
                 for s in spec.dependencies():
                     p = spack.repo.get(s)
                     p.do_install(**kwargs)
+
             else:
                 package = spack.repo.get(spec)
                 kwargs['explicit'] = True
                 package.do_install(**kwargs)
+
+        except InstallError as e:
+            if args.show_log_on_error:
+                e.print_context()
+                if not os.path.exists(e.pkg.build_log_path):
+                    tty.error("'spack install' created no log.")
+                else:
+                    sys.stderr.write('Full build log:\n')
+                    with open(e.pkg.build_log_path) as log:
+                        shutil.copyfileobj(log, sys.stderr)
+            raise
+
         finally:
             PackageBase.do_install = saved_do_install
 
-        # Dump log file if asked to
+        # Dump test output if asked to
         if args.log_format is not None:
             test_suite.dump(log_filename)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 86223688fa..897b5ea9bc 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -785,6 +785,14 @@ class PackageBase(with_metaclass(PackageMeta, object)):
         """Allow a stage object to be set to override the default."""
         self._stage = stage
 
+    @property
+    def env_path(self):
+        return os.path.join(self.stage.source_path, 'spack-build.env')
+
+    @property
+    def log_path(self):
+        return os.path.join(self.stage.source_path, 'spack-build.out')
+
     def _make_fetcher(self):
         # Construct a composite fetcher that always contains at least
         # one element (the root package). In case there are resources
@@ -1331,20 +1339,11 @@ class PackageBase(with_metaclass(PackageMeta, object)):
                     self.stage.chdir_to_source()
 
                     # Save the build environment in a file before building.
-                    env_path = join_path(os.getcwd(), 'spack-build.env')
-
-                    # Redirect I/O to a build log (and optionally to
-                    # the terminal)
-                    log_path = join_path(os.getcwd(), 'spack-build.out')
-
-                    # FIXME : refactor this assignment
-                    self.log_path = log_path
-                    self.env_path = env_path
-                    dump_environment(env_path)
+                    dump_environment(self.env_path)
 
                     # Spawn a daemon that reads from a pipe and redirects
                     # everything to log_path
-                    with log_output(log_path, echo, True) as logger:
+                    with log_output(self.log_path, echo, True) as logger:
                         for phase_name, phase_attr in zip(
                                 self.phases, self._InstallPhase_phases):
 
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index 80ecc1249d..66b79642a3 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -142,3 +142,18 @@ def test_install_with_source(
         spec.prefix.share, 'trivial-install-test-package', 'src')
     assert filecmp.cmp(os.path.join(mock_archive.path, 'configure'),
                        os.path.join(src, 'configure'))
+
+
+def test_show_log_on_error(builtin_mock, mock_archive, mock_fetch,
+                           config, install_mockery, capfd):
+    """Make sure --show-log-on-error works."""
+    with capfd.disabled():
+        out = install('--show-log-on-error', 'build-error',
+                      fail_on_error=False)
+    assert isinstance(install.error, spack.build_environment.ChildError)
+    assert install.error.pkg.name == 'build-error'
+    assert 'Full build log:' in out
+
+    errors = [line for line in out.split('\n')
+              if 'configure: error: cannot run C compiled programs' in line]
+    assert len(errors) == 2