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] Issues with the CMakeToolchain generator #11224

Open
1 task done
fredizzimo opened this issue May 11, 2022 · 6 comments
Open
1 task done

[feature] Issues with the CMakeToolchain generator #11224

fredizzimo opened this issue May 11, 2022 · 6 comments
Assignees
Milestone

Comments

@fredizzimo
Copy link
Contributor

fredizzimo commented May 11, 2022

We are building custom workspace functionality for Conan 1 based on the existing code, but using CMakeToolchain and the new layout method instead of of the old layout configuration files. It's not that generic at the moment, since it's generating everything dynamically based on our project structure, so it does not need a definition yml file. It's mostly working as it should now, but there are some issues with the toolchain generator.

  1. Toolchains are not really suitable for this use case. We want to use the same model that was used in the original workspaces model of using add_subdirectory. But that is problematic, since you can only specify the toolchain for the root level cmake file. Our current workaround for this is to just include the toolchain file, which seems to work, except for problem 2. But this is not how cmake toolchains should be used, and it's probably not guaranteed to work in the future, if more stuff is added to the toolchain. Additionally I'm not so sure about the whole idea of using toolchains for setting normal variables and project configuration, IMO toolchains should be reserved for what they actually are meant for, compiler toolchains and stuff. I think that we need a new additional generator that generates just plain cmake files with the project configuration, that can be then included manually. Then we could include a toolchain file at the top level, if needed, and this generated cmake file in each subproject. And when the package is actually exported, it would use both files.
  2. The variables are set to the cache and not force overwritten. In our case this means that you can't use the same variable in two different projects, which means that they can't be cached. Other use cases need the cache see, [bug] CMakeToolchain now requires double configure to set cache variables #7832, but even in those cases I believe that the variables should be force set, otherwise you have to delete the cache in order to be able to change the variable from the recipe. So you should probably provide an interface for setting different variable types.
  3. You can't provide a template file to the generator, only a static toolchain file. If a template file was allowed, then we would at least have a quite elegant workaround for the issue, by making a custom template that just sets normal variables instead of cache ones. Custom blocks could also be an alternative, but sadly an interface for setting that does not seem to be available either. So I think my current workaround would be to just make a custom class that overrides the internal _template, that's not elegant, nor future proof, but at least I think it will work.

I was also considering using cmake external projects instead of add_subdirectory, but sadly, the IDE integration for that is not great, so I'm not sure if that's a path I'm willing to take. I have read most of the issues related to workspaces here, and some people are suggesting that, but if you go that way with the official conan 2 integration, you should definitely make sure that the ide integration works properly.

At least for our use cases, the way I'm doing it now seems to be solving all the issues I had with the existing implementation, and why I decided to write my own. The existing workspaces are too decoupled from the actual recipes, but by utilizing the layout and generators, everything can be shared.

@memsharded
Copy link
Member

Hi @fredizzimo

Thanks for the feedback.
These are valid points, and the workspaces is something that needs to be resumed, because so far we have been focused on the building blocks, the editables.

It is true, that the way we are considering now for the workspaces is based on cmake external projects, this is our current thinking:

  • Every package will be mapped to its own project. As you realized, the integrations are following this project model. The variability that sharing the same project or not (for example, regarding the find_packages() vs directly using targets from another directory) pollutes the CMakeLists too much, doesn't seem worth it.
  • A workspace would be a "solution" or "project" that aggregates connected subprojects, but the subproject entity would still be necessary
  • Regarding the IDE integrations, it seemed that the project aggregation could work well with relative low friction, here is a demo we did, with zero "workspace" automation, just manually adding subprojects to the IDE, and it seemed really simple and effective: https://youtu.be/VzUJQw89U7o?t=2499

but sadly, the IDE integration for that is not great,

So maybe this is the first bit that should be investigated. Why this integration via subprojects is not great? Which IDEs? Can we automate something at a different place than CMakeLists (like generation VS .sln on the fly?)

@fredizzimo
Copy link
Contributor Author

Thanks, I have perhaps dismissed external projects too early, since I only did an initial research and found problems like these, but never tested it myself
https://discourse.cmake.org/t/externalproject-superbuilds-and-visual-studio/794
https://cmake.org/pipermail/cmake/2018-January/066957.html
https://developercommunity.visualstudio.com/t/cmake-cannot-build-project-with-non-target-debug-p/1228684
https://developercommunity.visualstudio.com/t/test-explorer-with-tests-from-cmake-externalprojec/1227332

All of them seems to be related to the fact that proper targets are not created for the external executables. But perhaps there's a way to generate some dummy targets in the root cmake file, and make it work properly. I will do some experimentation with that and what you showed in the video and come back to you in a couple of days.

But, I still think that suggestion 2. especially the ability to force overwrite cache variables, and 3. to a less degree is needed with this.

@memsharded
Copy link
Member

All of them seems to be related to the fact that proper targets are not created for the external executables. But perhaps there's a way to generate some dummy targets in the root cmake file, and make it work properly. I will do some experimentation with that and what you showed in the video and come back to you in a couple of days.

That could be relevant, yes. As Conan is generating the targets itself, then maybe there is a chance that this is not problematic.

But, I still think that suggestion 2. especially the ability to force overwrite cache variables, and 3. to a less degree is needed with this.

Yes, this could deserve a separate discussion. There are already some variables that are CACHE forced in the toolchain:

 {% if generator_platform %}
        set(CMAKE_GENERATOR_PLATFORM "{{ generator_platform }}" CACHE STRING "" FORCE)
        {% endif %}
        {% if toolset %}
        set(CMAKE_GENERATOR_TOOLSET "{{ toolset }}" CACHE STRING "" FORCE)
        {% endif %}

Should all of them be forced by default then?

@fredizzimo
Copy link
Contributor Author

Should all of them be forced by default then?

It's hard to comment about other projects, and even on our own without trying it out fully. But yes, if we can use external projects, then forcing them all seems to be a working solution. The most flexible would obviously be to expose different types of variables to the interface, but that's also tricky because they are set through a table and not a function.

@memsharded
Copy link
Member

But yes, if we can use external projects, then forcing them all seems to be a working solution.

Ok, I think we should investigate the potential new workspace approach via external projects or wider IDE or build systems solutions, and from there we will work out the details for specific implementations like the toolchains.

This will have to wait a bit, working on workspaces is not planned in the scope of 2.0, we can label it as 2.X.

@memsharded memsharded added this to the 2.X milestone May 11, 2022
@fredizzimo
Copy link
Contributor Author

I have tested this a bit more now. And I don't think I can make the external projects work with proper IDE support.

The way you demonstrated in the video is very specific to Visual Studio, and would not work with Visual Studio code for example, so I did not look into that any further. I have mostly looked at the "open folder" in both Visual Studio and Visual code. I haven't really looked at the cmake Visual Studio generators, since they are Visual Studio specific and as far as I know, they don't support compiling Linux configurations through WSL for example, which is something that we need. Microsoft themselves also only seem to document the "open folder" workflow.

Here are some of the major problems that I have found:

  1. Like I mentioned before I can confirm that Visual Studio and Visual Studio code don't see the external executables as proper executable targets, which makes debugging without handcrafted debug configurations hard. The debug configuration could be generated to overcome some of the difficulties, but I don't really like that approach, epecially since users might also want to edit the same files to add command line parameters for example. I did not find any way to fool the system by using add_custom_command, add_custom_target. add_executable with an imported exe file did not work either, it simple does not show up as a target.
  2. The unit test framework is even more problematic, and I did not even try to solve the problems with that.
  3. The code completion knows nothing about the external projects, which makes it very inconvenient to use. See https://gitlab.kitware.com/cmake/cmake/-/issues/22759 and External projects not handled as I would expect microsoft/vscode-cmake-tools#1616 for more information. For language servers that use compile_commands.json it might be possible to concatenate all of them into one in a custom build step, but that's not possible for all IDEs.
  4. Since the actual compile system does not know the full dependency graph it can't parallelize very efficiently. All the parallelism has to be done at the package level.

So due to these problems I will move back to using add_subdirectory instead of add_external_project. But feel free to do you own investigations, there might be things that I have overlooked.

It would be nice if a new generator was added, or if the CMakeToolchain generator was flexible enough to support both workflows. But meanwhile, I will try to just make a derived generator myself, which just replaces the template used and generates a file meant to be included. That file is then used when compiling in workspace mode, while the toolchain is used when actually exporting packages.

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

No branches or pull requests

2 participants