Civitasv / cmake-tools.nvim

CMake integration in Neovim
GNU General Public License v3.0
366 stars 59 forks source link

Add option to not pass the '--presets' option to the cmake command #210

Closed Townk closed 4 months ago

Townk commented 5 months ago

Changes Description

On CMake projects that use Conan as their package manager, there are two presets that are created with the same name. This always causes any cmake command issued by CMakeTools to fail. On those scenarios, the project usually specify the CMake options (including the preset location) as an argument to the cmake command.

This commit adds the option to not pass the --preset argument to the cmake command.

Civitasv commented 5 months ago

I don't think this is the right way to support Conan. Seems a little weird.

Townk commented 5 months ago

I think I should re-word this PR, is not really about conan. The problem happens when you use CMake older than 3.23 AND conan. On that case, conan generated presets won't work with CMake. During compilation Conan actually logs that if you're using CMake < 3.23 you should not use presets, and that is the reason why I want an option to not pass --preset to cmake commands

Townk commented 5 months ago

Just got time to gather more information regarding my reason for having this pull request.

First, about what I mentioned before, here is the relevant part of a Conan build log:

Calling generate()
conanfile.py (mypackage/0.1.0): Generators folder: /path/to/my/cmake/output/dir
conanfile.py (mypackage/0.1.0): CMakeToolchain generated: conan_toolchain.cmake
conanfile.py (mypackage/0.1.0): Preset 'conan-release' added to CMakePresets.json. Invoke it manually using 'cmake --preset conan-release' if using CMake>=3.23
conanfile.py (mypackage/0.1.0): If your CMake version is not compatible with CMakePresets (<3.23) call cmake like: 'cmake <path> -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE=/path/to/my/cmake/output/dir/conan_toolchain.cmake -DCMAKE_POLICY_DEFAULT_CMP0091=NEW -DCMAKE_BUILD_TYPE=Release'
conanfile.py (mypackage/0.1.0): CMakeToolchain generated: CMakePresets.json
conanfile.py (mypackage/0.1.0): CMakeToolchain generated: ../../../../CMakeUserPresets.json 

With that out of the way, I thought a bit more about why I want this pull request to go in. Of course, the main reason is that I want cmake-tools to work on my system, but besides that, I believe that making cmake-tools always use the --preset argument makes the plugin not as flexible as it could be. CMake itself does not imply --preset by default, so why should cmake-tools do it?

I'm not sure if this clarifies my reasons behind this pull request, but please let me know if you want further details about it.

I'm working on a second patch that will have this change rebased with the current master, and it should be ready around this week.

Civitasv commented 5 months ago

I understand. I think we can add an option use_presets. It receives a function as its value, if the return value of this function is true, we use presets when presets file exists. It's more flexible than current and can solve this problem. What do you think?

Townk commented 5 months ago

That is perfect, I would suggest the option to be a function or a boolean.

Do you want me to make this change in this PR?

Civitasv commented 5 months ago

function or a boolean.

Yeah, this is better.

Do you want me to make this change in this PR?

Yes.

Townk commented 4 months ago

Sorry for the delay here. I rebased the change and was able to simplify a bit. I made sure to let the use_presets option turned on by default to no break existing users.

Townk commented 4 months ago

Thanks for merging!