Approved.

Glad it was simple double quote and "==" usage, rather than requiring substantial change.

        Dave

On 4/3/2013 4:30 PM, Mike Duigou wrote:
I've received some feedback from Tim Bell and David Katleman.

Here's an updated version of the patch.

http://cr.openjdk.java.net/~mduigou/JDK-8011350/2

It turns out the real shell incompatibility was more straightforward. The 
outright conversion to bash isn't necessary. Replacement of '==' with '=' and 
double to single quotes in the cut command are sufficient.

The whichhg changes are based on an observation by Tim Bell in another bug that some versions of 
Solaris "/usr/bin/which" don't generate an exit code and instead output the message 
"no foo in PATH". This change better handles no mercurial being present.

Mike

On Apr 3 2013, at 08:56 , Mike Duigou wrote:

An alternative has been suggested: convert the hgforest.sh script to a bash 
script. I have tested this alternative on unbuntu linux 11.04, solaris 10u9, 
MacOS 10.7 and cygwin 1.7.17. This seems like less risk and there doesn't seem 
to be a compelling reason to stick with classic sh.

I have prepared an alternate webrev here:

http://cr.openjdk.java.net/~mduigou/JDK-8011350/1

We could still consider the original webrev if using bash turns out to have 
unexpected issues.

Mike

On Apr 2 2013, at 20:03 , Mike Duigou wrote:

Hello all;

Further testing on JDK-8011342 revealed that hgforest.sh can fail if the sh 
shell is not bash. The problem appears to be due to mixing of -o -a and ! in [] 
test expressions.

I have prepared a webrev here:

http://cr.openjdk.java.net/~mduigou/JDK-8011350/0/webrev/common/bin/hgforest.sh.udiff.html

This converts all of the potentially problematic [ expr -o expr ] [ expr -a expr ] and [ expr 
-{o|a} ! expr ] to use "test". My conversions are based on the advice of the autotools 
chapter on "Writing portable Bourne Shell" 
(http://sourceware.org/autobook/autobook/autobook_208.html#SEC208) for avoiding potential problems.

The other option is just to require bash which is already required by the new 
build process.

Mike

Reply via email to