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
