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] ability to call cppstd helpers in conan.tools.build within a package_id() method #16147

Closed
1 task
jcar87 opened this issue Apr 25, 2024 · 4 comments
Closed
1 task
Assignees
Milestone

Comments

@jcar87
Copy link
Contributor

jcar87 commented Apr 25, 2024

What is your question?

example use case

from conan.tools.build import valid_max_cppstd

def package_id():
    if self.info.settings.get_safe("compiler.cppstd") and not valid_max_cppstd(self, "14"):
        self.info.settings.compiler.cppstd = "14"

def generate():
        cppstd = self.settings.get_safe("compiler.cppstd")
        if cppstd and not valid_max_cppstd(self, "14"):
            tc.cache_variables["CMAKE_CXX_STANDARD"] = "14"

This fails because the implementation of valid_max_cppstd() accessses self.settings which is not allowed within a package_id()

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@valgur
Copy link
Contributor

valgur commented Apr 25, 2024

YES, PLEASE. This is quite a headache when trying to re-use the same checks in package_id and elsewhere. self.settings and self.options should not be modifiable in package_id(), but it would be great to be able to do read-only access to these values as mentioned in the issue.

Doing valid_max_cppstd(self.info, "14") can be used in the above snippet as a workaround, though.

@memsharded
Copy link
Member

@jcar87 I am checking this based on my PR #16456 because it is a bit dirty.

I am thinking that:

if self.info.settings.get_safe("compiler.cppstd") and not valid_max_cppstd(self, "14"):
     self.info.settings.compiler.cppstd = "14"

in the package_id() doesn't make a lot of sense anymore. With the new compatibility() method and the compatibility plugin the original information used to create the binary can be respected and maintained (otherwise the above produces information erasure) while the desired binary compatibility can be defined.

@memsharded
Copy link
Member

def generate():
        cppstd = self.settings.get_safe("compiler.cppstd")
        if cppstd and not valid_max_cppstd(self, "14"):
            tc.cache_variables["CMAKE_CXX_STANDARD"] = "14"

This wouldn't be aligned with the general strategy in ConanCenter packages @jcar87. This is overwriting and hardcoding defining settings, why many recipes do raise a validate() if my compiler.cppstd is not at least 17, and there would be other recipes that can ignore that and build with an hardcoded different cppstd than the defined one without even notifying users.

@memsharded memsharded added this to the 2.5.0 milestone Jun 21, 2024
@memsharded memsharded modified the milestones: 2.5.0, 2.6.0 Jul 1, 2024
@memsharded memsharded modified the milestones: 2.6.0, 2.7.0 Jul 31, 2024
@memsharded memsharded modified the milestones: 2.7.0, 2.8.0 Aug 27, 2024
@memsharded
Copy link
Member

This has been solved with the approach in #16871

The solution is to allow the --build=compatible new policy to be able to build a compatible one, instead of using a non correct cppstd information inpackage_id or forcing the cppstd value inside the recipe.

The above practices, doing tc.cache_variables["CMAKE_CXX_STANDARD"] = "14" and self.info.settings.compiler.cppstd = "14" in recipes are discouraged.

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.

3 participants