On 17/06/2013 1:41 PM, Mike Duigou wrote:

On Jun 16 2013, at 19:59 , David Holmes wrote:

Hi Mike,

I don't like the warnings. A warning implies something needs fixing. If you 
provide a default and I choose to use it why should it generate a warning?

I this case the warnings are currently provided as a gentler alternative to 
simply deleting the defaults. It is intended to provide an opportunity those 
using the default to switch to specifying the value in the invocation. For all 
of the variables with defaulted values the variable should already be set by 
some external context (JPRT, root makefile, etc.). If the warning does trigger 
then I hope it does catch people's attention.

Eventually I plan to have the test/Makefiles include the spec.gmk file to allow 
them to grab their context directly from the configuration. Hopefully the 
spec.gmk will be the sole configuration mechanism. For now we will continue to 
have some things defined via environment variables and I didn't want to 
immediately break existing usages.

As long as something provides a default as to where to find jtreg on SLASH_JAVA :)

Thanks,
David

Mike

David

On 14/06/2013 2:39 PM, Mike Duigou wrote:

On Jun 13 2013, at 15:33 , Jonathan Gibbons wrote:

Mike,

Thanks for looking at this.

I guess I am not a big fan of JT_ as an abbreviation of JTREG_.

I recently propagated the use of JT_HOME up to the configure script based upon 
it's usage in the jtreg scripts. I have no particular preference. Would you 
like to try to standardize on JTREG_ names? It would be understandable if JT_ 
got confused for JavaTest.

Lines 149-151 seem off-color to me.

I trust your judgement on this. It seemed strange to me to check for the 
dist/lib/classes.jar only if the TESTJAVA definition was coming from 
PRODUCT_HOME. I have restored it to only check in that case.

  Based on the name, I would never expect $TESTJAVA to contain 
dist/lib/classes.jar.    The previous code looked for distlib/classes.jaar in 
$PRODUCT_HOME which was a more accurate reflection of what was going on.


Updated webrev:

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

-- Jon


On 06/13/2013 01:11 PM, Mike Duigou wrote:
Hello all;

I have been working on improving the execution of tests via the root repo "make 
test" target (JDK-8009683). This work has so far mostly concerned the jdk repo but 
now I would like to increase the commonality of the test/Makefile for the various repos.

In the langtools/test/Makefile these vars are

JTREG_HOME
JT_JAVA
TESTJAVA

currently unconditionally set. They should only be set if they don't already 
have definitions. This is part of an effort to retire some of the JPRT specific 
vars and synchronize the jdk and langtools test/Makefile

Additionally

- JTREG_HOME is renamed to JT_HOME for consistency

- TESTBOOTCLASSPATH setting is made more uniform, ie. not just when 
PRODUCT_HOME is defined.

- Warnings are now printed for variables using the default "unset" value. In 
most cases these variable *should* already have a value so no default should be used. For 
now the defaults are retained but with a warning.

- An error is generated if PRODUCT_HOME seems to point at an invalid location.

A review webrev is posted here:

http://cr.openjdk.java.net/~mduigou/JDK-8016577/0/

Once this issue is completed I will follow up with JDK-8016573 which will 
replace JPRT_JAVA_HOME with JT_JAVA in the Main.gmk make file.

Cheers,

Mike



Reply via email to