support list in conan command during version check #668
Conversation
|
|
There was a problem hiding this comment.
Thanks for your contribution @oleksandr-prykhodko-quickbase
| @@ -560,7 +560,7 @@ macro(conan_provide_dependency method package_name) | |||
| get_property(_conan_install_success GLOBAL PROPERTY CONAN_INSTALL_SUCCESS) | |||
| if(NOT _conan_install_success) | |||
| find_program(CONAN_COMMAND "conan" REQUIRED) | |||
| conan_get_version(${CONAN_COMMAND} CONAN_CURRENT_VERSION) | |||
| conan_get_version("${CONAN_COMMAND}" CONAN_CURRENT_VERSION) | |||
There was a problem hiding this comment.
This is not very clear. The CONAN_COMMAND variable is the one defined in the above find_program(), not a custom defined one, but you said you defined it as set(CONAN_COMMAND "py;conan_command.py"). Where did you defined it? And how it gets to this location with that value?
There was a problem hiding this comment.
Your script defines this variable at this point, but you don't own it—we can define it too! :)
At the beginning of CMakeLists.txt, we set up various configurations, including the CONAN_COMMAND. Because of this setup, CONAN_COMMAND is not being set by the find_program() call mentioned above (as described in the documentation: 'If the program is found, the result is stored in the variable, and the search will not be repeated unless the variable is cleared.').
As a result, we define CONAN_COMMAND before any actual work is done, meaning that when find_program() is invoked later, it effectively does nothing.
The conan_provider is used as advised with -DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=../conan_provider.cmake. When CMake retrieves dependencies from Conan, conan_provide_dependency is called. By this point, our pre-setup has already executed, and CONAN_COMMAND has been set.
There was a problem hiding this comment.
But that approach is a bit risky, isn't it? If there is some other Conan installed in the system, the find_program() will find it and use it, while your intention was to use your own wrapper always, is that correct?
There was a problem hiding this comment.
Ah, no sorry, you are right, if the find_program() finds that it is set, it will not even try to find it, reusing, it.
Understood, thanks for the clarification!
There was a problem hiding this comment.
I think I am fine with this change, but will assign @jcar87 to review.
Thanks for your contribution @oleksandr-prykhodko-quickbase
We have custom wrapper around
conancommand and use it in cmake asset(CONAN_COMMAND "py;conan_command.py")Passing it to
conan_get_versionunwraps it as a list thus version check calls whatever is first in the list instead of the whole command.