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.s...@oracle.com
<mailto:xueming.s...@oracle.com>> wrote:
On 01/12/2017 01:46 PM, Mandy Chung wrote:
On Jan 10, 2017, at 10:00 PM, Xueming Shen <xueming.s...@oracle.com
<mailto:xueming.s...@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
