RFR: JDK-8065576: Enable pipefail in the shell used by make to better detect build errors
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Wed Nov 26 23:21:10 UTC 2014
On 2014-11-26 15:56, Erik Joelsson wrote:
> Please review this build reliability fix. In JDK-8065138, we would
> have caught the error much faster if the build had failed instead of
> silently generating bad output. To avoid this in the future, this
> patch activates pipefail and errexit in the shell, when available.
> This means that long command lines, consisting of multiple commands,
> chained together either by pipes or ';', will fail the build
> regardless of which of the sub commands that failed. Currently, all
> but the last command would be ignored.
> Since these features my not always be available in all versions of
> bash, I added a check to configure for each of them and only enable
> them if they are available. I also had to fix some instances where we
> have 'grep' and 'find' returning non zero without it being an error.
> Thanks to Martin who suggested this in the first place.
Thank you for fixing this so quickly!
The webrev looks good as such, but I think you have missed how this
interact with common/bin/shell-tracer.sh. Which, unfortunately, is
First of all, I realize that the existing line in MakeBase:
$$(if $$(findstring yes,$$(IS_GNU_TIME)),$$(TIME),-)
is incorrect. (It's my bad...) The last $$(BASH) is supposed to be the
old value of $(SHELL), according to shell-tracer.sh. So, even before
your fix, it should have been $$(SHELL), not $$(BASH).
However, even if you fix that, I'm not sure how this will work with a
$(SHELL) that has spaces in it. I think you can get it working it by
sending "$$(SHELL)" as last argument in WRAPPER_SHELL, and updating
shell-tracer.sh, so that the assignment to OLD_SHELL keeps the "" around
$3, but the calls to "$OLD_SHELL" (note, two places) removes the "". But
you'll have to verify that.
More information about the build-dev