Review request for JDK-8141338: Move jdk.internal.dynalink package to jdk.dynalink

Attila Szegedi szegedia at gmail.com
Mon Nov 23 15:27:06 UTC 2015


Folks,

I integrated the changes Mandy suggested; please review these (build related) changes:
jdk9 top level: <http://cr.openjdk.java.net/~attila/8141338/webrev.jdk9.top.2/index.html> 
jdk9/jdk: <http://cr.openjdk.java.net/~attila/8141338/webrev.jdk9.top.2/index.html> 

For the sake of completeness, the jdk/nashorn changes are here: <http://cr.openjdk.java.net/~attila/8141338/webrev.jdk9> but they have already been reviewed by Hannes and Sundar; only the above two (jdk9 and jdk9/jdk) have been modified compared to the original review request.

Sundar was kind enough to verify that JDK9 builds fine with these changes.

Thanks,
  Attila.

> On Nov 20, 2015, at 4:41 PM, Attila Szegedi <szegedia at gmail.com> wrote:
> 
> Thanks for pointing out these, Mandy; I will make the changes you suggested.
> 
> Attila.
> 
>> On Nov 20, 2015, at 6:10 AM, Mandy Chung <mandy.chung at oracle.com> wrote:
>> 
>> jdk.scripting.nashorn is loaded by the extension class loader.  Is jdk.dynalink expected to be loaded by the ext. class loader?
>> 
>> You need to edit this file to include the new module:
>>  jdk/make/src/classes/build/tools/module/ext.modules
>> 
>> This is an interim file to map modules to class loader and we will fix it when the changeset propagates to jake.
>> Mandy
>> 
>>> On Nov 19, 2015, at 7:00 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>> 
>>> I reviewed the top repo change.
>>> 
>>> modules.xml looks fine.
>>> 
>>> jdk.dynalink  should be in MAIN_MODULES since it has exported APIs.  jdk.scripting.nashorn should be moved too.  They are not sole service providers.  Since you are on this file, can you move jdk.scripting.nashorn to MAIN_MODULES as well?
>>> 
>>> Mandy
>>> 
>>>> On Nov 19, 2015, at 3:15 PM, Attila Szegedi <szegedia at gmail.com> wrote:
>>>> 
>>>> Please review JDK-8141338 "Move jdk.internal.dynalink package to jdk.dynalink" for <https://bugs.openjdk.java.net/browse/JDK-8141338>. This is basically the implementation step for integrating JEP 276. This changeset will introduce a new public API that has CCC approval (request 8075866), and is also the implementation step of JEP 276 which is now targeted for 9 and thus can be integrated.
>>>> 
>>>> The changes in this changeset fall into several categories:
>>>> - renaming of jdk.internal.dynalink.* package to jdk.dynalink.* package, with ripple effects in Nashorn classes that import from these packages
>>>> - changes to modules.xml and some build files to accommodate a new public module and a dependency of Nashorn on it
>>>> - new tests
>>>> 
>>>> I’m sending this webrev to several lists with the following rationales:
>>>> - nashorn-dev as the primary users and expected reviewers (also, the Dynalink module code lives in jdk9/nashorn/src/jdk.dynalink). A lot of newly added test code was contributed by Sundar.
>>>> - jigsaw-dev because of modules.xml changes
>>>> - jdk9-dev for build changes (build file changes were graciously contributed by Erik Joelsson and Sundar)
>>>> - core-libs-dev since that’s the designated JEP 276 discussion list.
>>>> 
>>>> Nashorn changes: <http://cr.openjdk.java.net/~attila/8141338/webrev.jdk9> 
>>>> top-level jdk9 changes: <http://cr.openjdk.java.net/~attila/8141338/webrev.jdk9.top>
>>>> 
>>>> Thanks,
>>>> Attila.
>>>> 
>>> 
>> 
> 



More information about the jigsaw-dev mailing list