RFR: JDK-8139657: Incremental build of jdk.vm.ci-gensrc creates repeated entries in services file

[Erik Joelsson]

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