-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Feature/improve cmake #347
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
Conversation
|
I had to make this minor change to build on Android: spauldingdevice@130e7b0 Before this change I would get the following compiler error: |
|
Are there any plans to merge this PR? I would certainly find it helpful so that I can fetch zlib via FetchContent. |
|
There are a lot of: CMake PRs: CMake Issues: |
|
True. I thought I'd prod just in case it might lead to these changes pushed through. It's not a problem anymore though, I've made my own fork with the changes I require. Thanks! |
977c9c1 to
a85ce74
Compare
|
These commits are out of date. |
It's ineffective to actually disable building the shared lib as of zlib v1.3. Enable it anyway in the hope that one day it might be fixed, e.g. by merging this PR opened in 2018: madler/zlib#347 This would allow an even less obtrusive workaround for the RC compilation bug, by allowing to disable building the DLL and thus not hitting that bug in the first place. All this via CMake and without the manual, GNU Make-level hackery and manual install phase.
|
@madler: @ClausKlein has done an update since 2018: |
|
@Neustradamus This is #347. |
|
@madler: Yes it has been recently updated |
|
@Neustradamus My comment was hours ago, and so was referring to the most recent updates. |
|
@madler: Sorry! @ClausKlein: Can you look? |
2cb00f8 to
b67c694
Compare
Sorry, I have rebased with current develop now. |
- remove unneeded asm obtions There are not asm files in source tree! - add export statement for cmake config - install zlib namespace zlib::zlib to support FetchContent cmake module - build tests and examples under clear condions
8eab66f to
b9a11c5
Compare
|
@ClausKlein: Thanks :) @madler: It is good? |
|
@madler: Can you look this updated PR? |
|
Can this be merged? |
craigscott-crascit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a few comments to clean up, maybe that will encourage getting this PR over the line.
README
Outdated
| well, "make install" should work for most flavors of Unix. For Windows, use | ||
| one of the special makefiles in win32/ or contrib/vstudio/ . For VMS, use | ||
| make_vms.com. | ||
| To compile all files and run the test program, follow this instructions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your example commands also install zlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but not to the destination that I have expected:
bash-5.2$ cmake --install build --prefix /tmp/zlib
-- Install configuration: "Release"
-- Installing: /usr/local/lib/cmake/zlib/zlibConfigVersion.cmake
-- Up-to-date: /usr/local/lib/libz.a
-- Installing: /usr/local/lib/cmake/zlib/zlibConfig.cmake
-- Installing: /usr/local/lib/cmake/zlib/zlibConfig-release.cmake
-- Up-to-date: /usr/local/include/zconf.h
-- Up-to-date: /usr/local/include/zlib.h
-- Up-to-date: /usr/local/share/man/man3/zlib.3
-- Up-to-date: /usr/local/share/pkgconfig/zlib.pc
bash-5.2$ gvim /usr/local/lib/cmake/zlib/*.cmakeCo-authored-by: Craig Scott <craig.scott@crascit.com>
Use gcc on OSX ci Use GNUInstallDirs to be more flexible
cf12420 to
c20611e
Compare
|
@craigscott-crascit replaced with #1004 |
| install(EXPORT ${PROJECT_NAME}Config | ||
| NAMESPACE zlib:: DESTINATION "${INSTALL_LIB_DIR}/cmake/${PROJECT_NAME}" | ||
| install(EXPORT zlibConfig | ||
| NAMESPACE zlib:: DESTINATION "${ZLIB_INSTALL_CMAKEDIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@craigscott-crascit Note: this namespace is exported here, not ZLIB::ZLIB?
add option to build either shared or static lib
fix build problems on msys2 and conan
see bincrafters/community#205
and https://github.com/conan-community/conan-zlib/issues/32