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

[Attila Szegedi]

On Nov 20, 2015, at 4:10 PM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 19/11/2015 23:15, Attila Szegedi 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
> Probably build-dev instead of jdk9-dev.
> 
> I'm curious if it's strictly necessary for module jdk.dynalink to be in the nashorn repo now, I assume not but it's probably convenient when working Nashorn.

Exactly; it isn’t necessary, but it has been beneficial for now seeing how changes in Dynalink usually had changes in Nashorn to go with them, so having them in a single repo for atomic update is a nice property. I guess someone might eventually give a bit of a thought to the issue of how modularization relates to the JDK forest organization…

> In any case, the module name and the changes to modules.xml look okay to me. As Mandy noted, this isn't a service provider API so in Images.gmk then you can add it to MAIN_MODULES rather than PROVIDER_MODULES.

Yep, will do that as Mandy suggested.

> Your webrevs don't have the changes to the jdk repo but I assume that make/src/classes/build/tools/module/ext.modules has been updated to list jdk.dynalink.

Because I haven’t updated the webrevs since Mandy pointed out that this file also has to be changed :-). Will do it soon.

> That is, I assume it needs to be defined to the ext loader because jdk.scripting.nashorn uses it. The ext.modules file is temporary and goes away when we bring in the module system.

I too believe that having it in ext loader is correct. It will hopefully become moot once the module system is in.

Thanks,
  Attila.

> -Alan.