RFR : 8015510 : (s) Improve JTReg location detection and provide location to test/Makefile

[Mike Duigou]

Thank you for the feedback. I have updated the webrev :

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

It's a lot simpler now.

The one part I couldn't make perfect was the check

      if test ! -f "$JTREGEXE"; then
        AC_MSG_ERROR([JTReg executable does not exist: $JTREGEXE])
      fi

which only checks for whether the executable exists and not whether it is executable. I understand the "-x" text condition can't be used because it doesn't work on windows. Alternatives?

Mike

On May 29 2013, at 01:06 , Erik Joelsson wrote:

> 
> 
> On 2013-05-29 00:30, Mike Duigou wrote:
>> Hello all;
>> 
>> As a follow on to JDK-8007129 recently pushed to the jdk8/build repo I've improved the detection logic for finding jtreg. In addition to using the provided path it will also try the location specified by JT_HOME and in the command path.
>> 
>> Attention people who just let magic happen and don't pay attention until something breaks: Currently if no location is defined for JT_HOME a default location of $(SLASH_JAVA)/re/jtreg/4.1/promoted/latest/binaries/jtreg is used. SLASH_JAVA is either /java on posix platforms or J:\\ on windows. This default location still works for now, but future changes will almost certainly remove it. You can avoid this pain by adding an appropriate --with-jtreg to your configure command today.
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-8015510/0/webrev/
> When trying to find a program on the path, please use the builtin AC_PATH_PROG or similar macro. Using "which" is bad as it works differently on all platforms and we have worked hard to remove its use in configure. It looks like at that point, JTREGEXE is required to be found, then you can use BASIC_REQUIRE_PROG which also fails with a suitable error message.
> 
> There is no need to repeat AC_SUBST calls and making them conditionals has no effect to my knowledge. I would move them out of the if statements and put them at the end of the block.
> 
> If JT_HOME is set in the environment, it is just accepted and not validated. Should it be?
>> If "--without-jtreg", "--with-jtreg=no" or no jtreg directive is used on the command line the spec.gmk vars JT_HOME and JTREGEXE will still present but will be empty. I am not sure if it's better (or possible) to avoid defining them in this case.
>> 
> If you want to have it undefined, you can achieve it with the following pattern (look at OPENJDK for an example):
> In .m4: SET_JT_HOME="JT_HOME=$JT_HOME"
> In spec.gmk.in: @SET_JT_HOME@
> 
> Keeping it undefined when not set would let the makefiles pick it up from the environment, a pattern the new build is trying to break.
> 
> /Erik
>> Also, since test/Makefile doesn't yet read spec.gmk it is necessary to provide it the JT_HOME location via an environment var in Main.gmk. This will eventually be fixed.
>> 
>> Rather than wait for 8007129 to propagate to TL I would like someone to sponsor this change into build repo.
>> 
>> Thanks,
>> 
>> Mike