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

Christian Thalinger christian.thalinger at oracle.com
Thu Oct 15 22:35:05 UTC 2015


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
> 




More information about the build-dev mailing list