-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Bringing Cmake back #470
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
Bringing Cmake back #470
Conversation
Anything you want me to improve upon with this PR? I think it doesn't really have any more to add to it, but I'm open to suggestions. |
@sum01, thank you very much for spending time for CMake support. I don't come up with any suggestion since I personally don't know much about CMake. But it looks good and complete to me. I would like to ask you to handle the following two things before merge the PR:
The reason why I am asking these things is to reduce necessary maintenance work as much as possible because I don't personally use it and I don't have time to support users about CMake. (That's why I removed the previous CMake support.) When it comes to README, I would like to put only information about features and examples in README. (I have actually received similar pull requests such as #416, #408, #195. But I didn't accepted any of them with the same reason.) I appreciate your understanding and thank you very much for your great job! |
I'm a little bummed that you don't want the version string, as it allows specific version requirements in a As for the README, would it make sense to put that info somewhere else? EDIT: Never mind, I'll just put the info into the CMakeLists.txt |
This reverts commit 8674555.
Previous behaviour is just a warning.
Adds automatic dependency finding (if they were used). Adds a way to require a specific version in the find_package(httplib) call. You should link against the httplib::httplib IMPORTED target, which is created automatically. Add options to allow for strictly requiring OpenSSL/ZLIB HTTPLIB_REQUIRE_OPENSSL & HTTPLIB_REQUIRE_ZLIB require the libs be found, or the build fails. HTTPLIB_USE_OPENSSL_IF_AVAILABLE & HTTPLIB_USE_ZLIB_IF_AVAILABLE silently search for the libs. If they aren't found, the build still continues, but shuts off support for those features.
Has info on all the available options and what targets are produced. Also put some things about installation on certain platforms.
Ok I removed the version file & README changes. The only thing that might ever need changing is the OpenSSL minimum version string, which I doubt you'll be doing often anyways. |
@sum01, thank you for the adjustments even if you don't fully agree with. The OpenSSL minimum version string (1.1.1) is good with me. Also, it's actually a good idea to include the installation guide in the CMakeLists.txt file itself, so that users don't need to look around in README to find the info. Hope it helps many users who depend on cmake. Thank you so much for the fine contribution! |
* Revert "Removed CMakeLists.txt. (Fix yhirose#421)" This reverts commit e59ffaa. * Fail if cmake version too old Previous behaviour is just a warning. * Improve CMakeLists Adds automatic dependency finding (if they were used). Adds a way to require a specific version in the find_package(httplib) call. You should link against the httplib::httplib IMPORTED target, which is created automatically. Add options to allow for strictly requiring OpenSSL/ZLIB HTTPLIB_REQUIRE_OPENSSL & HTTPLIB_REQUIRE_ZLIB require the libs be found, or the build fails. HTTPLIB_USE_OPENSSL_IF_AVAILABLE & HTTPLIB_USE_ZLIB_IF_AVAILABLE silently search for the libs. If they aren't found, the build still continues, but shuts off support for those features. * Add documentation to CMakeLists.txt Has info on all the available options and what targets are produced. Also put some things about installation on certain platforms.
I find having Cmake support very useful, so I reverted the deletion, and fixed the issue someone had with it in #421
Note: This PR namespaces the Cmake target, so it's now
httplib::httplib
I also took the time to improve it in a couple ways.
find_package(httplib)
is called so they don't have to remember to do that.find_pacakge
call.[2]httplib::httplib
so people building in-tree can link against the same target as users who installed.[1]. Specifically the following:
HTTPLIB_REQUIRE_OPENSSL
(default off)HTTPLIB_REQUIRE_ZLIB
(default off)HTTPLIB_USE_OPENSSL_IF_AVAILABLE
(default on)HTTPLIB_USE_ZLIB_IF_AVAILABLE
(default on)[2]. I was unsure if you follow SemVer or not, so I made the versioning allow anything >= request version.
[3]. See GNUInstallDirs, specifically
CMAKE_INSTALL_INCLUDEDIR