Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MZ_FETCH_LIBS option for non-WIN32 systems #686

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

lawadr
Copy link
Contributor

@lawadr lawadr commented Mar 29, 2023

Using cmake_dependent_option overrides a previously set directory scope MZ_FETCH_LIBS variable. This means if a project sets MZ_FETCH_LIBS before adding minizip-ng as a subdirectory in order to force it to fetch zlib, it will set the variable to OFF, only use find_library and never fetch zlib on non-WIN32 systems. This is a regression from previous behaviour.

Instead, revert back to using option as this obeys a directory scope MZ_FETCH_LIBS variable set by a parent directory, allowing said parent directory to control zlib version.

Using `cmake_dependent_option` overrides a previously set directory
scope MZ_FETCH_LIBS variable. This means if a project sets MZ_FETCH_LIBS
before adding minizip-ng as a subdirectory in order to force it to fetch
zlib, it will set the variable to OFF, only use `find_library` and never
fetch zlib on non-WIN32 systems. This is a regression from previous
behaviour.

Instead, revert back to using `option` as this obeys a directory scope
MZ_FETCH_LIBS variable set by a parent directory, allowing said parent
directory to control zlib version.
@nmoinvaz nmoinvaz changed the base branch from master to develop March 29, 2023 18:44
@nmoinvaz nmoinvaz merged commit b3a7cae into zlib-ng:develop Mar 30, 2023
@lawadr lawadr deleted the zlib-fetch-fix branch March 30, 2023 10:22
@nmoinvaz nmoinvaz added the build system Build system and script changes label Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build system Build system and script changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants