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

AutotoolsToolchain: empty None values from self fields + Refactor #11678

Merged

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Jul 20, 2022

Changelog: Bugfix: The AutotoolsToolchain now clears None values from the attributes .cxxflags, .cflags, .ldflags and .defines.
Changelog: Feature: The AutotoolsToolchain attributes .cxxflags, .cflags, .ldflags and .defines can be read at any moment, now is not needed to call .environment() to get them calculated. In the other hand, if you want to add additional flags the following attributes have to be used: .extra_cxxflags, .extra_cflags, .extra_ldflags and .extra_defines
Docs: conan-io/docs#2660

@lasote lasote added this to the 1.51 milestone Jul 20, 2022
@lasote lasote marked this pull request as ready for review July 20, 2022 13:53
@lasote lasote changed the title AutotoolsToolchain: empty None values from self fields AutotoolsToolchain: empty None values from self fields + Refactor Jul 20, 2022
Copy link
Contributor

@franramirez688 franramirez688 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great but I'm wondering if we really need those extra_*** attributes. The configurations self._conanfile.conf.get("tools.build:*****") are meant to add those extra flags so I think it could be a bit of a boilerplate mechanism. What do you think?

@lasote
Copy link
Contributor Author

lasote commented Jul 21, 2022

The mechanism was already there, I suppose that some recipes (only for the recipe) might need to introduce something in some configuration that Conan is not providing. I would love to remove the 80% of the customization power but I'm afraid we can't.

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

Successfully merging this pull request may close these issues.

None yet

3 participants