[develop2] General feedback

[ssrobins]

Thanks for supporting this very important workflow in Conan 2.0!

I used the original CMake wrapper in personal projects and I still use it at work. I only stopped using it so I could use Conan 2.0 beta, then I converted to something like this.

...and now I'm trying develop2 out!

Here are the main reasons why I use the CMake wrapper:

• I want exactly one interface to the build. For Conan recipes, that's Conan, but for apps built in CMake that use Conan packages, I prefer using CMake directly
• I always want the Conan package settings to match the CMake build settings as much as possible and not rely on external settings such as profiles, which may be incomplete or not match the CMake build
• I want every build configuration set in multi-config CMake generators to always have a Conan package automatically
• I want to keep it possible to use the CMake GUI and Visual Studio's 'Open Folder' functionalities which, last I checked, use CMake as its sole interface

Onto what I like about develop2:

• It's flexible, I can use it at the command line, I can include it into my CMake directly, or I can even call the individual functions to inject alternate settings such as alternate --build flag or add --update into conan install.
• The individual detect functions are an excellent abstraction. I even started adding additional settings to support iOS and Android and it was going together very smoothly! (happy to share)
• Generating the profiles is a great idea. When Conan 2.0 forced me to use profiles, I added the bare minimum to the files and then added the rest as command line parameters to conan install. Generating the whole file and then referencing it in conan install is way cleaner with fewer files.

Here's what I'd love for you to please consider changing (and with your blessing, I'm happy to contribute all of these changes):

• Add a check for the minimum supported version of Conan in the CMake wrapper and make it a one-line change to update it in the future. This would help new users get started faster. Right now, the errors you get with an unsupported Conan don't make it immediately apparent the Conan version is to blame. Check required Conan version #517
• Combine conan_provider.cmake and conan_support.cmake into one file so there's only one file to copy or download into my codebase. Feature/reorg #519
• Allow develop2 to be downloaded at CMake time just like develop does (and include the code snippet in documentation). I think it can still be done without affecting CMAKE_PROJECT_TOP_LEVEL_INCLUDES. Since there were objections to this, I'll just make the change in my repos.
• Add Android Add Android support #521 and iOS Add iOS, tvOS, and watchOS support #528 support
• Add support for hardcoding settings. I'm having trouble deducing certain settings from CMake such as os.sdk on iOS and compiler.runtime on Windows. If there's a function to just add a setting, the user could set unsupported values and/or override default behavior. Figured out a way, it's done in Add iOS, tvOS, and watchOS support #528 and 1ae6cdd
• Add support for hardcoding profile configurations. Some things can't be deduced from the build such as tools.system.package_manager:mode or tools.android:ndk_path (actually, found a way for NDK path!). If there's a function to append/override profile configuration settings, I'm all set!
• Add conan_add_remote() function from develop. I want to be able to add a remote automatically in cases where an alternate artifactory server houses the packages. It also makes it easy to directly specify the remote name in the conan install command. Abandoning this. For hobby projects, I converted to use conancenter instead of my own artifactory server so the default remote setting is all I need. For work stuff, I'm ok with adding some CMake code to do this because Conan is required so we have no motivation to try to separate the Conan and CMake code.

Anyway, thanks for reading. Happy to implement any of these things, though I totally understand if my needs are in the minority and that the tool needs to go in a different direction.

[memsharded]

Hi @ssrobins

Thanks very much for trying out and the feedback!
Happy to know that this seems to look something reasonable, and happy to know that you want to contribute with improvements. Lets start step by step, too many different things to discuss there:

Add a check for the minimum supported version of Conan in the CMake wrapper

Sounds good, I think it can be added

Combine conan_provider.cmake and conan_support.cmake into one file so there's only one file to copy or download into my codebase

Agree, we separated the files while trying out different things, in can be consolidated in a single file. Wdyt @jcar87 ?

Allow develop2 to be downloaded at CMake time just like develop does (and include the code snippet in documentation). I think it can still be done without affecting CMAKE_PROJECT_TOP_LEVEL_INCLUDES.

I am not sure this would work, develop does this after project(), which in this case is not possible. Furthermore, one of the goals for this integration would be to keep the CMakeLists.txt completely free of Conan things! So we see the process:

• Instead of modifying the CMakeLists.txt, the conan_provider.cmake is copied into the project (we can provide some helper for this)
• User can customize that conan_provider.cmake without changing the CMakeLists.txt

Add Android and iOS support

Detection of Android and iOS would be good, feel free to contribute it!

Add support for hardcoding settings. I'm having trouble deducing certain settings

We know that allowing to customize settings would be necessary, but we are not so sure about the interface. We think that probably not modifying the CMakeLists.txt but the provider.cmake file, but we need to discuss a bit about this first, please wait a bit.

[ssrobins]

Allow develop2 to be downloaded at CMake time just like develop does (and include the code snippet in documentation). I think it can still be done without affecting CMAKE_PROJECT_TOP_LEVEL_INCLUDES.

I am not sure this would work, develop does this after project(), which in this case is not possible. Furthermore, one of the goals for this integration would be to keep the CMakeLists.txt completely free of Conan things! So we see the process:

• Instead of modifying the CMakeLists.txt, the conan_provider.cmake is copied into the project (we can provide some helper for this)
• User can customize that conan_provider.cmake without changing the CMakeLists.txt

I was successfully able to do this in a local test on macOS, I just have to put the download commands and set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES ${CMAKE_BINARY_DIR}/conan_provider.cmake) before project(). Given that, is it something you'd be ok with being documented?

