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

Missing zconf.h when using zlib as a git submodule (with CMake) #133

Open
JPGygax68 opened this issue Mar 7, 2016 · 24 comments
Open

Missing zconf.h when using zlib as a git submodule (with CMake) #133

JPGygax68 opened this issue Mar 7, 2016 · 24 comments

Comments

@JPGygax68
Copy link

I used add_subdirectory() to integrate zlib into my CMake-controlled project. This works well, but there is the problem that git detects a "false update" and constantly asks me to commit a change that I didn't make.

Apparently, something in the build process renames zconf.h to zconf.h.included, so git detects the former as deleted and the latter as new.

Is there a way to work around that, or better yet, fix it ?

@mtl1979
Copy link

mtl1979 commented Mar 28, 2016

I think there was a commit in somewhere here that removed the rename from CMakeLists.txt, it did involve adding some includes in the source files... This however doesn't fix the issue that zconf.h gets modified during build process.

@minexew
Copy link

minexew commented Nov 13, 2016

I'm running into the same problem, except in my case, the missing zconf.h outright causes a fatal error whenever I try to #include "zlib.h" in my project.

How did you manage to solve this?

@mtl1979
Copy link

mtl1979 commented Nov 13, 2016

@minexew add_subdirectory() should automatically add directory containing zlib.h to include path... If it doesn't, you need to make sure cmake is upgraded to latest available and that zlib.h actually exists in directory specified in second parameter of add_subdirectory().

@minexew
Copy link

minexew commented Nov 13, 2016

Oh no, zlib.h is just fine. However, one of the first things it does inside is #include "zconf.h". This causes an error, because zconf.h had been renamed to zconf.h.included by CMake.

@mtl1979
Copy link

mtl1979 commented Nov 13, 2016

@minexew All cmake-generated files should be inside the binary directory, so it should find them if the second parameter is correctly defined.

@minexew
Copy link

minexew commented Nov 13, 2016

I'm starting to understand. Do you have an example of the intended usage when zlib is a submodule?

I'm currently (trying to) do this:

add_subdirectory(dependencies/zlib)
add_dependencies(${PROJECT_NAME} zlibstatic)
target_link_libraries(${PROJECT_NAME} zlibstatic)

@mtl1979
Copy link

mtl1979 commented Nov 13, 2016

@minexew Second parameter of add_subdirectory should be something like:

"${CMAKE_CURRENT_BINARY_DIR}/zlib_build"

@minexew
Copy link

minexew commented Nov 13, 2016

Ok.

Just wondering, why is it neccessary to add the include paths manually?
E.g. why does zlib's CMake declare
include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_SOURCE_DIR})
instead of
target_include_directories(zlib PUBLIC ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_SOURCE_DIR})?

The latter would automatically propagate include paths whenever somebody does target_link_libraries(... zlib)
Not only would that be more convenient, it would also better encapsulate the library and reduce the amount of "interfaces" (in this case directory paths) that the library user needs to worry about.

@mtl1979
Copy link

mtl1979 commented Nov 13, 2016

@minexew target_include_directories() didn't exist in older cmake versions.

@robiwano
Copy link

Maybe zconf.h should be added to the .gitignore file so that changes are hidden ?

@robiwano
Copy link

Regarding target_include_directories(), since it was available from CMake 3.0 (over 2 years ago) I would think it is safe to require at least CMake 3.0 for zlib now.

@mtl1979
Copy link

mtl1979 commented Nov 28, 2016

@robiwano I think it was added in 2.8.11 (see https://cmake.org/cmake/help/v2.8.11/cmake.html#section_Commands), but as zlib is all for backwards support for even oldest operating systems, I doubt @madler will want to require minimum CMake of 2.8.11 or 3.0. Removing zconf.h from git repository and adapting Makefiles for Windows and other non-bash operating systems to copy it from zconf.h.in would solve false update notifications. That's what I did for zlib-ng.

@minexew
Copy link

minexew commented Nov 28, 2016

Yeah, in Debian world for example, "2 years ago" is basically "yesterday".
Thanks for mentioning zlib-ng, I didn't know about it and it seems really nice.

@mtl1979
Copy link

mtl1979 commented Nov 28, 2016

@minexew Development on zlib-ng has been quite slow for last 13 months because @Dead2 has been busy with other projects, but it does support more (read: better) "recent" architectures than stock zlib. I've been mostly working on the MinGW/MSYS2 support and optimized ARM and Visual C++ code based on what was posted here.

@kymikoloco
Copy link

If zlib is not going to use even 2.8.12, then note that you should probably be setting the INTERFACE_INCLUDE_DIRECTORIES property of the zlib target(s). It has no effect unless someone is using CMake 2.8.11 or above, but will allow users of add_subdirectory() and the target name in target_link_libraries to get the interface that zlib requires without having to make manual include directory changes to their own projects.

set_target_properties(zlib PROPERTIES 
    DEFINE_SYMBOL ZLIB_DLL
    SOVERSION 1
+    INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR};${CMAKE_CURRENT_BINARY_DIR}"
)

