On 2013-05-30 20:05, Mike Duigou wrote:
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.
Now my concern is that configure will fail without either specifying
--without-jtreg or having it available. I doubt RE has it available on
their build machines and it's not a requirement to just build. It would
be better if it was optional by default and only required if you
specified --with-jtreg.
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?
You could try executing it with a known parameter that returns 0 and
outputs something known (-version or similar), and see that it gives the
expected result, but I wouldn't go that far.
/Erik
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
