Skip to content

Fix App.framework path in Podfile#63412

Merged
fluttergithubbot merged 1 commit intoflutter:masterfrom
jmagman:pod-add2app
Aug 12, 2020
Merged

Fix App.framework path in Podfile#63412
fluttergithubbot merged 1 commit intoflutter:masterfrom
jmagman:pod-add2app

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Aug 10, 2020 •
edited
Loading

Description

There was a bug with the path logic in creating the Debug fake static const int Moo = 88; App.framework. Fix the path.

Related Issues

Fixes #63176

Tests

Add a check to make sure the App.framework is really being embedded in the host app.
Added a flutter clean and flutter pub get to the integration test. Before, it was using the App.framework from a previous step building it from the module, which doesn't prove it can be generated from the host app without running flutter build in the module first.

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 read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • 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

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@jmagman jmagman added the a: existing-apps Integration with existing apps via the add-to-app flow label Aug 10, 2020
@jmagman jmagman self-assigned this Aug 10, 2020
@flutter-dashboard
Copy link

flutter-dashboard bot commented Aug 10, 2020

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

1 similar comment
@flutter-dashboard
Copy link

flutter-dashboard bot commented Aug 10, 2020

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard flutter-dashboard bot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Aug 10, 2020
christopherfujino
christopherfujino approved these changes Aug 10, 2020
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll LGTM anything with less ruby

@jmagman
Copy link
Member Author

jmagman commented Aug 10, 2020

I don't think this test is complete.

@jmagman jmagman changed the title Remove unused fake App.framework Fix App.framework path in Podfile Aug 12, 2020
@xster
Copy link
Member

xster commented Aug 12, 2020

LGTM. Thanks for adding tests

@jmagman
Copy link
Member Author

jmagman commented Aug 12, 2020

I was wrong, we still need the fake App.framework. I forgot about the -framework App in the OTHER_LDFLAGS Pods xcconfig.
I updated this to just fix the path.

Also, I added a flutter clean and flutter pub get to the integration test. Before, it was using the App.framework from a previous step building it from the module, which doesn't prove it can be generated from the host app without running flutter build in the module first.

This was referenced Aug 12, 2020
Closed
Closed
Closed
@szotp
Copy link

szotp commented Aug 12, 2020

LGTM and works on my machine

@fluttergithubbot fluttergithubbot merged commit 6b2ef0d into flutter:master Aug 12, 2020
pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request Aug 12, 2020
@jmagman
Fix App.framework path in Podfile (flutter#63412)
49a87cb
pcsosinski pushed a commit that referenced this pull request Aug 13, 2020
@Piinks @jmagman
@rami-a @darrenaustin
1.20.2 Cherrypicks (#63591)
bbfbf17 
* Fix SliverList scrollOffsetCorrection 0 case (#62615)

* Case insensitive check flavor names against Xcode schemes (#61140)

* Address misc time picker design issues (#62803)

* Update to the latest localizations (#63026)

* Removed the inputFormatters from the text input fields used by the Date Pickers (#63461)

* Fix App.framework path in Podfile (#63412)

* Update engine hash to 1.20.2

Co-authored-by: Kate Lovett <[email protected]>
Co-authored-by: Jenn Magder <[email protected]>
Co-authored-by: Rami <[email protected]>
Co-authored-by: Shi-Hao Hong <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request Aug 18, 2020
@jmagman
Fix App.framework path in Podfile (flutter#63412)
2e34f14
pcsosinski pushed a commit that referenced this pull request Aug 18, 2020
@darrenaustin @jmagman
1.21.0-9.1.pre cherrypicks (#64050)
be9bc8c 
* Update engine hash to 1.21.0-9.1.pre

* Removed the inputFormatters from the text input fields used by the Date Pickers (#63461)

* Fix App.framework path in Podfile (#63412)

Co-authored-by: Darren Austin <[email protected]>
Co-authored-by: Jenn Magder <[email protected]>
@jmagman jmagman added the platform-ios iOS applications specifically label Aug 21, 2020
@jmagman jmagman deleted the pod-add2app branch September 2, 2020 18:09
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@jmagman @mingwandroid
Fix App.framework path in Podfile (flutter#63412)
e8c92b4
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@xster xster Awaiting requested review from xster

1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jmagman jmagman

Labels

a: existing-apps Integration with existing apps via the add-to-app flow c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

upgrade to 1.20.1 app can not load the flutter module and creates a new folder

6 participants

@jmagman @xster @szotp @christopherfujino @googlebot @fluttergithubbot