Re: Condition is always false in 'ProcessHandleImpl.Info#toString'

Thanks for reporting. Filed: https://bugs.openjdk.java.net/browse/JDK-8274003 -Sundar From: core-libs-dev on behalf of Andrey Turbanov Sent: 20 September 2021 20:49 To: core-libs-dev Subject: Condition is always false in 'ProcessHandleImpl.Info#toString' H

Re: RFR: 8252725: Refactor jlink GenerateJLIClassesPlugin code

Looks good to me. Few minor comment: * traceFileStream (and even the preexisting mainArgument) is accessed only inside GenerateJLIClassesPlugin. Could be private? -Sundar On 04/09/20 3:41 am, Yumin Qi wrote: HI, Mandy   Thanks for review and comment. Yumin On 9/3/20 9:13 AM, Mandy Chung

Re: Review request for JDK-8251538: Modernize and lint Dynalink code

Looks good. Thanks for doing this cleanup! -Sundar On 21/08/20 2:10 am, Attila Szegedi wrote: Following up on the previous e-mail, here’s the modernization and linting work on the existing Dynalink codebase: Please review JDK-8251538 "Modernize and lint Dynalink code" at

Re: Review request for JDK-8252124: Restore Dynalink tests

Yes. no need for separate webrev. -Sundar On 23/08/20 6:02 pm, Attila Szegedi wrote: Im happy to update the copyright years before merging my changes. I’ll not put up a separate webrev for that though, if that’s okay. Attila. On 2020. Aug 21., at 07:32, Sundararajan Athijegannathan wrote

Re: Review request for JDK-8252124: Restore Dynalink tests

Looks good. Minor comment: not sure if you need to update copyright year on tests - especially because there have been changes like package removal (flat). -Sundar On 21/08/20 1:50 am, Attila Szegedi wrote: Hi folks, long time since I actively popped up here. I’m taking some time to maintai

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

Hi David. Thanks. -Sundar On 18/08/20 8:04 am, David Holmes wrote: Hi Sundar, On 18/08/2020 12:25 pm, sundararajan.athijegannat...@oracle.com wrote: Not a full review of fresh changes. But couple of comments: * src/hotspot/share/memory/lambdaFormInvokers.cpp and src/hotspot/share/memory/la

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

Not a full review of fresh changes. But couple of comments: * src/hotspot/share/memory/lambdaFormInvokers.cpp and src/hotspot/share/memory/lambdaFormInvokers.hpp miss "Classpath exception" clause in the copyright header * I had suggested a change GenerateJLIClassesPlugin.java in last round o

Re: Fwd: RFR: 8247536: Support pre-generated MethodHandle lambda forms in CDS

* File: InvokerBytecodeGeneratorHelper.java copyright header has GNU without the "Classpath exception" clause. * The comment could say this string prefix is known in hotspot (classFileParser.cpp hard codes it) + // The log prefix for tracing resolve + static final String CDS_LOG_PREFIX = "@la

RFR (JDK 15) 8251276: two jdeps files have extra whitespace in copyright header

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8251276 Webrev: http://cr.openjdk.java.net/~sundar/8251276/webrev.00/index.html Thanks, -Sundar

Re: RFR (JDK 15) 8248299: two jdeps files miss copyright header

Oops. Missed "Classpath exception" clause. Updated: http://cr.openjdk.java.net/~sundar/8248299/webrev.01/ -Sundar On 06/08/20 8:33 pm, sundararajan.athijegannat...@oracle.com wrote: Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8248299 Webrev: http://cr.openjdk.java.net/~sunda

RFR (JDK 15) 8248299: two jdeps files miss copyright header

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8248299 Webrev: http://cr.openjdk.java.net/~sundar/8248299/webrev.00/ Thanks, -Sundar

Re: RFR JDK-8250929: Missing "classpath exception" in LambdaProxyClassArchive.java

Looks good -Sundar On 05/08/20 10:56 pm, Mandy Chung wrote: Trivial update on license header. diff --git a/src/java.base/share/classes/java/lang/invoke/LambdaProxyClassArchive.java b/src/java.base/share/classes/java/lang/invoke/LambdaProxyClassArchive.java --- a/src/java.base/share/classe

Re: JDK 16 RFR of JDK-8250237: Address use of default constructors in the javax.script package

