-
Notifications
You must be signed in to change notification settings - Fork 989
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/message alias overridden #3456
Feature/message alias overridden #3456
Conversation
@@ -76,7 +78,7 @@ def _load_deps(self, node, down_reqs, dep_graph, public_deps, down_ref, down_opt | |||
param down_ref: ConanFileReference of who is depending on current node for this expansion | |||
""" | |||
# basic node configuration | |||
new_reqs, new_options = self._config_node(node, down_reqs, down_ref, down_options) | |||
new_reqs, new_options = self._config_node(node, down_reqs, down_ref, down_options, aliased) | |||
|
|||
self._resolve_deps(node, aliased, update, remote_name) |
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.
If _config_node
performs the alias substitution, then it is no longer needed in _resolve_deps
.
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.
Yes, it is necessary, because _resolve_deps
needs to check for alias AFTER the range has been resolved.
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.
And the work done in _config_node
does not require version ranges to be already resolved?
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.
Not necessary, this applies to normal requires that are aliased.
However, you are right, I need to review this, maybe could be improved, it is complicated the deps resolution...
Please wait until I can check it.
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 tried a few things, and tests are broken. Leaving it as-is.
95c7c8c
to
c990abf
Compare
* resolve alias earlier so they are not overridden * minor style
Close #3353
Changelog: Fix: Removed bad duplicated messages about dependency overriding when using conan alias.