On 25.11.2015 09:37, Erik Joelsson wrote:


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.

Ok. In this case what is the easiest way to perform in-situ merge? Eg. merging the services from jdk.jvmstat.rmi into jdk.jvmstat in the gensrc?


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
\
     #

Ok. Will change this.

-JB-


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-


Reply via email to