Closed prince-chrismc closed 5 months ago
as this is quite a repeating patter in conan-center-index (right now, 10 occurrences for _minimum_compilers_version
), I support to make a tool for it to help recipe authors.
depending which substring you look for you get different lists! With different variations!
❗ I think this search is the best representative of the problem
$ grep -r 'self.output.warn(' | grep 'compiler' | wc -l
37
Recently we noticed the version compare was failing when settings.compiler.version = 6
and was minimum_version = 6.4
Now everywhere where the aforementioned code exists could benefit from the following blob
https://github.com/conan-io/conan-center-index/pull/3472/files#diff- f262a4dd3c2a23c9985dbbcf5ed0ae18bc3b8bca3fa0f80ffa8dcad9c075b421R32-R38
IMHO, we already have this functionality implemented in the check_min_cppstd
tool. This necessity arises because we are not using profiles with different cppstd values but the default ones, don't you think so?
Yes, I think that would cover the majority of use cases.
There's a very slim number which require minimum versions because the CXX standard in some compilers is only partial, just because you set C++17 does not mean it's all there.
EDIT: This happens https://github.com/conan-io/conan-center-index/pull/4631
Having a check_min_compiler_version
(or something like that) could still prevent this boilerplate in such cases. CCI had quite some recipe bugs because of bugs in these checks
And not all consumers have the cppstd in their profiles either
I agree that not all consumers have cppstd
in their profiles. We don't so far. It seems to me that the C++ standard belongs to the recipe, not the profile. I use one profile to build packages with different C++ standard requirements. Some require C++11, others C++14, others C++17.
I am currently working on a recipe where some versions of the package require C++11 and later versions require C++17. It would be good if this tool supported that - for example if _minimum_compilers_version()
took a cppstd
parameter instead of using a hardcoded _minimum_cpp_standard()
.
For example:
def _minimum_compilers_version(self, cppstd):
standards = {
"11": {
"Visual Studio": "15",
"gcc": "4.8",
"clang": "4",
"apple-clang": "9",
},
"17": {
"Visual Studio": "16",
"gcc": "7",
"clang": "5",
"apple-clang": "10",
},
}
return standards.get(cppstd) or {}
def validate(self):
cppstd = "11" if Version(self.version) <= "0.17.6" else "17"
if self.settings.compiler.get_safe("cppstd"):
tools.check_min_cppstd(self, cppstd)
min_version = self._minimum_compilers_version(cppstd).get(str(self.settings.compiler))
if not min_version:
self.output.warn("{} recipe lacks information about the {} compiler support.".format(
self.name, self.settings.compiler))
else:
if tools.Version(self.settings.compiler.version) < min_version:
raise ConanInvalidConfiguration("{} requires C++{} support. The current compiler {} {} does not support it.".format(
self.name, cppstd, self.settings.compiler, self.settings.compiler.version))
I have a PR open for quite a long time with a comprehensive implementation. While it provides a lot of different things - you can salvage a smaller scope tool.
Also related to https://github.com/conan-io/conan-center-index/issues/54
Closing as not planned - As CCI reviewers, we're ok with recipes repeating this pattern where necessary. Note that in CCI, it's mostly used for cppstd checks in Conan 1. Conan 2 models this property way better, so that the check becomes irrelevant in most cases.
The cases that would still need it would not benefit from having this logic abstracted out, because as commented here, there are lots of variations of the expected behaviour
Thanks everyone for your input!
There is this piece of boiler plate that get's copied a LOT in CCI, but it's also something large project that migrate to newer compilers need to handle as well...
Would this be something that could be added to
conans.tools
?