Looks good -Sundar On 24/07/20 2:54 am, Joe Darcy wrote: Hello, One class in the javax.script package uses a default constructor; please review the patch below and CSR (https://bugs.openjdk.java.net/browse/JDK-8250239) to replace it with an explicit constructor. Thanks, -Joe diff -r d62

Re: request for review JDK-8242935

This is in response to https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/067571.html Hi, Your fix looks good to me. Thanks -Sundar

Re: RFR: JDK-8246697 Test: java/util/StringJoiner/StringJoinerTest.java failing with OOM

Looks good. -Sundar On 05/06/20 8:11 pm, Jim Laskey wrote: Can I get a quick review for the following. Getting some unexpected OOM from test. Pulling until I figure it out. jbs: https://bugs.openjdk.java.net/browse/JDK-8246696 diff -r 345693999a92 test/jdk/ProblemList.txt --- a/test/jdk/Prob

Re: RFR 8246040: java/foreign/TestAddressHandle fails on big endian platforms

Looks good to me. -Sundar On 29/05/20 4:51 pm, Maurizio Cimadamore wrote: Hi, following integration of second iteration of foreign memory access API, one of the new tests started acting up on big endian platforms (thanks to Thomas Stüfe for catching this!). Here's a fix: http://cr.openjdk.

Re: RFR 8246034: Remove java.base/share/classes/jdk/internal/jrtfs/jrtfsviewer.js and java.base/share/classes/jdk/internal/jrtfs/jrtls.js

Yes, checked. No config file refers to these .js files. -Sundar On 28/05/20 12:28 pm, Alan Bateman wrote: On 28/05/2020 07:14, sundararajan.athijegannat...@oracle.com wrote: Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8246034 Webrev: http://cr.openjdk.java.net/~sundar/824603

RFR 8246034: Remove java.base/share/classes/jdk/internal/jrtfs/jrtfsviewer.js and java.base/share/classes/jdk/internal/jrtfs/jrtls.js

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8246034 Webrev: http://cr.openjdk.java.net/~sundar/8246034/webrev.00/ Thanks -Sundar

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Hi, Looks good. I'll sponsor this fix. Thanks -Sundar On 27/04/20 4:15 pm, Ao Qi wrote: Thanks, Sundar! I updated a new webrev (patch is the same, only hg commit info was added): http://cr.openjdk.java.net/~aoqi/8242846/webrev.02/ Could someone help to sponsor this? Thanks, Ao Qi On Mon,

Re: RFR (T): 8242846: Bring back test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java (was: Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Looks good -Sundar On 27/04/20 12:24 pm, Ao Qi wrote: On Sun, Apr 26, 2020 at 12:00 AM Alan Bateman wrote: On 21/04/2020 04:58, Ao Qi wrote: On 2020/4/20 下午9:27, Alan Bateman wrote: On 20/04/2020 11:32, sundararajan.athijegannat...@oracle.com wrote: Hi Alan, I don't remember it now. Likel

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Hi Alan, I don't remember it now. Likely a mistake. The changeset http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42 has that file. Perhaps it may be useful to restore & check if the test passes. Thanks, -Sundar On 18/04/20 3:32 pm, Alan Bateman wrote: Sundar - do you recognize th

RFR 8242859: test/jdk/tools/jlink/JLinkTest.java uses nashorn module

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8242859 Webrev: http://cr.openjdk.java.net/~sundar/8242859/webrev.00/ Relevant JLinkTest.java section deleted as part of nashorn removal is as follows: http://cr.openjdk.java.net/~sundar/8241749/webrev.00/test/jdk/tools/jlink/JLink

RFR 8242860: test/jdk/tools/jlink/ModuleNamesOrderTest.java uses nashorn module

Hi, nashorn modules were used only as an example in this test. Using jdk.jshell module instead to test known module dependencies in jlink produced images. Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8242860 Webrev: http://cr.openjdk.java.net/~sundar/8242860/webrev.00/ Thanks

Re: RFR 8242931: Few more tests that use nashorn have been missed

Thanks Daniel. Fixed it. Updated webrev: http://cr.openjdk.java.net/~sundar/8242931/webrev.02/ PS. Submitting mach5 job concurrently. Thanks, -Sundar On 16/04/20 11:16 pm, Daniel Fuchs wrote: Hi Sundar, I spotted a typo (generial vs generic) test/jdk/ProblemList.txt:  936 java/util/Serv

Re: RFR 8242931: Few more tests that use nashorn have been missed

Please review updated webrev: http://cr.openjdk.java.net/~sundar/8242931/webrev.01/ Thanks, -Sundar On 16/04/20 6:49 pm, David Holmes wrote: +1 Please problem list the tests under the associated bug ids. Thanks, David On 16/04/2020 11:10 pm, Alan Bateman wrote: On 16/04/2020 13:59, sundara

RFR 8242931: Few more tests that use nashorn have been missed

Nashorn engine removal fix (8241749: Remove the Nashorn JavaScript Engine) missed updating few tests, config files. Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8242931 Webrev: http://cr.openjdk.java.net/~sundar/8242931/webrev.00/ PS I'm disabling tests by adding @ignore. I've

Re: RFR 8241749: Remove the Nashorn JavaScript Engine

Thanks Mandy. I'll add bug id next to @ignore before push. Thanks, -Sundar On 16/04/20 2:50 am, Mandy Chung wrote: This looks okay to me. Can you add the bug ID next to @ignore that will make it easier to find the JBS issue?   These test bugs are filed with P4 but I think they should be fix

Re: RFR 8241749: Remove the Nashorn JavaScript Engine

Thanks for spotting this Magnus! I'll fix this before push. Thanks. -Sundar On 15/04/20 10:44 pm, Magnus Ihse Bursie wrote: On 2020-04-15 17:56, sundararajan.athijegannat...@oracle.com wrote: Please review. Nashorn script engine modules (jdk.scripting.nashorn, jdk.scripting.nashorn.shell) a

RFR 8241749: Remove the Nashorn JavaScript Engine

Please review. Nashorn script engine modules (jdk.scripting.nashorn, jdk.scripting.nashorn.shell) and jjs tool are removed. Bug: https://bugs.openjdk.java.net/browse/JDK-8241749 JEP: https://openjdk.java.net/jeps/372 CSR: https://bugs.openjdk.java.net/browse/JDK-8241751 Webrev: http://cr.op

Re: RFR: JDK-8230047: Remove legacy java.lang.reflect.ProxyGenerator_v49

Looks good -Sundar On 05/02/20 3:18 am, Mandy Chung wrote: ProxyGenerator_v49 was the old proxy generator that has been replaced with ASM in JDK 14 [1].  This patch removes this old version. Webrev: http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8230047/webrev.00/ thanks Mandy [1] https://b

RFR 8222100: tools/jimage/JImageTest.java time out

Bumping the default timeout (other tests in the same dir have similar timeout settings). Bug: https://bugs.openjdk.java.net/browse/JDK-8222100 Webrev: http://cr.openjdk.java.net/~sundar/8222100/webrev.00/ Thanks, -Sundar

Re: RFR 8222098: tools/jlink/plugins/IncludeLocalesPluginTest.java time out

Hi Naoto, Thanks. I'll make those changes and push. -Sundar On 10/01/20 7:20 pm, naoto.s...@oracle.com wrote: Hi Sundar, You might want to add the bug id in the regression test, and change the copyright year to 2020. Otherwise, looks good. Naoto On 1/10/20 5:07 AM, sundararajan.athijegan

Re: RFR 8222098: tools/jlink/plugins/IncludeLocalesPluginTest.java time out

Adding core-libs-dev. -Sundar On 10/01/20 11:33 am, sundararajan.athijegannat...@oracle.com wrote: Please review. Not really a fix - but increasing the default timeout for the test. Bug: https://bugs.openjdk.java.net/browse/JDK-8222098 Webrev: http://cr.openjdk.java.net/~sundar/8222098/webre

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

Looks good. -Sundar On 04/06/19, 9:30 AM, Mandy Chung wrote: The launcher prints out the error message when the main class fails to initialize but java.launcher.module.error5 resource that intends to print the Caused-by exception message when running in a named module but prints an incorrect pa

Re: RFR (s): 8217412 deprecate rmic for removal

Looks good. -Sundar On 31/05/19, 6:40 AM, Stuart Marks wrote: Hi all, Please review this patch and CSR request for upgrading the deprecation status of the rmic to from ordinary to terminal (i.e., conceptually set forRemoval=true, though there are no actual annotations involved here). Ther

RFR 8224946: jrtfs URI to Path and Path to URI conversions are wrong

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8224946 Webrev: https://cr.openjdk.java.net/~sundar/8224946/webrev.00/ Earlier attempt for the fix of the same bug resulted in test failures and so the fix was reverted. The current webrev has the same fix - but fixes 2 jlink tests t

Re: RFR: Revert: 8216553: JrtFileSystemProvider getPath(URI) omits /modules element from file path

Thanks Jim for pushing the reverting changeset. PS. Sorry that I broke the build. I did run jrtfs, javac and javap tests. Didn't realise that faulty jrt: URIs are pervasive in tests (per JEP jrt: URIs should not have '/modules/' part - which is the bug fix is about) -Sundar On 29/05/19, 1:1

Re: RFR 8216553: JrtFIleSystemProvider getPath(URI) omits /modules element from file path

Updated for java test failures: https://cr.openjdk.java.net/~sundar/8216553/webrev.01/ PS. Test framework used wrong jrt: URI pattern. Fixed it. -Sundar On 27/05/19, 5:37 PM, Alan Bateman wrote: On 27/05/2019 10:18, Sundararajan Athijegannathan wrote: Please review. Bug: https

RFR 8216553: JrtFIleSystemProvider getPath(URI) omits /modules element from file path

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8216553 Webrev: https://cr.openjdk.java.net/~sundar/8216553/webrev.00/ Thanks, -Sundar

Re: Review Request: 8221530: Caller sensitive methods not handling caller = null when invoked by JNI code with no java frames on stack

Looks good. -Sundar On 28/03/19, 4:47 AM, Mandy Chung wrote: This is to fix a regression introduced in JDK 12 by JDK-8206240. This impacts only native application that creates JVM via JNI and also invokes Field::getField (or other reflection API) via JNI that triggers reflection access check ag

Re: RFR - JDK-8215489 Remove String::align

Looks good. -Sundar On 09/01/19, 8:56 PM, Jim Laskey wrote: Please review the removal of String::align from JDK12. Thank you. — Jim webrev: http://cr.openjdk.java.net/~jlaskey/8215489/webrev/index.html JBS: https://bugs.open

Re: RFR - JDK-8215682 - Remove compiler support for Raw String Literals from JDK 12

Looks good. -Sundar On 02/01/19, 11:32 PM, Jim Laskey wrote: Looking for reviewers to make sure this CSR passes muster. CSR: https://bugs.openjdk.java.net/browse/JDK-8215682 Thank you. Cheers, — Jim

Re: 8214696: Module class should be filtered by core reflection

Looks good -Sundar On 12/12/18, 10:28 PM, Alan Bateman wrote: I'd like add java.lang.Module to list of classes that has its fields filtered from core reflection. As with previous addition in this area, this is another class where hacking its fields leads to integrity and security issues. The

Re: RFR JDK-8213909: jdeps --print-module-deps should report missing dependences

13909/webrev.01-delta Thanks Mandy On 11/19/18 9:22 AM, Mandy Chung wrote: Thanks Sundar. Here is the CSR: https://bugs.openjdk.java.net/browse/JDK-8213915 Mandy On 11/19/18 6:48 AM, Sundararajan Athijegannathan wrote: Looks good to me. -Sundar On 15/11/18, 5:46 AM, Mandy Chung wrote: This

Re: RFR JDK-8213909: jdeps --print-module-deps should report missing dependences

Looks good to me. -Sundar On 15/11/18, 5:46 AM, Mandy Chung wrote: This patch improves `jdeps --print-module-deps`, `--list-deps` and `--list-reduced-deps` to report missing dependences and also do transitive dependence analysis as the default. Webrev at: http://cr.openjdk.java.net/~mchung/j

Re: RFR JDK-8211051: jdeps usage of --dot-output doesn't provide valid output for modular jar

+1 -Sundar On 15/11/18, 5:42 AM, Mandy Chung wrote: This is a small fix to jdeps --dot-output option that should name the dot file with the filename of the modular JAR rather than its module name. http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8211051/webrev.00/ Thanks Mandy

Re: RFR: 8209837: Avoid initializing ExpiringCache during bootstrap

Looks good -Sundar On 23/08/18, 7:39 PM, Claes Redestad wrote: Hi, it's a tiny startup improvement to rearrange code so that the ExpiringCaches used in the FileSystem implementations aren't created if they aren't going to be used: Webrev: http://cr.openjdk.java.net/~redestad/8209837/open.0

Re: RFR JDK-8066709 Make some JDK system properties read only

Looks good -Sundar On 04/06/18, 7:02 PM, Roger Riggs wrote: Please review a change to make the values of java.home, user.home, user.dir, and user.name effectively read-only for internal use. The values are cached during initialization and the cached values are used. Webrev: http://cr.open

Re: RFR: CSR - JDK-8203428 Predicate::not

Actually it would be: Predicate.of(String::isEmpty).negate() But not(String::isEmpty) reads almost like !str.isEmpty() -Sundar On 18/05/18, 10:41 PM, Daniel Fuchs wrote: Hi Jim, Have you thought of introducing something like: static Predicate Predicate.of(Predicate target) { re

Re: RFR: CSR - JDK-8203428 Predicate::not

+1 -Sundar On 18/05/18, 10:05 PM, Jim Laskey wrote: Introduce a new static method Predicate::not which will allow developers to negate predicate lambdas trivially. csr: https://bugs.openjdk.java.net/browse/JDK-8203428

Re: RFR: JDK-8200380 String::lines

+1 I think LinesSpliterator classes in StringLatin1 and StringUTF8 could be "private" and "final". -Sundar On 18/05/18, 7:14 PM, Jim Laskey wrote: String::lines instance method that returns a Stream with elements composed of substrings from the original string delimited by any recognized n

Re: RFR: JDK-8200436 - String::isBlank

+1 -Sundar On 14/05/18, 8:55 PM, Jim Laskey wrote: New string instance method that returns true if the string is empty or contains only white space, where white space is defined as any codepoint returns true when passed to Character::isWhitespace. webrev: http://cr.openjdk.java.net/~jlaskey/

Re: RFR: JDK-8200377 String::strip, String::stripLeading, String::stripTrailing

Looks good. http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/053034.html Thanks, -Sundar

Re: RFR JDK-8200372 - String::trim JavaDoc should clarify meaning of space

Looks good -Sundar On 08/05/18, 5:49 PM, Jim Laskey wrote: Comment change approved in CSR webrev: http://cr.openjdk.java.net/~jlaskey/8200372/webrev/index.html JBS: https://bugs.openjdk.java.net/browse/JDK-8200372 CSR: https://bugs.openjdk.java.net/browse/JDK-8196005

Re: RFR: 8198793:Add launcher support for preview features

Looks good. -Sundar On 12/04/18, 2:13 AM, Kumar Srinivasan wrote: Hello, Could I get a review to update the launcher help message. https://bugs.openjdk.java.net/browse/JDK-8198793 diff --git a/src/java.base/share/classes/sun/launcher/resources/launcher.properties b/src/java.base/share/clas

Re: RFR: 8200289: Reduce number of exceptions created when calling Lookup::canBeCached

Hi, +if (caller != null&& !VerifyAccess.isClassAccessible(refc, caller, allowedModes)) { +return false; +} +return true; can be return caller == null || VerifyAccess.isClassAccessible(refc, caller, allowedModes); +1 -Sundar On 27/0

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

Looks good -Sundar On 14/03/18, 5:49 AM, David Lloyd wrote: On Tue, Mar 13, 2018 at 7:16 PM, David Lloyd wrote: On Tue, Mar 13, 2018 at 6:31 PM, mandy chung wrote: I prefer to keep the original test case i.e. create a proxy class from Runnable and Observer. Instead add a new test case to

Re: 8193819: Error message when module does not have a ModuleMainClass attribute is confusing

+1 -Sundar On 15/02/18, 7:30 PM, Alan Bateman wrote: There's a typo in the launcher resource used for the error message when `java -m` fails because the module doesn't have a main class. The name of the attribute is "ModuleMainClass", not "MainClass". Trivial change. -Alan diff --git a/s

Re: RFR: 8196959: NullPointerException in discovery003.java

Looks good. -Sundar On 12/02/18, 5:31 PM, Srinivas Dama wrote: Hi, Please review http://cr.openjdk.java.net/~sdama/8196959/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8196959 This is small modification of fix for https://bugs.openjdk.java.net/browse/JDK-8011697. Fix is to handle

Re: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)

Looks good. -Sundar On 02/02/18, 2:23 PM, Srinivas Dama wrote: Hi Alan/Sundar, Please review the revised webrev with copyright header change at http://cr.openjdk.java.net/~sdama/8011697/webrev.02/ Regards, srinivas -Original Message- From: Sundararajan Athijegannathan Sent: Tuesday

Re: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)

+1 (with the copyright header change suggested by Alan) -Sundar On 30/01/18, 8:28 PM, Alan Bateman wrote: On 30/01/2018 09:17, Srinivas Dama wrote: Hi, Please review the revised webrev at http://cr.openjdk.java.net/~sdama/8011697/webrev.01/ for https://bugs.openjdk.java.net/browse/JDK-80

Re: [10] RFR 8194959: Correct test tag to move bugid from @test to @bug

+1 -Sundar On 12/01/18, 10:21 AM, Amy Lu wrote: Please review this minor test-tag-only change to move bugid from @test to @bug bug: https://bugs.openjdk.java.net/browse/JDK-8194959 webrev: http://cr.openjdk.java.net/~amlu/8194959/webrev.00/ Thanks, Amy

Re: RFR: 8190287: Update JDK's internal ASM to ASMv6

jlink changes look good. I ran jlink tests and all nashorn tests (jtreg as well as ant test/test262parallel) after applying the patch locally. All fine! +1 -Sundar On 27/10/17, 10:42 PM, Kumar Srinivasan wrote: Hello Remi, Sundar and others, Please review the webrev [1] to update JDK's int

Re: RFR 8189262: jdk.jlink module-info.java javadoc comment refers to the non-existent jmage tool doc

Thanks! Fixed in JIRA. -Sundar On 16/10/17, 6:03 PM, David Holmes wrote: Hi Sundar, Please fix the typo in the bug synopsis: jmage Thanks, David On 16/10/2017 10:22 PM, Sundararajan Athijegannathan wrote: Please review: Webrev: http://cr.openjdk.java.net/~sundar/8189262/webrev.00/ Bug

RFR 8189262: jdk.jlink module-info.java javadoc comment refers to the non-existent jmage tool doc

Please review: Webrev: http://cr.openjdk.java.net/~sundar/8189262/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189262 Thanks, -Sundar

Re: Review Request JDK-8189202: (jdeps) Need jdeps output format easy for jlink --add-modules to use

Looks good. -Sundar On 13/10/17, 9:10 PM, mandy chung wrote: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8189202/webrev.00/ This proposes to add a new jdeps --print-module-deps option to print a comma-separated list of modules that the analyzed classes depend on and such output can be t

Re: Review Request JDK-8188321:(jdeps) help message should say "requires transitive" rather than "requires public"

+1 -Sundar On 04/10/17, 12:37 AM, mandy chung wrote: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8188321/webrev.00/ This changes the jdeps help message fixing the typo, move -q and -version to the first section and group the module-related options together in a new section. thanks Mandy

Re: RFR: 8076417: Update test/jdk/asm/AsmSanity.java with modules

+1 -Sundar On 19/04/17, 12:09 AM, Kumar Srinivasan wrote: Hello, Please review [1] which addresses bug [2]. This test is unnecessary, as there are other components and tests within the jdk, which will fail if ASM is not included, therefore this test is removed. Thanks Kumar [1] http://cr.

Re: JDK 9 RFR of JDK-8173864: Problem list src/jdk/nashorn/api/tree/test/ParseAPITest.java for some platforms

+1 -Sundar On 03/02/17, 10:24 AM, Amy Lu wrote: src/jdk/nashorn/api/tree/test/ParseAPITest.java This nashorn test keeps failing at linux-i586, windows-i586 and solaris-sparcv9 (JDK-8173863) Please review this patch to put the test to problem list until issues addressed. bug: https://bugs

Re: RFR: JDK-8169505 - Update changes by JDK-8159393 to reflect CCC review

+1 -Sundar On 11/15/2016 11:27 PM, Jim Laskey (Oracle) wrote: > http://cr.openjdk.java.net/~jlaskey/8169505/webrev/index.html > https://bugs.openjdk.java.net/browse/JDK-8169505 > >

Re: RFR 8071588: The spec for javax.script.ScriptEngineFactory.getProgram() should specify NPEs thrown

Alan Bateman wrote: On 19/10/2016 06:06, Sundararajan Athijegannathan wrote: Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8071588 jdk webrev: http://cr.openjdk.java.net/~sundar/8071588/jdk/webrev.00/ nashorn webrev: http://cr.openjdk.java.net/~sundar/8071588/nashorn/webrev.00/

RFR 8071588: The spec for javax.script.ScriptEngineFactory.getProgram() should specify NPEs thrown

Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8071588 jdk webrev: http://cr.openjdk.java.net/~sundar/8071588/jdk/webrev.00/ nashorn webrev: http://cr.openjdk.java.net/~sundar/8071588/nashorn/webrev.00/ Thanks -Sundar

RFR 8071678: javax.script.ScriptContext setAttribute method should clarify behavior when GLOBAL_SCOPE is used and global scope object is null

Please review http://cr.openjdk.java.net/~sundar/8071678/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8071678 Thanks, -Sundar

Re: RFR:8055033: Shell tests for jrunscript don't pass through VM options

+1 -Sundar On 10/5/2016 7:51 PM, Srinivas Dama wrote: > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8055033/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8055033 > > When launching java and javac from shell script, ${TESTVMOPTS}, > ${TESTJAVAOPTS} and ${TESTTOOLVMOPTS}

Re: RFR 8165772: fix for 8165595 results in failure of jdk/test/tools/launcher/VersionCheck.java

Bug: https://bugs.openjdk.java.net/browse/JDK-8165772 This VersionCheck.java test failed after main class was added to nashorn modules. VersionCheck expects all jdk/bin tools to be derived from the standard launcher. But, jlink generates shell scripts and batch files for modules with main class.

RFR 8165726: fix for 8165595 revealed a bug in pack200 tool's handling of main class attribute of module-info classes

Please review http://cr.openjdk.java.net/~sundar/8165726/webrev.01/ for https://bugs.openjdk.java.net/browse/JDK-8165726 Thanks, -Sundar

Re: RFR: JDK-8161000 - GPL header incorrect - classfile/classpath

+1 -Sundar On 8/29/2016 5:46 PM, Jim Laskey (Oracle) wrote: > On behalf of Swamy > > http://cr.openjdk.java.net/~jlaskey/8161000/webrev/index.html > https://bugs.openjdk.java.net/browse/JDK-8161000

Re: http://cr.openjdk.java.net/~redestad/8164866/webrev.01/

+1 -Sundar On 8/26/2016 9:37 PM, Claes Redestad wrote: > Hi, > > JDK-8163371 forgot about the associated test. Changed it as necessary > as some of the options are no longer implemented as of the latest > changes: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8164866 > Webrev: http://cr.openj

Re: RFR:8163793: jlink has typo in copy-files plugin help text example

+1 On 8/24/2016 7:58 PM, Srinivas Dama wrote: > Hi, > > This patch fixes very small typo error. > > Please review http://cr.openjdk.java.net/~sdama/8163793/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8163793 > > > Regards, > Srinivas

Re: RFR: JDK-8164326: jrtfsviewer.js and jrtls.js does not work

+1 On 8/18/2016 8:48 PM, Yasumasa Suenaga wrote: > Hi all, > > This review request is related to [1]. > > jrtfsviewer.js and jrtls.js are not contained JDK/JRE binaries. > However, these tool might be used by JDK developers. So I think we > should fix this issue. > > I uploaded a webrev. Could yo

Re: Error at jrt*.js

Yes, those scripts are meant for developers and not part of binary JDK bits. Please do file a bug and send webrev for review. -Sundar On 8/18/2016 7:51 PM, Yasumasa Suenaga wrote: > Hi all, > > I tried to run jrtfsviewer.js and jrtls.js . But they did not work as > below: > > $

Re: [9] RFR: 8162797: Code clean-up in IncludeLocalesPlugin

+1 -Sundar On 8/2/2016 2:15 AM, Naoto Sato wrote: > Hello, > > Please review this small change for the bug: > > https://bugs.openjdk.java.net/browse/JDK-8162797 > > The fix is located at: > > http://cr.openjdk.java.net/~naoto/8162797/webrev.00/ > > Naoto

Re: JDK 9 RFR of 8161970, 8157664: Remove 4 tools tests from ProblemList.txt

+1 for the jlink test being removed from the problem test. -Sundar On 8/2/2016 11:27 AM, Amy Lu wrote: > tools/jlink/JLinkOptimTest.java > This test has been removed in JDK-8160829 > > sun/tools/jinfo/JInfoSanityTest.java > sun/tools/jinfo/JInfoRunningProcessFlagTest.java > sun/tools/jmap/hea

RFR 8159487: Add JAVA_VERSION, OS_NAME, OS_ARCH properties in release file

Please review http://cr.openjdk.java.net/~sundar/8159487/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8159487 OS_NAME, OS_ARCH, OS_VERSION properties are already added due to another fix. Just adding "JAVA_VERSION" and a test change to check these properties exist in release file. Than

RFR 8161055: Remove plugin ordering by isAfter, isBefore.

Please review http://cr.openjdk.java.net/~sundar/8161055/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8161055 Thanks -Sundar

Re: RFR: JDK-8160829 - Remove ASMPool support from jlink

+1 On 7/5/2016 7:48 PM, Jim Laskey (Oracle) wrote: > Much of the removed code seems unnecessary since the same functionality can > be accomplished with much simpler code. An example is provided with > ClassForNamePlugin.java (temporary.) A shipping byte code optimizer plugin > will be suppli

Re: RFR: JDK-8160459 - jlink minor code clean up

Looks good -Sundar On 6/28/2016 8:39 PM, Jim Laskey (Oracle) wrote: > http://cr.openjdk.java.net/~jlaskey/8160459/webrev/index.html > > https://bugs.openjdk.java.net/browse/JDK-8160459 >

Re: RFR: JDK-8160348 - jlink should use System.out for usage messages

+1 -Sundar On 6/27/2016 5:46 PM, Jim Laskey (Oracle) wrote: > Trivial patch > > http://cr.openjdk.java.net/~jlaskey/8160348/webrev/index.html > > https://bugs.openjdk.java.net/browse/JDK-8160348 >

Re: RFR: 8159031: jjs throws NoSuchFileException if ~/.jjs.history does not exist

+1 On 6/8/2016 4:48 PM, Hannes Wallnöfer wrote: > Please review the following changes: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8159031 > JDK webrev: http://cr.openjdk.java.net/~hannesw/8159031/jdk/webrev.00/ > Nashorn webrev: http://cr.openjdk.java.net/~hannesw/8159031/nashorn/webrev.00

Re: RFR[9]:MethodHandles.dropArgumentsToMatch(...) non-documented IAE

Looks good -Sundar On 6/6/2016 12:16 PM, Michael Haupt wrote: > Hi Shilpi, > > CCC approval is in. My review is lower-case, so another one is needed ere I > can sponsor the push. > > Best, > > Michael > >> Am 02.06.2016 um 14:53 schrieb Michael Haupt : >> >> Hi Shilpi, >> >> lower-case thumbs u

Re: RFR : 8073611 : javax.script.ScriptEngineFactory: formatting error in javadoc of getParameter

+1 -Sundar On 6/3/2016 5:51 PM, Muneer Kolarkunnu wrote: > > Hi All, > > > > Please review fix for https://bugs.openjdk.java.net/browse/JDK-8073611 > > > > Webrev : http://cr.openjdk.java.net/~sdama/8073611/webrev.00/ > > > > > Probl

Re: RFR 8154192: Deprivilege java.scripting module

Thanks. I'll make that change and push it. -Sundar On 5/18/2016 2:17 PM, Alan Bateman wrote: > On 18/05/2016 08:55, Sundararajan Athijegannathan wrote: >> Please review the updated webrevs. >> >> * Fixed Modules.gmk for order of modules: >> >> http://c

Re: RFR(M): 8156915: introduce MethodHandle factory for array length

+1 Minor comment: File: MethodHandleImpl.java If "checks" can be inside the assert. + if (access == ArrayAccess.SET) assert(mh.type().parameterType(2) == Object.class); + if (access == ArrayAccess.GET) { + assert(mh.type().returnType() == Object.class); + assert(correctType.parameterType(0).getC

Re: RFR 8154192: Deprivilege java.scripting module

http://cr.openjdk.java.net/~sundar/8154192/jdk/webrev.01/ Yes, we can still revisit AllPermission for java.scripting module in a future change. Thanks, -Sundar On 5/17/2016 5:54 PM, Alan Bateman wrote: > On 17/05/2016 13:04, Sundararajan Athijegannathan wrote: >> Please review fix for https://bugs.ope

Re: RFR 8157146: Add debug printlns to tests FieldSetAccessibleTest and VerifyJimage.java

Please review the updated webrev: http://cr.openjdk.java.net/~sundar/8157146/webrev.01/ Thanks, -Sundar On 5/17/2016 6:42 PM, Alan Bateman wrote: > On 17/05/2016 14:08, Aleksey Shipilev wrote: >> On 05/17/2016 03:54 PM, Sundararajan Athijegannathan wrote: >>>

RFR 8157146: Add debug printlns to tests FieldSetAccessibleTest and VerifyJimage.java

Please review http://cr.openjdk.java.net/~sundar/8157146/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8157146 This is test only change to debug printlns. Thanks, -Sundar

RFR 8154192: Deprivilege java.scripting module

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8154192 java.scripting module is assigned to platform class loader (instead of boot loader). And java.scripting module is given AllPermission [previously it had AllPermission implicitly because of being boot loader code] jdk repo: ht

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

Hi Paul: On default package: with the new rule a VM anonymous class has to be either [1] in the same package as that of the host class or [2] in the unnamed package. MethodHandleImpl's T class was earlier in java.lang.invoke package. So we can either fix it [1] to match the package of the host cla

Re: RFR: 8133549: Generalize jshell's EditingHistory

The nashorn and jdk repo changes look good to me. Thanks, -Sundar On 5/12/2016 3:55 PM, Jan Lahoda wrote: > Hello, > > In jshell, there's a special history mode that is intended to aid with > editing/re-entering multi-line snippets (EditingHistory). It works > like this: when the user goes back

Re: RFR[9]:Fix java/lang/invoke/MethodHandleImpl's use of Unsafe.defineAnonymousClass()

+1 -Sundar On 5/11/2016 10:01 PM, shilpi.rast...@oracle.com wrote: > Hi All, > > Please review the updated webrev- > http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/ > > Changed the anonymous class package with no package name. > > Regards, > Shilpi > > On 5/11/2016 8:22 PM, Paul Sandoz w

  1   2   3   >