[bug] Workspace package dependencies do not get installed when called from cmake-conan

[alex1234567890123456789]

Describe the bug

Not sure whether this is technically a bug or feature request but I'm calling it a bug because a user would expect this to work and it does not.

We have a complex setup involving a main project, a few in-house libraries we would like to co-develop with the main project, and a bunch of 3rd party dependencies. The in-house libraries also have their own 3rd party dependencies. The build is triggered by running cmake, which uses cmake-conan to install the dependencies. Multiple profiles are passed to conan, and we have defined our own deployer. We add the subprojects to the main project using FetchContent, as described in the docs.

The FetchContent portion works, but the subproject's dependencies are not installed. I see that there is a workspace super-install command, but our workflow is all cmake-based, and cmake-conan won't call workspace super-install. In addition, super-install doesn't support deployers, so even changing cmake-conan to call super-install won't work.

Really, the question is why there is a separate workspace super-install command? Why can't conan install just add workspace packages to the graph (and disable downloading/building for those packages)? As a user, it doesn't make sense to have multiple install commands that do the same thing (they might be technically different but the intent is the same - install everything I need) but you have to pick the right one depending on whether a project has a workspace or not.

If for some reason the workspace super-install needs to remain a separate command, then it should be updated to support the same options as conan install (add -d, --format, the positional arg for the directory, etc) and cmake-conan should be given the option to call one or the other.

How to reproduce it

Main project: requires zlib
Subproject in workspace: requires geographiclib
Add cmake-conan integration
Add FetchContent to add subproject
cmake configure -> only zlib is installed, and subproject fails to configure because geographiclib can't be found.

[memsharded]

Hi @alex1234567890123456789

Thanks for the feedback.

This doesn't seem a bug at the moment, it is not expected that cmake-conan would support workspace super-install. For workspaces super-builds the expected flow will be to call conan first.

This happens because workspaces will have the capability of adding and removing sections to the root CMakeLists.txt to add and remove new packages to the workspaces. This only works if conan is called first and cmake is call after that.

In any case, this would seem a possible feature request to cmake-conan, because it seems that potential attempts to provide some support for it should be done in the cmake-conan CMake dependency provider, not really in the Conan client. I am moving this ticket to cmake-conan repo.

[alex1234567890123456789]

Hi @memsharded,

Thanks for the quick reply. A few comments...

it is not expected that cmake-conan would support workspace super-install.

I strongly strongly disagree. As a user, I can 100% say that I expected cmake-conan to work with workspaces, and wasted several days trying to figure out what I was doing wrong. I can't find anything in the documentation saying it doesn't support workspaces. From everything I've read, cmake-conan is the preferred way to work with Conan, why would it all of a sudden not be if you're using a specific feature? We based our whole workflow on cmake-conan, changing it now would be a huge downgrade. It's an extra step that developers could forget, and every time they pulled code from git, or switched branches etc they would never know whether they needed to re-run conan or not. With cmake-conan, it's safe - if something changes, they will automatically get the new requirements. That was the make-or-break feature that pushed us to adopt Conan in the first place!

This happens because workspaces will have the capability of adding and removing sections to the root CMakeLists.txt to add and remove new packages to the workspaces. This only works if conan is called first and cmake is call after that.

Is this a thing that currently happens? I don't see any mention of it anywhere. It sounds terrible - my CMakeLists.txt is mine. It goes in version control. I don't want other tools modifying it and potentially messing it up, just like I don't want other tools modifying my cpp files. I'm also not sure why it's even necessary - is this an attempt to eliminate the FetchContent lines? Keep in mind users might want to do their own thing to integrate the projects, maybe they would prefer to use add_subdirectory, or ExternalProject. In our case, we just want the transitive dependencies installed!

In any case, this would seem a possible feature request to cmake-conan, because it seems that potential attempts to provide some support for it should be done in the cmake-conan CMake dependency provider, not really in the Conan client. I am moving this ticket to cmake-conan repo.

