-
Notifications
You must be signed in to change notification settings - Fork 993
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
python requires with alias #3957
Conversation
module, filename = parse_conanfile(path) | ||
|
||
# Check for alias | ||
conanfile = parse_module(module, filename) |
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 would want to use here the ConanFileLoader
class, but it requires a python_requires
member (and it is where I am!), see instantiation in conan_api.py here:
python_requires = ConanPythonRequire(self._proxy, resolver)
self._loader = ConanFileLoader(self._runner, self._user_io.out, python_requires)
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 think I'll prefer to really have an injected loader object into PythonRequires (yes, runtime cycle between objects).
We could make ConanFileLoader
be the one that really instantiates PythonRequires
object, passing a referent to itself to it.
This can't go to 1.9.2, a bit risky in my opinion.
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.
As commented previously, I think this needs architectural changes.
I think I'll prefer to really have an injected loader object into PythonRequires (yes, runtime cycle between objects).We could make ConanFileLoader be the one that really instantiates PythonRequires object, passing a referent to itself to it.
I'm having a look at this again, and it is not immediate. I think it is worth a different architectural PR, maybe creating the |
* fix python_requires recursive alias * context to make it cleaner * fix tests * unify load_class/module * remove imports * improved parse_conanfile
Please, review again (and consider merging)
python requires with alias
Changelog: Fix: Resolve aliased packages in python_requires