Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

[Mandy Chung]

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

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> 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

Re: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Igor Ignatyev]

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: 8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens

[Mandy Chung]

> 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

[Alan Snyder]

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

[Chris Hegarty]

> 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.

8177474: Do not emit warnings when illegal access is allowed by --add-exports/--add-opens

[Alan Bateman]

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: Better tools for adjusting to strong encapsulation

[Alan Bateman]

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: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules

[Mandy Chung]

> 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: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules

[Alan Bateman]

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: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules

[Mandy Chung]

> 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

[Mandy Chung]

> 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

[Andrey Nazarov]

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

[Mandy Chung]

> 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: Better tools for adjusting to strong encapsulation

[Keimpe Bronkhorst]

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: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Mandy Chung]

> 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: Making jdeps aware of exported API

[Mandy Chung]

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: Better tools for adjusting to strong encapsulation

[Stephen Colebourne]

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: Better tools for adjusting to strong encapsulation

[Simon Nash]

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: Review Request: JDK-8174826 jlink does not provide support for linking in service provider modules

[Alan Bateman]

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: RFR(S) : 8177374 : fix module dependency declaration in jdk_svc tests

[Daniel Fuchs]

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 > wrote:



On Mar 22, 2017, at 1:09 PM, Igor Ignatyev > 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

Making jdeps aware of exported API

[Gunnar Morling]

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: Better tools for adjusting to strong encapsulation

[Jochen Theodorou]

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

Re: Better tools for adjusting to strong encapsulation

[Jochen Theodorou]

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

[Alan Bateman]

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