Warn when gradle builds fail because of AndroidX #27566
Conversation
|
This is still a WIP until the website has updated docs to link to. Worth reviewing now to see if this basic approach is solid though. The AndroidX regex I added here is very broad and could have false positives. I'd rather have that than false negatives, and from the various reports in the wild about this it looks like the errors can manifest in a ton different ways. I don't think narrowing down the regexes to just perfectly capture the logs we've clearly seen so far is a good idea since there's potentially many more permutations of errors from this that we just haven't happened to see yet. We could theoretically try to do something smarter here than grepping for likely issues in the logs by checking for actual problems in the compileSdkVersion and gradle dependencies and confirming the exact mismatch. That approach would have the benefits of being able to completely confirm the issue in a way this naive grepping the error logs can't, but I'm wary about introducing the extra complexity. It would require running at least one gradle task after every build failure to confirm the dependency problems. So there would be an extra delay/use of resources every time a build failed. And it's possible that that secondary gradle task would also fail and spit out its own stream of hard to parse errors, so we'd need some extra backup handling for that. I'm also a little hesitant to totally trust gradle's reporting of its own dependency tree for something like this. |
There was a problem hiding this comment.
so sad that there's no one message :-(
|
approach seems solid, i'll defer to @jonahwilliams for detailed review |
There was a problem hiding this comment.
We should link to a wiki page or something that we can update if the situation changes
There was a problem hiding this comment.
Should we be overly broad, or overly conservative here? Do we know of any particular cases we can mis-identify as an AndroidX issue?
There was a problem hiding this comment.
At this point I think it's better to direct people to a help page that doesn't apply to them than to miss people who are completely blocked on this. The gradle messages here don't give enough information to debug this on their own. You can click through the issue links for some examples of what they end up looking like. I am only logging when Gradle exits with a failing status code to try and minimize the logs to when there's actually a crash.
Out of the super broad nets I'm throwing here:
- "androidx" will only be mentioned by Gradle if it's being brought into the project by something in the first place. It's highly likely that it's related. It could only be unrelated if the user already had brought it in and then had an error in Java files that use it directly.
- "android.support" could be mentioned as an unrelated error if it's being manipulated somewhere in Java code, like a broken import or calling a nonexistent method. So if users are manipulating Java directly to use the old support libraries (or their dependencies are) and the code breaks based on that this warning will get thrown up as a false positive. I think it's possible but probably unlikely, and when this happens it's likely that users will know that it's a false positive since they would probably have just been editing the Java files. (Unless they brought in a plugin that shipped in a broken state, but that also seems unlikely.)
- "AAPT" is really broad. It's just a reference to the step where resources are compiled. I don't know all the things that could cause this but I suspect that there's a lot of ways to get into a broken state here besides the AndroidX bug. Looking at our repo there's a large amount of closed issues that mention this in the past. If I look at open gradle issues in our repo Cleanup build #2464 is open for it still. Unfortunately this is also the main way this problem manifests right now.
There was a problem hiding this comment.
Appreciate the explanation.
jonahwilliams
left a comment
There was a problem hiding this comment.
Make sure to note which parts of this code are androidX specific, so that we can cleanly remove it once we have a long term solution
Done. I tagged |
jonahwilliams
left a comment
There was a problem hiding this comment.
LGTM to land when documentation is ready
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.
Fix logic so that plugin warnings for AndroidX aren't counted on their own as an indicator that this is an AndroidX related crash.
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
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