RFR: JDK-8158535: Configure script uses basic tools directly in many places

[Erik Joelsson]

Hello,

On 2016-06-06 01:36, David Holmes wrote:
> Hi Erik,
>
> On 4/06/2016 1:56 AM, Erik Joelsson wrote:
>> In the configure script we check for the presence of several basic
>> tools, like grep, sed, echo etc, and assign variables for them. In the
>> makefiles we are very diligent with always using the tools through these
>> variables to make sure we have a well functioning version of the tool at
>> hand. In many places in the configure script, we have been sloppy with
>> this.
>>
>> I recently stumbled over this because we change the PATH variable in
>> parts of the script (typically when looking for the toolchain) which can
>> then potentially change which instance of the tool that is being run
>> unless the resolved variables are used. This patch fixes all the
>> instances that I could find.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8158535
>> Webrev: http://cr.openjdk.java.net/~erikj/8158535/webrev.top.01/
>
> Two questions:
>
> 1. How do we know a $XXX variable actually exists for a given command?
Well, we would have declared it somewhere so it's manual inspection. 
It's most commonly done through "BASIC_REQUIRE_PROGS".
> 2. How do we know it has been set when modifying the .m4 files? I 
> presume there are some parts of at least one .m4 file that can't use 
> $XXX because it won't have been set yet.
>
That is true, but for the most basic tools, we set them up very early. 
There is BASIC_INIT and then BASIC_SETUP_FUNDAMENTAL_TOOLS. It should be 
very rare to edit the logic in BASIC_INIT though. For more complex 
tools, there is more of a careful ordering. In this patch I'm pretty 
sure I only changed basic tools however.
> I also noticed on Windows that there are still inconsistencies. For 
> example we use $ECHO in places, but I also see:
>
>  99     new_path=`cmd /c "for %A in (\"$input_path\") do @echo 
> %~sA"|$TR \\\\\\\\ / | $TR ...
>
This particular line is executing echo inside cmd, so it actually needs 
to be just "echo". It is however possible that I missed something in 
this patch. I started out by hunting down the ones I had an issue with 
in my particular setup that triggered this change. Then I greped for 
more instances of the same.

/Erik
> Thanks,
> David
>
>> /Erik