[GH-ISSUE #405] Broken logic in FindMINIZIP.cmake #321

New issue

Closed

opened 2026-05-05 12:08:58 -06:00 by gitea-mirror · 3 comments

gitea-mirror commented

2026-05-05 12:08:58 -06:00

Owner

Originally created by @hosiet on GitHub (Aug 2, 2023).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/405

We look into what FindMINIZIP.cmake does:

f43e53e018/cmake/FindMINIZIP.cmake (L92-L95)

When enabling USE_SYSTEM_MINIZIP and system libminizip is found, these lines basically look through system's /usr/include/zlib.h and look for strings like Version [0-9]+\\.[0-9]+(\\.[0-9]+)?.

However, the zlib.h header never had such lines:

-> % cat /usr/include/zlib.h | grep Version
#define zlib_version zlibVersion()
ZEXTERN const char * ZEXPORT zlibVersion OF((void));
/* The application can compare zlibVersion and ZLIB_VERSION for consistency.

This condition actually applies to all recent zlib releases.

I believe the logic in FindMINIZIP.cmake must be broken somewhere. Can you take a look?

Originally created by @hosiet on GitHub (Aug 2, 2023). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/405 We look into what `FindMINIZIP.cmake` does: https://github.com/jmcnamara/libxlsxwriter/blob/f43e53e018b6f90b06e8b7eadb26c30fbfcf0528/cmake/FindMINIZIP.cmake#L92-L95 When enabling `USE_SYSTEM_MINIZIP` and system libminizip is found, these lines basically look through system's `/usr/include/zlib.h` and look for strings like `Version [0-9]+\\.[0-9]+(\\.[0-9]+)?`. --------- However, the `zlib.h` header never had such lines: ``` -> % cat /usr/include/zlib.h | grep Version #define zlib_version zlibVersion() ZEXTERN const char * ZEXPORT zlibVersion OF((void)); /* The application can compare zlibVersion and ZLIB_VERSION for consistency. ``` This condition actually applies to all recent zlib releases. I believe the logic in `FindMINIZIP.cmake` must be broken somewhere. Can you take a look?

gitea-mirror closed this issue

2026-05-05 12:09:01 -06:00

gitea-mirror commented

2026-05-05 12:09:03 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 2, 2023):

Thanks. I'll take a look.

@jmcnamara commented on GitHub (Aug 2, 2023): Thanks. I'll take a look.

gitea-mirror commented

2026-05-05 12:09:05 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 3, 2023):

There was a "Version/version" typo in the regex that prevented FindMINIZIP.cmake from reporting the library found even if it was. I fixed that and the compilation works on macOS and Ubuntu 22.04. I want to do a bit more digging to see why this didn't turn up in the past since that code has been there since 2017 (maybe cmake behaviour has changed?).

Anyway, if you get a chance can you test main to see if it works on your OS.

@jmcnamara commented on GitHub (Aug 3, 2023): There was a "Version/version" typo in the regex that prevented `FindMINIZIP.cmake` from reporting the library found even if it was. I fixed that and the compilation works on macOS and Ubuntu 22.04. I want to do a bit more digging to see why this didn't turn up in the past since that code has been there since 2017 (maybe cmake behaviour has changed?). Anyway, if you get a chance can you test main to see if it works on your OS.

gitea-mirror commented

2026-05-05 12:09:06 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 3, 2023):

I think I found the reason this didn't fail before. It was due to another change/fix in #398.

Anyway, the good news is that the CI is passing again so I'm going to close this.

@jmcnamara commented on GitHub (Aug 3, 2023): I think I found the reason this didn't fail before. It was due to another change/fix in #398. Anyway, the good news is that the CI is passing again so I'm going to close this.