Keep Flutter.framework binaries writable so they can be code signed by jmagman · Pull Request #39539 · flutter/flutter

jmagman · 2019-08-30T01:35:19Z

Description

Flutter.framework files were made read-only to "Provide a strong hint to developers that editing Flutter framework headers isn't supported" (cb2b89c) However this is making the Flutter binary readonly, which is being copied into the final embedded Frameworks directories, then can't be codesigned.

Related Issues

Fixes #39507.

Tests

Turn back on add2app shard.

Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

jmagman · 2019-08-30T01:38:25Z

This is a bit speculative. See #39507 (comment).

The original logic was introduced in 2017, so what changed?
I wasn't able to reproduce this in a non-module, so what's the difference?
This seems flaky. Even though according to a git bisect this was exposed 20 days ago with 78cca62, there are many Cirrus runs of add2app that succeeded. So why is this flaky?

jmagman · 2019-08-30T17:25:04Z

First add2app-macos test passed. Will keep rebasing to see if it's still flaky.

jmagman · 2019-08-30T18:17:34Z

First add2app-macos test passed. Will keep rebasing to see if it's still flaky.

Passed a second time.

jmagman · 2019-08-30T23:19:38Z

First add2app-macos test passed. Will keep rebasing to see if it's still flaky.

Passed a second time.

add2app-macos test failed, but got way past signing permissions to a flaky unit test? So not from this change (though still concerning).
https://cirrus-ci.com/task/4538022030213120

jmagman · 2019-09-03T17:31:45Z

First add2app-macos test passed. Will keep rebasing to see if it's still flaky.

Passed a second time.

