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

[bug] conan.tools.build.check_min_cppstd does infer default cppstd as documented #16338

Closed
puetzk opened this issue May 24, 2024 · 3 comments · Fixed by #16341
Closed

[bug] conan.tools.build.check_min_cppstd does infer default cppstd as documented #16338

puetzk opened this issue May 24, 2024 · 3 comments · Fixed by #16341
Assignees
Milestone

Comments

@puetzk
Copy link

puetzk commented May 24, 2024

Describe the bug

https://docs.conan.io/2/reference/tools/build.html#conan-tools-build-check-min-cppstd says that check_*_cppstd should be able to infer the default cppstd from compiler.version in cases where it has not been overridden:

It not settings.compiler.cppstd, the tool will use compiler to compare (reading the default from cppstd_default)

And does contain a function to derive this information

def default_cppstd(conanfile, compiler=None, compiler_version=None):

However, it doesn't seem to actually call this function anywhere - if the profile does not expliticly set compiler.cppstd, it simply errors out with

This doesn't match the documentation, or the behavior of the old (conan 1.x) conans.tools.check_min_cppstd, where such defaulting was also documented and did actually work.

test/1.0: Invalid: The compiler.cppstd is not defined for this configuration

I found a note at #11003 (comment) suggesting that the intent might have been to make cppstd compulsory, but if so that seemingly never happened - the default settings.yml is still happy to allow compiler.cppstd =null,

cppstd: [null, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20, 23, gnu23]

and this still causes the same

current_cppstd = conanfile.settings.get_safe("compiler.cppstd")
if current_cppstd is None:
raise ConanInvalidConfiguration("The compiler.cppstd is not defined for this configuration")

in conan 2.3.1 that it does in conan 1.x

So it seems that either the code should support defaults (as its documentation says, and its predecessor did) or the documentation should at least not claim that it does. Preferably the former, since the documented behavior matches the old tools and seems desireable (there's really no reason validate() needs to fail if the compiler's defaults satisfy it...

How to reproduce it

ubuntu 22.04 (11.4.0)

from conan import ConanFile
from conan.tools.build import check_min_cppstd

class TestConan(ConanFile):
    name = "test"
    version = "0.1"

    def validate(self):
        check_min_cppstd(self, "14")

conan create .

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.libcxx=libstdc++11
compiler.version=11
os=Linux

Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.libcxx=libstdc++11
compiler.version=11
os=Linux

======== Computing dependency graph ========
Graph root
cli
Requirements
test/0.1#12fed99e8a78e059aec5714f29af6be4 - Cache

======== Computing necessary packages ========
Requirements
test/0.1#12fed99e8a78e059aec5714f29af6be4:da39a3ee5e6b4b0d3255bfef95601890afd80709 - Invalid
ERROR: There are invalid packages:
test/0.1: Invalid: The compiler.cppstd is not defined for this configuration

@memsharded
Copy link
Member

Hi @puetzk

Thanks for your report.

https://docs.conan.io/2/reference/tools/build.html#conan-tools-build-check-min-cppstd says that check_*_cppstd should be able to infer the default cppstd from compiler.version in cases where it has not been overridden:

I think this is a docs bug, it totally looks like a copy & paste from legacy Conan 1 docs of that function. The docs in https://docs.conan.io/2/reference/tools/build.html#conan-tools-build-check-min-cppstd are generated from the docstring, and such docstring was copied verbatim from Conan 1). In Conan 2 it was never the intention to deduce cppstd from compiler version, but compiler.cppstd is recommended to be explicitly defined in most cases.

I'll do a PR to change those docstrings.

@memsharded memsharded added this to the 2.4.0 milestone May 26, 2024
@memsharded memsharded self-assigned this May 26, 2024
@puetzk
Copy link
Author

puetzk commented May 30, 2024

Thanks for clarifying the intent. I just got surprised when converting conans.tools.check_min_cppstd to conan.tools.build.check_min_cppstd caused build failures, since the docs said the behavior was the same and that wasn't actually the case...

Now the docs at least match reality. I can certainly fix the failures by adding compiler.cppstd to all our profiles (like conan 2's default detection would have).

@memsharded
Copy link
Member

Now the docs at least match reality. I can certainly fix the failures by adding compiler.cppstd to all our profiles (like conan 2's default detection would have).

Sounds good, in Conan 2 it is expected that profiles define compiler.cppstd in most cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants