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] Consider passing the buildenv via CMake Presets #15012

Closed
1 task
jcar87 opened this issue Oct 25, 2023 · 6 comments · Fixed by #15192
Closed
1 task

[feature] Consider passing the buildenv via CMake Presets #15012

jcar87 opened this issue Oct 25, 2023 · 6 comments · Fixed by #15192

Comments

@jcar87
Copy link
Contributor

jcar87 commented Oct 25, 2023

What is your suggestion?

CMake presets allow setting the environment that is "activated" during the CMake run: https://cmake.org/cmake/help/latest/manual/cmake-presets.7.html#id4

For example:

      "environment": {
        "MY_ENVIRONMENT_VARIABLE": "Test",
        "PATH": "$env{HOME}/ninja/bin:$penv{PATH}"
      },

Currently, in Conan it is advised to 'activate' the run environment:

source ./Release/generators/conanbuild.sh
generators\conanbuild.bat
# (and the one for powershell)

if we are able to gather the information and express it in the generated CMake presets by Conan, the workflow could be the same for all platforms:

conan install .
cmake --preset conan-release
cmake --build --preset conan-release

and it would do TheRightThing ™️

this would narrow the gap of having to run different commands on Windows / macOS / Linux depending on whether the generator is multi-generator, without multigenerator, or whether we are using windows cmd prompt, powershell, or bash. Just some quality of life imprvement

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@jcar87 jcar87 added this to the 2.1 milestone Oct 25, 2023
@memsharded
Copy link
Member

Hi @jcar87

Maybe this would be a duplicate of #14271?

@thorntonryan
Copy link
Contributor

To help aid in discussion, some good points about this topic were brought up here:

CMake Preset support was introduced in CMake 3.19, but Conan 2.x wants to target CMake 3.15.

If I follow, this means that the Conan CMake build helper couldn't use cmake --preset , but as @puetzk pointed out, the build helper could still parse the CMakePresets.json file containing the buildenv info and preserve compatibility with CMake 3.15 while allowing folks using newer versions the opportunity to use/include the CMakePresets.json file directly.

@memsharded
Copy link
Member

Yes, the feature cannot be exclusive, in the sense that the integration relies on having CMake>=3.19 for all cases.
But that doesn't mean that the feature cannot be implemented and users with a modern CMake>=3.19 can enjoy it.

The solution cannot be just using the CMake build helper, because that means that a flow with conan install + activate environment + cmake ... cannot work and it will always require to call conan build, which is a no-go for all users doing local development, with an IDE; etc. So for older CMake versions, it is likely that the flow with the conanbuild virtualenv.

@puetzk
Copy link

puetzk commented Nov 14, 2023

I think Ryan was just nodding at the general principle that the CMake helper could do things its own way (e.g. conanbuild.sh), and remain compatible down to CMake 3.15, while the preset file carries the information to let an modern CMake using --preset (or IDE with similar support) obtain an equivalent information configuration/environment etc.

@puetzk
Copy link

puetzk commented Nov 14, 2023

For this specific case, it seems like one would also want runenv (i.e. bindirs of requirments in profile:host) would also make it into a test preset, so that IDE features like a unit test runner can work with requires that had shared .dll (or .so) libraries in them. This again parallels the way the conan.tools.cmake helper overlays the runenv on top of the buildenv for CMake.test().

@memsharded
Copy link
Member

#15192 has added environment to CMakePresets, it will be released in 2.0.15, feedback welcome

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

Successfully merging a pull request may close this issue.

6 participants