This repository was archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Fix annotation dependency #1917
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
62b3f4f
Fix annotation dependency
blasten d0d4156
Merge branch 'master' into add-annotations
blasten 26b87d6
[firebase_auth] Patch increment
blasten c121ce6
Merge branch 'master' into add-annotations
blasten bce47ec
Merge branch 'master' into add-annotations
blasten 9b60f94
Merge branch 'master' into add-annotations
blasten f319fb3
Merge remote-tracking branch 'upstream/master' into add-annotations
blasten File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether it's better to just remove the 2 usages of the annotations, at least until we fix Jettifier, to prevent apps that didn't migrate to androidx from blowing up when using this plugin. However I'm not sure I understand the condition for when gradle blows up? I just tried to depend on this plugin from an app that wasn't migrated to androidx and it seems to work.
@blasten do you happen to know when Gradle blows up? would it reduce build failure for some apps if we don't include this dependency?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all 1P plugins?
Yeah. The issue occurs when a plugin relies on any
apitransitive dependency listed here and then one of these cases happen:Case 1
Plugin A imports
android.support.annotation.NonNull, expecting thatMaven Dependency Bwill always contain that api dependency.A developer decides to migrate App to AndroidX, which means Android's auto migration tool "Jetifier" will translate
android.support.annotation.NonNullintoandroidx.annotation.NonNull. The built is broken. Building the plugins as AAR fixes this problem.Case 2
Similar to case 1, but in this case we update a plugin to use an AndroidX dependency e.g.
androidx.annotation.NonNull, but it still relies on "Dependency B", which providesandroid.support.annotation.NonNullinstead. This works today because the example app that each plugin has also uses AndroidX, which then enables Jetifier auto translation. However, if the App developer isn't using AndroidX, then the build fails indicating thatandroidx.annotation.NonNullis undefined.Case 3
Similar to case 1, but in this case "Dependency B" upgrades to "AndroidX" and doesn't bump the major version. Now, the plugin is broken.
Solution
Relying on
api/compiletransitive dependencies is "dangerous" IMO. This seems to be a common pattern in Android development though. In the meanwhile, building the plugins as AAR fixes these issues because Jetifier can operate at the plugin level and translate all references.