Re: Better tools for adjusting to strong encapsulation
On 22/03/2017 21:07, Jochen Theodorou wrote: the warnings you are seeing are from the groovy runtime trying to determine - in a pre JDK9 compatible way - if it is allowed to call the methods on Objecz. This happens in a first step by setAccessible on all methods using the array version, and if that fails by using the normal setAccessible on each method (which will contribute to much longer startup times). At no point our runtime or the program you wrote are trying to call finalize, clone or registerNatives. This sounds like Object.class.getDeclaredMethods() and then invoking setAccessible(true) on every method, is that right? If so then it will trigger warnings for non-public methods. Is there any reason why this code couldn't just look at the modifiers? Claes mentions the new canAccess which might be useful going forward. The only one we are to be blamed for is MethodHandles$Lookup(java.lang.Class,int), for which I haven´t implemented the alternative yet If you did other programs... like for example running a gradle build... you would see many many more such lines Lots of lines but not clear to me that it's Gradle that wants to hack into every non-public member of java.util.ArrayList. $ gradle NOTE: Picked up the following options via JDK_JAVA_OPTIONS: --permit-illegal-access WARNING: --permit-illegal-access will be removed in the next major release WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.getPackages() (permitted by --permit-illegal-access) Starting a Gradle Daemon, 1 stopped Daemon could not be reused, use --status for details WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.finalize() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.clone() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.registerNatives() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.vmplugin.v7.Java7$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to constructor java.lang.invoke.MethodHandles$Lookup(java.lang.Class,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractCollection.hugeCapacity(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractCollection.finishToArray(java.lang.Object[],java.util.Iterator) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.subListRangeCheck(int,int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.removeRange(int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.rangeCheckForAdd(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.outOfBoundsMsg(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.ArrayList.add(java.lang.Object,java.lang.Object[],int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.ArrayList.access$000(java.util.ArrayList) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.ArrayList.elementAt(java.la
Re: Better tools for adjusting to strong encapsulation
On 23.03.2017 01:16, Claes Redestad wrote: [...] the warnings you are seeing are from the groovy runtime trying to determine - in a pre JDK9 compatible way - if it is allowed to call the methods on Objecz. This happens in a first step by setAccessible on all methods using the array version, and if that fails by using the normal setAccessible on each method (which will contribute to much longer startup times). At no point our runtime or the program you wrote are trying to call finalize, clone or registerNatives. For determining capabilities in this manner it might be possible to use the new AccessibleObject.canAccess[1], which should avoid any warnings? /Claes [1] http://download.java.net/java/jdk9/docs/api/java/lang/reflect/AccessibleObject.html#canAccess-java.lang.Object- I guess so. I mean normally we request that you open up the Objects to the runtime and the runtime then decides by itself if class X has access to class Y via runtime. canAccess will avoid the warnings, and completely avoid them I think for the case of everything being on the classpath. It will not avoid the add-opens required for the runtime to access objects from class X in module M to class Y in the same Module via Groovy runtime. Because even if we use MethodHandles, the handle is still produced from a Method object in reflection and that will mean we still require the runtime accessing this. And actually I don't think we can exclude the reflective part, because we have to do runtime method selection. We cannot query for a method directly, we have to query for all methods of a certain name, which is impossible, so we have to take a look at all methods of a class. And I mean independent of the accessibility modifier. So does canAccess help? Yes, in the classpath scenario, not in the module scenario. But at least that scenario is currently not likely bye Jochen
Re: Better tools for adjusting to strong encapsulation
I will interpret your answer as that there is no additional per method invocation cost due to this. bye Jochen On 23.03.2017 09:12, Alan Bateman wrote: On 22/03/2017 21:07, Jochen Theodorou wrote: the warnings you are seeing are from the groovy runtime trying to determine - in a pre JDK9 compatible way - if it is allowed to call the methods on Objecz. This happens in a first step by setAccessible on all methods using the array version, and if that fails by using the normal setAccessible on each method (which will contribute to much longer startup times). At no point our runtime or the program you wrote are trying to call finalize, clone or registerNatives. This sounds like Object.class.getDeclaredMethods() and then invoking setAccessible(true) on every method, is that right? If so then it will trigger warnings for non-public methods. Is there any reason why this code couldn't just look at the modifiers? Claes mentions the new canAccess which might be useful going forward. The only one we are to be blamed for is MethodHandles$Lookup(java.lang.Class,int), for which I haven´t implemented the alternative yet If you did other programs... like for example running a gradle build... you would see many many more such lines Lots of lines but not clear to me that it's Gradle that wants to hack into every non-public member of java.util.ArrayList. $ gradle NOTE: Picked up the following options via JDK_JAVA_OPTIONS: --permit-illegal-access WARNING: --permit-illegal-access will be removed in the next major release WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.getPackages() (permitted by --permit-illegal-access) Starting a Gradle Daemon, 1 stopped Daemon could not be reused, use --status for details WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.finalize() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.clone() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.lang.Object.registerNatives() (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.vmplugin.v7.Java7$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to constructor java.lang.invoke.MethodHandles$Lookup(java.lang.Class,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.gradle.internal.reflect.JavaMethod (file:/gradle-3.4.1/lib/gradle-base-services-3.4.1.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractCollection.hugeCapacity(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractCollection.finishToArray(java.lang.Object[],java.util.Iterator) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.subListRangeCheck(int,int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.removeRange(int,int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.rangeCheckForAdd(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.AbstractList.outOfBoundsMsg(int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.ArrayList.add(java.lang.Object,java.lang.Object[],int) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/gradle-3.4.1/lib/groovy-all-2.4.7.jar) to method java.util.ArrayList.access$000(java.util.ArrayList) (permitted by --permit-illegal-access) WARNING: Illegal access by org.codehaus.groovy.reflection.CachedClass$3$1 (file:/
Making jdeps aware of exported API
Hi, I would like to suggest to add an option to jdeps for describing the intended exported API of descriptors created by --generate-module-info. This would have two benefits: * Limiting the number of exported packages in the generated descriptor to the intended public API * Adding the "transitive" modifiers only to those "requires" statements whose types are used in the exported API While it's relatively simple to rework descriptors after generation and remove any unwanted exports, that's not the case for removing superfluous "transitive" modifiers. Identifying them would require to parse the input JAR once more to find out whether a given dependence is used in public/protected signatures of the exported API or not. If the intended exported API could be passed to jdeps - e.g. in form of regular expressions or some other kind of exclude/include patterns - no further post-processing would be needed for generating descriptors with a well-defined API. I had a look at the --api-only option, but this seems only to be about public/protected access and not the exported packages of a module. Thanks for your consideration, --Gunnar
Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests
Hi Igor, small nit: 42 * @modules java.management 43 * jdk.attach 44 * jdk.management.agent/jdk.internal.agent I don't think java.management needs to be specified as a dependency when the test requires jdk.management.agent, because jdk.management.agent already requires java.management. best regards, -- daniel On 22/03/2017 23:09, Igor Ignatyev wrote: Hi Mandy, I've (re)sorted @modules in all the tests which I touched -- http://cr.openjdk.java.net/~iignatyev/8177374/webrev.01/index.html test/java/lang/management/PlatformLoggingMXBean/LoggingMXBeanTest.java This test should need java.management. Where is it declared? it's declared in test/java/lang/management/TEST.properties which has been pushed by JDK-8176176. Thanks, -- Igor On Mar 22, 2017, at 2:01 PM, Mandy Chung mailto:mandy.ch...@oracle.com>> wrote: On Mar 22, 2017, at 1:09 PM, Igor Ignatyev mailto:igor.ignat...@oracle.com>> wrote: http://cr.openjdk.java.net/~iignatyev/8177374/webrev.00/index.html 40 lines changed: 26 ins; 13 del; 1 mod; Hi all, could you please review this changeset which fixes in a few jdk_svc tests which were missed by JDK-8176176[1]? testing: :jdk_svc tests webrev: http://cr.openjdk.java.net/~iignatyev/8177374/webrev.00/index.html JBS: https://bugs.openjdk.java.net/browse/JDK-8177374 test/java/lang/management/PlatformLoggingMXBean/LoggingMXBeanTest.java This test should need java.management. Where is it declared? Does the webrev miss some new file? Can you keep @modules sorted? e.g. the following in a few files: 42 * @modules jdk.management.agent/jdk.internal.agent 43 * java.management 44 * jdk.attach Mandy
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
On 22/03/2017 19:11, Mandy Chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.00/ This is a proposal to resolve the open issue listed in JEP 282 about jlink and service binding. The output for --suggest-providers looks good. For --verbose then the "Providers: " list looks okay when used in conjunction with --bind-services. I would be tempted to leave it out when --bind-services is not specified because there is no service binding. A few random comments: - JImageTask is now using toUpperCase() which is locale specific and should be changed. - Jlink.moduleFinder() unconditionally uses the runtime version. Not a bug introduced by your changes but this method really needs to locate java.base and use its version when creating the module path. We should probably create an issue for that and fix it another time. - JlinkTask.uses then the stream() isn't needed as Set defines forEach. -Alan
Re: Better tools for adjusting to strong encapsulation
I think the distinction between the precise options and the big kill switch is important and I welcome this change. It will solve the immediate problem for my application. I still have reservations about the hard-wired use of System.err for warning messages for the reasons given by Roel Spilker. Michael Rasmussen mentioned other cases of this in JDK 9 but I'm not sure what these are. Simon On 23/03/2017 03:30, mark.reinh...@oracle.com wrote: Thanks to everyone for all the feedback on this topic. It appears that issuing warning messages for illegal-access operations enabled by the precise `--add-opens` and `--add-exports` options is a bit too aggressive, at least for JDK 9. Perhaps we can enable that in JDK 10 after there's been more time for libraries, frameworks, and even the JDK itself to adjust to the realities of strong encapsulation. For now I suggest that we revert to the previous behavior of these two options, so that they do not cause warning messages to be issued. The new `--permit-illegal-access` option will continue to generate warning messages, as proposed. If those messages are a problem in a particular scenario then they can be eliminated by switching to an appropriate set of `--add-opens` options, which can be constructed from the information contained in those messages. Comments? - Mark
Re: Better tools for adjusting to strong encapsulation
On 23 March 2017 at 03:30, wrote: > It appears that issuing warning messages for illegal-access operations > enabled by the precise `--add-opens` and `--add-exports` options is a > bit too aggressive, at least for JDK 9. Perhaps we can enable that in > JDK 10 after there's been more time for libraries, frameworks, and even > the JDK itself to adjust to the realities of strong encapsulation. > > For now I suggest that we revert to the previous behavior of these two > options, so that they do not cause warning messages to be issued. The > new `--permit-illegal-access` option will continue to generate warning > messages, as proposed. If those messages are a problem in a particular > scenario then they can be eliminated by switching to an appropriate set > of `--add-opens` options, which can be constructed from the information > contained in those messages. Sounds good. Stephen
Re: Making jdeps aware of exported API
I considered adding an option to specify the packages to be exported. It may work fine for simple cases. It may take a prefix to avoid listing the packages individually. It’s an idea on the list to implement. On the other hand, there will be cases that we would have to edit module-info.java to include `uses` and remove `exports` or qualify any export to a friend. One useful option, `jdeps —-check` option provides suggestion to `requires transitive` that may not be needed and any unused qualified exports. After you edit module-info.java, compile it and rerun jdeps —-check that will analyze the exported API packages and suggest `requires` and `requires transitive`. It requires a compilation step which would be needed to run as a module anyway. About jdeps —-api-only, when running on a module, it looks at the module descriptor and analyzes only exported API packages. If the classes are on classpath (-cp), module-info.class is skipped. Mandy > On Mar 23, 2017, at 2:24 AM, Gunnar Morling wrote: > > Hi, > > I would like to suggest to add an option to jdeps for describing the > intended exported API of descriptors created by > --generate-module-info. This would have two benefits: > > * Limiting the number of exported packages in the generated descriptor > to the intended public API > * Adding the "transitive" modifiers only to those "requires" > statements whose types are used in the exported API > > While it's relatively simple to rework descriptors after generation > and remove any unwanted exports, that's not the case for removing > superfluous "transitive" modifiers. Identifying them would require to > parse the input JAR once more to find out whether a given dependence > is used in public/protected signatures of the exported API or not. > > If the intended exported API could be passed to jdeps - e.g. in form > of regular expressions or some other kind of exclude/include patterns > - no further post-processing would be needed for generating > descriptors with a well-defined API. > > I had a look at the --api-only option, but this seems only to be about > public/protected access and not the exported packages of a module. > > Thanks for your consideration, > > --Gunnar
Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests
> On Mar 23, 2017, at 7:33 AM, Daniel Fuchs wrote: > > Hi Igor, > > small nit: > > 42 * @modules java.management > 43 * jdk.attach > 44 * jdk.management.agent/jdk.internal.agent > > I don't think java.management needs to be specified as > a dependency when the test requires jdk.management.agent, > because jdk.management.agent already requires java.management. That’s true. Igor - How do you analyze the dependency? Are you using jdeps —-list-reduced-deps? Mandy
Re: Better tools for adjusting to strong encapsulation
Thanks, Mark. We're in the middle of collecting necessary --add-opens, --add-exports and other options and only making some progress with removing the need for them. Warnings in JDK10 seems more appropriate. Keimpe Bronkhorst Oracle JDeveloper On 3/23/2017 2:13 AM, jigsaw-dev-requ...@openjdk.java.net wrote: -- Message: 4 Date: Wed, 22 Mar 2017 20:30:00 -0700 From: mark.reinh...@oracle.com To: jigsaw-dev@openjdk.java.net Subject: Re: Better tools for adjusting to strong encapsulation Message-ID: <20170322203000.424722...@eggemoggin.niobe.net> Content-Type: text/plain Thanks to everyone for all the feedback on this topic. It appears that issuing warning messages for illegal-access operations enabled by the precise `--add-opens` and `--add-exports` options is a bit too aggressive, at least for JDK 9. Perhaps we can enable that in JDK 10 after there's been more time for libraries, frameworks, and even the JDK itself to adjust to the realities of strong encapsulation. For now I suggest that we revert to the previous behavior of these two options, so that they do not cause warning messages to be issued. The new `--permit-illegal-access` option will continue to generate warning messages, as proposed. If those messages are a problem in a particular scenario then they can be eliminated by switching to an appropriate set of `--add-opens` options, which can be constructed from the information contained in those messages. Comments? - Mark
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
> On Mar 23, 2017, at 7:58 AM, Alan Bateman wrote: > > On 22/03/2017 19:11, Mandy Chung wrote: > >> Webrev: >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.00/ >> >> This is a proposal to resolve the open issue listed in JEP 282 >> about jlink and service binding. > The output for --suggest-providers looks good. > > For --verbose then the "Providers: " list looks okay when used in conjunction > with --bind-services. I would be tempted to leave it out when --bind-services > is not specified because there is no service binding. > When linking with explicit providers via —-add-modules, would you think it may be useful to see the list of providers? > A few random comments: > > - JImageTask is now using toUpperCase() which is locale specific and should > be changed. > Fixed. > - Jlink.moduleFinder() unconditionally uses the runtime version. Not a bug > introduced by your changes but this method really needs to locate java.base > and use its version when creating the module path. We should probably create > an issue for that and fix it another time. > I created https://bugs.openjdk.java.net/browse/JDK-8177471 to track this. > - JlinkTask.uses then the stream() isn't needed as Set defines forEach. Good catch. Fixed. Revised webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.01/ Mandy
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
Hi Mandy, TaskHelper.java, JLinkTest and IntegrationTest need new copyright year. It’s unclear why do you need concept of “terminal” option Instead of "copy-paste" code to run Jlink in tests ProcessTools and OutputAnalyzer can be used from standard test library test/lib/share/classes/jdk/test/lib/process —Andrey > On 22 Mar 2017, at 22:11, Mandy Chung wrote: > > Webrev: > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.00/ > > This is a proposal to resolve the open issue listed in JEP 282 > about jlink and service binding. > > jlink does not do service binding by default as it may be confusing > to the users. To link in service providers, users have to determine > the provider modules to be added at link time. > > The proposal is to continue not to do service binding by default > since full service binding may possibly cause many modules to be > linked in, which would surprise many users. Provide the following > jlink options to make it easier to cope with services when linking: > > $ jlink --help > : > --bind-services Do full service binding > > --suggest-providers [,...] Suggest providers of services used by > the modules that would be linked, or > of the given service types > > -v, --verbose Enable verbose tracing > > Some sample output that will show with and without —-bind-services > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.verbose.txt > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.suggested.providers.txt > - The providers are sorted by the service type name and then the > provider's module name. > > When —-bind-services is specified with —-suggest-providers, no > additional provider will be suggested. > > Thanks > Mandy
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
> On Mar 23, 2017, at 10:47 AM, Andrey Nazarov > wrote: > > Hi Mandy, > > TaskHelper.java, JLinkTest and IntegrationTest need new copyright year. > It’s unclear why do you need concept of “terminal” option > It’s a low risk and expedient way to support options with optional argument (-—suggest-providers) until jlink command-line processing has to be enhanced to support optional argument. > Instead of "copy-paste" code to run Jlink in tests ProcessTools and > OutputAnalyzer can be used from standard test library > test/lib/share/classes/jdk/test/lib/process > I agree that it’d be good to move it to a test library but this does not need to be the general one since it’s jlink-specific. jlink has a test library under jdk/tests/tools/lib/tests that needs big cleanup. At some point we should look at all jlink tests and see what makes sense. For these new tests, I can move the helper class to jdk/tests/tools/lib/tests. Mandy > > —Andrey > >> On 22 Mar 2017, at 22:11, Mandy Chung wrote: >> >> Webrev: >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/webrev.00/ >> >> This is a proposal to resolve the open issue listed in JEP 282 >> about jlink and service binding. >> >> jlink does not do service binding by default as it may be confusing >> to the users. To link in service providers, users have to determine >> the provider modules to be added at link time. >> >> The proposal is to continue not to do service binding by default >> since full service binding may possibly cause many modules to be >> linked in, which would surprise many users. Provide the following >> jlink options to make it easier to cope with services when linking: >> >> $ jlink --help >> : >> --bind-services Do full service binding >> >> --suggest-providers [,...] Suggest providers of services used by >>the modules that would be linked, or >>of the given service types >> >> -v, --verbose Enable verbose tracing >> >> Some sample output that will show with and without —-bind-services >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.verbose.txt >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8174826/jlink.suggested.providers.txt >> - The providers are sorted by the service type name and then the >> provider's module name. >> >> When —-bind-services is specified with —-suggest-providers, no >> additional provider will be suggested. >> >> Thanks >> Mandy >
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
> On Mar 23, 2017, at 11:07 AM, Mandy Chung wrote: > >> Instead of "copy-paste" code to run Jlink in tests ProcessTools and >> OutputAnalyzer can be used from standard test library >> test/lib/share/classes/jdk/test/lib/process >> > > I agree that it’d be good to move it to a test library but this does not need > to be the general one since it’s jlink-specific. jlink has a test library > under jdk/tests/tools/lib/tests that needs big cleanup. At some point we > should look at all jlink tests and see what makes sense. > > For these new tests, I can move the helper class to > jdk/tests/tools/lib/tests. > Having another look at jdk/tests/tools/lib/tests/*, I think it’s probably best to leave these new tests as is and avoid adding more helper classes until they are cleaned up/unified. Mandy
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
On 23/03/2017 17:04, Mandy Chung wrote: : When linking with explicit providers via —-add-modules, would you think it may be useful to see the list of providers? For the most part then this will just print out the providers specified to --add-modules, unless you are thinking about the less common case of a module that both exports an API and is a service provider module at the same time? -Alan
Re: Better tools for adjusting to strong encapsulation
If I understand JEP 264 correctly, it should be possible to direct platform generated error messages to locations other than the standard error stream (System.err). Is that correct? If so, would it not make sense for the default to be the (platform dependent) console log rather than System.err, which is used by applications for their own error messages? I would guess that any platform being used for development would have a console log facility. Providing access to the console log could be useful for applications as well. On macOS, output written to System.err currently is discarded, which means that any app that wants to log must provide its own facility. > On Mar 21, 2017, at 10:56 AM, Simon Nash wrote: > > These new messages are a big problem for an application that interacts with > the user via the console. I understand that the messages may be useful to > application developers who need to "identify which libraries and frameworks > on your class path require illegal access in order to work". For other > situations where the reason for the access is known to the application > developer and there is no way to avoid it, please consider giving the > application developer the option to suppress the messages or redirect them > to somewhere other than System.err. > > Simon > Roel Spilker wrote: > That said, I consider the output of my programs (the exit code, the output > stream, and the error stream) to be part of my public API. I consider extra > output, that is added to those streams outside my control, a breaking > change. Depending on my program flow and data in my program, these messages > appear in the middle of my output. > > This is especially a problem because the users of my program cannot do > anything themselves to get rid of it. In my opinion, they should not be > bothered by messages intended for the developer.
Re: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules
> On Mar 23, 2017, at 11:40 AM, Alan Bateman wrote: > > On 23/03/2017 17:04, Mandy Chung wrote: > >> : >> When linking with explicit providers via —-add-modules, would you think it >> may be useful to see the list of providers? > For the most part then this will just print out the providers specified to > --add-modules, and all of their transitive dependencies. Basically listing the providers and used by the modules linked in the resulting image provides you the information and avoid the need to run `java —list-modules m1,m2,m3 to inspect the module descriptor, if it wants to confirm what providers are in the image. > unless you are thinking about the less common case of a module that both > exports an API and is a service provider module at the same time? Not really in this context. Mandy
Re: Better tools for adjusting to strong encapsulation
On 23/03/2017 18:44, Alan Snyder wrote: If I understand JEP 264 correctly, it should be possible to direct platform generated error messages to locations other than the standard error stream (System.err). Is that correct? If so, would it not make sense for the default to be the (platform dependent) console log rather than System.err, which is used by applications for their own error messages? System.Logger can be configured to send log messages to whatever logging library you are using. However, is is not appropriate here, ditto for several other areas where you don't want arbitrary code to execute. -Alan
8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens
I'd like to get jdk9/dev updated to align with the latest proposal for --add-exports/--add-opens to not emit warnings. I've put the webrev with the changes here: http://cr.openjdk.java.net/~alanb/8177474/webrev/index.html Some of this is just reverting some of the changes that were in JDK-8174823. I've expanded the test coverage to cover cases where --permit-illegal-access is used with the precise options. These tests ensure that there isn't any warnings emitted when access is allowed via the precise options. Once these changes are in jdk9/dev then I will pull them down to jake. -Alan
Re: 8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens
> On 23 Mar 2017, at 19:16, Alan Bateman wrote: > > I'd like to get jdk9/dev updated to align with the latest proposal for > --add-exports/--add-opens to not emit warnings. I've put the webrev with the > changes here: > >http://cr.openjdk.java.net/~alanb/8177474/webrev/index.html These changes look good to me Alan. -Chris.
Re: Better tools for adjusting to strong encapsulation
OK, so the configuration idea does not work. How about the idea of using the console log instead of System.err for these messages, on systems that have such a thing? > On Mar 23, 2017, at 12:04 PM, Alan Bateman wrote: > > On 23/03/2017 18:44, Alan Snyder wrote: > >> If I understand JEP 264 correctly, it should be possible to direct platform >> generated error messages to locations other than the standard error stream >> (System.err). >> >> Is that correct? >> >> If so, would it not make sense for the default to be the (platform >> dependent) console log rather than System.err, which is used by applications >> for their own error messages? > System.Logger can be configured to send log messages to whatever logging > library you are using. However, is is not appropriate here, ditto for several > other areas where you don't want arbitrary code to execute. > > -Alan
Re: 8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens
> On Mar 23, 2017, at 12:16 PM, Alan Bateman wrote: > > I'd like to get jdk9/dev updated to align with the latest proposal for > --add-exports/--add-opens to not emit warnings. I've put the webrev with the > changes here: > >http://cr.openjdk.java.net/~alanb/8177474/webrev/index.html Looks good to me. Mandy
Re: Better tools for adjusting to strong encapsulation
On 23 March 2017 at 17:05, Simon Nash wrote: > Michael Rasmussen mentioned other cases of this in JDK 9 but I'm > not sure what these are. An example of those is regarding intrinsic methods and the new annotation for those. As mentioned in this thread on hotspot-dev: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-March/026232.html /Michael
Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests
Mandy/Daniel, yes, jdk.management.agent does require java.management, but not transitively. Shura and I have discussed that and agreed that in such cases a test should declare dependency explicitly, otherwise it can start to fail when some of transitive requires (which are not a part of the contract) are changed. I used jdeps with the post-proceccing which makes reduction similar to list-reduced-deps. I have run 'jdeps --list-reduced-deps' on classes from sun/management/jmxremote/bootstrap/CustomLauncherTest.java test run, and it showed the same: >java.management >jdk.attach >jdk.management.agent/jdk.internal.agent >unnamed module: > /tmp/run/jdk/sun/management/jmxremote/bootstrap/JTwork-sun-management-jmxremote-bootstrap-CustomLauncherTest-java_0/classes Thanks, -- Igor > On Mar 23, 2017, at 9:39 AM, Mandy Chung wrote: > > >> On Mar 23, 2017, at 7:33 AM, Daniel Fuchs wrote: >> >> Hi Igor, >> >> small nit: >> >> 42 * @modules java.management >> 43 * jdk.attach >> 44 * jdk.management.agent/jdk.internal.agent >> >> I don't think java.management needs to be specified as >> a dependency when the test requires jdk.management.agent, >> because jdk.management.agent already requires java.management. > > That’s true. > > Igor - How do you analyze the dependency? Are you using jdeps > —-list-reduced-deps? > > Mandy >
Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests
> On Mar 23, 2017, at 1:07 PM, Igor Ignatyev wrote: > > Mandy/Daniel, > > yes, jdk.management.agent does require java.management, but not transitively. > Shura and I have discussed that and agreed that in such cases a test should > declare dependency explicitly, otherwise it can start to fail when some of > transitive requires (which are not a part of the contract) are changed. > > I used jdeps with the post-proceccing which makes reduction similar to > list-reduced-deps. I have run 'jdeps --list-reduced-deps' on classes from > sun/management/jmxremote/bootstrap/CustomLauncherTest.java test run, and it > showed the same: >> java.management >> jdk.attach >> jdk.management.agent/jdk.internal.agent >> unnamed module: >> /tmp/run/jdk/sun/management/jmxremote/bootstrap/JTwork-sun-management-jmxremote-bootstrap-CustomLauncherTest-java_0/classes > It’s good you are using `jdeps --list-reduced-deps`. It includes java.management because the test references the exported APIs from java.management. I agree that @modules should list java.management even though the test selection would work properly without it. Mandy
Review Request: JDK-8173303: Add module-subgraph images to main platform documentation
Jon and I work on this patch as the first step to enable module graphs for inclusion in the javadoc. It includes a @moduleGraph taglet, an updated GenGraphs tool, and also updates javadoc of module-info.java to include @moduleGraph. I also take the opportunity to add simple javadoc to a few JDK modules preparing for JEP 299 that will generate module summary page for JDK modules. Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.00/ Here shows some sample of the javadoc with module graph images: http://cr.openjdk.java.net/~mchung/jdk9/module-graph-docs/overview-summary.html Click on a module and it will show a module graph icon. Mouse over it will show the full size image. java.base is a special case that has no hovered image since it has one node and the full size image is already shown. The build work to generate PNG file from .dot file will be done separately [1]. Mandy [1] https://bugs.openjdk.java.net/browse/JDK-8176785