-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Improve CMake integration #14
Comments
Some of this is lost on me, so you'll have to walk me through it a little bit.
Do you mean things like
Yes. You can override this by getting a package manager to get the sources yourself, and then overriding the FetchContent source directory so that no download/update step is done, which is the usual CMake Practice now (
I'd like to avoid this if at all possible. I can get rid of CMAKE_CXX_STANDARD (which only affected executables internally, and not the interface libraries) but I have no intention on giving someone a build error if some check for C++17 fails; I would much rather document we use some C++17 features and if someone back-fills or poly-fills what they need, that's fine.
I'll fix them! Albeit I'm not sure what's wrong: the install directory looks okay to me? I think the only thing missing is the singles. Are the docs not supposed to be nested in "sphinx"? What happens if I have other outputs? -
I'm going to be honest with you; if your FS / build system doesn't support globbing or doesn't support letting CMake re-invoke itself to check the generated file list, it's probably not going to have a good time if I start using generator expressions and other things (which I plan to do!). That being said, maybe when I hit 1.0.0 I'll use a raw file list since the file names and everything will have stabilized by then. |
|
Sure, I can work on that or something. The scratch variables and scratch target will stay, if only because I don't feel like adding a whole new independent CMakeLists.txt just for debugging purposes.
FetchContent isn't for "developer" workflow, as far as I understand it. It's supposed to be THE workflow, for everyone. Letting someone specify the source with
Well, okay. I'll figure something out that hopefully won't error people on C++14-with-polyfills.
I still don't get this part but I'll dig into it and figure out how to make that apply here.
Well, okay. It seems to be going to the right place right now for the
I am just going to have to say that it's unfortunate; the globs are going to stay. They're fast, convenient, and work with plenty of build systems. CMake even has a special "glob checking" step is performs for build systems that aren't glob-smart yet, so it can just regenerate the content before-hand! I'm not really worried about I'll get to work on everything else! |
Build tooling for C++ using CMake is a topic I have been heavily looking into for the better half of the last year and cmake-init culmination of that effort. I can also probably count on one hand who cares about build tooling as much as I do besides myself and it's no secret that it shows very much on the ecosystem. |
From the article:
This project is not even in version 1 yet. I am literally in the process of tearing out an enormous amount of the shims and detail code to be placed somewhere else. I am absolutely not going to stop globbing, as I said earlier, until the project's structure becomes more fixed
If and when this changes, I'd be more than happy to fully explore adding As for "dev mode", I still do not understand what you are getting at, at all. The "dev mode" is something I have by having my own CMakeUserPresets.json that turns on all the variables I'm interested in. Otherwise, they are off and the user only gets the library targets and the install targets. I still need to figure out how to export a CMake package declaration or whatever, but that's still a separate thing. I also do not understand half of the output from cmake-init, so I need to spend some time studying that. |
Sure, early in the development globbing might be more convenient, however this is a header-only library and unless you add new .cpp files to the tests often, there is still no advantage to globbing. There is no need to add header files to I'd be curious where you get that information regarding # "Backtrace" when importing a dependency:
# find_package(example REQUIRED) in your CMakeLists.txt
# include("${CMAKE_CURRENT_LIST_DIR}/exampleTargets.cmake") in exampleConfig.cmake
# In the CMake generated exampleTargets.cmake script:
add_library(example::example INTERFACE IMPORTED)
set_target_properties(example::example PROPERTIES
INTERFACE_COMPILE_FEATURES "cxx_std_17"
INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/example"
) You can completely forget about find modules, those are only for dependencies that don't use CMake and don't provide CMake support. You also don't need to copy-paste anything, CMake has for the longest time provided a way to generate CMake packages. You can already substitue dependencies that aren't installed anywhere on your system using the technique outlined here. The dev mode is something that I came up to exclude absolutely everything not necessary to build a library, which also means less things to pollute the build tree with in case the project is vendored (e.g. using FetchContent). The less cache variables there are the better, because anyone looking at the cache variables as a consumer using ccmake or cmake-gui will see less options to mess with. Same goes for targets that are only relevant for developers, like the ones that come with |
At the moment, the CMake build files are messy.
This makes it impossible to build/install/package the library without heavily patching build files, which is completely avoidable. It's not nice to force consumers to pass 10 flags just so they can dodge everything but the library itself.
If you wish this library to be reach a wide audience, this is an absolute no go. For dependencies like Catch2, you can use vcpkg, even for those you grab from GitHub, like the other ztd libraries.
I had to find out from Library Concepts? #13 that this library is targetting C++17. A simple
target_compile_features(ztd_text INTERFACE cxx_std_17)
would be helpful. HardcodingCMAKE_CXX_STANDARD
is also wrong, because it's purpose is to be used from the CLI to control the build.This is mainly for scripting. Again, if you wish this library to be usable by a wide audience, this is an absolute no go. Mainly because it doesn't even work reliably for most build systems. Please read the big note from the CMake developers and heed their words.
Consumer side of lists files should do nothing more than describe the build and usage requirements of a library and the CMake build scripts should be as minimal as possible with no hardcoded details. It is noone's benefit to force people trying to build/package/use your library to patch things.
I recommend taking a look at cmake-init, which is designed to solve all of these issues. Since you have dependencies from GitHub as well, I recommend taking a look at the vcpkg example repository.
A better solution would be however, if instead of introducing potentially exponential complexity to building the library with optional dependencies, just make a separate library that depends on both this one and that optional dependency.
The text was updated successfully, but these errors were encountered: