Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reland] Add migrator to upgrade gradle version when conflict with Android Studio bundled Java version is detected #125836

Merged
merged 13 commits into from May 3, 2023

Conversation

gmackall
Copy link
Member

@gmackall gmackall commented May 1, 2023

This is an attempt to reland #124085.

Differences from this attempt and the last:

  1. Adds a check for null android studio versions and a test for this case.
  2. Wraps the migrate code in a try-catch per the suggestion here.

Old PR description:
This PR adds an android project migrator that checks the version of android studio and the version of gradle for conflicts, and upgrades to 7.4 if a conflict is detected. For more detail about the particular conflict, see #122376.

The PR also upgrades older gradle versions being used in integration testing to 7.4.

Fixes/related to: #122376 and #123636

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels May 1, 2023
migration.migrate();
expect(gradleWrapperPropertiesFile.readAsStringSync(), gradleWrapperToMigrateTo);
expect(bufferLogger.statusText, contains('Conflict detected between Android Studio Java version and Gradle version, '
'upgrading Gradle version from 6.7 to 7.6.1.'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using the constants defined here so if they upgrader uses a different version this test passes.

Copy link
Member Author

@gmackall gmackall May 1, 2023

Choose a reason for hiding this comment

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

I've changed to use the constant defined for version we upgrade to, 7.6.1 (the other version, 6.7, is parsed in the migration method itself so it can't be used here).

@@ -20,7 +20,7 @@ import '../gradle_utils.dart';
@visibleForTesting
final Version androidStudioFlamingo = Version(2022, 2, 0);

const String _gradleVersion7_6_1 = r'7.6.1';
const String gradleVersion7_6_1 = r'7.6.1';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be annotated @VisibleForTesting

@gmackall gmackall added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels May 2, 2023
@gmackall
Copy link
Member Author

gmackall commented May 2, 2023

Removing autosubmit briefly to test an assumption about RegExp group counts.

@gmackall
Copy link
Member Author

gmackall commented May 2, 2023

Since last approval, I wrapped two different values returned by RegExpMatch group().

The main reason is that it is possible in a case like

final RegExp r = RegExp(r'ab(c)?');
final String s = 'ab';
final RegExpMatch m = r.firstMatch(s);

for m.groupCount to be 1, but m[1] to be null. It isn't possible with the RegExp as written, but given that the regex is in a different file it would be good for this check to be resilient to changes.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
gmackall added a commit to gmackall/flutter that referenced this pull request May 4, 2023
…droid Studio bundled Java version is detected (flutter#125836)

This is an attempt to reland flutter#124085.

Differences from this attempt and the last:
1. Adds a check for null android studio versions and a test for this case.
2. Wraps the migrate code in a try-catch [per the suggestion here](https://github.com/flutter/flutter/pull/125728/files#r1181747899).

Old PR description:
This PR adds an android project migrator that checks the version of android studio and the version of gradle for conflicts, and upgrades to 7.4 if a conflict is detected. For more detail about the particular conflict, see flutter#122376.

The PR also upgrades older gradle versions being used in integration testing to 7.4.

Fixes/related to: flutter#122376 and flutter#123636
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
gmackall added a commit to gmackall/flutter that referenced this pull request May 5, 2023
…droid Studio bundled Java version is detected (flutter#125836)

This is an attempt to reland flutter#124085.

Differences from this attempt and the last:
1. Adds a check for null android studio versions and a test for this case.
2. Wraps the migrate code in a try-catch [per the suggestion here](https://github.com/flutter/flutter/pull/125728/files#r1181747899).

Old PR description:
This PR adds an android project migrator that checks the version of android studio and the version of gradle for conflicts, and upgrades to 7.4 if a conflict is detected. For more detail about the particular conflict, see flutter#122376.

The PR also upgrades older gradle versions being used in integration testing to 7.4.

Fixes/related to: flutter#122376 and flutter#123636
XilaiZhang pushed a commit that referenced this pull request May 15, 2023
…d Studio bundled Java version is detected (and 3 others) (#126093)

This is a cherry pick of the following four PRs (the first one depends
on changes made in the latter three).
1. #125836
2. #124771
3. #123034
4. #123644

For the first PR, 
Original issue: #122376
Cherry-pick issue: #125693

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

---------

Co-authored-by: Reid Baker <reidbaker@google.com>
Co-authored-by: Mitchell Goodwin <58190796+MitchellGoodwin@users.noreply.github.com>
Co-authored-by: Greg Spencer <gspencergoog@users.noreply.github.com>
Co-authored-by: Victoria Ashworth <vashworth@google.com>
Co-authored-by: Jackson Gardner <jacksongardner@google.com>
Co-authored-by: Rydmike <m.rydstrom@gmail.com>
Co-authored-by: keyonghan <54558023+keyonghan@users.noreply.github.com>
Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>
Co-authored-by: Taha Tesser <tessertaha@gmail.com>
Co-authored-by: Ben Konyi <bkonyi@google.com>
Co-authored-by: engine-flutter-autoroll <engine-flutter-autoroll@skia.org>
Co-authored-by: hellohuanlin <41930132+hellohuanlin@users.noreply.github.com>
Co-authored-by: Danny Tuppeny <danny@tuppeny.com>
Co-authored-by: Chris Bracken <chris@bracken.jp>
Co-authored-by: Kate Lovett <katelovett@google.com>
Co-authored-by: Michael Goderbauer <goderbauer@google.com>
Co-authored-by: Elias Yishak <42216813+eliasyishak@users.noreply.github.com>
Co-authored-by: Christopher Fujino <fujino@google.com>
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
…ct with Android Studio bundled Java version is detected (flutter/flutter#125836)
FlutterSu pushed a commit to flutter-rosita/flutter-rosita that referenced this pull request Sep 14, 2023
…d Studio bundled Java version is detected (and 3 others) (flutter#126093)

This is a cherry pick of the following four PRs (the first one depends
on changes made in the latter three).
1. flutter#125836
2. flutter#124771
3. flutter#123034
4. flutter#123644

For the first PR,
Original issue: flutter#122376
Cherry-pick issue: flutter#125693

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

---------

Co-authored-by: Reid Baker <reidbaker@google.com>
Co-authored-by: Mitchell Goodwin <58190796+MitchellGoodwin@users.noreply.github.com>
Co-authored-by: Greg Spencer <gspencergoog@users.noreply.github.com>
Co-authored-by: Victoria Ashworth <vashworth@google.com>
Co-authored-by: Jackson Gardner <jacksongardner@google.com>
Co-authored-by: Rydmike <m.rydstrom@gmail.com>
Co-authored-by: keyonghan <54558023+keyonghan@users.noreply.github.com>
Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>
Co-authored-by: Taha Tesser <tessertaha@gmail.com>
Co-authored-by: Ben Konyi <bkonyi@google.com>
Co-authored-by: engine-flutter-autoroll <engine-flutter-autoroll@skia.org>
Co-authored-by: hellohuanlin <41930132+hellohuanlin@users.noreply.github.com>
Co-authored-by: Danny Tuppeny <danny@tuppeny.com>
Co-authored-by: Chris Bracken <chris@bracken.jp>
Co-authored-by: Kate Lovett <katelovett@google.com>
Co-authored-by: Michael Goderbauer <goderbauer@google.com>
Co-authored-by: Elias Yishak <42216813+eliasyishak@users.noreply.github.com>
Co-authored-by: Christopher Fujino <fujino@google.com>
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
FlutterSu pushed a commit to flutter-rosita/flutter-rosita that referenced this pull request Dec 27, 2023
…d Studio bundled Java version is detected (and 3 others) (flutter#126093)

This is a cherry pick of the following four PRs (the first one depends
on changes made in the latter three).
1. flutter#125836
2. flutter#124771
3. flutter#123034
4. flutter#123644

For the first PR,
Original issue: flutter#122376
Cherry-pick issue: flutter#125693

*If you had to change anything in the [flutter/tests] repo, include a
link to the migration guide as per the [breaking change policy].*

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

---------

Co-authored-by: Reid Baker <reidbaker@google.com>
Co-authored-by: Mitchell Goodwin <58190796+MitchellGoodwin@users.noreply.github.com>
Co-authored-by: Greg Spencer <gspencergoog@users.noreply.github.com>
Co-authored-by: Victoria Ashworth <vashworth@google.com>
Co-authored-by: Jackson Gardner <jacksongardner@google.com>
Co-authored-by: Rydmike <m.rydstrom@gmail.com>
Co-authored-by: keyonghan <54558023+keyonghan@users.noreply.github.com>
Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>
Co-authored-by: Taha Tesser <tessertaha@gmail.com>
Co-authored-by: Ben Konyi <bkonyi@google.com>
Co-authored-by: engine-flutter-autoroll <engine-flutter-autoroll@skia.org>
Co-authored-by: hellohuanlin <41930132+hellohuanlin@users.noreply.github.com>
Co-authored-by: Danny Tuppeny <danny@tuppeny.com>
Co-authored-by: Chris Bracken <chris@bracken.jp>
Co-authored-by: Kate Lovett <katelovett@google.com>
Co-authored-by: Michael Goderbauer <goderbauer@google.com>
Co-authored-by: Elias Yishak <42216813+eliasyishak@users.noreply.github.com>
Co-authored-by: Christopher Fujino <fujino@google.com>
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
@gmackall gmackall deleted the reland_gradle_migrator branch January 17, 2024 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants