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

Add implements attribute in ConanFile to provide opt-in automatic management of some options and settings. #14320

Merged

Conversation

czoido
Copy link
Contributor

@czoido czoido commented Jul 18, 2023

Changelog: Feature: Add implements attribute in ConanFile to provide automatic management of some options and settings.
Docs: conan-io/docs#3303

Closes: #14308

@czoido czoido added this to the 2.0.9 milestone Jul 18, 2023
@czoido czoido changed the title [WIP] Explore other more explicit alternatives of managing the options automatically Explore other more explicit alternatives of managing the options automatically Jul 18, 2023
@czoido czoido changed the title Explore other more explicit alternatives of managing the options automatically Explore other more explicit alternatives to automatically manage the options Jul 18, 2023
@memsharded
Copy link
Member

Did some minor changes:

  • Replaced getattr for normal access
  • Changed GenConanFile specialized for generic with_class_attribute

Add options_auto_handle=True to the conan new template?

Probably not yet? The advantage of the current template is that it will work too as-is in latest Conan 1.X

@@ -10,7 +10,7 @@ def run_configure_method(conanfile, down_options, profile_options, ref):
if hasattr(conanfile, "config_options"):
with conanfile_exception_formatter(conanfile, "config_options"):
conanfile.config_options()
else:
elif conanfile.activate_default_behaviors:
Copy link
Member

@memsharded memsharded Jul 18, 2023

Choose a reason for hiding this comment

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

Don't love the name, but challenging to find a better one. Some ideas:

  • auto_configuration = True
  • manage_options = True
  • auto_manage_library_type_configuration = True
  • auto_manage_library_configuration = True
  • auto_library_configuration = True
  • library_type_defaults = True

Generic list of things?

  • auto_manage = ["library_type_configuration", "app_type_configuration"]

  • auto_manage = ["library_configuration", "app_configuration"]

  • auto = ["library_configuration", "app_configuration"]

  • policies = ["auto_shared_fpic", "auto_header_only", "auto_cmake"]

  • auto_manage = ["shared_fpic", "header_only"]

  • rules, auto, defaults, implicits, auto_behaviors, mods, builtins, mixin, recipe_auto, recipe_defaults, config_templates

  • implements = ["auto_shared_fpic", "cmake_template"]

  • applies = [

  • sidecar = ["auto_shared_fpic", ]

  • recipe_presets = [

  • salt_and_pepper = ["auto_shared_fpic"

  • pre-cooked = ["auto_shared_

build_policy = "missing", upload_policy = "skip"

The language case would be a different one most likely:

  • languages = "C", "C++", "fortran"

Maybe "configuration" and "options" should be in the var name?

FINAL DECISION:

  • implements = ["auto_shared_fpic", "auto_header_only"]

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. We should think of another shorter one if possible.

Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

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

I like the attribute name "activate_default_behavior" (without S at the end).

@@ -20,7 +20,7 @@ def run_configure_method(conanfile, down_options, profile_options, ref):
if hasattr(conanfile, "configure"):
with conanfile_exception_formatter(conanfile, "configure"):
conanfile.configure()
else:
elif conanfile.activate_default_behaviors:
default_configure(conanfile)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
default_configure(conanfile)
if ... => auto_shared_fpic_configure(conanfile)
if ... => auto_header_only_configure(conanfile)

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Drop the public methods



def auto_shared_fpic_configure(conanfile):
if conanfile.options.get_safe("header_only"):
Copy link
Member

Choose a reason for hiding this comment

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

Should this if be part of the header_only automation? Maybe not, I guess the meaning of auto-shared_fpic is "you can remove these options if they are not necessary"

Copy link
Member

Choose a reason for hiding this comment

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

I think is fine as is but would require a clear explanation in the docs

conans/client/conanfile/configure.py Outdated Show resolved Hide resolved
Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

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

I like it!

@czoido czoido marked this pull request as ready for review July 19, 2023 12:20
@uilianries
Copy link
Member

Just like I asked before, a magic attribute 😝

@czoido czoido merged commit 2dde121 into conan-io:release/2.0 Jul 19, 2023
@czoido czoido changed the title Explore other more explicit alternatives to automatically manage the options Add implements attribute in ConanFile to provide opt-in automatic management of some options and settings. Jul 19, 2023
@czoido czoido mentioned this pull request Jul 19, 2023
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 this pull request may close these issues.

fPIC
5 participants