#123 Custom attributes must be protected

[uilianries]

• I need to compare if the attributes in the conanfile passed to this hook, contains more attributes and if they are protected. However, I need to compare using a default ConanFile object, which is not loaded by basic_load, because it requires a conan recipe (file) to be loaded. Also, the loaded injects some attributes, which are not present in the Conanfile, so I checked them adding the attributes by their names (conan_data and python_requires)

closes #123

Wiki:

#KB-H035: "CUSTOM ATTRIBUTES"

When a new attribute have to be added to the recipe, and it is not a default one provided by ConanFile, it must added as protected member:

class Recipe(ConanFile):
    settings = "os", "arch" # default attributes are public
    _my_attr = "foobar" # custom attributes are protected
    __private_attr = "qux" # private should not be used. Use protected instead.

[jgsogo]

A little bit afraid of this one. It can break if we add to Conan any field (also dynamic ones) and we might not notice it until all the PRs in ConanCenter fail. For example, here you are missing python_requires_extend, but there are other fields we all might not be aware right now.

This is a nice hook, but I will make it a warning, not an error. wdyt?

[uilianries]

It can break if we add to Conan any field (also dynamic ones) and we might not notice it until all the PRs in ConanCenter fail.

Do you mean protected Conan fields? But we are not allowed to use it in CCI because we can't ensure them for breaking, we only use public and exported methods.

For example, here you are missing python_requires_extend, but there are other fields we all might not be aware right now.

I didn't understand, python_requires_extend is public, it won't be blocked by this hook. Is there any protected field that we are using and should be public?

This is a nice hook, but I will make it a warning, not an error. wdyt?

The problem using warning level is that we need to build when reviewing and checking the log, OR read output logs from CI. IMO it doesn't work, repetitive human review is susceptible to failure.

[SSE4]

python_requires_extend is mentioned twice in the docs, but appears to be undocumented on its own. what does it do?

[uilianries]

@SSE4 Indeed the example on https://docs.conan.io/en/latest/extending/python_requires.html#extending-base-classes shows its usage, but I didn't find any reference with details.

[jgsogo]

My concern is that you have added conan_data and python_requires by hand. Any other field can pop up in the future and this hook will fail. Probably this is not as bad as I'm thinking about because we can iterate really fast in this repo to handle the new member if needed. So ok, not a problem.

@SSE4 python_requires_extend is the way to inherit from a base class inside a python requires. Conan injects this class as a base class to the conanfile in runtime. Why? We need to be able to load the python file with the recipe and the python requires information before resolving the classes/methods from python_requires (if we use Python inheritance we need to retrieve python_requires first and the interpreter will use the base class). This let Conan control the python_requires.

Agree it needs better docs, it should appear in the text, not only in the example.

[uilianries]

Nice explanation, thanks @jgsogo !

[SSE4]

@jgsogo thanks for the explanation. I believe python_requires_extend should be documented there. I'll create an issue for docs.

[jgsogo]

At least, here we need python_requires_extend too. And for 1.26 we should handle new attributes/functions that could result of this PR conan-io/conan#6945