On Nov 20, 2015, at 4:10 PM, Alan Bateman <alan.bate...@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.
