OpenJDK fails to build with GCC when the #include<time.h> inside zip.cpp comes from a non-sysroot path

[Roger Riggs]

Hi Patrick,

Please re-post the entire proposed patch based on the JDK 13 repo.

BTW, there is already a "#ifndef _MSC_VER" at line 36.

Thanks, Roger


On 01/14/2019 09:02 AM, Magnus Ihse Bursie wrote:
> On 2019-01-02 00:11, David Holmes wrote:
>> Hi Patrick,
>>
>> On 13/12/2018 1:23 pm, Patrick Zhang wrote:
>>> Ping...
>>>
>>> Apply for a sponsor for this simple patch.
>>
>> Seems no one wants to own this one :(
> For what it's worth, it looks good to me. (But I'd like to see a core 
> library developer chime in as well.)
>
> /Magnus
>
>>
>>> I doubt if I could have permission to file the issue/bug report 
>>> anywhere, could anyone kindly give a guidance? Thanks.
>>
>> I have filed:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8215976
>>
>> to cover this and linked to the discussion threads.
>>
>> The appropriate owners still need to review this, but I'm not sure 
>> who that may be these days.
>>
>> Cheers,
>> David
>> -----
>>
>>>
>>> Regards
>>> Patrick
>>>
>>> -----Original Message-----
>>> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On 
>>> Behalf Of Patrick Zhang
>>> Sent: Thursday, December 6, 2018 4:28 PM
>>> To: core-libs-dev at openjdk.java.net; David Holmes 
>>> <david.holmes at oracle.com>
>>> Cc: Florian Weimer <fweimer at redhat.com>
>>> Subject: RE: OpenJDK fails to build with GCC when the 
>>> #include<time.h> inside zip.cpp comes from a non-sysroot path
>>>
>>> To All,
>>> Who could help sponsor this simple patch (may require a wide range 
>>> of building tests)? Thanks in advance.
>>>
>>> Regards
>>> Patrick
>>>
>>> -----Original Message-----
>>> From: David Holmes <david.holmes at oracle.com>
>>> Sent: Monday, December 3, 2018 8:11 AM
>>> To: Patrick Zhang <patrick at os.amperecomputing.com>; Florian Weimer 
>>> <fweimer at redhat.com>
>>> Cc: core-libs-dev at openjdk.java.net
>>> Subject: Re: OpenJDK fails to build with GCC when the 
>>> #include<time.h> inside zip.cpp comes from a non-sysroot path
>>>
>>> Hi Patrick,
>>>
>>> On 30/11/2018 11:41 pm, Patrick Zhang wrote:
>>>> Thanks Florian, the "-isystem /usr/include" is helpful to my case, 
>>>> I see gcc.gnu.org says that "it gets the same special treatment 
>>>> that is applied to the standard system directories". As such the 
>>>> issue gets hidden (error suppressed).
>>>>
>>>> Hi David,
>>>> Thanks for your suggestion. My intention was to limit the influence 
>>>> range as far as I could since I don't have other systems except 
>>>> CentOS/Fedora to verify (even just smoke test) all paths.
>>>
>>> You'd need some assistance testing a wider range of platforms but 
>>> that can be arranged.
>>>
>>>> In addition, if we make below update, does it mean the macro " 
>>>> _REENTRANT " can be removed too? This is probably the only place 
>>>> where _REENTRANT gets used AFAIK.
>>> _REENTRANT is also examined by system headers. It's probably legacy 
>>> but would require more investigation.
>>>
>>> Cheers,
>>> David
>>>
>>>> #ifdef _MSC_VER // Windows
>>>> #define gmtime_r(t, s) gmtime(t)
>>>> #endif
>>>>
>>>> Regards
>>>> Patrick
>>>>
>>>> -----Original Message-----
>>>> From: Florian Weimer <fweimer at redhat.com>
>>>> Sent: Thursday, November 29, 2018 8:02 PM
>>>> To: David Holmes <david.holmes at oracle.com>
>>>> Cc: Patrick Zhang <patrick at os.amperecomputing.com>;
>>>> jdk-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
>>>> Subject: Re: OpenJDK fails to build with GCC when the #include<time.h>
>>>> inside zip.cpp comes from a non-sysroot path
>>>>
>>>> * David Holmes:
>>>>
>>>>> This should really be being discussed on core-libs-dev.
>>>>
>>>> Okay, moving the conversation.
>>>>
>>>>>> diff -r 70a423caee44 
>>>>>> src/share/native/com/sun/java/util/jar/pack/zip.cpp
>>>>>> --- a/src/share/native/com/sun/java/util/jar/pack/zip.cpp Tue Oct 
>>>>>> 09 08:33:33 2018 +0100
>>>>>> +++ b/src/share/native/com/sun/java/util/jar/pack/zip.cpp Wed Nov 
>>>>>> 28 22:13:12 2018 -0500
>>>>>> @@ -415,9 +415,7 @@
>>>>>>         ((uLong)h << 11) | ((uLong)m << 5) | ((uLong)s >> 1);
>>>>>>     }
>>>>>>     -#ifdef _REENTRANT // solaris
>>>>>> -extern "C" struct tm *gmtime_r(const time_t *, struct tm *); -#else
>>>>>> +#if !defined(_REENTRANT) // linux
>>>>>>     #define gmtime_r(t, s) gmtime(t)
>>>>>>     #endif
>>>>>>     /*
>>>>>
>>>>> Under the theme "two wrongs don't make a right" the use of _REENTRANT
>>>>> here is totally inappropriate AFAICS. It seems to be a misguided
>>>>> attempt at determining whether we need the thread-safe gmtime_r or
>>>>> not
>>>>> - and the answer to that should always be yes IMHO.
>>>>>
>>>>> We define _REENTRANT for:
>>>>> - linux
>>>>> - gcc
>>>>> - xlc
>>>>>
>>>>> So the original code will define:
>>>>>
>>>>> extern "C" struct tm *gmtime_r(const time_t *, struct tm *);
>>>>>
>>>>> for linux (and AIX with xlc?) but not Solaris, OS X or Windows.
>>>>>
>>>>> But Solaris has gmtime_r anyway. So the existing code seems a really
>>>>> convoluted hack. AFAICS we have gmtime_r everywhere but Windows
>>>>> (where gmtime is already thread-safe). So it seems to me that all we
>>>>> should need here is:
>>>>>
>>>>> -#ifdef _REENTRANT // solaris
>>>>> -extern "C" struct tm *gmtime_r(const time_t *, struct tm *); -#else
>>>>> +#ifdef _MSC_VER // Windows
>>>>>    #define gmtime_r(t, s) gmtime(t)
>>>>>    #endif
>>>>
>>>> That looks much cleaner.
>>>>
>>>> Thanks,
>>>> Florian
>>>>
>