Looks .patch didn't work as an attachment, retry with .txt and paste the text
below as well. Sorry for one more message here.
# HG changeset patch
# User patrickz
# Date 1546508379 -28800
# Thu Jan 03 17:39:39 2019 +0800
# Node ID 0a90eb1de8e4e6c87a6643072e00645d5dde9da2
# Parent 0b2574a2a6c7816e6a7b6fc05de18318a2b6e561
8215976: Fix gmtime_r declaration conflicts in zip.cpp with linux header files
diff -r 0b2574a2a6c7 -r 0a90eb1de8e4
src/jdk.pack/share/native/common-unpack/zip.cpp
--- a/src/jdk.pack/share/native/common-unpack/zip.cpp Tue Jan 15 08:03:30
2019 +0100
+++ b/src/jdk.pack/share/native/common-unpack/zip.cpp Thu Jan 03 17:39:39
2019 +0800
@@ -416,9 +416,11 @@
((uLong)h << 11) | ((uLong)m << 5) | ((uLong)s >> 1);
}
-#ifdef _REENTRANT // solaris
-extern "C" struct tm *gmtime_r(const time_t *, struct tm *);
-#else
+/*
+ * For thread-safe reasons, non-Windows platforms need gmtime_r
+ * while Windows can directly use gmtime that is already thread-safe.
+ */
+#ifdef _MSC_VER
#define gmtime_r(t, s) gmtime(t)
#endif
/*
Regards
Patrick
-----Original Message-----
From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> On Behalf Of
Patrick Zhang
Sent: Wednesday, January 16, 2019 10:23 AM
To: Roger Riggs <roger.ri...@oracle.com>; David Holmes <david.hol...@oracle.com>
Cc: core-libs-dev <core-libs-dev@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
Re-generated the patch in pure text using “hg export -o” instead of webrev,
perhaps this could work (see attached please).
Comments had been updated as well according to David’s input. Thanks
Regards
Patrick
From: Roger Riggs <roger.ri...@oracle.com>
Sent: Tuesday, January 15, 2019 11:26 PM
To: Patrick Zhang <patr...@os.amperecomputing.com>
Cc: core-libs-dev <core-libs-dev@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,
Attaching just the patch file that is text works fine and it can be applied
directly using patch.
I don't find a patch in your emails that I can apply directly (or am using the
wrong tool).
The allowed attachment types are occasionally mentioned. [1]
Thanks, Roger
[1] http://mail.openjdk.java.net/pipermail/code-tools-dev/2018-March/000378.html
On 01/15/2019 03:57 AM, Patrick Zhang wrote:
An attachment in the email has been found to contain executable code and has
been removed.
File removed : jdk-8215976.webrev.01.zip,
zip,01/totalchangedlines,list,html,html,html,html,html,html,html,html,html,html,html,js
----------------------------------------------------------------------
Hi Roger,
Thanks for your review firstly.
Attached is the latest patch rebased on today's tip of jdk/jdk
(http://hg.openjdk.java.net/jdk/jdk/rev/0b2574a2a6c7).
<<JDK-8215976.webrev.01.zip>>
Yes there is a "#ifndef _MSC_VER" at line 36, while I think we'd better keep
this "#ifdef _MSC_VER" here, as such the declaration of gmtime_r can be near to
where it gets used (line 436 in the patch).
FYI. I sent out an almost same patch early on 3 Jan.
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/057679.html
<<JDK-8215976.webrev.00.zip>>
Regards
Patrick
From: Roger Riggs <roger.ri...@oracle.com><mailto:roger.ri...@oracle.com>
Sent: Monday, January 14, 2019 11:49 PM
To: Patrick Zhang
<patr...@os.amperecomputing.com><mailto:patr...@os.amperecomputing.com>
Cc: core-libs-dev
<core-libs-dev@openjdk.java.net><mailto:core-libs-dev@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,
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 mailto:core-libs-dev-boun...@openjdk.java.net On Behalf Of
Patrick Zhang
Sent: Thursday, December 6, 2018 4:28 PM
To: mailto:core-libs-dev@openjdk.java.net; David Holmes
mailto:david.hol...@oracle.com
Cc: Florian Weimer mailto:fwei...@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 mailto:david.hol...@oracle.com
Sent: Monday, December 3, 2018 8:11 AM
To: Patrick Zhang mailto:patr...@os.amperecomputing.com; Florian Weimer
mailto:fwei...@redhat.com
Cc: mailto:core-libs-dev@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 mailto:fwei...@redhat.com
Sent: Thursday, November 29, 2018 8:02 PM
To: David Holmes mailto:david.hol...@oracle.com
Cc: Patrick Zhang mailto:patr...@os.amperecomputing.com;
mailto:jdk-...@openjdk.java.net; mailto:core-libs-dev@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
# HG changeset patch
# User patrickz
# Date 1546508379 -28800
# Thu Jan 03 17:39:39 2019 +0800
# Node ID 0a90eb1de8e4e6c87a6643072e00645d5dde9da2
# Parent 0b2574a2a6c7816e6a7b6fc05de18318a2b6e561
8215976: Fix gmtime_r declaration conflicts in zip.cpp with linux header files
diff -r 0b2574a2a6c7 -r 0a90eb1de8e4
src/jdk.pack/share/native/common-unpack/zip.cpp
--- a/src/jdk.pack/share/native/common-unpack/zip.cpp Tue Jan 15 08:03:30
2019 +0100
+++ b/src/jdk.pack/share/native/common-unpack/zip.cpp Thu Jan 03 17:39:39
2019 +0800
@@ -416,9 +416,11 @@
((uLong)h << 11) | ((uLong)m << 5) | ((uLong)s >> 1);
}
-#ifdef _REENTRANT // solaris
-extern "C" struct tm *gmtime_r(const time_t *, struct tm *);
-#else
+/*
+ * For thread-safe reasons, non-Windows platforms need gmtime_r
+ * while Windows can directly use gmtime that is already thread-safe.
+ */
+#ifdef _MSC_VER
#define gmtime_r(t, s) gmtime(t)
#endif
/*
