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] Generate cmake version files with different policies #13809

Closed
1 task done
ilya-lavrenov opened this issue May 3, 2023 · 13 comments · Fixed by #14176
Closed
1 task done

[feature] Generate cmake version files with different policies #13809

ilya-lavrenov opened this issue May 3, 2023 · 13 comments · Fixed by #14176
Assignees
Milestone

Comments

@ilya-lavrenov
Copy link

ilya-lavrenov commented May 3, 2023

What is your suggestion?

Hi,

I'm working on migration our project to Conan dependencies and faced with the following issue:

  • Conan generates default cmake version with policy called SameMajorVersion
  • But this is not valid policy for all the projects. E.g. oneTBB (former TBB) has policy called AnyNewerVersion in cmake. Actually, TBB (versions 2017-2020) and oneTBB (2021) are compatible
  • So, in our project we can use any TBB with version 2017 and higher:
find_package(TBB 2017.0 REQUIRED)

but when we started to run cmake, it failed to find oneTBB, because 2017 != 2021 => version is not satisfied from Conan point of view.

What is recommendation here? Should Conan support different policies for version file generation (as cmake provides) or any other solution can be provided here?

I understand that I can try to search for 2021 version first, then try older ones, but it's a work around which I don't want to have in our product. My criteria of Conan success is that users should not rewrite their cmake scripts specifically because of the fact, that Conan ignores cmake generated files.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded
Copy link
Member

Hi @ilya-lavrenov

Thanks for your suggestion.
When CMakeDeps generates the xxxVersion.cmake file, it knows exactly the version it is installing, for example if you required in your conanfile requires = "tbb/2021.3", then, that will be the version that will be installed, and the only version that can be found. Why shouldn't doing a find_package(oneTBB 2017) not fail if the version that is trying to find is not the one installed?

If the find_package() is intended to also find the 2021 version then it should do something like?

find_package(oneTBB 2017...<2022 )

@ilya-lavrenov
Copy link
Author

ilya-lavrenov commented May 4, 2023

Hi @memsharded,

Thanks for your answer, I really like Conan and want to continue this discussion understand why some (unclear for me) decisions are made in Conan.

Why shouldn't doing a find_package(oneTBB 2017) not fail if the version that is trying to find is not the one installed?

Because original TBBConfig.cmake does not fail. Please, have a look how TBB defines its version file.

find_package(oneTBB 2017...<2022 )

It works only for cmake 3.19 or later (while Conan requires 3.15), while we require 3.13 (available out of the box in most of Linux OS distributions). I understand that the suggestion could be to increase minimum required version, but I suppose this is not what Conan should suggest to its users.

My points are:

  • User's cmake scripts work with TBB / oneTBB packages installed from sources, brew, apt, yum / dnf / zypper, conda, vcpkg and probably other package managers. And do not work with cmake scripts generated by Conan => it's a Conan issue, users should not adapt their generic cmake scripts because of Conan.
  • Library / component developers better know how <package>Config.cmake and <package>Version.cmake should be generated (note, library developer != Conan recipe developer => we can have discrepancies in the results), why does Conan set new rules here? And always generates SameMajorVersion? Conan's CMakeDeps is not powerful enough to repeat what is generated natively. E.g. one more issue I had is that in case of components are used like in onetbb, Conan generates TBB::tbb target to represent a component and for libs inside the component it generates weird CONAN_LIB::onetbb_TBB_tbb_tbb_RELEASE target, which means I cannot take properties of TBB::tbb (like location, etc; yes, sometimes users need to do it) while I can do it with original TBB::tbb, which represents real target.

I like Conan, but what I really don't understand - why original cmake files generated during install phase cannot be reused by Conan? For this, Conan either need:

  • Be smart enough to parse <package>Config.cmake and generate content defined in package_info automatically. I understand, that it's hard to do, but otherwise Conan recipe developers fully duplicate all dependencies between the components / libraries, etc which are already defined by projects. Look at OpenCV's Conan recipe - it's really sad to maintain.
  • More realistic approach - Conan does not remove original package cmake files and reuses them. In this case developers still need to fill in package_info section, but the amount of information is significantly less (the minimal information required by the Conan itself, e.g. dependencies between components, something else; but the extra information required to generate cmake or pkg-config files can be omitted). From the first glance, I don't see a problem to keep and install original cmake scripts and reuse them in conan_toolchain.cmake. The main difference is that currently all Conan-generated <package>Config.cmake files are located in the same directory as Conan toolchain, but with proposed approach cmake files are located in the package folders (e.g. ~/conan2/p/**) and conan_toolchain.cmake just has multiple paths (e.g. ~/conan2/p/xxx/cmake, ~/conan2/p/yyy/lib/cmake, etc) in CMAKE_PREFIX_PATH variable. Only iff project does not provide cmake interface, I will have to add extra information in package_info, but in numerous cases, we don't need it and OpenCV's recipe will be significantly simpler.

