-
Notifications
You must be signed in to change notification settings - Fork 988
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
[feature] Manage shared, fPIC and header_only options automatically #14194
[feature] Manage shared, fPIC and header_only options automatically #14194
Conversation
Is it possible to add a new recipe attribute later, only working with Conan 2.x, that could be a shortcut to those new methods? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a comment about the Conan private methods, to see what you all think, but otherwise it is looking great so far :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far!
@uilianries I think we already discussed this before starting the implementation and preferred not to bother with attributes as it will be less intuitive. Maybe in the future, we change our minds, but at the moment it is very straightforward to just declare the options or package type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great, good job @danimtb! Thanks for taking the initiative on it and implementing it :)
I have two minor questions besides the reviews that everyone else has already done, the rest looks great, congrats!
conan/tools/options/helpers.py
Outdated
|
||
|
||
def handle_common_package_id_options(conanfile): | ||
if conanfile.options.get_safe("header_only") or conanfile.package_type == PackageType.HEADER: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that if by the point this gets executed the package type has already been properly computed, why rely on the option? I don't actually know if having it is better or worse, so happy to hear your opinion :)
Co-authored-by: James <memsharded@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :)
Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es>
Changelog: Feature: Manage shared, fPIC, and header_only options automatically.
Changelog: Feature: Manage package ID for header-library package type automatically.
Docs: conan-io/docs#3296
develop
branch, documenting this one.