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

[David Holmes]

On 29/03/2022 7:20 pm, Magnus Ihse Bursie wrote:
> 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.

Our sources are for a very old version:

1.2.11 (15 Jan 2017)

which doesn't appear to support building on macOS. The webpage for zlib 
states [1]:

"zlib for macOS (Mac OS X): zlib is already included as part of macOS"

so perhaps it was never intended to be built directly on macOS.

David
-----

[1] https://www.zlib.net/

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