As a Conan user / recipe developer I want to provide configure options, build and install steps for the product, but I don't want to rewrite the content of <package>Config.cmake and <package>Version.cmake files using a Conan notation in package_info section. I don't write anything similar in conda, I don't do it in brew, why should I write it in Conan?

@memsharded
Copy link
Member

I like Conan, but what I really don't understand - why original cmake files generated during install phase cannot be reused by Conan?

We have had this discussion tons of times. Basically:

  • Original cmake files are often not great. They might and will find dependencies in the system, instead of Conan ones. So you think that you are linking with the Conan package openssl/3.0, but some package is finding and using the openssl 1 from the system because it found it.
  • Original cmake files doesn't allow multi-config, while Conan generated ones allow to do multi-config, letting Windows users (MSVC) switch from the IDE the Debug/Release, and also for OSX XCode users. Multi-config is very important for many Conan users and CMake doesn't allow multi-config find_package()
  • Conan is not forbidding it. Actually it allows and support using generated config.cmake files. We have tests that cover it, and we will also properly document it in conan 2.0. It is just ConanCenter recipes that do not package the generated config.cmake files. This is necessary and important to support not only the above cases, but also to be able to provide good support for other build systems. There are tons of users using MSBuild, Meson, Bazel and other build systems out there. Relying on the packaged config.cmake files implies in practice that packages will only work for CMake, but not for other build systems, and that is not right.

@ilya-lavrenov
Copy link
Author

ilya-lavrenov commented May 4, 2023

We have had this discussion tons of times

Maybe it's good to take into account and incorporate user's feedback :)

Original cmake files doesn't allow multi-config

How it's possible if those files are generated by cmake automatically? (I mean files with targets, the rest of the config file should be just find_dependency) I understand what user's CMakeLists.txt sometimes are not multi-config compliant and have conditions like if(CMAKE_BUILD_TYPE STREQUAL ..), but we cannot address this issue inside the Conan tool.

Original cmake files are often not great. They might and will find dependencies in the system, instead of Conan ones

Conan recipes do this as well. I've tried to Conan install ade in an empty docker container with only Conan installed. The build has failed, because cmake has not been found in the system, which means the recipe missed build dependency.

It is just ConanCenter recipes that do not package the generated config.cmake files

Sorry, I was talking about ConanCenter specifically. What if I know that my cmake scripts are 100% compatible with Conan rules and find only Conan dependencies? or the package does not have dependencies at all like TBB? Can I use my config.cmake and .pc file in ConanCenter? while files for other build systems will be auto-generated

also to be able to provide good support for other build systems

Is it possible to classify and document what information is required depending on Conan supported build systems? I still assume that other than cmake build systems don't require so much information, so:

  • for build systems, while are supported by product itself (e.g. cmake, pkg-config), files are re-used.
  • for other build systems, the minimum required information is provided.

@memsharded
Copy link
Member

How it's possible if those files are generated by cmake automatically?

Because they assume building from source mostly and generating the different configs together if necessary, but this doesn't work for a package manager that will have Debug and Release package managers. So the a given xxxConfig.cmake can only find 1 configuration either Release or Debug but never both, and that breaks multi-config users, which are a large number of users.

Conan recipes do this as well. I've tried to Conan install ade in an empty docker container with only Conan installed. The build has failed, because cmake has not been found in the system, which means the recipe missed build dependency.

