Fix platform check for macOS #895
Conversation
`zutils.h` contains configurations that are no longer exercised for the macOS target. - The target OS conditional macros are misused. Specifically `TARGET_OS_MAC` covers all Apple targets, including iOS, and it should not be checked with `#if defined` as they would always be defined (to either 1 or 0) on Apple platforms. - The assumption that macOS does not have `fdopen`, or defines `fdopen` as a macro is no longer valid. The null definition in `zutils.h` would conflict with the macOS SDK and cause a compilation failure. This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke zlib builds on Apple platforms.
zlib still compiles fine on the latest clang that Apple delivers, 15.0.0. What version of clang does it break on, and exactly how does it break? |
Thanks for taking a look Mark! before the |
|
Thanks. Fixed. |
|
@madler: I think that some commits are missing since 2023-11-15, can you push it? |
|
Note that the |
| @@ -137,15 +137,11 @@ extern z_const char * const z_errmsg[10]; /* indexed by 2-zlib_error */ | |||
| # endif | |||
| #endif | |||
|
|
|||
| #if defined(MACOS) || defined(TARGET_OS_MAC) | |||
| #if TARGET_OS_OSX | |||
| # define OS_CODE 7 | |||
| # ifndef Z_SOLO | |||
| # if defined(__MWERKS__) && __dest_os != __be_os && __dest_os != __win32_os | |||
There was a problem hiding this comment.
The history of this code is interesting.
zlib 0.71 (the first version in git) had:
#ifdef MACOS
# define OS_CODE 0x07
#endif
zlib 1.0.8 added the fdopen thing, but differently:
#if defined(MACOS) || defined(TARGET_OS_MAC)
# define OS_CODE 0x07
# ifndef fdopen
# define fdopen(fd,mode) NULL /* No fdopen() */
# endif
#endif
#if defined(__MWERKS__) && !defined(fdopen)
# if __dest_os != __be_os && __dest_os != __win32_os
# define fdopen(fd,mode) NULL
# endif
#endif
The corresponding changelog:
- check for TARGET_OS_MAC in addition to MACOS (Brad Pettit)
- do not use fdopen for Metrowerks on Mac (Brad Pettit))
zlib 1.1.3 changed it to:
#if defined(MACOS) || defined(TARGET_OS_MAC)
# define OS_CODE 0x07
# if defined(__MWERKS__) && __dest_os != __be_os && __dest_os != __win32_os
# include <unix.h> /* for fdopen */
# else
# ifndef fdopen
# define fdopen(fd,mode) NULL /* No fdopen() */
# endif
# endif
#endif
I think the corresponding changelog entry is:
- Support gzdopen on Mac with Metrowerks (Jason Linhart)
Presumably, Metrowerks was defining TARGET_OS_MAC, and probably didn't define TARGET_OS_OSX, so my assumption here is that this change breaks Metrowerks. However, it's also a long gone compiler, so maybe this part can go entirely, which leaves us with two different OS_CODE values for macOS, one of which was added in ce12c5c
In a similar manner as zlib (madler/zlib#895), libpng contains a header configuration that's no longer valid and hasn't been exercised for the macOS target. - The target OS conditional macros are misused. Specifically `TARGET_OS_MAC` covers all Apple targets, including iOS, and it should not be checked with `#if defined` as they would always be defined (to either 1 or 0) on Apple platforms. - `#include <fp.h>` no longer works for the macOS target and results in a compilation failure. macOS ships all required functions in `math.h`, and clients should use `math.h` instead. This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke libpng builds on Apple platforms. The failure can be reproduced now by adding `#include <TargetConditionals.h>` before the block.
In a similar manner as zlib (madler/zlib#895), libpng contains a header configuration that's no longer valid and hasn't been exercised for the macOS target. - The target OS conditional macros are misused. Specifically `TARGET_OS_MAC` covers all Apple targets, including iOS, and it should not be checked with `#if defined` as they would always be defined (to either 1 or 0) on Apple platforms. - `#include <fp.h>` no longer works for the macOS target and results in a compilation failure. macOS ships all required functions in `math.h`, and clients should use `math.h` instead. This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke libpng builds on Apple platforms. The failure can be reproduced now by adding `#include <TargetConditionals.h>` before the block. Signed-off-by: Cosmin Truta <ctruta@gmail.com>
|
It is good with the current devel branch? |
I believe 4bd9a71 resolved the issue so yeah it should be good. |
zutils.hcontains configurations that are no longer exercised for the macOS target.TARGET_OS_MACcovers all Apple targets, including iOS, and it should not be checked with#if definedas they would always be defined (to either 1 or 0) on Apple platforms.fdopen, or definesfdopenas a macro is no longer valid. The null definition inzutils.hwould conflict with the macOS SDK and cause a compilation failure.This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke zlib builds on Apple platforms.