inolen added a commit to rtissera/libchdr that referenced this issue Aug 9, 2017
…ore committing renamed this

disabled zconf.h renaming functionality which is in a problematic state madler/zlib#133
@azisso
Copy link

azisso commented Nov 9, 2017

Hey guys, do you have any status update for this issue?
I currently can't build my project that has zlib as a submodule (of grpc as a submodule), see my comment here:
grpc/grpc#11581

@mtl1979
Copy link

mtl1979 commented Nov 10, 2017

@l33ds It's not a bug in zlib, it's an error in CMakeFiles.txt on the project using zlib as subproject. I already explained it in previous comments. It doesn't happen with just zlib, but any project that generates include files.

@robiwano
Copy link

@mtl1979 Problem is that the generated include files are within the source tree. I know many projects do this, but it's not a good way to do it nevertheless :) Wouldn't it be possible to generate it out of the source tree, i.e. in ${CMAKE_CURRENT_BINARY_DIR} somewhere ?

@mtl1979
Copy link

mtl1979 commented Nov 10, 2017

@robiwano I already explained how to do it exactly that way...

@robiwano
Copy link

Ah, sorry :)

@strattist
Copy link

Hi guys,

I'm sorry to reactivate this thread but I just ran on something close to this issue I think. I use zlib as a git submodule and it always give me modifications from the source repo which is not of my doing but zlib directly since there is a renaming step in its CMakeLists.txt:

if(NOT CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_CURRENT_BINARY_DIR)
    # If we're doing an out of source build and the user has a zconf.h
    # in their source tree...
    if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h)
        message(STATUS "Renaming")
        message(STATUS "    ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h")
        message(STATUS "to 'zconf.h.included' because this file is included with zlib")
        message(STATUS "but CMake generates it automatically in the build directory.")
        file(RENAME ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h.included)
    endif()
endif()

I believe in a lot of cases CMAKE_CURRENT_SOURCE_DIR will never be equal to CMAKE_CURRENT_BINARY_DIR.
I also believe if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h) will always evaluate to true since this file is in the repo since the first version in this github page.

How to get rid of these forced changes to the submodule ? @mtl1979 you said you gave a solution, were you talking about the one with the add_subdirectory binary directory argument ?

@mtl1979
Copy link

mtl1979 commented Jun 12, 2018

@strattist zconf.h is only needed there because Makefile for Visual C++ (and other compilers not using configure or cmake) needs to have unmodified copy of the file for a successful build... In my solution I made it copy zconf.h.in to zconf.h only when needed, so zconf.h is no longer needed in source tree of the repository.

@mtl1979
Copy link

mtl1979 commented Jun 12, 2018

@strattist I think this is the wrong place to talk about repository of zlib-ng... Some people would think we are spamming.

jinfeihan57 pushed a commit to minchai23/zlib that referenced this issue Sep 15, 2020
* set(CMAKE_VERBOSE_MAKEFILE ON)

* -D CMAKE_AR=/usr/bin/ar
idzm added a commit to idzm/ptusa_main-1 that referenced this issue Oct 21, 2022
Use fork of zlib to solve issue with CMake processing (madler/zlib#133).
AlexandrBehunkov pushed a commit to savushkin-r-d/ptusa_main that referenced this issue Oct 26, 2022
Use fork of zlib to solve issue with CMake processing (madler/zlib#133).
@RandallFlagg
Copy link

I had this same problem. It was solved by adding the following line before linking:
target_include_directories(myapp PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/${ZLIB_LIBRARY}")

I am not sure if I solved it correctly but it would be great if this can be simplified in the future.

My project tree is as follows:

myapp
-|_ zlib

I think that #781 is also related to this issue and I also commented there with the same comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants