refactor flutter upgrade to be 2 part, with the second part re-entrant#38325
Merged
christopherfujino merged 3 commits intoflutter:masterfrom Aug 13, 2019
Merged
refactor flutter upgrade to be 2 part, with the second part re-entrant#38325christopherfujino merged 3 commits intoflutter:masterfrom
flutter upgrade to be 2 part, with the second part re-entrant#38325christopherfujino merged 3 commits intoflutter:masterfrom
Conversation
…trant via `flutter upgrade --continue`
fluttergithubbot
added
the
tool
Contributor
|
Can you add some more context to the PR description on how this helps to resolve the underlying issue for Catalina? |
Contributor
Author
Done. |
Codecov Report
@@ Coverage Diff @@
## master #38325 +/- ##
==========================================
- Coverage 56.09% 55.13% -0.96%
==========================================
Files 194 194
Lines 18181 18195 +14
==========================================
- Hits 10198 10032 -166
- Misses 7983 8163 +180
Continue to review full report at Codecov.
|
Hixie
reviewed
Aug 14, 2019
| ) | ||
| ..addFlag( | ||
| 'continue', | ||
| hide: true, |
Contributor
There was a problem hiding this comment.
we should expose this in verbose mode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, if you run flutter upgrade on macOS Catalina and the upgrade path has a Dart SDK revision change, you will get an exception with a misleading VersionCheckError: Command exited with code -9: git log -n 1 --pretty=format:%ad --date=iso. The actual cause is calling the Flutter tool re-entrantly, which deletes the current Dart SDK (which the flutter upgrade process is using), after which any forked sub-processes would be killed (code -9 from the VersionCheckError message) by the macOS anti-malware software. See #33890 and dart-lang/sdk#37662 for more info.
This PR breaks up the
flutter upgradeflow into two parts, the second half invoked re-entrantly via the hidden flagflutter upgrade --continue. This fixes the issue because after there are no steps following the re-entrant call offlutter upgrade --continuefrom the first dart process.Description
Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change. If you're changing visual properties, consider including before/after screenshots (and runnable code snippets to reproduce them).
Related Issues
Fixes #33890
Tests
I re-wrote the existing
upgrade_test.dartfile to accommodate the new re-entrant command.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.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?