[modules-dev] Review please for launcher bugs

[Dave Bristor]

Stanley M. Ho wrote:
> Hi Kumar,
> 
> Looks good. Several comments inline:
> 
> - src/share/classes/sun/module/ModuleLauncher.java
> 
>   188         String fname = new File(jamFileName).getName();
>   189         String moduleName = fname.substring(0, 
> fname.lastIndexOf('.'));
>   190         List<ModuleArchiveInfo> ilist =  repository.list();
>   191         for (ModuleArchiveInfo i : ilist) {
>   192             if (i.getFileName().endsWith(fname + 
> JamUtils.EXPANDED_EXT)) {
>   193                 definition = getModuleDefinition(repository, 
> i.getName());
> 
> This looks a bit funny. ModuleArchiveInfo.getFileName() should retain 
> the original filename of the jam, not the internal cooked filename. 
> Perhaps Dave could clarify.

When a ModuleArchiveInfo is created, we don't have an "original filename", we 
have a URL.  ExpandedJamRepository creates the ModuleArchiveInfo from
     new File(url.getFile()).getAbsolutePath()
I don't recall why it particularly does that, probably should be 
getCanonicalPath() if anything.

The URLRepository doesn't have an "original filename" at all; currently it 
provides just the module name: this should be fixed to refer to the downloaded 
  JAM but that's difficult at best: When the ModuleArchiveInfo is created, the 
repository has not yet downloaded the JAM.

	Dave

> 
> 238                 repository.initialize();
> 
> Assuming Dave will putback his changes soon, and we won't need to call 
> initialize() manually anymore.
> 
> - Stanley
> 
> 
> Kumar Srinivasan wrote:
>> Hi,
>>
>> I need a review of these fixes asap.
>> http://javaweb.sfbay/~ksrini/webrevs/j2se-tools/jsr277-6/
>> [The webrev contains the CRs].
>>
>> Built and tested on Solaris and Windows all the regresssion
>> tests in test/java/module pass.
>>
>> Thanks
>>
> _______________________________________________
> modules-dev mailing list
> modules-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/modules-dev