-
Notifications
You must be signed in to change notification settings - Fork 76
feat: Add CMake installation support with CPM-based dependency management #585
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
base: main
Are you sure you want to change the base?
Conversation
How is fetch_content or CPM not "normal" CMake? And CPM supports caching.
Explain - a key reason for using CPM is to avoid duplicate copies of the library since CPM allows for lock files and full version control.
If I were convinced to accept this, the changes to cpp-library would be a prerequisite (I've been actively removing support for install from cpp-library to simplify it). Along with CI support and refactoring this PR so the cmake for install is in a separate file. My current read is that this is too much, and there is not yet any CI. I'm a firm believer in "build everything from source with the same compiler and same settings at the same time." Previous versions of the library supported (at various times) CMake install, vcpkg, and conan. All were a PITA to keep running in CI across all platforms. |
|
Thanks for the detailed feedback. Here are my thoughts on your points.
You're right, they are normal. My point is about the interface vs. the implementation. The "normal" CMake mechanism for a consumer to declare a dependency is find_package(). What that command does is the implementation detail. It could find exported targets from an install(), or it could (as in my first patch) be a FindModule.cmake that falls back to using CPM. The goal is that the caller doesn't have to care how a dependency is retrieved. The second patch achieves the same result by delegating the find_package call to CPM. As long as the consumer is aware of CPM_USE_LOCAL_PACKAGES, the behavior is identical: the consumer calls find_package.
CPM supports source caching. An install() target enables binary caching. To be clear, binary caching is still built from source, compiling everything with the same compiler and flags, just as you want. With vcpkg, this binary cache simply becomes shareable between machines. This is what solves the "enormous configuration time" problem you get from parsing hundreds of CMakeLists.txt files at configure time.
Gladly. You're right, that works perfectly, but only in a fully hermetic build where your project and all its dependencies use CPM in the same configuration run. As you know, that isn't the reality for most large-scale projects. The standard CMake abstraction is find_package precisely to avoid the problems of a monolithic configuration. Once a project relies on find_package for even one dependency, the hermetic model breaks and creates conflicts. A concrete example:
This ODR violation is a symptom of the two larger reasons why the single-stage, add_subdirectory model doesn't scale:
For theses reasons we rely on installed dependencies. And as it was described above, CPM does not handle that.
I agree with this 100%. We do the same, we don't rebuild Qt for example, but most dependencies are built from source in the same environment. Installable does not contradict this. It just means we build in stages rather than one single, monolithic configuration.
Yes, I saw that, which is why I tried to move quickly on this.
This seems to be the real issue. As long as a project install()s correctly, writing a vcpkg port is trivial. And we will be maintaining a vcpkg port for stlab, that won't cost us much, as long as it install correctly. The goal is a system with minimal maintenance. Splitting the library will add complexity, but that install logic can likely be centralized in cpp_library. To help address your actual pain points: what were the specific CI issues you encountered, and on what platforms? |
The issue was that the dependencies had to be updated in multiple places. Adding a new dependency or even changing a version caused a ripple. Then vcpkg itself was another dependency, and we went through a couple rounds of vcpkg changing how it was installed. My goal is for dependencies to be listed only in one place, with their version number, and to keep as much as possible out of the CI install phase (I think I'm down to just installing doxgen for the documentation build). One possible approach would be:
|
|
Let's first have an installation approach you are happy with before thinking about having official vcpkg support, integrating it too soon may be confusing, especially if you want to have multiple libraries.
This is the maintenance cost I wrote about earlier, the current approach require you to update the find_dependency every time you change either the find_package or the CPMAddPackage. I think the best solution would be (and I hate to say that) make a wrapper for adding dependencies, that will be defined in the cpp_library, stlab_add_dependency or something similar.
You mean changing version in one place and not the other ? This would be solved by the previous solution, though maybe you are describing something else ?
It is possible, though they could use just the wrapper and the install logic for now from cpp_library, this would minimise duplicated code and boilerplate.
That would be up to you, though I don't see reason why it would fail. We could use components to define what get installed.
In my head, to test the install, you just call install on stlab (with proper environment so that it find qt, libdispatch, Threads etc). And then find_package it in your test project. No need to duplicate dependencies, the one found via find_package won't be installed as they are imported target, the one added with add_subdirectory via cpm will have their own install rules.
Community driven still sounds better than a fork to me. I will discuss this with our tech leads to make sure I can allocate time to implement it. But from my POV, we will have to do it one way or another, and I prefer a way that is as upstreamable as possible. |
Short Version
Adds
install()+ generated config files so stlab can be consumed withfind_package()instead of onlyadd_subdirectory(). Keeps CPM with the expectation that consumers will use eitherCPM_USE_LOCAL_PACKAGESorCPM_LOCAL_PACKAGES_ONLYwhen they provide the dependencies themselves.What
stlab-config.cmake+ version fileSTLAB_INSTALL, defaults to OFF (to be decided)CPM_SOURCE_CACHEand put it insidePROJECT_IS_TOP_LEVEL, otherwise it corrupts consumer CPM cache when they use stlab via CPMWhy
Not Removing CPM
CPM stays.
CPM_USE_LOCAL_PACKAGESwas tested and working as expected.Scope
No vcpkg-specific code; just standard CMake packaging mechanics.
Optional Next Step
I can add the copy-on-write and enum-ops logic to the cpp_library to automatically apply it to any of its users.
Maintenance Cost
The
find_dependencymust be correctly added in the template if new dependencies are added.Feedback Welcome
Happy to tweak naming or change install layout if you prefer.