This is unrelated, the fact that recipes in ConanCenter do not always requires a tool_requires("cmake/... so it is not added as a dependency doesn't mean that package generated cmake files won't find dependencies in the system, just because they will hardcode assumptions about dependencies or other problems.

Sorry, I was talking about ConanCenter specifically. What if I know that my cmake scripts are 100% compatible with Conan rules and find only Conan dependencies? or the package does not have dependencies at all like TBB? Can I use my config.cmake and .pc file in ConanCenter? while files for other build systems will be auto-generated

This is utopic. Sounds good, but in practice it doesn't work, because the resources are not infinite, the contributors efforts are not infinite, the CI is not infinite. So it is needed to focus on 1 solution that is general for all. The package_info() would be mandatory anyway, it really needs to be defined if we want those packages to be consumed by MSBuild, Autotools or Meson. There is no way to avoid this, either we drop support for other build systems, or we need to mandate a fully functional package_info() definition. The information there is already the necessary abstraction to work for any build system, basically includedirs, libdirs, libs, flags... all build systems needs this. And dropping support for other build systems is not planned.

@ilya-lavrenov
Copy link
Author

Thanks for your answers @memsharded

Returning back to original topic about default version file generated by Conan. In this video I've seen that Conan 1.x mistakenly implied that semver scheme works for all packages and later some breaking changes were done and in 2.0 version Conan has different modes for dependencies to clearly defined package_id.
Do you think that during generation of cmake's XX-Version.cmake files Conan's implications are also too naive? As I wrote:

  • TBB defines different rules for its cmake version file and uses AnyNewerVersion. I don't see any reason why Conan takes TBB's source code and ignores compatibility rules which are provided by library developers themselves.
  • Some other libraries break their API even in minor releases and they are not source compatible between X.y and X.y+1, so SameMajorVersion will not work here.

for example if you required in your conanfile requires = "tbb/2021.3"

What if one day TBB version will 2022.x? And it's still compatible with 2021.x (in the same way as 2021 is compatible with 2017, 2018 etc). Yes, some products can be source compatible independently on their major versions, because major versions can be incremented because:

  • New year - some packages increment their major versions each year, but it does not affect compatibility.
  • New release with a lot of features (new epoch in the product), but library is still source compatible.

Do you think Conan needs to introduce new options which are provided by cmake?

@memsharded
Copy link
Member

The problem here is the compounding complexity of every corner case to be managed in the Conan codebase and documentation that ultimately is a maintenance liability slowing down the team and the project.

  • Users wanting to have the originally created tbb-version.cmake can package it in their packages and use it. It is only in ConanCenter packages that we don't allow at the moment packaging it for the reasons above.
  • Maybe I didn't comment it above, but the reality is that the vast majority of Conan users, specially those that would be interested in this kind of things, are not using ConanCenter packages directly in production. I am talking about 70-80% users creating their own packages. Many of them will work from a fork of conan-center-index and others creating their recipes from scratch. Allowing the library original cmake generated files to be package is removing one rmdir() in the recipe.
  • Few open source libraries actually introduce these kind of policies/rules for their xxx-version.cmake files (for a start, only around 50% of recipes in ConanCenter use CMake at all). This is the first case of an open source library in ConanCenter that I am aware of that actually defines this.

The cost of implementing a definition for this is far from trivial, and the utility is limited to very few corner cases in practice.
Handling this on the user side is quite simple, just aligning the version in the CMakeLists.txt with the Conan one. It can even be automated, and the version passed via the CMakeToolchain. Even if this is not automated in the recipe, yes, the day of tomorrow, a new tbb 2024.1 might be released. And libraries and packages that contains a requires = "onetbb/2021.3 will need to change it, modify it to requires = "onetbb/2024.1", and when they build it, it might raise an error because the version is not aligned, then they can change the CMakeLists.txt to find_package(oneTBB 2024.1).

As everything in SW engineering it is a matter of tradeoffs. Having to implement, document, test and maintain such a feature for this edge case is a quite large effort, to avoid an extremely small fraction of users having to bump the version in their CMakeLists.txt (a 5 seconds thing) when they bump the version in their conanfile.py, when they want to use a new version of that specific library is released (and that often will require other changes too, even in the source code).

@ilya-lavrenov
Copy link
Author

ilya-lavrenov commented May 18, 2023

find_package(oneTBB 2017...<2022 )

Unfortunately, it does not work. 2021 version is ignored here
See similar issue https://stackoverflow.com/questions/71411736/with-cmakes-find-package-why-does-version-range-0-3-3-0-5-0-not-accept-0-4

The problem is in versions files generated by Conan - they don't support version ranges. See https://gitlab.kitware.com/cmake/cmake/-/issues/24581

@memsharded
Copy link
Member

#14172 filed by @jcar87 seems to be the same issue.

I think I didn't fully understood the scope and need of this, and also overestimated the effort to implement it, which seems reasonable, sorry for that.

Let's add a cmake_version_config property that allows defining this.

@memsharded
Copy link
Member

This was implemented in #14176, it has been merged, so it will be in 2.0.8. Thanks!

@ilya-lavrenov
Copy link
Author

Hi @memsharded
Thanks for implementing this feature. What is the recommended way to use it in existing recipes? E.g. I want to add to override default version policy for oneTBB. I suppose I have to set minimal Conan version in the recipe, otherwise the feature will not take an affect? How users of older Conan versions are supposed to work with such updated recipes?
Thanks.

@memsharded
Copy link
Member

Hi @ilya-lavrenov

To be clear, only the policy has been implemented. Version ranges have not been implemented yet, let's see how it goes with the policies first.

As like any other property, the package recipe can define its policy in:

# policy in ["AnyNewerVersion", "SameMajorVersion", "SameMinorVersion", "ExactVersion"])

        from conan import ConanFile
        class Test(ConanFile):
            def package_info(self):
                self.cpp_info.set_property("cmake_config_version_compat", "{policy}")

And consumers can override it in generate():

def generate(self):
      deps = CMakeDeps(self)
      deps.set_property("onetbb", "cmake_config_version_compat", ...)
      deps.generate()

Older Conan versions are not aware of this property, so they will still need to match the version in their CMakeLists.txt, as they had to do so far, nothing changes for them (but it won't break), and they need to update to the latest Conan to avoid having to match the version exactly. But they can perfectly use the new recipes, it won't break, as unknown properties are ignored.

@paulharris
Copy link
Contributor

Thank you for this feature, I needed it for VTK.
VTK's cmake files assume their version is the minimum required (higher major versions are also acceptable).

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