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

feat: conan 2.x recipe #1308

Merged
merged 15 commits into from
Oct 25, 2024
Merged

feat: conan 2.x recipe #1308

merged 15 commits into from
Oct 25, 2024

Conversation

braindigitalis
Copy link
Contributor

@braindigitalis braindigitalis commented Oct 23, 2024

This cannot be used to create a pr on conan-center until after the next release (10.0.34) as it relies on this release's tarball to function, which is not yet in place.

Code change checklist

  • I have ensured that all methods and functions are fully documented using doxygen style comments.
  • My code follows the coding style guide.
  • I tested that my change works before raising the PR.
  • I have ensured that I did not break any existing API calls.
  • I have not built my pull request using AI, a static analysis tool or similar without any human oversight.

@braindigitalis braindigitalis self-assigned this Oct 23, 2024
Copy link

netlify bot commented Oct 23, 2024

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit a9b007a
🔍 Latest deploy log https://app.netlify.com/sites/dpp-dev/deploys/671b86c2cb1d5300082c7805
😎 Deploy Preview https://deploy-preview-1308--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the build Issue or Pull Request related to the build process label Oct 23, 2024
@braindigitalis braindigitalis marked this pull request as draft October 23, 2024 22:57
@braindigitalis braindigitalis added the packages Issue or Pull Request related to packaging label Oct 23, 2024
@braindigitalis
Copy link
Contributor Author

@MikeRavenelle i would appreciate some feedback on this one if you have time please!

Copy link

@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.

Looking good, just a few possible ideas for improvements or simplifications.

conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
library/CMakeLists.txt Show resolved Hide resolved
@braindigitalis
Copy link
Contributor Author

Looking good, just a few possible ideas for improvements or simplifications.

Thanks, I've made these changes!

When it comes to preparing this for a PR on conan center, how do i go about this? Is there a page on the docs saying how to submit a new package to conan center as i must have missed it.

@memsharded
Copy link

The main docs are in https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/README.md

We are just in the process to moving to a Conan 2-only pipeline, a matter of a couple of weeks. Probably you might want to wait until this happens before submitting it.

I'd say that the recipe is quite good, it shouldn't be very difficult to add it, just some minor things ,like using a conandata.yml with the data, probably removing the export capture of the scm commit.

@braindigitalis
Copy link
Contributor Author

Also after completing a build of D++ through conan i get the following deprecation warnings about several of the dependencies, is this something to be concerned about, if the dependencies never updated to Conan 2.x and you're deprecating Conan 1.x soon won't this break all D++'s dependencies via Conan?

dpp/10.0.33: Package '70ff0fab11b405566a3aa83746595e4f42384a03' created
dpp/10.0.33: Full package reference: dpp/10.0.33#ceb89c69974c5d471818a4f592cfe2c5:70ff0fab11b405566a3aa83746595e4f42384a03#13a094550d605b980d4c81686fa3496e
dpp/10.0.33: Package folder /home/brain/.conan2/p/b/dpp1c4bd0de58a7b/p
WARN: deprecated: Usage of deprecated Conan 1.X features that will be removed in Conan 2.X:
WARN: deprecated:     'cpp_info.names' used in: opus/1.4, openssl/3.1.2, zlib/1.3
WARN: deprecated:     'cpp_info.build_modules' used in: openssl/3.1.2
WARN: deprecated:     'env_info' used in: openssl/3.1.2

@memsharded
Copy link

Also after completing a build of D++ through conan i get the following deprecation warnings about several of the dependencies, is this something to be concerned about, if the dependencies never updated to Conan 2.x and you're deprecating Conan 1.x soon won't this break all D++'s dependencies via Conan?

Don't worry too much. We will make sure to remove the legacy parts in ConanCenter recipes before completely deprecating them in the Conan client (that would raise an error). Hopefully this happens relatively fast after we move to the Conan 2-only pipeline, but it is true that there is also no intent to be aggressive about this, because the backlog there is very large, no need to overwhelm the system and reviewers with "stylistic/deprecation" only changes.

@braindigitalis braindigitalis marked this pull request as ready for review October 25, 2024 11:54
@braindigitalis braindigitalis linked an issue Oct 25, 2024 that may be closed by this pull request
11 tasks
@braindigitalis braindigitalis merged commit b5ae650 into dev Oct 25, 2024
53 checks passed
@braindigitalis
Copy link
Contributor Author

@memsharded having difficulty getting this to work on Conan center. everything I've done seems to conflict with what they are saying there e.g. must still support Conan 1.x? and ancient cmake from before 2019 seems to be a thing on their ci!

conan-io/conan-center-index#25745

I would appreciate some input if you have time please

also is Conan center different from Conan itself with different policies???

@memsharded
Copy link

Yes, this is why my advice above:

We are just in the process to moving to a Conan 2-only pipeline, a matter of a couple of weeks. Probably you might want to wait until this happens before submitting it.