Ok, but cmake-conan can't do it alone, without some Conan changes as I mentioned above - Conan would need to add support for deployers and json formatting etc to the workspace super-install command to bring it up to parity with the install command. There should probably be an issue in each repo.

[memsharded]

From everything I've read, cmake-conan is the preferred way to work with Conan

Where have you read that?

cmake-conan is practically not mentioned in the official Conan docs in docs.conan.io. It is just mentioned as a side thing when talking about CLion and VS IDE integrations.

In all the docs, tutorials, examples, even the new Academy video trainings, 100% of the time, the flow conan install + cmake ... is used and recommended, not a single time the cmake-conan is used in any reading material.

If you read old Conan tickets, you will see that we often recommend not using cmake-conan and using it only when there is no other possibility, and we always recommend the conan install + cmake ... explicit flow.

We based our whole workflow on cmake-conan, changing it now would be a huge downgrade.

We are pushing no one or forcing anyone to drop or abandon cmake-conan. But we are introducing a new incubating feature, the Workspaces, that can be pretty challenging, and one of its features is the dynamic definition of CMakeLists.txt, because for example the FetchContent doesn't respect the topological order of the graph. This is not a Conan issue, but a CMake limitation.

That means that for large workspaces with many packages it will be extremely necessary to run conan first to be able to dynamically compute the correct topological order, otherwise it can be a nightmare to have to define it manually. And for those flows, it is completely necessary to run conan workspace super-install first. It is not possible to launch it from cmake, because it will modify the CMakeLists.txt itself. You can see ongoing work here: conan-io/conan#18693

I am not saying that we will not consider this feedback.
But this is definitely not a bug, and as a feature, it will not be a high priority for the workspace feature in the short term, because there are other higher priorities, to make workspaces work with the recommended default conan ... + cmake flow.

[alex1234567890123456789]

Thanks for the added details, that is helpful to know the background. I understand the desire to have conan manage the build order, but please try to see how Conan modifying your CMakeLists.txt might not be behavior that all users want (too intrusive). What I'm looking for is much much simpler - just a way to install the requirements from more than one conanfile at once. I think anyone who works with git submodules would find that useful.

I just finished adding it locally in Conan and cmake-conan, and it works well. It was just a couple of lines. I'd love to submit a PR if you are willing to look at it but I'll need to check first whether I can.

[memsharded]

Thanks for the added details, that is helpful to know the background. I understand the desire to have conan manage the build order, but please try to see how Conan modifying your CMakeLists.txt might not be behavior that all users want (too intrusive).

The PR in conan-io/conan#18693 is considering to not modify the CMakeLists.txt, but generate it in an auxiliary .cmake file that the main CMakeLists.txt can include(build_order_conan.cmake) and get the necessary order for the FetchContent declaration.
It will avoid the direct edition of the user CMakeLists.txt, but for the evaluation point of view, it is conceptually the same, Conan needs to run first to define the order, then CMake can execute, but the execution order must be that.

We understand this might not be necessary for all users, but we do estimate that it will be almost necessary for a large fraction of the workspace feature users, mostly all users that have relatively large workspaces (like larger than 10 packages), and it will be specially critical if the workspace evolves regularly, by adding and removing packages to the workspace.

So we need to figure out the "canonical" way, at least one path that can be succesfully used by all Conan users. Maybe it is not the optimal one for all cases, but it should be quite a good one for a majority of cases, and it should be conceptually complete for all of them. This flow is so far the conan xxx + cmake ..., and not the cmake-conan one, and this is why the cmake-conan one is not cited or used in the docs or any learning material, because the learning material focuses on explaining concepts, and it does it using the "canonical" way.

Said that, it is what I commented above, it is not that we will not consider to improve cmake-conan and try to support and implement more use cases. This is the main reason why cmake-conan is there, to try to satisfy those users with some constraints. It is not the "canonical" way of running Conan, but we created and maintain it, try to document some of its limitations, and many users find it useful. It is just a matter of priorities, when the resources are limited, efforts like the Workspace feature focus on the "canonical" way, and other auxiliary flows are typically less priority.

