[flutter_tools] enable `flutter upgrade` to support force pushed branches #55594

christopherfujino · 2020-04-24T19:20:42Z

Description

In #53715 I removed support for flutter upgrade-ing forward through a force pushed branch assuming that we would not be force pushing in the future, and to allow the downgrade_upgrade_integration_test to properly test upgrading.

However, because of #55576, we will have to again force push past hotfixes. The way upgrading from hotfixed branches previously worked was:

reset backward to the last non-hotfix tag on the branch (e.g. v1.2.3+hotfix.4 -> v1.2.3)
do a git pull --ff fast forward to the tip of the branch (e.g. v1.2.3 -> 1.3.0).

However, with the new release scheme, finding the point to reset to is not as simple as stripping out the +hotfix.x from the tag (I'm guessing we could use git merge-base master <channel>. However this would regress the downgrade_upgrade_integration_test, and since we're doing a hard reset, we might as well just hard reset to the upstream HEAD:

fetch the upstream version of the branch, git fetch
retrieve the revision of the upstream HEAD, git rev-parse --verify @{u}
hard reset to the upstream HEAD, git reset --hard <revision from last step>

Related Issues

Fixes #55576.

Tests

I updated mocks to the unit tests and the downgrade_upgrade_integration_test still passes.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

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

Did any tests fail when you ran them? Please read [Handling breaking changes].

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.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

…he upstream branch

jonahwilliams · 2020-04-24T20:15:32Z

packages/flutter_tools/lib/src/commands/upgrade.dart

    @required bool testFlow,
  }) async {
-    await verifyUpstreamConfigured();
+    final String upstreamRevision = await fetchRemoteRevision();


I know we used to have this check, could you remind me of the reason we added? Is there git console spam on no-op upgrades otherwise?

I think before, we never grabbed the revision that we wanted to upgrade to, so the way that we checked if an upgrade wasn't necessary was that we saved the version we were at, did an upgrade, and checked to see if we were on the same commit. if so, the upgrade was an essential no-op, and we didn't have to do the second half.

Since we're grabbing the desired upgrade target revision from the start, just exit early without messing with their repo.

That makes sense, thanks Chris!

jmagman

LGTM

jonahwilliams

LGTM

zanderso

dbc

zanderso · 2020-04-27T17:59:16Z

packages/flutter_tools/lib/src/commands/upgrade.dart

-        newFlutterVersion.frameworkRevision == oldFlutterVersion.frameworkRevision;
-    } on Exception catch (e) {
-      globals.printTrace('Failed to determine FlutterVersion after upgrade fast-forward: $e');
+    if (result.exitCode != 0) {


throwOnError is true, so you'll get a ProcessException on an error and this condition won't ever be true. Do we have tests with git invocations mocked out and failing?

Good point, lemme look into it.

Created this PR to address.

…ches (flutter#55594)

* [flutter_tools] fix type error in symbolize (#55212) * [flutter_tools] check first for stable tag, then dev tag (#55342) * [flutter_tools] enable `flutter upgrade` to support force pushed branches (#55594) * [flutter_tools] catch ProcessException and throw ToolExit during upgrade (#55759) * Update engine version to 376ad6a64b08aa26005e3f82aed26de2e290b572 Co-authored-by: Jonah Williams <campfish91@gmail.com> Co-authored-by: Christopher Fujino <fujino@google.com> Co-authored-by: Christopher Fujino <christopherfujino@gmail.com>

rather than fast forward to upgrade, to a hard reset to the HEAD of t…

444a317

…he upstream branch

fluttergithubbot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Apr 24, 2020

christopherfujino marked this pull request as ready for review April 24, 2020 20:13

christopherfujino requested review from jmagman, jonahwilliams and zanderso April 24, 2020 20:13

jonahwilliams reviewed Apr 24, 2020

View reviewed changes

jmagman approved these changes Apr 24, 2020

View reviewed changes

christopherfujino removed the work in progress; do not review label Apr 24, 2020

jonahwilliams approved these changes Apr 24, 2020

View reviewed changes

make output of downgrade_upgrade_integration_test easier to debug

7cc05e1

googlebot added the cla: yes label Apr 24, 2020

christopherfujino added 3 commits April 24, 2020 13:52

cleanup

c764683

explicitly fetch tags

beafdc6

fix test

4ddbc5c

christopherfujino added the waiting for tree to go green label Apr 24, 2020

christopherfujino merged commit 4552af1 into flutter:master Apr 25, 2020

christopherfujino deleted the fix-upgrade branch April 25, 2020 00:55

zanderso reviewed Apr 27, 2020

View reviewed changes

christopherfujino mentioned this pull request Apr 27, 2020

Flutter upgrade should support upstream branches that have been force pushed #55576

Closed

pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request Apr 28, 2020

[flutter_tools] enable flutter upgrade to support force pushed bran…

e86eb1a

…ches (flutter#55594)

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flutter_tools] enable `flutter upgrade` to support force pushed branches #55594

[flutter_tools] enable `flutter upgrade` to support force pushed branches #55594

Uh oh!

christopherfujino commented Apr 24, 2020 •

edited

Loading

Uh oh!

jonahwilliams Apr 24, 2020

Uh oh!

christopherfujino Apr 24, 2020 •

edited

Loading

Uh oh!

jonahwilliams Apr 24, 2020

Uh oh!

jmagman left a comment

Uh oh!

jonahwilliams left a comment

Uh oh!

zanderso left a comment

Uh oh!

zanderso Apr 27, 2020

Uh oh!

christopherfujino Apr 27, 2020

Uh oh!

christopherfujino Apr 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[flutter_tools] enable flutter upgrade to support force pushed branches #55594

[flutter_tools] enable flutter upgrade to support force pushed branches #55594

Uh oh!

Conversation

christopherfujino commented Apr 24, 2020 • 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!

jonahwilliams Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

christopherfujino Apr 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

zanderso left a comment

Choose a reason for hiding this comment

Uh oh!

zanderso Apr 27, 2020

Choose a reason for hiding this comment

Uh oh!

christopherfujino Apr 27, 2020

Choose a reason for hiding this comment

Uh oh!

christopherfujino Apr 27, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[flutter_tools] enable `flutter upgrade` to support force pushed branches #55594

[flutter_tools] enable `flutter upgrade` to support force pushed branches #55594

christopherfujino commented Apr 24, 2020 •

edited

Loading

christopherfujino Apr 24, 2020 •

edited

Loading