-
Notifications
You must be signed in to change notification settings - Fork 254
Make the workaround for absolute paths in depfiles compatible with macOS #809
Conversation
2dba836 to
39d7fc4
Compare
jmagman
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.
On master on macOS:
/fml/platform/android/fml.scoped_java_ref.o && sed -i 's@/b/f/w@../..@g' obj/flutter/fml/platform/android/fml.scoped_java_ref.o.d
sed: 1: "obj/flutter/fml/platfor ...": invalid command code o
On this PR: success.
| coverage_flags = "-fprofile-instr-generate -fcoverage-mapping" | ||
| } | ||
|
|
||
| if (host_os == "mac") { |
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'' that is, an empty string with no space after the -i should work on both, but maybe it's better to be explicit like 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.
In my local testing on macOS -i'' did not work.
It looked like it was being parsed as equivalent to -i, causing the next argument (the sed command) to be used as the backup file extension.
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.
Ah, maybe it only works with -e before the command to delineate the empty string from the command like: sed -i'' -e 's@/b/f/w@../..@g' $depfile.
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 tried that originally but found that it was creating a backup file with the extension -e added to the 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.
Hahaha. Okay. I'm glad you checked =)
|
New errors in roll: |
|
Fix for the unused variable error: #810 |
flutter/buildroot#804 flutter/buildroot#806 flutter/buildroot#808 flutter/buildroot#809 flutter/buildroot#810 Engine tests were already running on iOS 13. Part of flutter/flutter#140474 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
See flutter/engine#49542