Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?

[Magnus Ihse Bursie]

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
>