I've always been ok with putting Conan in my CMake, but I understand there are situations where the code author wants to let the user decide how they will manage the project dependencies. I wouldn't want to deal with an extra -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=conan_provider.cmake in my cmake command, though I suppose it could be set in CMake presets.

Add support for hardcoding settings. I'm having trouble deducing certain settings

We know that allowing to customize settings would be necessary, but we are not so sure about the interface. We think that probably not modifying the CMakeLists.txt but the provider.cmake file, but we need to discuss a bit about this first, please wait a bit.

No problem, feel free to tag me into any discussions or design docs if you want feedback!

[memsharded]

I was successfully able to do this in a local test on macOS, I just have to put the download commands and set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES ${CMAKE_BINARY_DIR}/conan_provider.cmake) before project(). Given that, is it something you'd be ok with being documented?

The thing is that this probably wouldn't be best practice. For example, the moment it is not a pure consumer, but you want to create a package out of it, this would be extremely problematic, require conditionals to activate it or not, further polluting the CMakeLists.

. I wouldn't want to deal with an extra -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=conan_provider.cmake in my cmake command, though I suppose it could be set in CMake presets.

Yes, defining it in presets seems the way to go, clean and convenient from both sides, maintainers of the CMakeLists.txt and users (developers) building the project.

[mkviatkovskii]

@memsharded
I wonder if there is a proper way to set up a correct Conan Profile using -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=conan_provider.cmake approach.

For instance, now on MSVC it automatically sets compiler.runtime=dynamic in conan_host_profile, no matter what we really need, and it results with MD and MT conflict during linking.

[memsharded]

@mkviatkovskii that could be a different issue. If the VS runtime is defined by CMake, then the automatic detection in the provider should take it into account. Most likely it is hardcoded at the moment, and it requires to be implemented. The idea is not that you have to define it, but if it is defined by CMake, then it should be automatic.

[redradist]

Thanks for supporting this very important workflow in Conan 2.0!

I used the original CMake wrapper in personal projects and I still use it at work. I only stopped using it so I could use Conan 2.0 beta, then I converted to something like this.

...and now I'm trying develop2 out!

Here are the main reasons why I use the CMake wrapper:

* I want exactly one interface to the build. For Conan recipes, that's Conan, but for apps built in CMake that use Conan packages, I prefer using CMake directly

I also want the same, single source of truth, one inte4rface to build ...
It is unfortunate that conan remove raw cmake generators that makes it easier to integrates with CMakeLists.txt
]

[jcar87]

Hi @ssrobins thanks a lot for your feedback and for contributing to move this forward.

I have a few comments on some of the points.

Allow develop2 to be downloaded at CMake time just like develop does (and include the code snippet in documentation). I think it can still be done without affecting CMAKE_PROJECT_TOP_LEVEL_INCLUDES.

On the one hand, indeed one shouldn't alter CMAKE_PROJECT_TOP_LEVEL_INCLUDES from within a CMakeLists.txt ahead of the call to project() - doing so would likely cause issues or remove a user's freedom to inject CMake code this way. This can be very headache inducing for repository maintainers as well.

On the other hand, as per CMake documentation, the dependency provider via cmake_language(SET_DEPENDENCY_PROVIDER ...) can only be set in one of the files that are listed in CMAKE_PROJECT_TOP_LEVEL_INCLUDES, so I don't think any other way would work. If CMake were to relax this restriction, the order in which the file is loaded is also important - it's critical that conan_provider.cmake is loaded after the toolchain file is loaded as documented by CMake - an inclusion ahead of project() may not work the same, or at all.

Our recommendation would be that for projects who wish to do this, to include their copy of conan_provider.cmake- we are happy to explore options to make it easy, including an easy way to update it as it changes.

Add conan_add_remote() function from develop. I want to be able to add a remote automatically in cases where an alternate artifactory server houses the packages. It also makes it easy to directly specify the remote name in the conan install command.

I would say that we need to be very careful about enabling such functionality - I would say that is beyond the scope of the tool, and the tool assumes that Conan is minimally configured to satisfy the required dependencies. We need to consider that conan_add_remote() mutates the user's own configuration in a persistent way - and since cmake-conan obscures the use of Conan completely, a user may not even notice. Adding a new remote, especially if it's added with more priority than the existing ones, is a potential security risk. Adding remotes should only be done explicitly by users. Perhaps I'd consider a helper function where a project can specify required remotes, and query the current configured remotes, and display a very friendly error informing the user on how to resolve this.

Once again, thanks for your feedback. We'll continue working on the other points as well :)

[ssrobins]

Hi @ssrobins thanks a lot for your feedback and for contributing to move this forward.

