On May 31 2013, at 07:14 , Erik Joelsson wrote: > > > On 2013-05-31 15:47, Mike Duigou wrote: >> On May 31 2013, at 01:06 , Erik Joelsson wrote: >> >>> >>> 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. >> It is optional. Not specifying anything is the same as specifying >> "--without-jtreg". >> >> --without-jtreg disables >> --with-jtreg=no disables >> (nothing) disables >> --with-jtreg=path uses path >> $JT_HOME --with-jtreg uses $JT_HOME location >> (on PATH) --with-jtreg uses PATH location >> > I tried applying your patch and ran just "bash configure" and it failed on > missing jtreg, so i think you might have missed something in the logic for > the default case.
One more try then... http://cr.openjdk.java.net/~mduigou/JDK-8015510/2/webrev/ I added a default for absent in the AC_ARG_WITH Thanks, Mike > /Erik >>>> 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. >> OK, I will leave it as is then I think this patch is complete then. >> >>> /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
