-
Notifications
You must be signed in to change notification settings - Fork 6k
Actually print out if xcrun metal fails #39926
Conversation
My previous commit was exiting with an unhelpful error that did not contain the failure output. This prints the failure output more nicely.
jonahwilliams
left a comment
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.
RSLGTM
|
|
||
| subprocess.check_output(command, stderr=subprocess.STDOUT) | ||
| try: | ||
| subprocess.check_output(command, stderr=subprocess.STDOUT, text=True) |
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.
The presubmits will tell, but hopefully CI has a recent enough python for text=True.
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'm pretty sure we do. I was going to use subprocess.run here but I'm not sure we have a recent enough for that. If it fails I'll change this.
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.
impeller/tools/build_metal_library.py:116:2: C0103: Variable name "e" doesn't conform to '[a-z_][a-z0-9_]{2,30}$' pattern (invalid-name)
For real?
|
auto label is removed for flutter/engine, pr: 39926, due to - The status or check suite Linux Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
auto label is removed for flutter/engine, pr: 39926, due to - The status or check suite Mac iOS clang-tidy has failed. Please fix the issues identified (or deflake) before re-applying this label. |
My previous commit was exiting with an unhelpful error that did not contain the failure output. This prints the failure output more nicely.