RFR [XXS] : JDK-8081616: build fixes for --disable-warnings-as-errors

[David Holmes]

On 2/06/2015 10:36 PM, Magnus Ihse Bursie wrote:
> On 2015-06-02 13:29, David Holmes wrote:
>> On 2/06/2015 8:45 PM, Magnus Ihse Bursie wrote:
>>> On 2015-06-02 12:35, Magnus Ihse Bursie wrote:
>>>> On 2015-06-02 12:27, David Holmes wrote:
>>>>> These should be removed instead.
>>>>>
>>>>> Okay but I don't think it reasonable to expect Bertrand to make such
>>>>> changes. In the interest of moving forward can we use his current
>>>>> patch and file a follow up bug to get this done properly by the build
>>>>> team?
>>>> It's not worth the trouble to put in bad code just to rip it out. I'll
>>>> have the patch done in five minutes, brb. :)
>>>
>>> Here's the webrev:
>>> http://cr.openjdk.java.net/~ihse/JDK-8081616-respect-disable-warnings-as-errors/webrev.01
>>>
>>>
>>>
>>> No top level changes are needed. I kept the disabling of
>>> unused-parameter in libsctp for pragmatic reasons. On my computer it
>>> compiled fine without it, but it requires more work to determine if it
>>> can be removed.
>>
>> I'm missing something - where does CFLAGS_WARNINGS_ARE_ERRORS get
>> added in when it is enabled ??
>
> It is used in SetupNativeCompilation.gmk. It is one of the "control
> flags" that are tightly coupled with the SetupNativeCompilation code,
> like SHARED_LIBRARY_FLAGS.
>
> The "control flags" are used to control the behavior of the compiler
> rather than to affect the resulting build. We have been doing a bad job
> in the past of separating "product flags" (e.g. -DPRODUCT_NAME=JDK) from
> control flags. This is the reason I didn't want
> CFLAGS_WARNINGS_ARE_ERRORS removed.

Thanks for clarifying.

>> Also we will need to this to go in via hs-rt please (using JPRT).
> Okidoki!

Please use "-testset hotspot".

Thanks - and good night :)

David
-----


> /Magnus
>>
>> Thanks,
>> David
>>
>>> /Magnus
>>>
>>>>
>>>> /Magnus
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>> I also checked for -Werror in the code, but could only find the
>>>>>> already
>>>>>> discovered location in Lib-jdk.sctp.gmk. For that, I believe the
>>>>>> proper
>>>>>> solution is:
>>>>>> diff --git a/make/lib/Lib-jdk.sctp.gmk b/make/lib/Lib-jdk.sctp.gmk
>>>>>> --- a/make/lib/Lib-jdk.sctp.gmk
>>>>>> +++ b/make/lib/Lib-jdk.sctp.gmk
>>>>>> @@ -30,12 +30,8 @@
>>>>>>   ifeq ($(OPENJDK_TARGET_OS_TYPE), unix)
>>>>>>
>>>>>>     ifeq (, $(filter $(OPENJDK_TARGET_OS), macosx aix))
>>>>>> -
>>>>>> -    # Suppress unused parameters required by exported JNI functions.
>>>>>> -    SCTP_WERROR := -Werror -Wno-error=unused-parameter
>>>>>> -    ifeq ($(OPENJDK_TARGET_CPU_ARCH), ppc)
>>>>>> -      SCTP_WERROR :=
>>>>>> -    endif
>>>>>> +    # DISABLED_WARNINGS_gcc := unusused-parameter needed to
>>>>>> +    # suppress unused parameters required by exported JNI functions.
>>>>>>
>>>>>>       $(eval $(call SetupNativeCompilation,BUILD_LIBSCTP, \
>>>>>>           LIBRARY := sctp, \
>>>>>> @@ -49,7 +45,7 @@
>>>>>>               $(LIBJAVA_HEADER_FLAGS) \
>>>>>>               -I$(SUPPORT_OUTPUTDIR)/headers/jdk.sctp \
>>>>>>               -I$(SUPPORT_OUTPUTDIR)/headers/java.base, \
>>>>>> -        CFLAGS_linux := $(SCTP_WERROR), \
>>>>>> +        DISABLED_WARNINGS_gcc := unusused-parameter, \
>>>>>>           MAPFILE :=
>>>>>> $(JDK_TOPDIR)/make/mapfiles/libsctp/mapfile-vers, \
>>>>>>           LDFLAGS := $(LDFLAGS_JDKLIB) \
>>>>>>               $(call SET_SHARED_LIBRARY_ORIGIN), \
>>>>>>
>>>>>>
>>>>>> However, as I said, it should probably be verified that it is correct
>>>>>> that the unused-parameter warning is still triggered.
>>>>>>
>>>>>> /Magnus
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> I can see that libsctp is a special case that hard-codes -Werror.
>>>>>>>> But in
>>>>>>>> this case we should remove the hard-coding and relying on the
>>>>>>>> system
>>>>>>>> setting. This is probably a remnant from before the overall -Werror
>>>>>>>> usage, where the authors of a specific lib wanted to enforce a
>>>>>>>> higher
>>>>>>>> standard. Also, it might be worth revisiting if
>>>>>>>> -Wno-error=unused-parameter is really needed. These things tend to
>>>>>>>> bit-rot.
>>>>>>>>
>>>>>>>> /Magnus
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>
>>>>>>>>> Bertrand.
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>>
>