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

CMakeLists.txt - fix zconf.h not being found in case of out-of-source… #532

Merged
merged 1 commit into from Dec 4, 2020

Conversation

eiszapfen2000
Copy link
Contributor

@eiszapfen2000 eiszapfen2000 commented Nov 25, 2020

…-tree build

zlib CMakeLists.txt does some strange things with zconf.h.in and zconf.h
To make sure zconf.h is found with an out-of-source-tree build, we have to adjust the add_subdirectory call, and give the source directory as binary directory.

ZLIB_BINARY_DIR is empty anyways, but that is caused by another issue in the clone_repo cmake helper, and even if it would have a valid value, for zlib we need to point to the source dir.

@ptc-tgamper
Copy link
Contributor

ptc-tgamper commented Dec 3, 2020

Could we merge this. Until now we were keeping this one-liner around as a local patch, because we don't run CMake in mini zip's source directory. If I am not mistaken, this fix for the "zconf.h not found" error with out-of-source-tree builds stems from the following discussion: madler/zlib#132

@nmoinvaz
Copy link
Member

nmoinvaz commented Dec 3, 2020

According to this line in the CMakeLists.txt for madler/zlib, it is configured to the binary directory.

https://github.com/madler/zlib/blob/master/CMakeLists.txt#L84-L85

What are the cmake commands you are using that reproduce the error?

@eiszapfen2000
Copy link
Contributor Author

eiszapfen2000 commented Dec 3, 2020

I created a separate build directory and executed the follwing command in git bash:
cmake ../minizip -DMZ_BZIP2=OFF -DMZ_LZMA=OFF -DMZ_ZSTD=OFF

Afterwards I open the generated Visual Studio solution, and when building minzip I get the follwowing compile error:
"zconf.h": No such file or directory

From what I found, the following line in zlib cmake is related to the issue:
https://github.com/madler/zlib/blob/cacf7f1d4e3d44d871b605da3b647f07d718623f/CMakeLists.txt#L69

The following discussion is also related to the issue:
grpc/grpc#11581 (comment)

@nmoinvaz
Copy link
Member

nmoinvaz commented Dec 3, 2020

It looks like FetchContent is using BINARY_DIR of the SOURCE_DIR.

@nmoinvaz nmoinvaz added 2.0 build system Build system and script changes labels Dec 4, 2020
@nmoinvaz nmoinvaz merged commit b81e65c into zlib-ng:dev Dec 4, 2020
@nmoinvaz
Copy link
Member

nmoinvaz commented Dec 6, 2020

This PR broke the Windows builds.

@eiszapfen2000
Copy link
Contributor Author

Could you provide the cmake line to reproduce?

@nmoinvaz
Copy link
Member

nmoinvaz commented Dec 6, 2020

Actually it looks like for whatever reason cmake is finding an installed version of LZMA on Windows machines (probably GitHub changes the images) and it is failing. So not this PR.

@eiszapfen2000
Copy link
Contributor Author

eiszapfen2000 commented Dec 6, 2020

Yep, looks like it is not checking out liblzma:
https://github.com/nmoinvaz/minizip/runs/1507335552?check_suite_focus=true#step:7:51

@nmoinvaz
Copy link
Member

nmoinvaz commented Dec 6, 2020

I have made some additional changes that should fix things. Also I have added a CI build that checks that out-of-source-tree builds work.

@eiszapfen2000
Copy link
Contributor Author

Thank you, much appreciated

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.

None yet

3 participants