Conversation
becker33
left a comment
There was a problem hiding this comment.
This looks good to me, approved pending testing.
lib/spack/spack/util/log_parse.py
Outdated
|
|
||
| log_events = [] | ||
| for i, line in enumerate(lines): | ||
| if re.search('error', line, re.IGNORECASE): |
There was a problem hiding this comment.
I would highly recommend using error: instead of error. The latter will pick up hundreds of lines of the compiler linking to error-related modules.
There was a problem hiding this comment.
I think this will evolve into lots of heuristics (like eventually getting warnings as well). I'll change it to error for now.
alalazo
left a comment
There was a problem hiding this comment.
This will be a huge usability improvement. Probably we'll get neater bug report on failed builds as a side-effect. 👍 💯
| context from the Python code. | ||
|
|
||
| SpackError handles displaying the special traceback if we're in debug | ||
| mode with spack -d. |
| if self.build_log: | ||
| events = parse_log_events(self.build_log) | ||
| out.write("\n%d errors in build log:\n" % len(events)) | ||
| out.write(make_log_context(events)) |
There was a problem hiding this comment.
Parsing all the errors and showing them is great!
lib/spack/spack/util/log_parse.py
Outdated
| logfile (str): name of the build log to parse | ||
| context (int): lines of context to extract around each log event | ||
|
|
||
| Currently looks for lines that contain the string 'error', ignoring case. |
08b80a6 to
cb3c5ee
Compare
|
Ok I think I've addressed all comments here -- I'm about to add tests but it needs a rework of I/O redirection. Currently I can't get the output I need to test this functionality. I'll push a new PR and rebase on it. |
cb3c5ee to
f29f82d
Compare
- If a failure comes from an external command and NOT the Python code, display errors highlighted with some context. - Add some rudimentary support for parsing errors out of the build log (not very sophisticated yet). - Build errors in Python code will still display with Python context.
f29f82d to
0df994a
Compare
- install and probably other commands were designed to run once, but now
we can to test them from within Spack with SpackCommand
- cmd/install.py assumed that it could modify do_install in PackageBase
and leave it that way; this makes the decorator temporary
- package.py didn't properly initialize its stage if the same package had
been built successfully before (and the stage removed).
- manage stage lifecycle better and remember when Package needs to
re-create the stage
- Update handling of ChildError so that its output is capturable from a SpackCommand - Update cmd/install test to make sure Python and build log output is being displayed properly.
|
@tgamblin : I am trying this with latest develop and bit confused about how this works. I don't see error/log message : I thought with this PR we will get behaviour of using This is useful for for |
Nope. Depending on the build, this would result in thousands of lines of useless output. The goal of this PR is to grep the build log for things like
Hmm. Could you do something like: That would give you the behavior you desire. I just don't know where to get |
|
thanks for clarification!
Hmm...typically I look at last
yes, currently my travis config has something like: and I thought I don't need this anymore. |
|
@pramodskumbhar: are there errors this isn't catching? If so you could add them to the filter so they're printed out without -v. Note that it's not just the lines but also context and line numbers that are printed by his command. Also, once the test-suite PR is done, that command will be able to upload full logs to cdash with errors highlighted, and install will be able to spit out a cdash log as well. So you could potentially use those from travis and view logs externally. |
Simple test I am trying to use with compilers that require dirty. For example, if I try installing zlib as: But actual error is: So my expectation was to capture I have related question: In our internal jenkins/travis plans we would like to But if the dependency is failed then this doesn't work. So I would like to know how to get the path of |
Currently, our regex only looks for |
|
@adamjstewart True, but if we also look for |
|
I'm tempted to look at what ctest does in the next couple weeks. I want this for cdash error output anyway. Ctest has some magic heuristics it uses and I'm sure they're better tested than what we could come up with. But for @pramodskumbhar right now, we could just add an option to install that dumps the log on error. Seems less cumbersome than the shell scripting alternative, and it's easy enough to do... thoughts? |
this is my / our developers expectation for nightly travis/jenkins builds of different software components. |
Currently, if you get a build error in Spack, you get this:
Spack is trying to be helpful by showing Python context for the package, but usually you don't get a Python error. You get some type of build error, and you just want to look at the build log. Also, with our various build system classes, the Python context is often in a class the user knows nothing about (like the
configuremethod inAutotoolsPackageshown here), and doesn't really tell you what the issue is.This PR changes the behavior. If the error raised is a
ProcessErrorfrom some script or tool invoked by the build process (e.g.,makeorconfigure), we'll show errors from the build log instead. If the error is some other error (e.g. one that actually comes from Python code) we'll show Python context. So, here's the new output:Users now get a chunk of the build log with the error lines highlighted. If there are multiple errors, each will be shown with 6 lines of context before and after (with
[...]in between the sections). If color is enabled, the errors are highlighted in red.Hopefully this means people won't need to take the extra step of opening the build log in their editor to see what happened, even if they didn't use
spack install -v.Tasks: