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

Consider adding cmakeExecutable to presets #15447

Merged
merged 6 commits into from
Jan 26, 2024

Conversation

czoido
Copy link
Contributor

@czoido czoido commented Jan 12, 2024

Changelog: Feature: Add cmakeExecutable to configure preset.
Docs: conan-io/docs#3548

Closes: #15427

@czoido czoido added this to the 2.1 milestone Jan 12, 2024
@czoido czoido requested a review from memsharded January 12, 2024 09:21
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.

looks good

conan/tools/cmake/toolchain/toolchain.py Outdated Show resolved Hide resolved
Co-authored-by: James <james@conan.io>
Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

Very good! just one comment, but it shouldn't be a problem.

I wonder if it would be useful to have a an attribute in CMakeToolchain that the users could set in the generate() method, if they so wish? , in case they're in a universe where cmake is not in the system path, and also not a tool_requirement (think a CI machine that has multiple CMake versions installed in different places)

conan/tools/cmake/toolchain/toolchain.py Show resolved Hide resolved
@memsharded
Copy link
Member

I wonder if it would be useful to have a an attribute in CMakeToolchain that the users could set in the generate() method, if they so wish? , in case they're in a universe where cmake is not in the system path, and also not a tool_requirement (think a CI machine that has multiple CMake versions installed in different places)

We already have

self._cmake_program = conanfile.conf.get("tools.cmake:cmake_program", default="cmake")

Inside the CMake helper, to override which CMake is being used. Maybe that is what could go directly to the presets as well?

@czoido
Copy link
Contributor Author

czoido commented Jan 24, 2024

I wonder if it would be useful to have a an attribute in CMakeToolchain that the users could set in the generate() method, if they so wish? , in case they're in a universe where cmake is not in the system path, and also not a tool_requirement (think a CI machine that has multiple CMake versions installed in different places)

We already have

self._cmake_program = conanfile.conf.get("tools.cmake:cmake_program", default="cmake")

Inside the CMake helper, to override which CMake is being used. Maybe that is what could go directly to the presets as well?

I made changes to read that conf with preference

@czoido czoido marked this pull request as ready for review January 24, 2024 15:24
@memsharded memsharded merged commit 5a8cc86 into conan-io:develop2 Jan 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] configurePresets.cmakeExecutable in CMakePresets.json should be filled in
3 participants