Skip to content

Cp for gradle 8.0 support in flutter 3.7 #124990

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.

Sign up for GitHub

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

Jump to bottom
Merged
CaseyHillers merged 2 commits into from
Apr 18, 2023
Merged

Cp for gradle 8.0 support in flutter 3.7 #124990

CaseyHillers merged 2 commits into from
Apr 18, 2023

Conversation

@reidbaker
Copy link
Contributor

@reidbaker reidbaker commented Apr 17, 2023 •
edited
Loading

Pull the minimal set of changes from flutter 3.10 (in beta) into flutter 3.7 to support gradle 8.0
124838

Original PR

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.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 17, 2023
@flutter-dashboard
Copy link

flutter-dashboard bot commented Apr 17, 2023

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 (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@reidbaker reidbaker mentioned this pull request Apr 17, 2023
Closed
@beroso @reidbaker
Fix Gradle 7 warnings that are now errors in Gradle 8 (#121958)
e69253c 
Fix Gradle 7 warnings that are now errors in Gradle 8
mossmana
mossmana reviewed Apr 17, 2023
View reviewed changes
dev/benchmarks/complex_layout/android/build.gradle

dependencies {
classpath 'com.android.tools.build:gradle:7.2.0'
classpath 'com.android.tools.build:gradle:7.3.0'
Copy link
Contributor

@mossmana mossmana Apr 17, 2023

Choose a reason for hiding this comment

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

Out of curiosity, if there any reason why we couldn't start using a variable for version number like we do for the kotlin version?

Copy link
Contributor Author

@reidbaker reidbaker Apr 17, 2023

Choose a reason for hiding this comment

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

Agp version is more likely to cause you to have to make updates to your app than kotlin version or gradle version. Nothing mechanically stops us but I think leaving this version where it is easy for the Android Studio tooling to update is the best policy. That said it does probably make sense to have a version that can be shared for packages/packages.

@reidbaker reidbaker requested a review from camsim99 April 17, 2023 20:33
gmackall
gmackall reviewed Apr 17, 2023
View reviewed changes
dev/benchmarks/complex_layout/android/gradle/wrapper/gradle-wrapper.properties
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
Copy link
Member

@gmackall gmackall Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I notice this changes from ***all.zip to ***bin.zip. Is there one we should generally prefer, or does it depend on the situation?

As I understand, its the difference of whether sources/documentation come bundled, but I'm not sure of the implications of that for all these gradle wrappers in /benchmarks, for example.

Copy link
Contributor Author

@reidbaker reidbaker Apr 17, 2023

Choose a reason for hiding this comment

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

I think for normal users all is probably safe since the size is not large. That said this is what is in the 3.10 branch and we dont want to make any changes.

camsim99
camsim99 reviewed Apr 17, 2023
View reviewed changes
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

The gradle changes look good, just had one question!

dev/benchmarks/complex_layout/android/settings.gradle

include ':app'

enableFeaturePreview('ONE_LOCKFILE_PER_PROJECT')
Copy link
Contributor

@camsim99 camsim99 Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

@reidbaker reidbaker Apr 17, 2023

Choose a reason for hiding this comment

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

This was removed in the original pr because one lockfile per project was an experimental feature that was promoted to stable.

@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Apr 17, 2023
@reidbaker
Copy link
Contributor Author

reidbaker commented Apr 17, 2023

Mac builds are failing because Exception on command "checkout e69253c6bd718486fdb5669a0c181900940e70f7". I think that is a an infra issue because my cherry pick is commit e69253c

@CaseyHillers CaseyHillers merged commit 4d9e56e into flutter:flutter-3.7-candidate.1 Apr 18, 2023
@reidbaker reidbaker deleted the i124838-gradle-8-backport-flutter-3-7 branch April 18, 2023 14:18
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 20, 2023
@engine-flutter-autoroll
4d9e56e69 [CP] Gradle 8.0 support in flutter 3.7 (flutter/flutter#124990
2b871c4 
)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 20, 2023
@engine-flutter-autoroll
4d9e56e69 [CP] Gradle 8.0 support in flutter 3.7 (flutter/flutter#124990
1b05bd0 
)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2023
@engine-flutter-autoroll
4d9e56e69 [CP] Gradle 8.0 support in flutter 3.7 (flutter/flutter#124990
adb7816 
)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 26, 2023
@engine-flutter-autoroll
4d9e56e69 [CP] Gradle 8.0 support in flutter 3.7 (flutter/flutter#124990
d271787 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmackall gmackall gmackall left review comments

@camsim99 camsim99 camsim99 approved these changes

+1 more reviewer

@mossmana mossmana mossmana left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. 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.

6 participants

@reidbaker @mossmana @gmackall @camsim99 @CaseyHillers @beroso