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

[Magnus Ihse Bursie]

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