RFR: JDK-8065576: Enable pipefail in the shell used by make to better detect build errors

[Magnus Ihse Bursie]

On 2014-11-26 15:56, Erik Joelsson wrote:
> Hello,
>
> 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 
non-trivial. :-(

First of all, I realize that the existing line in MakeBase:
     WRAPPER_SHELL:=$$(BASH) $$(SRC_ROOT)/common/bin/shell-tracer.sh 
$$(if $$(findstring yes,$$(IS_GNU_TIME)),$$(TIME),-) 
$$(OUTPUT_ROOT)/build-trace-time.log $$(BASH)
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.

/Magnus