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 handle the Standard version requirement in recipes? #54

Closed
Minimonium opened this issue Sep 15, 2019 · 8 comments
Closed
Labels
question Further information is requested

Comments

@Minimonium
Copy link
Contributor

Recipes should handle problems of incompatible standard version flags gracefully and raise ConanInvalidConfiguration appropriately.

@Minimonium Minimonium mentioned this issue Sep 15, 2019
4 tasks
@Minimonium
Copy link
Contributor Author

Minimonium commented Sep 15, 2019

@lasote

That's a good question, because we are not applying different cppstd settings values in the input profiles. Catch2 is header only right? So it needs the "test_package" code to enable de c++11 support. But we don't want to raise ConanInvalidConfiguration in the catch2, if the recipe is correct, the package_id for all the applied configurations should be the same (header only).
We don't have an answer yet, could you please open an issue?
In the meantime might be ok to enable the CXX11 in the CMakeLists of the test_package.

We already have the mechanism to hash packages with aligned default standard versions with the same value: https://docs.conan.io/en/latest/reference/conanfile/methods.html#self-info-default-std-matching-self-info-default-std-non-matching

I think we don't have the same mechanism to put into the configure function (to raise the ConanInvalidConfiguration as @SSE4 mentioned) to check for the minimum required value (standard version range if you want)? I think it's the right way to address the problem.

@danimtb danimtb added the question Further information is requested label Sep 16, 2019
@lasote
Copy link
Contributor

lasote commented Sep 17, 2019

We could follow 2 approaches, not necessarily incompatible:

  • We would need probably a tool that report the applied compiler.cppstd setting taking into account that the input could be None and the applied one is the default from the applied compiler. That way we could raise if the minimum is not satisfied and the conan-index CI service will automatically skip the profiles with the compiler without a compiler that supports the minimum cppstd. But if the library only supports >=17 we might have an issue if we don't explicitly apply a profile with the cppstd.

  • We could always add something to the config.yml file to indicate the cppstd value (vaues? which ones?) to apply for that version.

@Minimonium
Copy link
Contributor Author

I was thinking about the first approach, yes. Problems >=17 could arise when redistributing such package, since now it'd be wrong, isn't it?
I'm a bit confused about the second approach. Could you please explain it a bit more?

@lasote
Copy link
Contributor

lasote commented Sep 25, 2019

Sure. Currently, the CI system is applying a default set of profiles. None of them contain compiler.cppstd declared. So no binary packages are created with other compiler.cppstd than the default of the compiler. So, if a library is only compatible with >17 what should we do? We could somehow add information to the config.yml file to force the system to create profiles for 17 (why not 20?). But anyway, we won't generate binaries for the rest of libraries for 17 standard, so... I don't know

@Minimonium
Copy link
Contributor Author

Okay. I'm not sure I agree with such an approach since the question of the standard version is on the toolchain, not a global configuration. I'm not sure how to reliably generate a default version even.

So I'd consider the first approach. Thanks for the input. 🙂

@Minimonium
Copy link
Contributor Author

For recipes that require a C++11, should we throw an error when the cppstd setting is missing on compilers with an older default standard?

@Minimonium
Copy link
Contributor Author

Okay, I think now I get it how we should do.

The thing with cppstd is that we really need to consider the case when it's not specified to be the case of using the default standard version of the current compiler. The reason for that is because while standard libraries don't usually break ABI, projects are free to do it between different versions. For example to conditionally use std::string_view is the standard version is allowed.
On the other hand, projects that are compatible with older standard versions should probably delete the setting in the package_id.

All it means that indeed if a project requires a >=17 than we need to handle it in CCI builds. How can we achieve it?

About the algorithm to handle the package configuration:

  • We check the minimal standard version required with the check_min_cppstd. We need to handle if the tool doesn't know about the current compiler, it could be a customer provided one.
  • We check the minimal compiler version supported. The standard version support is a very fluid thing, can't rely just on that.
  • We need to warn if we lack information about any of these.

We can nicely wrap it into an additional tool.

datalogics-robb pushed a commit to datalogics-robb/conan-center-index that referenced this issue May 31, 2023
conan-center-bot pushed a commit that referenced this issue Dec 14, 2023
* xmlsec: bump

* Bump/xmlsec/all (#54)

* xmlsec/all: bump deps

Generated and committed by [Conan Center Bump Deps](https://github.com/ericLemanissier/conan-center-index-bump-deps)
Find more updatable recipes in the [GitHub Pages](https://ericLemanissier.github.io/conan-center-index-bump-deps/)

* rebump

* unbump libxml2

* simplify test package

* remove 1.3.2

* Update conanfile.py

* Update conanfile.py
hoyhoy pushed a commit to hoyhoy/conan-center-index that referenced this issue Dec 18, 2023
* xmlsec: bump

* Bump/xmlsec/all (conan-io#54)

* xmlsec/all: bump deps

Generated and committed by [Conan Center Bump Deps](https://github.com/ericLemanissier/conan-center-index-bump-deps)
Find more updatable recipes in the [GitHub Pages](https://ericLemanissier.github.io/conan-center-index-bump-deps/)

* rebump

* unbump libxml2

* simplify test package

* remove 1.3.2

* Update conanfile.py

* Update conanfile.py
valgur pushed a commit to valgur/conan-center-index that referenced this issue Jan 1, 2024
* xmlsec: bump

* Bump/xmlsec/all (conan-io#54)

* xmlsec/all: bump deps

Generated and committed by [Conan Center Bump Deps](https://github.com/ericLemanissier/conan-center-index-bump-deps)
Find more updatable recipes in the [GitHub Pages](https://ericLemanissier.github.io/conan-center-index-bump-deps/)

* rebump

* unbump libxml2

* simplify test package

* remove 1.3.2

* Update conanfile.py

* Update conanfile.py
@Croydon
Copy link
Contributor

Croydon commented Jun 9, 2024

This issue can be closed

conan-io/conan#8002 (comment)

@danimtb danimtb closed this as completed Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants