RFR: JDK-8172432,jar cleanup/update for module and mrm jar

[Xueming Shen]

On 01/12/2017 03:44 PM, Mandy Chung wrote:
> http://cr.openjdk.java.net/~sherman/8172432/webrev.01 <http://cr.openjdk.java.net/%7Esherman/8172432/webrev.01>
>
> Validator.java
>     159 if (entryName.endsWith(MODULE_INFO)) {
>
> Does this have the same issue as the isModuleInfoEntry method addresses (i.e. META-INF/versioned/n/module-info.class)?

let's use the Main.isModuleInfoEntry for consistency.

  159         // validate the versioned module-info
  160         if (isModuleInfoEntry(entryName)) {
  161             if (entryName.length() != MODULE_INFO.length())
  162                 checkModuleDescriptor(je);
  163             return;
  164         }

webrev has been updated accordingly.

-sherman


>
> I’m okay if you want to address this together with JDK-8165640.   This patch is a good improvement to the existing code anyway and good to push it.
>
> Mandy
>
>
>> On Jan 12, 2017, at 3:10 PM, Xueming Shen <xueming.shen at oracle.com <mailto:xueming.shen at oracle.com>> wrote:
>>
>> On 01/12/2017 01:46 PM, Mandy Chung wrote:
>>>
>>>> On Jan 10, 2017, at 10:00 PM, Xueming Shen <xueming.shen at oracle.com <mailto:xueming.shen at oracle.com>> wrote:
>>>>
>>>>
>>>> webrev has been updated to catch IMDE and fails the jar as other fatal error handing.
>>>>
>>>> http://cr.openjdk.java.net/~sherman/8172432/webrev
>>>>
>>>
>>> This version includes new fixes for JDK-8171830 and JDK-8165640.  Thanks for doing that.  The fix for JDK-8171830 looks fine.
>>>
>>> For JDK-8165640, it looks like checkModuleInfo can be refactored / moved to Validator so that the validation code is consistent and shared for checking in both places (MMR validation and module-info.class).
>>
>>
>>>
>>> Since it’s a separate issue, you should consider just to pushing the changeset for JDK-8172432 and JDK-8171830.   Resolve  JDK-8165640 in a separate patch that will make the review easier too.
>>
>> OK. as suggested, I have pulled out the latest changes related JDK-8165640,
>>
>> which includes the extra "service provider impl" check in the Validate.java, when
>> there is no root module-info.class. and 2 extra test case in modularJar.Basic.java.
>> (that webrev has been renamed to
>> http://cr.openjdk.java.net/~sherman/8172432/webrev.02)
>>
>> Since now there is no leverage for checkModuleInfo() (check service provider impl) in MMR
>> validation, I leave it in Main.java asis.
>>
>> The latest webrev is at
>>
>> http://cr.openjdk.java.net/~sherman/8172432/webrev/
>>
>> (you can compare it to the webrev you reviewed without JDK-8171830 at
>> http://cr.openjdk.java.net/~sherman/8172432/webrev.01)
>>
>> I will address JDK-8165640 in a separate issue.
>>
>> Thanks,
>> Sherman
>>
>