From c7a789e2d61b85d081f65100f0d5ee40bf78928f Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 17 Sep 2017 15:07:44 -0700 Subject: 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. --- lib/spack/spack/build_environment.py | 17 +++++++++++++++-- lib/spack/spack/cmd/install.py | 20 +++++++++++++++++++- lib/spack/spack/package.py | 21 ++++++++++----------- lib/spack/spack/test/cmd/install.py | 15 +++++++++++++++ 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 @@ -68,6 +70,9 @@ the dependencies""" subparser.add_argument( '--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") @@ -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 -- cgit v1.2.3-70-g09d2