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

[question] How to pass cmake configuration option with modern conan? #10515

Closed
julienalofs opened this issue Feb 4, 2022 · 11 comments · Fixed by #10573
Closed

[question] How to pass cmake configuration option with modern conan? #10515

julienalofs opened this issue Feb 4, 2022 · 11 comments · Fixed by #10573

Comments

@julienalofs
Copy link

I need to pass cmake configuration option to be able to modify the project version. Unfortunately, by using toolchain variables, those variable aren't already loaded when the project() cmake function is called.

How could I achieve that?

@memsharded
Copy link
Member

Interesting question, I didn't know that the project() call would not be able to use toolchain.cmake defined vars. I see projects like https://github.com/tinyalsa/tinyalsa/blob/515be35688a76edee90b892150ea22fa9516638c/CMakeLists.txt#L7 doing some crazy things like reading things from a file.

I think I would try something in the line of:

  • The generate() method is in general the responsible of this task, as it should prepare everything to the project build correctly, both in the cache and in user folder. A cmake .... build should get the same result in both cases.
  • The simplest approach could be a regex replace over project() in CMakeLists.txt directly. This can probably be done in a couple of lines in the generate() method.
  • If an explicit approach is desired (something that is explicit to the CMakeLists.txt), then probably generate() writing a file with set(MY_PROJECT_VERSION <myversion>), and having CMakeLists.txt include the file and use the MY_PROJECT_VERSION var.

Alternatively, we could add to the modern CMake helper the capability to pass some variables in the command line. I don't love this approach, because abusing it (and it is something that very easily will lead to abuse) results in projects building different things when you call conan build and when you call cmake, unless you correctly pass all the -DVAR=VALUE that the build() method would be adding, and that is something that is difficult to remember for developers, and in turn results later in many hours of debugging and support because something is unexpectedly slightly different.

@julienalofs
Copy link
Author

Thank you for the quick reply @memsharded

I'm not sure to understand what you mean by unless you correctly pass all the -DVAR=VALUE that the build() method would be adding.

Like you, I think that the generat() methode is responsible of this task. But cmake options can passed with arguments to the cmake ... call, isn't it ?

@memsharded
Copy link
Member

The alternative is to implement something that adds command line cmake variables in the cmake.configure() call that happens inside the build() method. Something like c = CMake(self), c.vars["MYVERSION"]=self.version, c.configure(), will pass the -DMYVERSION=xxx to the command line when the build() method executes.

But the goal of toolchains is to be able to do native builds, so basically conan install + cmake ..... That is, NOT executing the build() method. Unless developers remember to add all possible c.vars to the cmake ... -DMY_VERSION=xxx, without doing mistakes (like a typo in the version number), then the build that they will do locally will be different to what Conan is building when doing a package conan create.

Please let me know if this clarifies it a bit.

@julienalofs
Copy link
Author

Yes this clarifies.

It is just that it doesn't really fit with the cmake phylosophy. Cmake doesn't offer a way to configure options through toolchain because we can't be sure when the toolchain is loaded. I mean not in a generic/global solution.

From my side, I think something like c = CMake(self), c.vars["MYVERSION"]=self.version, c.configure() would be the correct answer but, that is just my opinion, in my (very restricted) context.

@memsharded
Copy link
Member

It is just that it doesn't really fit with the cmake phylosophy. Cmake doesn't offer a way to configure options through toolchain because we can't be sure when the toolchain is loaded. I mean not in a generic/global solution.

Not sure I follow. In general toolchains seems a good approach to define cmake inputs, and it seems only the project() version thing could be one exception. But the rest of possible options, configurations, etc, seems to be well defined, it is possible to configure many different things in a toolchain, define cmake variables, compilation flags, shared/static and more. If there are more things like the project version that will not work, it would be very interesting to know, please let us know.

From my side, I think something like c = CMake(self), c.vars["MYVERSION"]=self.version, c.configure() would be the correct answer but, that is just my opinion, in my (very restricted) context.

Yes, I am not opposed to adding this feature, it should be simple and low risk from the implementation point of view. But it is very important to clearly explain the implications, and what it means, because otherwise misuse/abuse will result in unnecessary pain for users and a burden on maintenance and support for us.

@julienalofs
Copy link
Author

Not sure I follow. In general toolchains seems a good approach to define cmake inputs, and it seems only the project() version thing could be one exception. But the rest of possible options, configurations, etc, seems to be well defined, it is possible to configure many different things in a toolchain, define cmake variables, compilation flags, shared/static and more. If there are more things like the project version that will not work, it would be very interesting to know, please let us know.

I made some more investigations:

  • It appears that variables set in the toolchain cannot be read before the project() call. Nevermind.
  • If i have a variable set in cache by my CMakeLists.txt (to expose an option). Variable with the same name in the toolchain will not override it.

So toolchain is great to add new variable but not to override existing one.