The support for Conan 1.X will stop in the following week hopefully. So it is better to wait a bit, so it is not necessary to pass Conan 1.X builds at all.

Modern cmake can be used by defining a self.tool_requires("cmake/[>=3.xxx <4]) in your recipe. The assumed default is CMake 3.15, yes, backward compatibility with older CMake versions is very important for a lot of users out there

also is Conan center different from Conan itself with different policies???

Yes, ConanCenter is quite large. It got like 6000 PRs in 2023 alone. Every PR might build a few times, for many different configurations in Linux, Mac, Windows, different compiler versions, architectures, etc. All PRs are reviewed by maintainers, and there is automation for quality checks, security, etc.

That also means that at any given time, like now, there are more than 500 PRs open, which is a huge load on the team. But lowering the standards or reviewing less things, going faster, is not an option at the moment. We have shifted from that model in previous years to a new way of working in which we prioritize stability, security and robustness with respect to other things like adding new packages or versions.

So the policies and requirements there for packages might be sometimes a bit overwhelming at first, but also necessary to be able to manage the scale and provide reasonable guarantees for users of ConanCenter in different platforms. The single major effort of 2024 has been a new CI pipeline for Conan 2-only that simplifies a lot of things, both internally and externally that will empower the team so we can also help contributors better to get things moved forward, and will allow contributors to have a better experience, with less noise and more clear information, faster builds, better logging.

@jcar87
Copy link

jcar87 commented Oct 29, 2024

@memsharded having difficulty getting this to work on Conan center. everything I've done seems to conflict with what they are saying there e.g. must still support Conan 1.x? and ancient cmake from before 2019 seems to be a thing on their ci!

Regarding the CMake version - when Conan 2 was designed, it was done in consultation with industry experts - and Conan 2 itself aims to be compatible (where possible, for the broadest compatibility) with CMake 3.15
https://github.com/conan-io/tribe/blob/main/design/004-tools-cmake.md

Please bear in mind that Conan Center has thousands of users, and what is "an ancient CMake from 2019" for some users, may actually be the version of CMake that users in enterprise environments are actually using - and we aim to support those as well. That's why the default in Conan Center CI is 3.15 in at least one of the platforms, so that we can also ensure broad compatibility with the recipes. It does not mean that all recipes need to be compatible with 3.15 - the recipes can express a requirement on a higher CMake version for this, as @memsharded mentions above.

@braindigitalis
Copy link
Contributor Author

braindigitalis commented Oct 29, 2024

@memsharded having difficulty getting this to work on Conan center. everything I've done seems to conflict with what they are saying there e.g. must still support Conan 1.x? and ancient cmake from before 2019 seems to be a thing on their ci!

Regarding the CMake version - when Conan 2 was designed, it was done in consultation with industry experts - and Conan 2 itself aims to be compatible (where possible, for the broadest compatibility) with CMake 3.15 conan-io/tribe@main/design/004-tools-cmake.md

Please bear in mind that Conan Center has thousands of users, and what is "an ancient CMake from 2019" for some users, may actually be the version of CMake that users in enterprise environments are actually using - and we aim to support those as well. That's why the default in Conan Center CI is 3.15 in at least one of the platforms, so that we can also ensure broad compatibility with the recipes. It does not mean that all recipes need to be compatible with 3.15 - the recipes can express a requirement on a higher CMake version for this, as @memsharded mentions above.

we simply cannot support 3.15 in dpp. we make extensive use of 3.16 features in our makefiles, and do not support any compilers/toolchains with a cmake this old. Our oldest supported compiler is gcc 8 and msvc 2019, msvc 2019 comes with cmake 3.36 (iirc).

If supporting 3.15 is a requirement, we would simply not list on conan until this version is incremented, this library is not intended to run on systems that old.

For now, i'll put in the 3.16 requirement as mentioned.

@memsharded
Copy link

we simply cannot support 3.15 in dpp. we make extensive use of 3.16 features in our makefiles, and do not support any compilers/toolchains with a cmake this old. Our oldest supported compiler is gcc 8 and msvc 2019, msvc 2019 comes with cmake 3.36 (iirc).

No, but you don't have to! That is a requirement by Conan client, and a default base version for ConanCenter, but not mandatory at all.

You can very easily request and use any CMake version in ConanCenter, and you can do it with a simple tool_requires = "cmake/[>=3.27 <4]" in your recipe (or the minimum version you need). You don't even need to stick to 3.16, you can request any higher version and use in your CMakeLists.txt the newer CMake features you want.

So please, just add to your conanfile.py that tool_requires = "cmake/.... Thanks for the feedback!

@braindigitalis braindigitalis deleted the conan-the-librarian branch November 3, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issue or Pull Request related to the build process packages Issue or Pull Request related to packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package manager support
4 participants