Re: RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

On 7/6/18, 5:43 PM, Martin Buchholz wrote: I would use different timestamps for the 4 possible times used in this test, if only to make it clearer which value comes from where. webrev has been updated accordingly. +// ze.setLastModifiedTime(now); +

Re: RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

I would use different timestamps for the 4 possible times used in this test, if only to make it clearer which value comes from where. +// ze.setLastModifiedTime(now);+ ze.setTime(now.toMillis()); setTime only sets the DOS time? Which only has a granularity of 2 seconds? If so, how

RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

Hi Please help review the changeset for JDK-8206389 issue: https://bugs.openjdk.java.net/browse/JDK-8206389 webrev: http://cr.openjdk.java.net/~sherman/8206389/webrev Cause: ZipOutputStream.writeCEN().writeCEN() incorrectly calculate the length of bytes needed for the "unix timestamps" when

Re: Prototype of jpackager in jdk/sandbox [was: Draft JEP proposal: JDK-8200758: Packaging Tool]

I've just taking a look at the patch, i don't see how this can be integrated soon, the code is consistently inconsistent as one of my colleague would say, even the coding conventions are not respected. i believe that's it's because the code have been written first in Java 6 an without

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

Thanks a lot for reviewing, Mandy! Jiangli > On Jul 6, 2018, at 1:40 PM, mandy chung wrote: > > Hi Jiangli, > > On 6/28/18 4:15 PM, Jiangli Zhou wrote:> webrev: > http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/ >> RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921 > >

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

> On Jul 6, 2018, at 1:36 PM, Ioi Lam wrote: > > Hi Jiangli, > > The VM changes look good to me. Thanks! > > For the tests: I think we need a comment here saying that "mods" is > intentionally empty, and also an explanation why it's not necessary to > actually fill with actual modules?

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

Hi Jiangli, On 6/28/18 4:15 PM, Jiangli Zhou wrote:> webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/ RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921 Good work. I'm glad to see a pretty good startup improvement. I reviewed java.base change that looks good.

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

Hi Jiangli, The VM changes look good to me. For the tests: I think we need a comment here saying that "mods" is intentionally empty, and also an explanation why it's not necessary to actually fill with actual modules? Thanks - Ioi 3) ArchivedModuleComboTest.java 55 Path

Prototype of jpackager in jdk/sandbox [was: Draft JEP proposal: JDK-8200758: Packaging Tool]

An initial prototype of the jpackager tool has been pushed to a new 'JDK-8200758-branch' branch in the JDK sandbox [1]. If anyone is interested in taking a look, you can clone it as follows:     hg clone http://hg.openjdk.java.net/jdk/sandbox     cd ./sandbox     hg update JDK-8200758-branch

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

Hi Calvin, Thanks for the review! Here is the updated webrevs that address the feedbacks from you and Ioi: http://cr.openjdk.java.net/~jiangli/8202035/webrev_inc.01/ Full webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev_full.01/ > On Jul 6, 2018, at 9:15 AM, Calvin Cheung wrote: >

Re: RFR: 8206495 [testbug] Redundant System.setProperty(null) tests

Hi Roger, Looks fine. > On Jul 6, 2018, at 3:08 PM, Roger Riggs wrote: > > Please review the removal of a duplicate test for System.setProperty(null). > > Webrev: >http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/ > > Thanks, Roger >

Re: RFR: 8206495 [testbug] Redundant System.setProperty(null) tests

+1 Mandy On 7/6/18 12:08 PM, Roger Riggs wrote: Please review the removal of a duplicate test for System.setProperty(null). Webrev:    http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/ Thanks, Roger

RFR: 8206495 [testbug] Redundant System.setProperty(null) tests

Please review the removal of a duplicate test for System.setProperty(null). Webrev:    http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/ Thanks, Roger

RFR: jsr166 integration 2018-07

http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html Some smallish changes; all of them already seen review elsewhere. 8206123: ArrayDeque created with default constructor can only hold 15 elements

Re: RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

Changed. Thank you. > On Jul 6, 2018, at 2:31 PM, Roger Riggs wrote: > > Hi Jim, > > The phrase: > > "All Character#isWhitespace(int) white space characters, including tab, are > treated as a single space." > > Can be mis-interpreted to mean collectively they are replaced by a single >

Re: RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

Hi Jim, The phrase: "All Character#isWhitespace(int) white space characters, including tab, are treated as a single space." Can be mis-interpreted to mean collectively they are replaced by a single space. Perhaps replace "All" with "Each". Regards, Roger On 7/6/2018 1:16 PM, Jim Laskey

RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

csr: https://bugs.openjdk.java.net/browse/JDK-8200435

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

Hi Jiangli, Thanks for this start-up improvement. The changes look good overall. I've the following minor comments. 1) make/hotspot/symbols/symbols-unix 134 JVM_InitializeFromArchive If you want the symbols to be in alphabetical order, the above should be moved after

Re: RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors

Hi, On 07/05/2018 01:01 AM, David Holmes wrote: I dispute "they will understand this might have happened in another thread". What if the stack trace was like the following... Before patch: 1st attempt [ForkJoinPool.commonPool-worker-3]: java.lang.ExceptionInInitializerError     at

Re: Adding new IBM extended charsets

Hi Sherman, Thank you for your valuable inputs. I would like to clarify some of your points. > I would assume the best option might be (3), in which only keeps those > ibm charsets that might be used/configured to be the default charset for > vm startup on IBM platform, and leaves the rest to

RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

Hi Alan ,so it looks likeJDK-8204233 added a switch (system property) to enable the enhanced socket IOException messages . That would be an option as well for 8205525 . 8205525 adds the jar file name and the line number info to the exception message . In case that only the

Re: Adding new IBM extended charsets

Hi Florian, Thank you for your response. iconv is platform dependent and not good for the platform agnostic nature of Java. Also, many charsets in Java are not available across platforms. I believe Java decided to have its own charsets due to those reasons so that it can work seamlessly on any

RFR[jdk8u-dev]: 8134124: sun/security/tools/jarsigner/warnings.sh fails when using Hindi locale

Hi all, Please review this trivial test fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8134124 Webrev: http://cr.openjdk.java.net/~rpatil/8134124/webrev.00/ The test is made locale independent now. Regards, Ramanand.

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

Hello. I'm not reviewer, but I tested your fix with some locales (non-UTF8 locales) on AIX platform. $ LANG=C javac TestIBMBugs.java $ LANG=ko_KR javac TestIBMBugs.java $ LANG=zh_CN javac TestIBMBugs.java Also testcase worked fine as expected. Thanks, Ichiroh Takiguchi IBM Japan, Ltd. On

Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java failed in Windows.

On 6/07/2018 6:03 PM, Srinivas Dama wrote: Hi, This patch fixes JImageExtractTest.java but nothing done specific to JImageListTest.java as I couldn't reproduce this failure earlier. So I just removed JImageListTest.java from ProblemList.txt. Now I am reverting this change and raised a new bug

Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java failed in Windows.

Hi, This patch fixes JImageExtractTest.java but nothing done specific to JImageListTest.java as I couldn't reproduce this failure earlier. So I just removed JImageListTest.java from ProblemList.txt. Now I am reverting this change and raised a new bug specific to this failure[JDK-8206445].

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

Hi Mikael, Thomas, Alan, thanks a lot for the quick reviews. I've just fixed the typo and pushed the fix. Sorry for the trouble, Volker On Fri, Jul 6, 2018 at 9:06 AM, Alan Bateman wrote: > On 06/07/2018 08:03, Mikael Vidstedt wrote: >> >> Fix the typo while at it? >> >> +// and [yen]

Re: JDK 12 RFR of JDK-8206440: Remove javac -source/-target 6 from jdk regression tests

On 06/07/2018 04:37, joe darcy wrote: Hello, With javac support for -source/-target 6 and 1.6 planned to be removed later in JDK 12 (JDK-8028563), please review the removal of usage of these options in the non-manual jdk regression tests: This looks okay.

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

On 06/07/2018 08:03, Mikael Vidstedt wrote: Fix the typo while at it? +// and [yen] an [overscore] are encoded to 0x5c and 0x7e an -> and I agree, otherwise looks okay to me. -Alan

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

Hi Volker, looks good and trivial. I did not test-compile it, but I trust you did that already. Best Regards, Thomas On Fri, Jul 6, 2018 at 8:56 AM, Volker Simonis wrote: > Hi, > > can I please have a review for this trivial test fix? > >

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

Fix the typo while at it? +// and [yen] an [overscore] are encoded to 0x5c and 0x7e an -> and Cheers, Mikael > On Jul 5, 2018, at 11:56 PM, Volker Simonis wrote: > > Hi, > > can I please have a review for this trivial test fix? > >

[11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

Hi, can I please have a review for this trivial test fix? http://cr.openjdk.java.net/~simonis/webrevs/2018/8206436/ https://bugs.openjdk.java.net/browse/JDK-8206436 The problem is that the test contains some non-ASCII characters in comments which makes the compilation fail for non-Unicode