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