[modules-dev] Review request for 6560271 (embedded JAR files)

[Dave Bristor]

Even more followup: I just noticed in issue regarding uninstallation of legacy 
jars: please hold off on any review until I get to look at it tomorrow morning...
	Dave

Dave Bristor wrote:
> Thanks for the feedback, my followup inline.
> 
> Kumar Srinivasan wrote:
>> Hi Dave,
>>
>> Generally looks good. I am quite sure Stanley will do a more
>> thorough review, here are some comments:
>>
>> 1. I noticed you were using buffers of 4096 and other places
>> 8192, I think we should put a static in JamUtils.BUFFER_SIZE = 8192
>> and simply use this everywhere or if we want different sizes for
>> whatever reason we could do this:
>> JamUtils.LARGE_BUFFER_SIZE = 8192
>> JamUtils.MEDIUM_BUFFER_SIZE = 4096
>> JamUtils.SMALL_BUFFER_SIZE = 2048
> 
> I added a single BUFFER_SIZE = 8192 in JamUtils, and made use of it in files 
> that are already part of this webrev.  I updated 6566724 to explicitly mention 
> this for other code too:
> 
> 6566724 Add constants for oft-used strings such as MODULE.METADATA
> 
> That'll make a nice cleanup task for a rainy day ;-)
> 
>> 2. LocalJamDefinitionContent.java s/cpde/code/g
> 
> Webrev updated:
> http://analemma.sfbay.sun.com/java/jdk/ws/libs/rev/6560271
> 
> Fixed, thanks.
> 
> 	Dave
>>
>>
>>
>>> Hi folks,
>>>
>>> This set of changes lets embedded legacy JAR files work, including with the 
>>> JarLibraryPath annotation.  I plan to build on this in making changes to 
>>> support native libraries.  Comments welcome!
>>>
>>> bugster: http://monaco.sfbay/detail.jsp?cr=6560271
>>> webrev: http://analemma.sfbay.sun.com/java/jdk/ws/libs/rev/6560271
>>>
>>> Description of changes:
>>>
>>> Several bits of cleanup, such as for javadoc, consistency, etc.
>>>
>>> JamUtils: Added copyFile() which is therefore removed from URLRepository.
>>>
>>> LocalRepository: Improve location of expansionDir.
>>>
>>> URLRepository: Just cleanup.
>>>
>>> LocalJamDefinitionContent: Support for legacy/embedded JAR files.  Code 
>>> cleanup to make it more consistent with URLModuleDefinitionContent.  I think 
>>> this should be renamed to LocalModuleDefinitionContent for consistency (or 
>>> rename the other class); opinions?
>>>
>>> URLModuleDefinitionContent: Support for legacy/embedded JAR files.
>>>
>>> RepositoryUtils: This new class has static methods which were formerly 
>>> duplicated in others, especially {Local,URL}Repository.  In theory, this could 
>>> be a pair of base classes, one for the *Repository implementations, and one 
>>> for the *ModuleDefinition implementations.  The former is particularly likely.
>>>
>>> JamBuilder: now public constructor and can add annotations to the JAM that's 
>>> to be built.
>>>
>>> LegacyJarTest: makes sure the above works ;-)
>>>
>>> Thanks,
>>> 	Dave
>>> _______________________________________________
>>> modules-dev mailing list
>>> modules-dev at openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/modules-dev
>>>   
>>
> _______________________________________________
> modules-dev mailing list
> modules-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/modules-dev