add2app-macos test failed, but got way past signing permissions to a flaky unit test? So not from this change (though still concerning).
https://cirrus-ci.com/task/4538022030213120
Cirrus CI
[Remove](#)


  [Instruction test_all failed in 02:52](https://cirrus-ci.com/task/4538022030213120)




  
    ✅ 01:41 clone
✅ 00:01 print_date
✅ 00:03 install_cocoapods
✅ 01:06 git_fetch
✅ 05:45 setup
✅ 00:02 setup_xcpretty
❌ 02:52 test_all

> > [[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Full Screen (Cold)")]
> > performAction:grey_tap()];
> > ```
> > 
> > testFullScreenCanPop, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
> > /tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:41
> > ```
> > 
> > 
> > [[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Full Screen (Cold)")]
> > performAction:grey_tap()];
> > ```
> > 
> > testHybridView, Exception: NoMatchingElementException
> > /tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:114
> > ```
> > 
> > 
> > [[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Hybrid View (Warm)")]
> > performAction:grey_tap()];
> > ```
> > 
> > testHybridView, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
> > /tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:114
> > ```
> > 
> > 
> > [[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Hybrid View (Warm)")]
> > performAction:grey_tap()];
> > ```
> > 
> > 
> >    Executed 4 tests, with 6 failures (6 unexpected) in 22.883 (22.919) seconds
> > 2019-08-30 14:50:15.653 xcodebuild[16360:33968] [MT] IDETestOperationsObserverDebug: 103.533 elapsed -- Testing started completed.
> > 2019-08-30 14:50:15.654 xcodebuild[16360:33968] [MT] IDETestOperationsObserverDebug: 0.000 sec, +0.000 sec -- start
> > 2019-08-30 14:50:15.654 xcodebuild[16360:33968] [MT] IDETestOperationsObserverDebug: 103.533 sec, +103.532 sec -- end
> > 
> > Failing tests:
> >   ios_add2appTests:
> >   	-[FlutterTests testDualFlutterView]
> >   	-[FlutterTests testDualFlutterView]
> >   	-[FlutterTests testFullScreenCanPop]
> >   	-[FlutterTests testFullScreenCanPop]
> >   	-[FlutterTests testHybridView]
> >   	-[FlutterTests testHybridView]
> > 
> > Test session results and logs:
> >   /Users/anka/Library/Developer/Xcode/DerivedData/ios_add2app-csiwhdwugybmwjeggtidemdjpolj/Logs/Test/Test-ios_add2appTests-2019.08.30_14-47-54--0700.xcresult
> > 
> > ** TEST FAILED **
> > 
> > ▌14:50:22▐ ELAPSED TIME: 3min 53.429s for ../../../build_and_test.sh  in dev/integration_tests/ios_add2app
> > ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > ERROR: Last command exited with 65 (expected: zero).
> > Command: ../../../build_and_test.sh 
> > Relative working directory: dev/integration_tests/ios_add2app
> > ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> > ```
> > 
> > 
> >       
> >       
> >        Show more
> >        Show less

Test Suite ios_add2appTests.xctest started
FlutterTests
    ✗ testDualFlutterView, Exception: NoMatchingElementException
    ✗ testDualFlutterView, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
    ✗ testFullScreenCanPop, Exception: NoMatchingElementException
    ✗ testFullScreenCanPop, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
    ✗ testHybridView, Exception: NoMatchingElementException
    ✗ testHybridView, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
ViewControllerRelease
    ✓ testReleaseFlutterViewController (2.455 seconds)
FlutterTests
  testDualFlutterView, Exception: NoMatchingElementException
  /tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:96

// Verify that there are two Flutter views with the expected marquee text.
[[[EarlGrey
selectElementWithMatcher:grey_accessibilityLabel(@"This is Marquee")]

testDualFlutterView, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
/tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:96

// Verify that there are two Flutter views with the expected marquee text.
[[[EarlGrey
selectElementWithMatcher:grey_accessibilityLabel(@"This is Marquee")]

testFullScreenCanPop, Exception: NoMatchingElementException
/tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:41

[[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Full Screen (Cold)")]
performAction:grey_tap()];

testFullScreenCanPop, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
/tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:41

[[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Full Screen (Cold)")]
performAction:grey_tap()];

testHybridView, Exception: NoMatchingElementException
/tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:114

[[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Hybrid View (Warm)")]
performAction:grey_tap()];

testHybridView, failed: caught "EarlGreyInternalTestInterruptException", "Immediately halt execution of testcase"
/tmp/flutter sdk/dev/integration_tests/ios_add2app/ios_add2appTests/IntegrationTests.m:114

[[EarlGrey selectElementWithMatcher:grey_buttonTitle(@"Hybrid View (Warm)")]
performAction:grey_tap()];

   Executed 4 tests, with 6 failures (6 unexpected) in 22.883 (22.919) seconds

jmagman · 2019-09-03T19:59:45Z

First add2app-macos test passed. Will keep rebasing to see if it's still flaky.

Passed a second time.

Passed (the step in question) a fourth time. Removing WIP.

@cbracken Does any of this make sense to you? #39539 (comment)

cbracken

cbracken · 2019-09-04T00:07:32Z

packages/flutter_tools/bin/xcode_backend.sh

    mkdir "${derived_dir}/engine"
    RunCommand cp -r -- "${flutter_podspec}" "${derived_dir}/engine"
    RunCommand cp -r -- "${flutter_framework}" "${derived_dir}/engine"
-    RunCommand find "${derived_dir}/engine/Flutter.framework" -type f -exec chmod a-w "{}" \;


Maybe add a comment noting this is marking all non-binary files as read-only so that we can code-sign the binaries.

I also wonder if there's any way to get the same list using only options to find rather than feeding it through grep -Il then into xargs.

Looking at the manpage, I don't see anything promising though.

That was my first thought but I couldn't find any such flag.

jmagman · 2019-09-04T01:00:38Z

Even though I can't prove this will fix the flaky add-to-app test, I'm going to merge it since I still think it's a good change. If we continue to see the flakes we can try to gather more info about the environments where this is reproducing.

Hixie · 2019-09-05T00:33:44Z

This broke devicelab, because now we can't delete the flutter repo anymore.

…signed (#39539)" This reverts commit 72cacb4.

…signed (#39539)" (#39859) This reverts commit 72cacb4.

jmagman added tool Affects the "flutter" command-line tool. See also t: labels. a: existing-apps Integration with existing apps via the add-to-app flow t: xcode "xcodebuild" on iOS and general Xcode project management labels Aug 30, 2019

jmagman self-assigned this Aug 30, 2019

googlebot added the cla: yes label Aug 30, 2019

xster requested a review from cbracken August 30, 2019 03:45

jmagman force-pushed the writable branch from 143849f to c1a1eb7 Compare August 30, 2019 17:25

jmagman force-pushed the writable branch from c1a1eb7 to 94cba92 Compare August 30, 2019 21:22

Keep Flutter.framework binaries writable so they can be code signed

1d94dbf

jmagman force-pushed the writable branch from 94cba92 to 1d94dbf Compare September 3, 2019 17:32

jmagman changed the title ~~WIP Keep Flutter.framework binaries writable so they can be code signed~~ Keep Flutter.framework binaries writable so they can be code signed Sep 3, 2019

cbracken approved these changes Sep 4, 2019

View reviewed changes

Add comment

b8f3438

jmagman merged commit 72cacb4 into flutter:master Sep 4, 2019

jmagman deleted the writable branch September 4, 2019 01:00

Hixie added a commit that referenced this pull request Sep 5, 2019

Revert "Keep Flutter.framework binaries writable so they can be code …

64dcffb

…signed (#39539)" This reverts commit 72cacb4.

Hixie mentioned this pull request Sep 5, 2019

Revert "Keep Flutter.framework binaries writable so they can be code signed" #39859

Merged

Hixie added a commit that referenced this pull request Sep 5, 2019

Revert "Keep Flutter.framework binaries writable so they can be code …

2efbc11

…signed (#39539)" (#39859) This reverts commit 72cacb4.

jmagman mentioned this pull request Sep 10, 2019

Keep Flutter.framework binaries writable so they can be code signed #40174

Merged

9 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep Flutter.framework binaries writable so they can be code signed#39539

Keep Flutter.framework binaries writable so they can be code signed#39539
jmagman merged 2 commits intoflutter:masterfrom
jmagman:writable

jmagman commented Aug 30, 2019 •

edited

Loading

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Sep 3, 2019

Uh oh!

jmagman commented Sep 3, 2019

Uh oh!

cbracken left a comment

Uh oh!

cbracken Sep 4, 2019

Uh oh!

cbracken Sep 4, 2019

Uh oh!

jmagman Sep 4, 2019

Uh oh!

jmagman commented Sep 4, 2019

Uh oh!

Hixie commented Sep 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jmagman commented Aug 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Aug 30, 2019

Uh oh!

jmagman commented Sep 3, 2019

Uh oh!

jmagman commented Sep 3, 2019

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

jmagman Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

jmagman commented Sep 4, 2019

Uh oh!

Hixie commented Sep 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmagman commented Aug 30, 2019 •

edited

Loading