Looks good. Since you are a JDK 9 Reviewer, will you push this fix?
> On Oct 15, 2015, at 2:39 AM, Erik Joelsson <erik.joels...@oracle.com> wrote: > > Hello, > > Sorry for not noticing this earlier. Here is a fix for the problem with > repeating lines in the services file. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8139657 > Patch: > diff -r 9ab5571ccea8 make/gensrc/Gensrc-jdk.vm.ci.gmk > --- a/make/gensrc/Gensrc-jdk.vm.ci.gmk > +++ b/make/gensrc/Gensrc-jdk.vm.ci.gmk > @@ -108,7 +108,11 @@ > ($(CD) $(GENSRC_DIR)/META-INF/jvmci.providers && \ > for i in $$($(LS)); do \ > c=$$($(CAT) $$i | $(TR) -d '\n\r'); \ > - $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c; \ > + $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c.tmp; \ > + done) > + ($(CD) $(GENSRC_DIR)/META-INF/services && \ > + for i in $$($(LS) *.tmp); do \ > + $(MV) $$i $${i%.tmp}; \ > done) > $(TOUCH) $@ > > /Erik > > On 2015-10-08 04:42, Christian Thalinger wrote: >>> On Oct 5, 2015, at 2:47 AM, Magnus Ihse Bursie >>> <magnus.ihse.bur...@oracle.com> wrote: >>> >>> On 2015-09-29 03:12, Christian Thalinger wrote: >>>>> On Sep 27, 2015, at 11:25 PM, Magnus Ihse Bursie >>>>> <magnus.ihse.bur...@oracle.com> wrote: >>>>> >>>>> On 2015-09-25 22:00, Christian Thalinger wrote: >>>>>> Btw. we found a bug in creating the OptionDescriptors files; we get >>>>>> duplicate entries: >>>>>> >>>>>> $ cat >>>>>> build/macosx-x86_64-normal-server-release/jdk/modules/java.base/META-INF/services/jdk.internal.jvmci.options.OptionDescriptors >>>>>> jdk.internal.jvmci.compiler.Compiler_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotConstantReflectionProvider_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaFieldImpl_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaMethodImpl_OptionDescriptors >>>>>> jdk.internal.jvmci.compiler.Compiler_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotConstantReflectionProvider_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaFieldImpl_OptionDescriptors >>>>>> jdk.internal.jvmci.hotspot.HotSpotResolvedJavaMethodImpl_OptionDescriptors >>>>>> … >>>>>> >>>>>> Would this be the right fix? >>>>>> >>>>>> diff -r db1a815d2f6c make/gensrc/Gensrc-java.base.gmk >>>>>> --- a/make/gensrc/Gensrc-java.base.gmkThu Sep 24 15:35:49 2015 -1000 >>>>>> +++ b/make/gensrc/Gensrc-java.base.gmkFri Sep 25 18:18:35 2015 +0200 >>>>>> @@ -94,6 +94,7 @@ >>>>>> $(GENSRC_DIR)/_gensrc_proc_done >>>>>> $(MKDIR) -p $(@D) >>>>>> ($(CD) $(GENSRC_DIR)/META-INF/jvmci.options && \ >>>>>> + $(RM) -f $@; \ >>>>>> for i in $$(ls); do \ >>>>>> echo $${i}_OptionDescriptors >> $@; \ >>>>>> done) >>>>>> >>>>> That seems like a reasonable fix, yes. >>>> Thanks, but… (see below) >>>> >>>>>> And I see the same behavior for HotSpotJVMCIBackendFactory: >>>>>> >>>>>> $ cat >>>>>> build/macosx-x86_64-normal-server-release/jdk/modules/java.base/META-INF/services/jdk.internal.jvmci.hotspot.HotSpotJVMCIBackendFactory >>>>>> jdk.internal.jvmci.hotspot.amd64.AMD64HotSpotJVMCIBackendFactory >>>>>> jdk.internal.jvmci.hotspot.sparc.SPARCHotSpotJVMCIBackendFactory >>>>>> jdk.internal.jvmci.hotspot.amd64.AMD64HotSpotJVMCIBackendFactory >>>>>> jdk.internal.jvmci.hotspot.sparc.SPARCHotSpotJVMCIBackendFactory >>>>>> … >>>>>> >>>>>> So I think a similar fix needs to be applied there. >>>> …I’ve look at the code that creates this file and it isn’t obvious to me >>>> how to fix it. Any good ideas? >>> Try this: >>> ($(CD) $(GENSRC_DIR)/META-INF/jvmci.providers && \ >>> for i in $$($(LS)); do \ >>> c=$$($(CAT) $$i | $(TR) -d '\n\r'); \ >>> + $(RM) $(GENSRC_DIR)/META-INF/services/$$c; \ >>> $(ECHO) $$i >> $(GENSRC_DIR)/META-INF/services/$$c; \ >>> done) >>> $(TOUCH) $@ >>> >>> I have not tested it but it should work. >> No, this won’t work. Both implementations of HotSpotJVMCIBackendFactory >> (AMD64HotSpotJVMCIBackendFactory and SPARCHotSpotJVMCIBackendFactory) >> contain the same service file name: >> >> jdk.internal.jvmci.hotspot.HotSpotJVMCIBackendFactory >> >> since we need to collect all available factories in one service. >> >>> /Magnus >