Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add a gradle warning to the AndroidX plugins #1138

Merged
merged 4 commits into from
Feb 8, 2019
Merged

Add a gradle warning to the AndroidX plugins #1138

merged 4 commits into from
Feb 8, 2019

Conversation

mklim
Copy link
Contributor

@mklim mklim commented Jan 30, 2019

This warning is only printed once per plugin on build failure.

flutter/flutter#27106

@mklim
Copy link
Contributor Author

mklim commented Jan 30, 2019

The website hasn't been updated with the docs on opening a Flutter app for Android editing yet so this is still WIP, but the instructions are available here for now.

This is what it looks like on build failure:

failure

This is what it looks like on build success:

success

@mklim mklim requested review from amirh, dnfield and Hixie January 30, 2019 19:45
@dnfield
Copy link
Contributor

dnfield commented Jan 30, 2019

Would it be feasible to check whether their build.gradle is using incompatible libraries/sdks before printing this and exiting?

@mklim mklim changed the title [WIP] Add a gradle warning to the AndroidX plugins Add a gradle warning to the AndroidX plugins Jan 30, 2019
@mklim
Copy link
Contributor Author

mklim commented Jan 30, 2019

The IDE instructions are available at https://flutter.io/docs/development/tools/android-studio#android-ide.

I also found a hook for build failure. Now it doesn't print anything when the build succeeds, and looks like the previous screenshot still when the build fails. This could still log when the build fails for reasons unrelated to AndroidX though.

@Hixie
Copy link
Contributor

Hixie commented Feb 2, 2019

I found a reason why this won't be enough: we don't control all the plugins. I depend on flutter_local_notifications, and when I tried to upgrade to the latest version today, I got the horrific and inexplicable Gradle error.

@mklim
Copy link
Contributor Author

mklim commented Feb 4, 2019

@Hixie this PR won't fix build failures in that case, but I don't think that will stop this PR from at least printing out a warning at the top of the console. Since it's still a gradle error the various scripts from our plugins will still get the on gradle failure hook and log a message.

@mklim
Copy link
Contributor Author

mklim commented Feb 4, 2019

@Hixie this PR won't fix build failures in that case, but I don't think that will stop this PR from at least printing out a warning at the top of the console. Since it's still a gradle error the various scripts from our plugins will still get the on gradle failure hook and log a message.

Discussed offline, I misunderstood the case here. The remaining problem is when somebody has picked up an AndroidX plugin from somewhere other than flutter/plugins. Since this patch just applies warnings to the plugins in that repo, it won't cover any other plugin migration failures. I'll follow up with a PR to flutter's tooling to catch build errors globally and direct to a help page for this class of error. I still think this PR is worth merging since it catches cases for anyone using a flutter/plugins package who isn't on the latest version of flutter itself.

@dnfield
Copy link
Contributor

dnfield commented Feb 7, 2019

This LGTM - I understand it as an improvement for at least some things we control, even if it's not the full solution

Michael Klimushyn added 4 commits February 7, 2019 16:17
This warning is only printed once per plugin, but prints every single
time the APK is built regardless of whether or not the crash is
required.
@Hixie
Copy link
Contributor

Hixie commented Feb 8, 2019

LGTM assuming that we land the tool side that hides this message very soon. (Maybe first.)

We should probably document how to do this so that other plugin authors can do the same.

mklim pushed a commit to flutter/flutter that referenced this pull request Feb 8, 2019
Try to detect Gradle error messages that hint at AndroidX problems, and
warn in the logs about the potential problem and point to documentation
on how to fix the issue.

Unfortunately the Gradle errors based on this root issue are varied and
project dependent. It's probably better to still leave the message
intact in case the problem is unrelated.

Also filters out the plugin warning message pending in
flutter/plugins#1138. It's still valuable to add that for people on
previous versions of Flutter, but this link should override that message
for anyone on an up to date version of Flutter.

#27106
@mklim
Copy link
Contributor Author

mklim commented Feb 8, 2019

LGTM assuming that we land the tool side that hides this message very soon. (Maybe first.)

We should probably document how to do this so that other plugin authors can do the same.

Tool PR has landed so I'm going to merge this. I added some notes for plugin maintainers to our website but didn't mention this kind of Gradle warning, I'll update it in a follow up to mention it.

@mklim mklim merged commit 5169655 into flutter:master Feb 8, 2019
@mklim mklim deleted the gradle_warning branch February 8, 2019 01:26
@sjmcdowall
Copy link

BTW -- is the Flutter team going to write up (or if they did, where is it?) how to upgrade a normal Flutter application -- in Flutter Speak -- not Android speak -- when upgrading plugins and then hitting the dreaded AndroidX cascade of errors? I've tried TWICE now upgrading some of the core packages that are now AndroidX and had to revert after spending 2-3 hours chasing down obscure Gradle error messages ..

This is a pretty normal (?) app that has about 20+ pubspec.yaml dependencies -- many 3rd party -- which aren't converted (and may never be?) ..

I think there needs to be a concise "Conversion" document -- what things must be changed in an existing projects gradle files ..

This is on the beta branch BTW.. I tried creating a new project and comparing the Gradle files but don't see any changes that I think are "significant" .. it's still using SDK 27 stuff, etc.

Anyway, so far this has been pretty frustrating .. almost as much as using a Swift Plugin within the app .. :)

Cheers

@mklim
Copy link
Contributor Author

mklim commented Feb 8, 2019

Hey @sjmcdowall. I'm really sorry that this has been so frustrating. Thanks for the feedback.

This PR and the related plugins one link to this doc on our website: https://flutter.io/docs/development/packages-and-plugins/androidx-compatibility. Does this fit what you're thinking of? The goal is that this should work as a conversion doc for Flutter apps specifically. We're trying to iterate and improve it, very open to any input on it.

@sjmcdowall
Copy link

@mklim -- let me take a look and see what that links says... and more importantly -- let me try (after reading it) how the upgrade to the latest AndroidX plugins (that are converted) works..

If I still run into issues -- is this an appropriate place to report them or start a new issue?

Cheers!

@mklim
Copy link
Contributor Author

mklim commented Feb 9, 2019

@sjmcdowall thanks! If you're still running into issues it would be best if you could open a new issue in the flutter/flutter repo explaining the problem. Feel free to cc me.

kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
Try to detect Gradle error messages that hint at AndroidX problems, and
warn in the logs about the potential problem and point to documentation
on how to fix the issue.

Unfortunately the Gradle errors based on this root issue are varied and
project dependent. It's probably better to still leave the message
intact in case the problem is unrelated.

Also filters out the plugin warning message pending in
flutter/plugins#1138. It's still valuable to add that for people on
previous versions of Flutter, but this link should override that message
for anyone on an up to date version of Flutter.

flutter#27106
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
This warning is only printed once per plugin on build failure.

flutter/flutter#27106
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
This warning is only printed once per plugin on build failure.

flutter/flutter#27106
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
This warning is only printed once per plugin on build failure.

flutter/flutter#27106
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
This warning is only printed once per plugin on build failure.

flutter/flutter#27106
Akachu pushed a commit to Akachu/flutter_camera that referenced this pull request Apr 27, 2020
This warning is only printed once per plugin on build failure.

flutter/flutter#27106
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants