Reland "Identify and re-throw our dependency checking errors in flutter.groovy" by gmackall · Pull Request #150128 · flutter/flutter

gmackall · 2024-06-12T18:14:22Z

The original approach in #149609 didn't work when the Flutter Gradle plugin was applied using the deprecated script apply - the kotlin portion couldn't resolve the custom exception defined in flutter.groovy:

e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:238:23: Unresolved reference: DependencyValidationException
e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:263:23: Unresolved reference: DependencyValidationException
e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:288:23: Unresolved reference: DependencyValidationException
e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:313:23: Unresolved reference: DependencyValidationException
Warning: Flutter was unable to detect project Gradle, Java, AGP, and KGP versions. Skipping dependency version checking. Error was: org.gradle.internal.exceptions.LocationAwareException: Script '/Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts' line: 238
Script compilation errors:

  Line 238:                 throw DependencyValidationException(errorMessage)
                                  ^ Unresolved reference: DependencyValidationException

  Line 263:                 throw DependencyValidationException(errorMessage)
                                  ^ Unresolved reference: DependencyValidationException

  Line 288:                 throw DependencyValidationException(errorMessage)
                                  ^ Unresolved reference: DependencyValidationException

  Line 313:                 throw DependencyValidationException(errorMessage)
                                  ^ Unresolved reference: DependencyValidationException

This new approach of setting one of the extra properties works in both cases (tested with the camera_android example app, which uses the script apply, and a freshly created counter app).

It also removes some brittleness in that we don't have to unwrap the exception anymore, and aren't subject to breaking if Gradle decides one day to wrap our custom exception 1 layer deeper in additional exceptions.

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

…tter.groovy` (flutter#149609)" (flutter#149918) This reverts commit 15307a9.

…king

reidbaker · 2024-06-13T14:20:08Z

packages/flutter_tools/gradle/src/main/groovy/flutter.groovy

-                        + ignored)
+            } catch (Exception e) {
+
+                if (!project.failedDependencyChecks) {


This if statement is a bit difficult to read since you are negating "failed" then logging an error.

Suggested change

if (!project.failedDependencyChecks) {

// Exception means something went wrong.

if (project.failedToCheckDependencies) {

// Possible bug in dependency checking code warn and do not block build.

...

} else {

// If failedToCheckDependencies is unset or false then the exception is thrown by us and/or an an error that should block the build.

throw e.

}

Made the property a constant, and changed it to usesUnsupportedDependencyVersions so that the check reads better as:

if (!project.usesUnsupportedDependencyVersions) { ... } else { ... }

packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts

gmackall · 2024-06-13T19:20:18Z

cc @bartekpacia - no action needed, just figured you might be interested in the reason for the revert+reland as it relates to the work of re-writing the FGP to Kotlin.

…in flutter.groovy" (flutter/flutter#150128)

flutter/flutter@01db23b...349ec71 2024-06-14 [email protected] Add tests for navigator.0.dart (flutter/flutter#150034) 2024-06-14 [email protected] Switch to `Iterable.cast` instance method (flutter/flutter#150185) 2024-06-14 [email protected] Include transform in static Gradient lerp methods (flutter/flutter#149624) 2024-06-14 [email protected] Validate the `contrastLevel` during `ColorScheme` creation (flutter/flutter#150176) 2024-06-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.9 to 3.25.10 (flutter/flutter#150228) 2024-06-13 [email protected] Fix leaky test. (flutter/flutter#150235) 2024-06-13 [email protected] Document CIPD role & login for upgrading Android engine (flutter/flutter#149433) 2024-06-13 [email protected] Update doc for `ColorScheme.surface` (flutter/flutter#150212) 2024-06-13 [email protected] Roll pub packages (flutter/flutter#150206) 2024-06-13 [email protected] Bump new release for a11y_assessment (flutter/flutter#150213) 2024-06-13 [email protected] Use --(no-)strip-wams instead of --(no-)-name-section in `dart compile wasm` (flutter/flutter#149641) 2024-06-13 [email protected] Reland "Identify and re-throw our dependency checking errors in flutter.groovy" (flutter/flutter#150128) 2024-06-13 [email protected] Use --(no-)strip-wams instead of --(no-)-name-section in `dart compile wasm` (flutter/flutter#150180) 2024-06-13 [email protected] Suppress Flutter update check if `--machine` is present at all. (flutter/flutter#150138) 2024-06-13 [email protected] [Reland] Introduce `ChipAnimationStyle` to override default chips animations durations (flutter/flutter#149876) 2024-06-13 [email protected] Update framework and flutter fix flutter.dev/docs links (flutter/flutter#150174) 2024-06-13 [email protected] Roll Flutter Engine from 4cb3025d3abf to 8167dffd1914 (2 revisions) (flutter/flutter#150208) 2024-06-13 [email protected] Replace InputDecorator M3 golden test (flutter/flutter#150111) 2024-06-13 [email protected] Roll Packages from 260102b to 7805455 (2 revisions) (flutter/flutter#150198) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

bartekpacia

That's an interesting approach! Thanks for the ping :-)

…er.groovy" (flutter#150128) The original approach in flutter#149609 didn't work when the Flutter Gradle plugin was applied using the deprecated script apply - the kotlin portion couldn't resolve the custom exception defined in `flutter.groovy`: ``` e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:238:23: Unresolved reference: DependencyValidationException e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:263:23: Unresolved reference: DependencyValidationException e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:288:23: Unresolved reference: DependencyValidationException e: /Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts:313:23: Unresolved reference: DependencyValidationException Warning: Flutter was unable to detect project Gradle, Java, AGP, and KGP versions. Skipping dependency version checking. Error was: org.gradle.internal.exceptions.LocationAwareException: Script '/Users/mackall/development/flutter/flutter/packages/flutter_tools/gradle/src/main/kotlin/dependency_version_checker.gradle.kts' line: 238 Script compilation errors: Line 238: throw DependencyValidationException(errorMessage) ^ Unresolved reference: DependencyValidationException Line 263: throw DependencyValidationException(errorMessage) ^ Unresolved reference: DependencyValidationException Line 288: throw DependencyValidationException(errorMessage) ^ Unresolved reference: DependencyValidationException Line 313: throw DependencyValidationException(errorMessage) ^ Unresolved reference: DependencyValidationException ``` This new approach of setting one of the [`extra` properties](https://docs.gradle.org/current/dsl/org.gradle.api.Project.html#N14FF7) works in both cases (tested with the `camera_android` example app, which uses the script apply, and a freshly created counter app). It also removes some brittleness in that we don't have to unwrap the exception anymore, and aren't subject to breaking if Gradle decides one day to wrap our custom exception 1 layer deeper in additional exceptions.

…in flutter.groovy" (flutter/flutter#150128)

Gray Mackall added 3 commits June 11, 2024 11:57

Reapply "Identify and re-throw our dependency checking errors in `flu…

db0c9bd

…tter.groovy` (flutter#149609)" (flutter#149918) This reverts commit 15307a9.

wipundo

80df9c1

new approach: set a property in kotlin, check that in groovy

c2c748b

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 12, 2024

Gray Mackall added 3 commits June 12, 2024 11:15

undo private->public

6e826ea

make it so that we always fail, unless we didn't fail dependency chec…

21f1160

…king

words

09719d0

gmackall marked this pull request as ready for review June 12, 2024 22:22

gmackall requested a review from reidbaker June 12, 2024 22:22

gmackall mentioned this pull request Jun 12, 2024

[image_picker]: Bump org.jetbrains.kotlin:kotlin-bom from 1.8.22 to 1.9.22 in /packages/image_picker/image_picker_android/android flutter/packages#5752

Closed

reidbaker approved these changes Jun 13, 2024

View reviewed changes

Gray Mackall added 2 commits June 13, 2024 12:05

review

1bbfcc1

doc

7924038

gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 13, 2024

auto-submit bot merged commit 7e97cf7 into flutter:master Jun 13, 2024

engine-flutter-autoroll mentioned this pull request Jun 14, 2024

Roll Flutter from 01db23b82233 to 349ec7183cd0 (19 revisions) flutter/packages#6926

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 14, 2024

7e97cf7 Reland "Identify and re-throw our dependency checking errors …

3192584

…in flutter.groovy" (flutter/flutter#150128)

bartekpacia reviewed Jun 14, 2024

View reviewed changes

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024

7e97cf7 Reland "Identify and re-throw our dependency checking errors …

ffdcad5

…in flutter.groovy" (flutter/flutter#150128)

gmackall mentioned this pull request Aug 26, 2024

Cannot build apk from new project after upgrade #153668

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland "Identify and re-throw our dependency checking errors in flutter.groovy"#150128

Reland "Identify and re-throw our dependency checking errors in flutter.groovy"#150128
auto-submit[bot] merged 8 commits intoflutter:masterfrom
gmackall:reland_rethrow_dependency_errors

gmackall commented Jun 12, 2024 •

edited

Loading

Uh oh!

reidbaker Jun 13, 2024

Uh oh!

gmackall Jun 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmackall commented Jun 13, 2024

Uh oh!

bartekpacia left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-                if (!project.failedDependencyChecks) {
+                // Exception means something went wrong.
+                if (project.failedToCheckDependencies) {
+                   // Possible bug in dependency checking code warn and do not block build.
+                   ...
+                } else {
+                  // If failedToCheckDependencies is unset or false then the exception is thrown by us and/or an an error that should block the build.
+                  throw e.
+                }

Conversation

gmackall commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

reidbaker Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

gmackall Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmackall commented Jun 13, 2024

Uh oh!

bartekpacia left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gmackall commented Jun 12, 2024 •

edited

Loading