I just finished adding it locally in Conan and cmake-conan, and it works well. It was just a couple of lines. I'd love to submit a PR if you are willing to look at it but I'll need to check first whether I can

Of course, even if it might not be possible to merge it in the short term, it would be very valuable feedback, knowing what it took from your side to support it can help us a lot to consider the different angles moving forward. So even if you cannot "legally" do a PullRequest, feel free to paste here a diff of your changes if that is easier for you.

Many thanks for your feedback!

[alex1234567890123456789]

Hi @memsharded,

The PR in conan-io/conan#18693 is considering to not modify the CMakeLists.txt, but generate it in an auxiliary .cmake file that the main CMakeLists.txt can include(build_order_conan.cmake) and get the necessary order for the FetchContent declaration. It will avoid the direct edition of the user CMakeLists.txt, but for the evaluation point of view, it is conceptually the same, Conan needs to run first to define the order, then CMake can execute, but the execution order must be that.

I was actually about to suggest that. That sounds much better (and way easier!) than modifying the CMakeLists. I think doing that actually allows the cmake-conan workflow to work as well - it could run conan multiple times if needed, generate and even automatically include a cmake script that adds the workspace projects.

I was able to get permission to share my changes. Most of them are changes to Conan so I'm going to open an issue and related PR in the Conan repo. I think they'd be useful regardless of whether cmake-conan is used or not. I'll link it here shortly.

The only change to cmake-conan was to add a cache variable to control whether to call conan install or conan workspace super-install. I added the word experimental in the variable and also output a warning just to be clear that it's not a stable feature. I could add a PR here but I'm not sure how to indicate that it depends on the Conan PR, so instead I'll just paste the diff:

diff --git a/conan_provider.cmake b/conan_provider.cmake
index 5ed6f84..b1916b8 100644
--- a/conan_provider.cmake
+++ b/conan_provider.cmake
@@ -475,7 +475,14 @@ function(conan_install)
         set(ENV{PATH} "$ENV{PATH}:${PATH_TO_CMAKE_BIN}")
     endif()
 
-    execute_process(COMMAND ${CONAN_COMMAND} install ${CMAKE_SOURCE_DIR} ${conan_args} ${ARGN} --format=json
+    if(CONAN_USE_EXPERIMENTAL_WORKSPACE_SUPERINSTALL)
+        set(CONAN_SUBCOMMAND "workspace;super-install")
+        message(WARNING "CMake-Conan: using experimental workspace super-install command")
+    else()
+        set(CONAN_SUBCOMMAND "install;${CMAKE_SOURCE_DIR}")
+    endif()
+
+    execute_process(COMMAND ${CONAN_COMMAND} ${CONAN_SUBCOMMAND} ${conan_args} ${ARGN} --format=json
                     RESULT_VARIABLE return_code
                     OUTPUT_VARIABLE conan_stdout
                     ERROR_VARIABLE conan_stderr
@@ -673,6 +680,7 @@ cmake_language(DEFER DIRECTORY "${CMAKE_SOURCE_DIR}" CALL conan_provide_dependen
 set(CONAN_HOST_PROFILE "default;auto-cmake" CACHE STRING "Conan host profile")
 set(CONAN_BUILD_PROFILE "default" CACHE STRING "Conan build profile")
 set(CONAN_INSTALL_ARGS "--build=missing" CACHE STRING "Command line arguments for conan install")
+set(CONAN_USE_EXPERIMENTAL_WORKSPACE_SUPERINSTALL False CACHE BOOL "Use experimental 'conan workspace super-install' command instead of 'conan install'")
 
 find_program(_cmake_program NAMES cmake NO_PACKAGE_ROOT_PATH NO_CMAKE_PATH NO_CMAKE_ENVIRONMENT_PATH NO_CMAKE_SYSTEM_PATH NO_CMAKE_FIND_ROOT_PATH)
 if(NOT _cmake_program)

[alex1234567890123456789]

See conan-io/conan#18792