Report compile errors in shared library #327
Conversation
# Conflicts: # CHANGELOG.md
| got_shared_library = true | ||
| attempt_multiline("Build shared library with #{gcc_binary} for #{p}") do | ||
| exe = cpp_library.build_shared_library( | ||
| config.aux_libraries_for_unittest, | ||
| gcc_binary, | ||
| config.gcc_config(p) | ||
| ) | ||
| unless exe | ||
| puts "Last command: #{cpp_library.last_cmd}" | ||
| puts cpp_library.last_out | ||
| puts cpp_library.last_err | ||
| got_shared_library = false | ||
| end | ||
| next got_shared_library | ||
| end | ||
| next unless got_shared_library |
There was a problem hiding this comment.
Can you explain this change a bit? I'm not opposed to it, it just seems weird to move a small function into a big function
There was a problem hiding this comment.
To be honest, I don't understand it myself. But the symptom was that a failure in the compile shared library function (which called the attempt_multiline() method) did not get properly reported out when it failed. That is, we got a compile error in the shared library then went on to compile the examples and when the examples compiled okay the overall test reported as passed. It seemed to me that when the attempt_multiline() method recognized a failure it reported it out one level but it didn't get out two levels. That is, by putting the call into a small, well-named method (which would otherwise be a good practice), it swallowed the error. But by moving it up a level the error got properly reported.
Of course, this is all speculation and since I don't really understand what is going on I'm not real comfortable with the explanation. In an case, I added tests that confirmed the problem and verified the fix. But I really would welcome a better understanding of what changed and how this fixed it!
| puts cpp_library.last_err | ||
| return false | ||
| end | ||
| return true |
There was a problem hiding this comment.
@jgfoster I think the actual problem was this return true which should just be an implicit return of exe 🤦 I'll push up this change in a bit and see what happens
Add test for compile errors.