I have a few comments on some of the points.

Allow develop2 to be downloaded at CMake time just like develop does (and include the code snippet in documentation). I think it can still be done without affecting CMAKE_PROJECT_TOP_LEVEL_INCLUDES.

On the one hand, indeed one shouldn't alter CMAKE_PROJECT_TOP_LEVEL_INCLUDES from within a CMakeLists.txt ahead of the call to project() - doing so would likely cause issues or remove a user's freedom to inject CMake code this way. This can be very headache inducing for repository maintainers as well.

On the other hand, as per CMake documentation, the dependency provider via cmake_language(SET_DEPENDENCY_PROVIDER ...) can only be set in one of the files that are listed in CMAKE_PROJECT_TOP_LEVEL_INCLUDES, so I don't think any other way would work. If CMake were to relax this restriction, the order in which the file is loaded is also important - it's critical that conan_provider.cmake is loaded after the toolchain file is loaded as documented by CMake - an inclusion ahead of project() may not work the same, or at all.

Our recommendation would be that for projects who wish to do this, to include their copy of conan_provider.cmake- we are happy to explore options to make it easy, including an easy way to update it as it changes.

I think simply setting CMAKE_PROJECT_TOP_LEVEL_INCLUDES is going to work for a lot of people who are totally fine with conan references in their CMake:
ssrobins@ada05ab

I could even have it within an if(NOT CMAKE_PROJECT_TOP_LEVEL_INCLUDES) block and only set it when it's not already set by the user. Or append to the list, though yet ordering would need to be considered. So I think there'd be ways to still allow the user freedom. Though, an argument could be made that sometimes limiting freedom can reduce developer problems by simplifying the build system. This is particularly useful in an enterprise environment where there could be an agreed upon one way to do things. I just think it would be valuable to document some other use cases so people know they are possible, especially since they were possible in the develop branch and people may be used to them. Going forward, I will probably include conan_provider.cmake in the codebase (or download an unchanging stable release if one exists like it does for the develop branch) where stability is key and probably download the latest from develop2 in a reference repo so CI can tell me when it stops working so I know there's a problem and have an opportunity to contribute a fix and perhaps improved test coverage.

Add conan_add_remote() function from develop. I want to be able to add a remote automatically in cases where an alternate artifactory server houses the packages. It also makes it easy to directly specify the remote name in the conan install command.

I would say that we need to be very careful about enabling such functionality - I would say that is beyond the scope of the tool, and the tool assumes that Conan is minimally configured to satisfy the required dependencies. We need to consider that conan_add_remote() mutates the user's own configuration in a persistent way - and since cmake-conan obscures the use of Conan completely, a user may not even notice. Adding a new remote, especially if it's added with more priority than the existing ones, is a potential security risk. Adding remotes should only be done explicitly by users. Perhaps I'd consider a helper function where a project can specify required remotes, and query the current configured remotes, and display a very friendly error informing the user on how to resolve this.

This is not beyond the scope of what I would expect from the tool, especially since develop branch allowed me to do this. The ideal solution would be having a way for Conan to set the remote url just in scope of the given conan install command, either directly in the command line itself or in a configuration file that could be included in the repo. Would that be an option? I'm not a fan of having to touch the user's configuration, but I have made the choice to do so sometimes since I have no other option.

Checking for the remote and telling the user to fix it is...just ok, in my view. I want developers to have as few of manual interventions needed as possible to get a working build. These sorts of things have a way of adding up and creating a lot of developer toil that delays developers from getting to the task at hand.

Once again, thanks for your feedback. We'll continue working on the other points as well :)

[tim-goto]

@ssrobins #510 would allow you to set any conan install parameter, which would also be the remote you want to use. If this is something that you need, please let the maintainer of the project know in the PR.

[redradist]

@memsharded @czoido Is it possible to return previous approach for cmake-conan wrapper ?

I really like previous interface for cmake-conan that allow to use only cmake, without knowing someone the dependecies was installed using conan ...

[ssrobins]

@memsharded @czoido Is it possible to return previous approach for cmake-conan wrapper ?

I really like previous interface for cmake-conan that allow to use only cmake, without knowing someone the dependecies was installed using conan ...

This sounds very much like what the develop2 iteration of cmake-conan is trying to do, can you expand on what specifically about the previous interface you think was better? I've used both interfaces, maybe I can offer some help in getting it to do what you want it to do.

[ssrobins]

I updated my original post with links to PRs for the work that's already done, put strikethroughs and additional notes for things that I didn't need, and made separate issues/PRs for everything else:

• [develop2] Design for further 'conan install' command customization
• Allow customization of the --build flag
• [develop2] append_compiler_executables_configuration() breaks libiconv package build

Therefore, I will close this issue.

Not sure if this is feasible, but I'd absolutely love to wrap up the list above before the end of the year. This would help with Conan 2.x migration at work and set up some hobby projects very well. I'd also love to contribute a video demo of CMake dependency provider to social media to put out the good word :) And of course, going forward, I would be proud to continue supporting this project so feel free to tag me in on things. Conan rocks!

-Steve