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

[Erik Joelsson]

I will push it to hs-comp. Thanks for the review!

/Erik

On 2015-10-16 00:35, Christian Thalinger wrote:
> 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.joelsson at 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.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