[modules-dev] Review please for launcher bugs
Dave Bristor
David.Bristor at Sun.COM
Wed Jul 25 10:50:04 PDT 2007
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
More information about the modules-dev
mailing list