RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]

[Lance Andersen]

On Wed, 11 May 2022 15:03:56 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - copyright years
>>  - disable format-nonliteral warning when building LIBSPLASHSCREEN with bundled zlib
>>  - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file
>>  - Magnus' suggestion - Disable format-nonliteral in build section of zlib instead of source code
>
> make/autoconf/lib-bundled.m4 line 220:
> 
>> 218:   if test "x$USE_EXTERNAL_LIBZ" = "xfalse"; then
>> 219:     LIBZ_CFLAGS="$LIBZ_CFLAGS -I$TOPDIR/src/java.base/share/native/libzip/zlib"
>> 220:     if test "x$OPENJDK_TARGET_OS" = xmacosx; then
> 
> Please add a comment here as to why we are doing this

> @LanceAndersen Is that really needed? We normally don't comment on the reason why certain code needs certain defines. We tried to document some compiler flags in the beginning, but it turned out that most command lines ended up as essays, and this were not helpful. I think it's quite obvious what this code does: libz requires the define HAVE_UNISTD_H on macOS. I'm not even sure how you'd formulate a "why" -- "otherwise it does not work properly"?

The  zlib configure script, which needs to be run prior  running make to build the upstream repository, will determine if HAVE_UNISTD_H is needed and generate zconf.h replacing the macro with "1".    My reason for adding a comment as this is a variant from how zlib is built upstream.  Perhaps just updating open/src/java.base/share/native/libzip/zlib/ChangeLog  is enough.  I was just trying to document why we are doing this.


Another question would it make sense to set LIBZ_DISABLED_WARNINGS_CLANG in make/autoconf/lib-bundled.m4 so that the value in the case of zlib is set in one location?  In addition, I  can log a request to address this in the upstream project so we do not need to worry about this warning going forward.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8651