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

[Magnus Ihse Bursie]

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.

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

/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.
>>>>>>>>
>>>>>>>
>>>>>
>>>
>>