RFR 8043138: Attach API should not require jvmstat rmi protocol

[Jaroslav Bachorik]

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.bachorik at 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-
>