[modules-dev] Review request: 6560281

[Dave Bristor]

Stanley M. Ho wrote:
> Hi Dave,
> 
> Dave Bristor wrote:
>> They are uppercase so that they stand out: the intent is that they be 
>> treated as reserved words, and that other respository implementation 
>> specific properties can be in lower case.  Perhaps make them lower 
>> case and/or use a case-insensitive comparison?
> 
> Sounds reasonable.

I made them lower case, and they *must* be specified that way: at times, they 
are used as keys in maps, which don't support case-insensitivity.

> 
>> Related: the default jre extension repository directory is
>>     ${java.home}/lib/module/ext
>> Do you know into which Makefile I should cause that directory to be 
>> created? I've modified LocalRepository to create the sourceLocation if 
> 
> Try make/java/module/Makefile.

I updated that to create the directory.

>> it does not exist (which will presumably suffice for the user 
>> repository).
> 
> If the location doesn't exist, I think it's better to ignore it. It 
> would be annoying if the user just mistypes the repository path in the 
> launcher and then finds the directory being created as a side effect.

This presents a conundrum: As we agreed Monday, the standard 
repository.properties has
     user.source=file:${user.home}/.java/module
and that does not exist, then the JVM will exit.  Something needs to create 
that directory first.  So along with your mention in earlier email (attached) 
about having system properties to define the locations of repositories, we 
still need something to create them.

One option, for now, is to comment out the user repository entry in 
repository.properties, and uncomment it once we know how its directory will be 
created.  For now, this is what's at the updated webrev.  What do you think?

> 
>> Fixed.  In line with your suggestion of "classname" above, shouldn't 
>> these all be "filename"?
> 
> Apparently no. "file" has been the convention used in other 
> configurations in the JDK.

Thanks for checking.

>> Thanks, I didn't know about PropertyExpander: fixed.  Well, maybe not: 
>> if you give PropertyExpander.expand this string:
>>     ${user.home/foo/bar
>> it doesn't throw an exception, which doesn't seem reasonable (IMHO).  
>> At your suggestion, the code currently uses PropertyExpander, but I 
>> disabled the test which checked for the condition above.  Please let 
>> me know if I should restore the code I had before.
> 
> Let's use PropertyExpander for now. If it turns out we need better error 
> handling, then we will migrate all the usages of PropertyExpander into 
> something better.

Agreed; PropertyExpander it is.

webrev updated: http://javaweb.sfbay/java/jdk/ws/libs/rev/6560281/

Thanks,
	Dave

> 
> - Stanley
-------------- next part --------------
An embedded message was scrubbed...
From: "Stanley M. Ho" <Stanley.Ho at Sun.COM>
Subject: Re: [modules-dev] Issue re location of extension & user repositories
Date: Tue, 24 Jul 2007 16:05:36 -0700
Size: 6704
Url: http://mail.openjdk.java.net/pipermail/modules-dev/attachments/20070725/6b89e401/attachment.mht