Remove chmod to make Flutter framework headers unwritable#45189
Remove chmod to make Flutter framework headers unwritable#45189jmagman merged 1 commit intoflutter:masterfrom
Conversation
fluttergithubbot
added
the
tool
| RunCommand cp -r -- "${flutter_podspec}" "${derived_dir}/engine" | ||
| RunCommand cp -r -- "${flutter_framework}" "${derived_dir}/engine" | ||
| # Make headers, plists, and modulemap files read-only to discourage editing. | ||
| RunCommand find "${derived_dir}/engine/Flutter.framework" -type f \( -name '*.h' -o -name '*.modulemap' -o -name '*.plist' \) -exec chmod a-w "{}" \; |
There was a problem hiding this comment.
Could we prepend a warning message onto the file if there isn't one there already? Like:
... -exec cat <(echo "DO NOT EDIT") "{}" \;
Or maybe put a message there directly in the engine repo.
There was a problem hiding this comment.
LGTM!
It's worth noting that this was less a gentle hint, and more us spending a fairly large amount of time dealing with confusing bug reports that we eventually got traced to developers having unintentionally edited the headers in ways that caused compile errors (typically through jump to header in Xcode then accidentally typing/deleting things).
If this is causing issues with code signing though, an alternative would be to check for changes in the tool (e.g. verify shas or check diffs against the SDK version, etc.). I'm not super convinced that adding "DO NOT EDIT" will solve the issue (though it can't hurt!) since several of these turned out to be developers who could verify the headers had diffs, but had no recollection of editing them. Maybe accidental keypresses or something.
I wish |
6 participants
Description
I believe code signing issues are being manifested as permission errors on these files. Remove the chmod--it's become more problematic than helpful (it was trying to give a gentle hint to not edit those header files) and is hiding potentially actionable code signing error messages.
Related Issues
Fixes #40146.
Also see #39507.
Originally introduced with #8748
Checklist
///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change