On 2022-03-29 03:42, Jaikiran Pai wrote:
Hello Magnus,
On 28/03/22 5:21 pm, Magnus Ihse Bursie wrote:
On 2022-03-28 09:03, David Holmes wrote:
On 28/03/2022 4:56 pm, Alan Bateman wrote:
On 28/03/2022 07:46, David Holmes wrote:
Hi Jai,
It isn't obvious to me that the bundled sources are actually
intended to build on macOS. There's no include of unistd.h to get
the lseek definition.
I think the context here is that Jai is chasing an issue that may
be bug in the libz on macOS. Building the bundled version and
comparing results would lead to useful information. AFAIK, the
system zlib has always been used since 7u4 when the macOS was
added. I think the Apple JDK did the same. So there may be a small
bit of "porting" to do, adds include files, etc.
Okay, I suggest adding the include of unistd.h as a starter then.
But Jai should note that this is not a build issue per-se - making
those sources buildable on all desired platforms is the job of the
owners of that src in the OpenJDK.
I agree fully, but I'd also like to add that it is not clear that
this is not "supposed" to work. If bundled zlib is not buildable on
macOS (that was news to me), then the correct build system behavior
should have been to block it in configure. So, Jaikiran, if you
intend to get this to work on macOS, great! If you're not taking that
route, please let me know so I can open a bug on prohibiting bundled
zlib on maccOS from configure.
I used David's suggestion to add that include header:
diff --git a/src/java.base/share/native/libzip/zlib/gzlib.c
b/src/java.base/share/native/libzip/zlib/gzlib.c
index a814dd8c7b6..9df629d4205 100644
--- a/src/java.base/share/native/libzip/zlib/gzlib.c
+++ b/src/java.base/share/native/libzip/zlib/gzlib.c
@@ -35,6 +35,7 @@
#if defined(_LARGEFILE64_SOURCE) && _LFS64_LARGEFILE-0
# define LSEEK lseek64
#else
+# include <unistd.h>
# define LSEEK lseek
#endif
#endif
That did help me get past the lseek error, but there are some more
errors (as noted in that log) which need to be addressed too. I don't
have the necessary knowledge of C ecosystem to decide what set of
includes and whether those includes need to be conditional for macOS,
are required to get this building. The other thing to consider with
this code is that it is bundled code and the real code lies in a
separate upstream zlib project[1]. So I think whatever changes we do
here might have to be co-ordinated back to that project.
Maybe. But when we bundle code we also kind of rip it out of context. It
might very well be that the upstream code has an automake system which
generates a config.h that has proper includes and defines to get this to
build. I mean, obviously there is an existing system zlib on macOS, so
*someone* has gotten it to build.
Please go ahead with your plan to create a JBS issue to prohibit this
combination in the meantime.
Ok, done that: https://bugs.openjdk.java.net/browse/JDK-8283844
What surprises me a bit is that looking at the commit history within
this source area, there haven't been any commits in this code for
around 4 years now. So I suspect this hasn't been working at least
that long or maybe the compiler rules got more stricter during that
period.
This has most likely never been working, yes. :) We've had a quite
strict policy of bundling-or-system-zlib depending on OS, and I don't
think anyone has ever tried to override that before. (With the exception
of linux, where I'm sure both options will work fine.)
/Magnus
[1] https://github.com/madler/zlib/tree/v1.2.11
-Jaikiran
