-
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
Delegate error handling to command framework instead of graph formatters #16415
Delegate error handling to command framework instead of graph formatters #16415
Conversation
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 👍
return install_order_serialized | ||
|
||
return {"build_order": install_order_serialized, | ||
"conan_error": result.get_errors()} |
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.
Is this even possible? How can a build-order-merge have errors?
Make conan graph <subcommand> a real "install" dry-run. Return same errors and give same messages
1c4a009
to
d89b5b9
Compare
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 better, getting closer! :)
conans/client/graph/install_graph.py
Outdated
def get_errors(self): | ||
try: | ||
self.raise_errors(basic_output=True) | ||
except ConanException as e: | ||
return str(e) | ||
return None |
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.
It looks like that instead of modifying the _raise_missing
and doing this, it would be better to craft its own simpler message here.
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.
But raise_errors
could potentially raise invalid binaries errors also.
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, sure, it can iterate all packages and add an error message line for each invalid and missing.
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.
Good, it can be moved forward
Co-authored-by: Abril Rincón Blanco <git@rinconblanco.es>
Changelog: Bugfix: Make conan graph a real "install" dry-run. Return same errors and give same messages
Docs: omit
Close #16393
develop
branch, documenting this one.