Skip to content
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

Fix different formatting of msvc runtime value conan_provider.cmake #613

Open
wants to merge 1 commit into
base: develop2
Choose a base branch
from

Conversation

Bobini1
Copy link

@Bobini1 Bobini1 commented Jan 27, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Bobini1

Thanks for your contribution.
It would be good to understand where this value comes from. The values that there are already defined are the canonical/standard ones by CMake, it is not very clear where a different value comes from and what could be other potential implications.

@Bobini1
Copy link
Author

Bobini1 commented Jan 28, 2024

Hi @Bobini1

Thanks for your contribution. It would be good to understand where this value comes from. The values that there are already defined are the canonical/standard ones by CMake, it is not very clear where a different value comes from and what could be other potential implications.

Yes, I understand. This is a proposition that solves the problem for me. It is probably worth investigating why I have it in the first place. Unfortunately, I don't think I know how to find that out. Do you think you could help me check why I have this problem? What can I do?

@Bobini1
Copy link
Author

Bobini1 commented Jan 28, 2024

I would like to add that my CMake's version is 3.26.4.
Running with cmake_minimum_required(VERSION 3.21).

@memsharded
Copy link
Member

Yes, I understand. This is a proposition that solves the problem for me. It is probably worth investigating why I have it in the first place. Unfortunately, I don't think I know how to find that out. Do you think you could help me check why I have this problem? What can I do?

Maybe the first thing that we could try to rule out is something that would be specific to a custom toolchain file or CMakeLists.txt file. Does it fail for you with a very straightforward CMakeLists.txt file and you need this patch even for that case?

Second thing would be to double check the CLion configuration, to see if there could be some specific configuration related to the build-types that could be affecting.

@Danleb
Copy link

Danleb commented Jun 22, 2024

I have recently reproduced this issue with Visual Studio (not with CLion like OP).

Versions:
Conan 2.4.1
CMake 3.30.0-rc1
Visual Studio 2022 version 17.10.3

I have an explicit dependency on conan_provider.cmake and invoke it from inside my CMakeLists.txt by command:

set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES "conan_provider.cmake")

The conan_provider.cmake invokes Conan install which creates a conan_toolchain.cmake file which contains the next line:

set(CMAKE_MSVC_RUNTIME_LIBRARY "$<$<CONFIG:Debug>:MultiThreadedDebugDLL>")

The value of CMAKE_MSVC_RUNTIME_LIBRARY is later used in conan_provider.cmake which fails:

1> [CMake] CMake Error at conan_provider.cmake:232 (message):
1> [CMake]   CMake-Conan: unable to map MSVC runtime:
1> [CMake]   $<$<CONFIG:Debug>:MultiThreadedDebugDLL> to Conan settings

This is exactly the value $<$<CONFIG:Debug>:MultiThreadedDebugDLL> which @Bobini1 wants to add to the list of _KNOWN_MSVC_RUNTIME_VALUES.

Merging this PR would help greatly for using conan_provider with Visual Studio.

@memsharded What do you think?

@memsharded
Copy link
Member

Hi @Danleb

Thanks for your info

The value of CMAKE_MSVC_RUNTIME_LIBRARY is later used in conan_provider.cmake which fails:

I am a bit confused by this. The conan_provider.cmake does not generate the conan_toolchain.cmake, nor it reads and use its values. Could you please clarify this point?

@Danleb
Copy link

Danleb commented Jun 26, 2024

I meant the conan_provider.cmake invokes conan install which generates conan_toolchain.cmake. Later, when the conan_toolchain.cmake is available and used by the project, it starts providing value of CMAKE_MSVC_RUNTIME_LIBRARY which is not supported with conan_provider.cmake.

@memsharded
Copy link
Member

Later, when the conan_toolchain.cmake is available and used by the project, it starts providing value of CMAKE_MSVC_RUNTIME_LIBRARY which is not supported with conan_provider.cmake.

This is what I don't understand, the conan_toolchain.cmake is not magically passed. There should be someone passing the argument, but if using the conan_provider.cmake, this doesn't happen, the provider do not use the toolchain at all. This is why I am confused.

@Danleb maybe in your case this could be the problem:

set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES "conan_provider.cmake")

If you define this in your CMakeLists.txt then you are blocking any possibility to later create a Conan package from it. This might only work for the "consumer" case, and only when using the provider, but not when calling conan install + cmake or conan build. I don't think this is the intended usage of CMAKE_PROJECT_TOP_LEVEL_INCLUDES, which exist for being able to inject files from outside the build scripts (it could be defined in a CMake presets, but not included in CMakeLists.txt)

@Danleb
Copy link

Danleb commented Jun 27, 2024

Thanks for the explanation. Yes, I have a "consumer" case. I really liked the approach with transparent integration to CMake in Conan1, so I replicated it by set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES "conan_provider.cmake") line.

As I found, the CMakeToolchain generator creates a CMakeUserPresets.json which includes conan_toolchain.cmake. Visual Studio uses this preset and then conflict happens.

In my case, the CMakeToolchain generator was redundant and causing this issue. I removed it and kept only CMakeDeps generator which is enough for my needs. It resolved my isssue. Sorry for bothering you.

@memsharded
Copy link
Member

Ok, sounds good, but please recall that the set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES "conan_provider.cmake") is in general not recommended, as it will be problematic if at any point you want to make a Conan package from that code, as it will produce re-entrancy and this will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants