On 2015-11-25 09:11, Staffan Larsen wrote:
On 24 nov. 2015, at 17:03, Jaroslav Bachorik <jaroslav.bacho...@oracle.com>
wrote:
Please, review the following change
Issue : https://bugs.openjdk.java.net/browse/JDK-8043138
Webrevs:
* top level: http://cr.openjdk.java.net/~jbachorik/8043138/webrev.00/top
* jdk: http://cr.openjdk.java.net/~jbachorik/8043138/webrev.00/jdk
This patch splits up the jdk.jvmstat module to jdk.jvmstat and jdk.jvmstat.rmi
to make the basic jvmstat functionality available without requiring
dependencies on RMI.
The split is pretty straightforward - all the RMI dependent implementation is
moved to the new module as well as 'jstatd' implementation.
The change requires changes in the makefile (for merging META-INF/services
resources from jdk.jvmstat and jdk.jvmstat.rmi) and therefore I am posting this
request also to the build-dev mailing list.
In Gensrc-jdk.jvmstat.gmk, please don't copy files into jdk/modules in a
gensrc file. I understand you used Gensrc-jdk.jdi.gmk as a template for
this, but that one is special because of jdk.hotspot.agent being built
in hotspot and imported. The correct behavior of a gensrc make file is
to just put files in the gensrc dir. The java compilation step will make
sure things move from gensrc to jdk/modules.
A minor style nit, we tend to not align lists of strings but instead use
a strict 4 spaces continuation indentation. The reason is that we want
to avoid spurious whitespace changes on for example variable renaming
changes. If you still want them aligned, the recommended pattern is:
SERVICE_SRC += \
$(JDK_TOPDIR)/src/jdk.jvmstat/share/classes/META-INF/services/sun.jvmstat.monitor.MonitoredHostService
\
$(JDK_TOPDIR)/src/jdk.jvmstat.rmi/share/classes/META-INF/services/sun.jvmstat.monitor.MonitoredHostService
\
#
For more details, see
http://openjdk.java.net/groups/build/doc/code-conventions.html
Can you explain what the makefile is doing? Why do we need to merge these files?
The merging is needed because of the module system not yet being present
and dealing with service providers with the same name from different
modules. Currently the runtime will find one of the provider files at
random and for it to work we need to make sure they contain the same thing.
/Erik
Thanks,
-JB-
