[modules-dev] Review request: 6560281

[Dave Bristor]

Stanley M. Ho wrote:
> Hi Dave,
> 
> Dave Bristor wrote:
> 
>>Why must the source location be a URL?  In repository.properties it 
>>given as a file.
> 
> 
> Source location could be either a URL or file path. However, the 
> repository.properties in the shipping JRE will be the same for all 
> platforms. Because file path is platform specific, the default source 
> locations come with the repository.properties in the JRE would be in the 
> form of URL.

Ahhh...makes sense.  Fixed.

>>CLASS could be sun.module.repository.LocalRepository.
>>
>>Or, we could check the URL: if it's a file: URL, create a LocalRepository.
> 
> 
> This would exclude the case where you want to use URLRepository with a 
> file URL. In fact, this is what we might end up using for the extension 
> repository.

I've got it working by implementing support for the CLASS attribute.  Take a 
look at repository.properties, and notice in 
RepositoryConfig.createRepositories() that there's now support for CLASS.

(As an aside, I think it should be possible to make better use of generics in 
the CLASS-supporting code, but my attempts thus far failed.  Any pointers 
would be appreciated.)

As per our discussion today, the entry for extension repository in 
repository.properties is perhaps incorrect, and we may need multiple such 
repositories: do you have suggestions to make now, or shall we defer this part 
of the decision?

>>In the process of running tests, I found a failure, fixed it, but wonder 
>>about a couple things.  Take a look at the changes to ModuleLauncher.  
>>It seems odd to me that we don't have a method Query.version(Version).  
>>Is that intentional?
> 
> 
> Yes. Try Query.version(Version.toVersionConstraint()).

Fixed.

>>Second, repository.install returns a ModuleArchiveInfo, but we need a 
>>ModuleDefinition, so we find() it.  Would it make sense for a method
>>    ModuleDefinition Repository.get(ModuleArchiveInfo)
>>based on the identity of the ModuleArchiveInfo?
> 
> 
> No. Installing a jam file does not mean you could always use its 
> ModuleDefinition at runtime, e.g. the ModuleDefinition might be shadowed 
> by another ModuleDefinition with the same name and version in a parent 
> repository. Try Repository.find(mainfo.getName(), mainfo.getVersion()).

OK: I'd used what you suggest; I was just hoping to avoid the cost of find().

webrev updated with all tests passing (I added a couple more since last review):
http://javaweb.sfbay/java/jdk/ws/libs/rev/6560281/

Thanks,
	Dave

> 
> - Stanley
> _______________________________________________
> modules-dev mailing list
> modules-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/modules-dev