@puetzk
Copy link

puetzk commented Feb 4, 2022

I didn't know that the project() call would not be able to use toolchain.cmake defined vars.

This is because CMAKE_TOOLCHAIN_FILE is loaded in CmakeSystem.cmake, which is loaded by enable_language(). So toolchain variables are not brought in until at least one language has been enabled. I think they do it this way since CMakeSystemSpecificInformation often uses some things (like CMAKE_LIBRARY_ARCHITECTURE) that aren't really language-specific (they really come from ld, not gcc/g++/clang), but aren't known until the CMake*DetermineABI calls have done some work. One can of course do an enable_language(...) before project(...), but it's a little weird looking.

But yes, I've also occasionally missed the fact that conan.tools.cmake doesn't have anything to replace CMake.definitions in places where I was passing additional variables (that weren't settings-related and don't quite belong in a toolchain).

One idea: what if conan generated the CMakePresets.json format, and conan.tools.cmake.CMake just used cmake --preset conan? That has ways to can set generator and cacheVariables.CMAKE_TOOLCHAIN_FILE, so this could be a drop-in replacement for the functionality of conanbuild.conf. But it's also got the abilities conanbuild.conf current lacks, like loading additional vars in the cache (as requested here), and passing along configure/build/run environment variables (the latter using buildPresets/testPresets.environment).

Since it's an upstream thing rather than a conan-specific format, so you'd also get IDE integration: cmake-gui of course, but also visual studio, CLion, QtCreator not yet but it's on their roadmap.

@memsharded
Copy link
Member

memsharded commented Feb 4, 2022

But yes, I've also occasionally missed the fact that conan.tools.cmake doesn't have anything to replace CMake.definitions in places where I was passing additional variables (that weren't settings-related and don't quite belong in a toolchain).

Ok, another vote for this, I think we can implement it, no prob.

One idea: what if conan generated the CMakePresets.json format, and conan.tools.cmake.CMake just used cmake --preset conan? That has ways to can set generator and cacheVariables.CMAKE_TOOLCHAIN_FILE, so this could be a drop-in replacement for the functionality of conanbuild.conf. But it's also got the abilities conanbuild.conf current lacks, like loading additional vars in the cache (as requested here), and passing along configure/build/run environment variables (the latter using buildPresets/testPresets.environment).

Presets is CMake 3.19. We have stabilized and approved on CMake 3.15 support in the Tribe 2.0. (conan-io/tribe#4) We might add some more powerful capabilities for presets, but the "canonical" integration should be the one supported by CMake 3.15

@puetzk
Copy link

puetzk commented Feb 4, 2022

Presets is CMake 3.19. We have stabilized and approved on CMake 3.15 support in the Tribe 2.0.

Hmm, true. Not something I care much about, but I had forgotten there was already an official ruling. Others certainly do care.

On the other hand, that really just means conan.tools.cmake.CMake can't use cmake --preset, not that it couldn't use the CMakePresets.json format (and parse the JSON itself for the generator argument/variables dictionary/apply_env/etc) instead of defining its own conanbuild.conf file format. To an older CMake, that looks like conan just doing its own thing as it currently does; to anyone using IDEs and newer CMake it would serve a nice ulterior motive.

In fact, the IDE Integration Guide explicitly endorses this sort of thing, saying

The --preset= option is intended only as a convenient frontend for command line users, and should not be used by the IDE.

And they give schemas/test cases for doing your own parsing. If conan's helper object did this (interfacing as they recommend an IDE should), that ought to be a stable integration and would make this compatible with older CMake (while also getting the benefits for newer IDEs).

@memsharded
Copy link
Member

Uhm, that is a really interesting approach. If in fact everything is possible via command line, and no other internal logic, then this looks like feasible. Thanks for the idea, let's have a look and try it!

@memsharded memsharded added this to the 1.46 milestone Feb 4, 2022
@puetzk
Copy link

puetzk commented Feb 4, 2022

I don't know if every feature of an arbitrary preset is possible. Certainly that seems unlikely in be a guarantee - if a newer CMake added some feature that doesn't exist before, it's likely that CMakePresets.json would be extended with a new key to support it. But it at least started as way of presetting features that cmake has had for a long time, it didn't arrive along an entirely different paradigm of configuration options. So there should be a large subset that would be compatible back to 3.15.

And certainly every feature conan.tools.cmake.CMake supports today (via conanbuild.conf) is something 3.15 can do, as are the requested addition of cache variables (via -Dvar=value). If we wanted to do the build/run environment variables you'd just use environment_append before you call exec.

But here conan would own both sides (a CMakePresets.json generator and conan.tools.cmake.CMake). So it should be straightforward to limit itself to generating only things that are compatible with CMake 3.15. And this would still deliver the value-add that it's something other tools would also understand, so you could conan install and then open it right up in your IDE.

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.

3 participants