[modules-dev] Review request: 6559092

[Dave Bristor]

Stanley M. Ho wrote:
> Hi Dave,
> 
> Looks good. Several comments inline:
> 
> -  src/share/classes/sun/module/repository/RepositoryConfig.java
> 
> I think rather than creating a fake system repository if one doesn't yet 
> exist, it would be better to simply use the bootstrap repository as the 
> default system repository, i.e.
> 
>     private static Repository systemRepository =              
>             Repository.getBootstrapRepository();
> 
> Then if the module launcher needs to change the system repository later, 
> it could simply reset it, e.g.
> 
>         repository = Modules.newLocalRepository(
>                 RepositoryConfig.getSystemRepository(),
>                 "application",
>                 tmp.getCanonicalFile().toURI().toURL()
>                 );
>     RepositoryConfig.setSystemRepository(repository);

That would mean that the system repository could be changed at will.  The 
intention of the proposed change was that the system repository could be set 
at most once.  Do we really want to allow that?  If so, should we do any 
checks to be sure that the bootstrap repository is reachable from the argument 
given to setSystemRepository?  Or, we could add some other field to indicate 
whether setSystemRepository has ever been invoked, and allow it to be invoked 
at most once/JVM.  Comments?

> 
> 
> - test/java/module/repository/LocalRepositoryTest.java
> 
> 81         check(systemRepo.getParent() == 
> Repository.getBootstrapRepository());
> 
> This check would break as soon as the repository configuration file 
> support is in place, but should be okay for now.

Thanks: I'm working on the configuration file support now, and will be sure to 
make that change!

	Dave

> 
> - Stanley