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

[feature] [toolchain.cmake] Set platform and toolset only if generator matches Visual Studio #7485

Closed
Minimonium opened this issue Aug 3, 2020 · 8 comments · Fixed by #8192
Milestone

Comments

@Minimonium
Copy link
Contributor

Use case: Using MSVC with Ninja as CMake Generator. It doesn't support PLATFORM and TOOLSET variables and errors out on them.

Consider generating the template like this:

if(CMAKE_GENERATOR MATCHES "Visual Studio")
        {% if generator_platform %}
        set(CMAKE_GENERATOR_PLATFORM "{{ generator_platform }}" CACHE STRING "" FORCE)
        {% endif %}
        {% if toolset %}
        set(CMAKE_GENERATOR_TOOLSET "{{ toolset }}" CACHE STRING "" FORCE)
        {% endif%}
endif()
@Minimonium
Copy link
Contributor Author

Related to #5737

@memsharded memsharded added this to the 1.29 milestone Aug 3, 2020
@memsharded
Copy link
Member

It seems that this will not be resolved until build-time, when cmake is actually called.
Maybe it makes sense to model that in the toolchain already, that is, that the toolchain is aware that it will be using Ninja instead of MSBuild. It is likely that there will be other things to fix as well, and it might be very useful to not have to wait until build time and things are more explicit and simpler. Just an idea to consider, lets have a look for next Conan 1.29 if possible.

@Minimonium
Copy link
Contributor Author

The issue is that we should consider how to integrate toolchains with IDEs first and foremost, it's crucial if we want gain popularity with a wider user base.
For example, my case is VSCode and CMake Tools which sets the environment for you, so you don't expect it to hijack the generator since it's configurated via their specific mechanisms. A potential Conan extension in there should absolutely be compatible with it to not reinvent the wheel.
We need to collect experience from users of other CMake compatible IDEs such as CLion, Visual Studio, QtCreator, and such to work from there first.

@memsharded
Copy link
Member

The issue is that we should consider how to integrate toolchains with IDEs first and foremost, it's crucial if we want gain popularity with a wider user base.

Possibly the biggest goal of toolchain() is to provide a better integration with build systems and IDEs. Being the core concept of toolchains that they generate files that can be used by the build systems, instead of calling the build systems and passing a lot of stuff via command line and environment, I would say that toolchains should definitely help in a better integration with the IDEs too. The plan for next release is to do some more steps and provide preliminary support for MSBuild, Autotools and Meson toolchains, and hopefully more feedback starts to come.

@memsharded memsharded modified the milestones: 1.29, 1.30 Aug 31, 2020
@memsharded
Copy link
Member

I am having a look and trying to add testing that includes the Ninja build system: https://github.com/memsharded/conan/tree/feature/toolchain_cmake_ninja

There are important design issues here, which require new solutions. Nothing that can be done right now, will keep working in next 1.31.

@memsharded memsharded modified the milestones: 1.30, 1.31 Sep 29, 2020
@memsharded memsharded modified the milestones: 1.31, 1.32 Oct 29, 2020
@Nipheris
Copy link

Nipheris commented Nov 10, 2020

Hi @memsharded !

It seems that this will not be resolved until build-time, when cmake is actually called.

What do you mean when say "will not be resolved"? The if block that @Minimonium propose to use is processed by CMake itself at the cmake-generation step (when files for the underlying build system like MSBuild/Ninja/etc are generated), and CMAKE_GENERATOR is already defined at this moment. We've met the same problem now, and after patching the generated cmake_toolchain.cmake (wrapping the set(CMAKE_GENERATOR_PLATFORM.. in the if statement) Ninja build files are generated without any issues.

Why not merge this solution in the next release? :)

Edit: to be precise, I think we should implement the same logic like in is_generator_platform_supported, but using generated CMake code. Green Hills MULTI should also be considered.

@memsharded
Copy link
Member

It seems that #8333 is not really necesary. This was fix earlier, and the toolchain is checking that the generator is "Visual" one, otherwise it will not add platform and toolset variables.

@memsharded
Copy link
Member

memsharded commented Jan 14, 2021

It was possibly fixed in #8192, that is also part of 1.33.

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