Improve Android SDK and NDK mistmatch warning message #147809

bartekpacia · 2024-05-03T22:26:02Z

This PR resolves #147806

List plugin that want to be compiled against a higher Android SDK version
List plugins that depend on a different NDK version (we don't have a way to compare them)
Small formatting and wording improvements
Update syntax to work for both Groovy and Kotlin
If project uses build.gradle.kts, then it is mentioned in the warning message (previously always build.gradle was mentioned)

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.

flutter-dashboard · 2024-05-03T22:26:05Z

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 "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use 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.

…for build.gradle.kts)

bartekpacia · 2024-05-04T13:18:05Z

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

+ }
+ }
+ project.logger.error("""\
+ Fix this issue by compiling against the highest Android SDK version (they are backward compatible).


Some resources I found to support the "they are backward compatible" statement:

https://stackoverflow.com/q/31493552/7009800

https://stackoverflow.com/q/42330768/7009800

don't you mean to say "that are backwards compatible"

No. I mean "Android SDK versions are backward compatible".

okay, it wasn't so clear to me. maybe it can be clearer

https://stackoverflow.com/q/42330768/7009800

I think it'd be great to include the guide linked here in the error message.

Maybe something like this could work:

Almost all changes to the Android framework API are additive, so older plugins can generally be compiled with the latest SDK version without compatibility issues. (see https://developer.android.com/guide/topics/manifest/uses-sdk-element#fc) Consider using the latest SDK version listed, or changing/removing the above dependencies.

I'm not sure if the wording is right here:

so older plugins can generally be compiled with the latest SDK version without compatibility issues.

The problem in this case is not "older plugins", but the app that compiles using Android SDK that is too low.

nate-thegrate

The first time I tried releasing an Android app, I was met with that vague error message and it led to hours of frustration.

I'm a huge fan of the work being done here 👍👍

nate-thegrate · 2024-05-04T22:46:14Z

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

+ }
+ }
+ project.logger.error("""\
+ Fix this issue by compiling against the highest Android SDK version (they are backward compatible).


https://stackoverflow.com/q/42330768/7009800

I think it'd be great to include the guide linked here in the error message.

Maybe something like this could work:

Almost all changes to the Android framework API are additive, so older plugins can generally be compiled with the latest SDK version without compatibility issues. (see https://developer.android.com/guide/topics/manifest/uses-sdk-element#fc) Consider using the latest SDK version listed, or changing/removing the above dependencies.

nate-thegrate · 2024-05-04T22:47:38Z

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


 numProcessedPlugins--
 if (numProcessedPlugins == 0) {
 if (maxPluginCompileSdkVersion > projectCompileSdkVersion) {
- project.logger.error("One or more plugins require a higher Android SDK version.\nFix this issue by adding the following to ${project.projectDir}${File.separator}build.gradle:\nandroid {\n compileSdkVersion ${maxPluginCompileSdkVersion}\n ...\n}\n")
+ project.logger.error("Your project is configured to compile against Android SDK $projectCompileSdkVersion, but the following plugin(s) require to be compiled against a higher Android SDK version:")


A source I found regarding wording:

https://stackoverflow.com/q/65278287

Suggested change

project.logger.error("Your project is configured to compile against Android SDK $projectCompileSdkVersion, but the following plugin(s) require to be compiled against a higher Android SDK version:")

project.logger.error("Your project is configured to compile using Android SDK $projectCompileSdkVersion, but the following plugin(s) require a higher Android SDK version:")

On a second thought, I'm not sure if "using" is always a right replacement. For example:

Fix this issue by compiling using the highest Android SDK version (they are backward compatible).

I don't think it sounds well – "compiling using something" sounds (at least to me) as if this something was a compiler, build tool, or similar thing that does work.

But in this case, Android SDK are merely header files - so we compile against them, not using them, because they are not useful on its own, i.e. they do not perform any work.

Fix this issue by compiling against the highest Android SDK version (they are backward compatible).

I have pretty much zero knowledge of the Android SDK (I had no idea that it's more of a reference than something actively involved in the compilation), so I really appreciate your explanation.

My concern is that beginners won't understand this colloquialism, and they rely on error messages more than others do.

Outside of the software industry, it's much more common to say something like "My husband fixed the printer using the owner's manual" rather than "My husband fixed the printer against the owner's manual"—the manual was still "used" as a reference, even if it wasn't actively doing any of the work.

And even within the software industry, the word "using" appears to be more common: you configure the SDK version with the tag <uses-sdk> rather than <against-sdk>.

But this is all extremely pedantic—I'd still be very happy to see this PR merged as-is.

Thank you for this extensive explanation and discussion. However this time I choose to stand by wording! :P

I think "compile against" is good enough – it stresses that there's no direct dependency (as e.g. Gradle's implementation), but rather the SDK.

I have pretty much zero knowledge of the Android SDK (I had no idea that it's more of a reference than something actively involved in the compilation)

Yes, it's confusing and not well explained. But compileSdk is actually a good name - it's the SDK version that will be used when compiling your app - and nothing more. It will not be present in the final APK's AndroidManifest (only minSdk and targetSdk are).

The <uses-sdk> AndroidManifest attribute you mention has 3 values:

minSdkVersion - this comes from minSdk in build.gradle

targetSdkVersion - this comes from targetSdk in build.gradle

maxSdkVersion - The docs also say it should almost never be used.

So compileSdk is only used at compilation-time.

FWIW @bartekpacia I like the phrasing you selected.

Co-authored-by: Nate <[email protected]>

This reverts commit 4b4aa3e.

gmackall

Mostly LGTM (just a couple short questions). This is a great change!

gmackall · 2024-05-06T19:03:27Z

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

- project.logger.error("One or more plugins require a higher Android SDK version.\nFix this issue by adding the following to ${project.projectDir}${File.separator}build.gradle:\nandroid {\n compileSdkVersion ${maxPluginCompileSdkVersion}\n ...\n}\n")
+ project.logger.error("Your project is configured to compile against Android SDK $projectCompileSdkVersion, but the following plugin(s) require to be compiled against a higher Android SDK version:")
+ for (Tuple2<String, String> pluginToCompileSdkVersion : pluginsWithHigherSdkVersion) {
+ if (pluginToCompileSdkVersion.second > projectCompileSdkVersion) {


We don't really need this check, right? Isn't the point of pluginsWithHigherSdkVersion that we know every element has passes this if check?

Also, if you want to keep it, its doing a comparison of string and int, right? I don't actually know how that works in Groovy but I would want to cast to be doing a check on the same type.

Good catch, you're right. I missed it when I was moving code around. Fixed.

gmackall · 2024-05-06T19:03:44Z

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

- project.logger.error("One or more plugins require a higher Android NDK version.\nFix this issue by adding the following to ${project.projectDir}${File.separator}build.gradle:\nandroid {\n ndkVersion \"${maxPluginNdkVersion}\"\n ...\n}\n")
+ project.logger.error("Your project is configured with Android NDK $projectNdkVersion, but the following plugin(s) depend on a different Android NDK version:")
+ for (Tuple2<String, String> pluginToNdkVersion : pluginsWithDifferentNdkVersion) {
+ if (pluginToNdkVersion.second != projectNdkVersion) {


Same comment about this check not being needed

(Same response as above)

…ter/flutter#147809)

flutter/flutter@04424e1...7920a52 2024-05-07 [email protected] Add tests for callback_shortcuts.0.dart API example. (flutter/flutter#147536) 2024-05-07 [email protected] Improve Android SDK and NDK mistmatch warning message (flutter/flutter#147809) 2024-05-07 [email protected] Add tests for shortcuts.dart API examples. (flutter/flutter#147433) 2024-05-07 [email protected] Added missing tests for ButtonStyle example (flutter/flutter#147457) 2024-05-07 [email protected] Roll Flutter Engine from ece1d686e3ba to 150f694a7816 (1 revision) (flutter/flutter#147912) 2024-05-07 [email protected] DropdownMenu cleanup (flutter/flutter#147860) 2024-05-07 [email protected] Roll Flutter Engine from 80470584e1e1 to ece1d686e3ba (1 revision) (flutter/flutter#147910) 2024-05-07 [email protected] Roll Flutter Engine from b7bfd94af743 to 80470584e1e1 (1 revision) (flutter/flutter#147906) 2024-05-07 [email protected] Roll Flutter Engine from b64e2300bcd0 to b7bfd94af743 (1 revision) (flutter/flutter#147905) 2024-05-07 [email protected] fix MenuItemButton if child is null (flutter/flutter#147485) 2024-05-07 [email protected] Fix document generation, eliminate template support from snippets tool. (flutter/flutter#147893) 2024-05-07 [email protected] Roll Flutter Engine from 4cb9e02c06ce to b64e2300bcd0 (1 revision) (flutter/flutter#147904) 2024-05-07 [email protected] test focus example 0 (flutter/flutter#147564) 2024-05-07 [email protected] Roll Flutter Engine from 422f92b992c5 to 4cb9e02c06ce (1 revision) (flutter/flutter#147899) 2024-05-06 [email protected] Roll pub packages (flutter/flutter#147896) 2024-05-06 [email protected] Roll Flutter Engine from 463ff7d2d4d5 to 422f92b992c5 (5 revisions) (flutter/flutter#147895) 2024-05-06 [email protected] MultiSelectableSelectionContainerDelegate documentation fixes. (flutter/flutter#147843) 2024-05-06 [email protected] Roll pub packages (flutter/flutter#147891) 2024-05-06 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.4 to 4.1.5 (flutter/flutter#147888) 2024-05-06 [email protected] Fix janks and memory leaks in `CupertinoPageTransition` and `CupertinoFullscreenDialogTransition` (flutter/flutter#146999) 2024-05-06 [email protected] Roll Flutter Engine from 960a0c8fecbe to 463ff7d2d4d5 (1 revision) (flutter/flutter#147886) 2024-05-06 [email protected] Roll Flutter Engine from 70cc300f9ad8 to 960a0c8fecbe (1 revision) (flutter/flutter#147884) 2024-05-06 [email protected] [new gallery] Reisze gallery images (flutter/flutter#147882) 2024-05-06 [email protected] Roll Flutter Engine from d0f99b35eac6 to 70cc300f9ad8 (1 revision) (flutter/flutter#147880) 2024-05-06 [email protected] Fix leak in a test. (flutter/flutter#147846) 2024-05-06 [email protected] Roll Flutter Engine from a30ae7729c95 to d0f99b35eac6 (3 revisions) (flutter/flutter#147878) 2024-05-06 [email protected] Roll Packages from f4719ca to 2dfe645 (3 revisions) (flutter/flutter#147866) 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 added 2 commits May 3, 2024 23:07

create pluginsToNdkVersions and pluginsToSdkVersions lists

c77f39f

finish implementation

5dd9751

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

bartekpacia added 2 commits May 3, 2024 23:40

improve formatting of the warning message

f965898

use correct Gradle build file in the warning message (i.e. also work …

d9298ae

…for build.gradle.kts)

bartekpacia changed the title ~~Feature/android sdk or ndk mistmatch better error msg~~ Improve Android SDK and NDK mistmatch warning message May 3, 2024

bartekpacia added 3 commits May 4, 2024 00:28

fix android_plugin_ndkversion_mismatch_test

2db1cdb

fix android_plugin_compilesdkversion_mismatch_test

400fdd8

fix typo

47230b6

bartekpacia requested review from reidbaker and gmackall May 3, 2024 23:48

bartekpacia added 2 commits May 4, 2024 05:04

fix android_plugin_skip_unsupported_test

296726d

retrigger CI (flaky)

bce879b

bartekpacia commented May 4, 2024

View reviewed changes

nate-thegrate reviewed May 4, 2024

View reviewed changes

bartekpacia and others added 2 commits May 5, 2024 13:29

fix "compile against" wording

4b4aa3e

Co-authored-by: Nate <[email protected]>

Revert "fix "compile against" wording"

6c7df79

This reverts commit 4b4aa3e.

gmackall reviewed May 6, 2024

View reviewed changes

remove redundant if-statements

b860dc4

reidbaker approved these changes May 7, 2024

View reviewed changes

gmackall approved these changes May 7, 2024

View reviewed changes

bartekpacia added the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024

auto-submit bot merged commit 7cf1969 into flutter:master May 7, 2024
129 checks passed

engine-flutter-autoroll mentioned this pull request May 7, 2024

Roll Flutter from 04424e10fe28 to 7920a52dd568 (27 revisions) flutter/packages#6683

Merged

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

7cf1969ad Improve Android SDK and NDK mistmatch warning message (flut…

3d09544

…ter/flutter#147809)

bartekpacia deleted the feature/android_sdk_or_ndk_mistmatch_better_error_msg branch May 7, 2024 17:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Android SDK and NDK mistmatch warning message #147809

Improve Android SDK and NDK mistmatch warning message #147809

bartekpacia commented May 3, 2024 •

edited

flutter-dashboard bot commented May 3, 2024

bartekpacia May 4, 2024

pretizy May 4, 2024

bartekpacia May 4, 2024

pretizy May 4, 2024

nate-thegrate May 4, 2024

bartekpacia May 5, 2024

nate-thegrate left a comment

nate-thegrate May 4, 2024

nate-thegrate May 4, 2024

bartekpacia May 5, 2024

bartekpacia May 5, 2024 •

edited

nate-thegrate May 5, 2024

bartekpacia May 6, 2024

reidbaker May 7, 2024

gmackall left a comment

gmackall May 6, 2024

bartekpacia May 6, 2024

gmackall May 6, 2024

bartekpacia May 6, 2024

	project.logger.error("Your project is configured to compile against Android SDK $projectCompileSdkVersion, but the following plugin(s) require to be compiled against a higher Android SDK version:")
	project.logger.error("Your project is configured to compile using Android SDK $projectCompileSdkVersion, but the following plugin(s) require a higher Android SDK version:")

Improve Android SDK and NDK mistmatch warning message #147809

Improve Android SDK and NDK mistmatch warning message #147809

Conversation

bartekpacia commented May 3, 2024 • edited

Pre-launch Checklist

flutter-dashboard bot commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nate-thegrate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartekpacia May 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmackall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartekpacia commented May 3, 2024 •

edited

bartekpacia May 5, 2024 •

edited