RE: [EXTERNAL] JDK-8234076 bug fix candidate

2019-12-03 Thread Nikola Grcevski
Hi Henry and Kumar, Thanks again for your comments! I have updated the test to be part of test/jdk/tools/launcher/modules/basic, it took a lot less code to achieve the same amount of testing. I added a new test method inside BasicTest.java and tested on both Windows and Linux. Please find my

Re: RFR(XS): 8234696: tools/jlink/plugins/VendorInfoPluginsTest.java times out

2019-12-03 Thread Mandy Chung
Hi Arno, On 12/3/19 1:13 AM, Zeller, Arno wrote: Hi all, could someone please review this small improvement for the test VendorInfoPluginsTest.java? The change avoids writing a core file and might reduce the memory footprint . JBS: https://bugs.openjdk.java.net/browse/JDK-8234696 Webrev:

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-03 Thread Mandy Chung
Hi Vicente, I reviewed jvm.h, jvm.cpp, and the changes in java.base but only skimmed on the serialization change from this version: http://cr.openjdk.java.net/~vromero/records.review/all_code/webrev.01/ Class::getRecordComponents    - JVM_GetRecordComponents creates a new RecordComponent

Re: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-03 Thread David Holmes
Hi Christoph, On 3/12/2019 7:26 pm, Langer, Christoph wrote: Hi David, thanks for taking care of this. This would be my updated patch that could hopefully be enabled by just adding the include directory where "jdk_util.h" is located. It would be really great if that'd work. I think it would

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-03 Thread Vicente Romero
Hi David, On 12/2/19 7:36 PM, David Holmes wrote: Hi Vicente, I have also re-examined all the hotspot related code and everything looks fine - and very neat (kudos to you and Harold!). thanks, yep Harold has made most of the work here, I just wrote the initial prototype a while ago, A

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

2019-12-03 Thread Philip Race
+1 (Approved). I think we are now just waiting on the CSR to be approved : https://bugs.openjdk.java.net/browse/JDK-8213087 -phil. On 12/3/19, 12:31 PM, Kevin Rushforth wrote: Updated version (with the one change mentioned in reply to Phil) looks good. +1 -- Kevin On 12/3/2019 11:35 AM,

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

2019-12-03 Thread Kevin Rushforth
Updated version (with the one change mentioned in reply to Phil) looks good. +1 -- Kevin On 12/3/2019 11:35 AM, Andy Herrick wrote: Please review the revised cumulative jpackage webrev at [3] /Andy [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11D/ On 11/18/2019 12:01 PM,

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

2019-12-03 Thread Andy Herrick
On 12/2/2019 2:07 PM, Phil Race wrote: This makes it very difficult to do more than a cursory re-review. There is one thing I just spotted. I believe these two scripts http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/test/jdk/tools/jpackage/test_jpackage.sh.html

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

2019-12-03 Thread Andy Herrick
Please review the revised cumulative jpackage webrev at [3] /Andy [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11D/ On 11/18/2019 12:01 PM, Andy Herrick wrote: On 11/13/2019 7:23 PM, Andy Herrick wrote: Please review  changes for [1] which is the implementation bug for

Re: [EXTERNAL] JDK-8234076 bug fix candidate

2019-12-03 Thread Henry Jen
Kumar, Great to have you look at this, you are correct, this patch doesn’t address the wildcard expansion issue, but only to address the potential crash if a main class is not specified as Nikola pointed out. We definitely need a follow up to fix wildcard expansion. The pointer to simplify

RE: [EXTERNAL] Re: JDK-8234076 bug fix candidate

2019-12-03 Thread Nikola Grcevski
Hi Kumar, Thank you so much for your input! You are absolutely correct, this fix doesn’t fix the argument expansion issue on Windows, it only fixes the crash. I made a similar comment in my original email, fixing the argument expansion would be a bit more substantial change. I’m more than

Re: [EXTERNAL] Re: JDK-8234076 bug fix candidate

2019-12-03 Thread Kumar Srinivasan
Hi, Sorry for chiming in late in the review process, for what it's worth 1. It is not at all clear to me if this solution is correct, yes it averts the problem of not finding the main-class and subsequent crash, but it does not address wildcard arguments expansion. What if we

Re: RFR: JEP 359: Records (Preview) (full code)

2019-12-03 Thread Chris Hegarty
Hi Joe, > On 3 Dec 2019, at 05:36, Joe Darcy wrote: > > ... > > As a non-essential cleanup, in > src/java.base/share/classes/java/io/ObjectOutputStream.java: > > 1444 final boolean isRecord = isRecord(obj.getClass()) ? true : > false; > 1445 if (isRecord) { > 1446

Multi Part publisher for java.net.http.HttpClient

2019-12-03 Thread Behrang Saeedzadeh
Hi all, Are there any plans to implement a "multipart/form-data" BodyPublisher in Java 14 or later? Best regards, { "blog" : "https://blog.behrang.org;, "twitter" : "https://twitter.com/behrangsa;, "github": "https://github.com/behrangsa/;, "stackoverflow" :

RE: RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

2019-12-03 Thread Langer, Christoph
Hi David, thanks for taking care of this. This would be my updated patch that could hopefully be enabled by just adding the include directory where "jdk_util.h" is located. It would be really great if that'd work. I think it would open up a path to automatically include io_util_md.h by

RFR(XS): 8234696: tools/jlink/plugins/VendorInfoPluginsTest.java times out

2019-12-03 Thread Zeller, Arno
Hi all, could someone please review this small improvement for the test VendorInfoPluginsTest.java? The change avoids writing a core file and might reduce the memory footprint . JBS: https://bugs.openjdk.java.net/browse/JDK-8234696 Webrev: http://cr.openjdk.java.net/~azeller/webrevs/8234696.0/