Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency resolution doesn't work like you think it does #303

Closed
consp1racy opened this issue Sep 20, 2017 · 7 comments
Closed

Dependency resolution doesn't work like you think it does #303

consp1racy opened this issue Sep 20, 2017 · 7 comments

Comments

@consp1racy
Copy link

OneSignal automatically adds the following dependencies;

com.google.android.gms - Version 11.2.+
com.android.support - Version 26.1.+
If you get mixed version warnings like above in your build.gradle please make sure to update your other dependencies to match these versions.

If you must keep using an older version of these decencies add the following 4 lines, replacing the versions with the ones you require.

compile 'com.google.android.gms:play-services-gcm:11.2.+'
compile 'com.google.android.gms:play-services-location:11.2.+'

compile 'com.android.support:support-v4:26.1.+'
compile 'com.android.support:customtabs:26.1.+'

Seen here: https://documentation.onesignal.com/v3.0/docs/troubleshooting-android#section-error-all-gms-firesbase-libraries-must-use-the-exact-same-version-specification

Aftermath: https://stackoverflow.com/questions/46316151/android-sdk-26-build-error

Issue

Dependency resolution doesn't work that way. Gradle automatically picks the latest version specified in any library or module. If you say your library requires 11.2.+, Gradle will always pick the newest available, completely ignoring what the consumer wrote in their build.gradle.

Fix

Only specify the oldest dependency you own library can do with. Seems to me that OneSignal worked well with older libraries so depending on the newest available is just vanity and is bound to break your consumers' builds.

I've talked about the issue in depth here: https://stackoverflow.com/questions/42949974/android-support-repo-46-0-0-with-android-studio-2-3/42957234#42957234

@jkasten2
Copy link
Member

@consp1racy We have updated the documentation page to use force = true if you must downgrade to older com.google.android.gms and com.android.support libraries than what OneSignal defines.

In the 3.6.0 version of this SDK we had to bump the min version of com.android.support to 26.0.0 and com.google.android.gms to 10.2.1 to be compatible with projects that are targeting 26 (Android 8 Oreo).
targetSdkVersion 26
If you are using an older targetSdkVersion it will be safe to force older versions for OneSignal at least.

@consp1racy
Copy link
Author

Ah, I didn't know about the force. Many thanks! However you shouldn't use ranges in depedencies at all.

26.0.0 and 10.2.1 is fine by me, without the plus.

@jkasten2
Copy link
Member

jkasten2 commented Sep 21, 2017

@consp1racy I believe using version ranges on library's dependencies should be used when the developers of the library can confirm the range. Instead of just depending on an exact version. This makes the library more compatible with others when the package manager implements ranges correctly.

However, Gradle unfortunately always picks the latest as you noted. Instead of finding the latest version that meets all range requires for a group:module. This exact issue however is tracked on Gradle's repo gradle/gradle#2869 and it seems like something they do plan to fix.

Until then using a resolutionStrategy or using forced = true to lower versions looks to be the best options.

@consp1racy
Copy link
Author

I respectfully disagree with the last statement.

It seems to me it would be better to shift the heavy lifting from consumer to the author of library, i.e. instead of having to force everything, the library would depend on the oldest dependency it can do with and has been tested on, leaving the dependency upgrade on the consumer.

This is what the consumer expects, this behaves predictably.

@consp1racy
Copy link
Author

As for the dependency resolution, I looked at this and tried to understand how it works in Gradle.
https://maven.apache.org/enforcer/enforcer-rules/versionRanges.html

[26.0.0,27.0.0) - Use at least version 26, but prefer newer up until 27.
26.0.0 - Use at least version 26, but don't force newer.

It feels like the second option best describes your dependency.


https://cwiki.apache.org/confluence/display/MAVENOLD/Dependency+Mediation+and+Conflict+Resolution#DependencyMediationandConflictResolution-DependencyVersionRanges

Default strategy: Of the overlapping ranges, the highest soft requirement is the version to be used. If there are no soft requirements inside the prescribed ranges, the most recent version is used. If that does not fit the described ranges, then the most recent version number in the prescribed ranges is used. If the ranges exclude all versions, an error occurs.

If I understand correctly, Gradle does not respect the first statement. The best course of action (i.e. easier on consumer and requiring no consumer action once it's fixed) seems not to use ranges in libraries.

On the other hand your proposal requires the consumer acknowledge this issue. Is this what you're going for?

@jkasten2
Copy link
Member

@consp1racy Defaulting to the oldest possible dependencies that a plugin can support I don't believe is the best strategy either. Google makes regular updates to Android Support Library and Google Play services that includes bugs fixes the app could be missing out on. Now since these two dependencies are used so many other libraries out there and directly by the app the chances are low that OneSignal will be the only one including these.

OneSignal will probably be the only one to have a dependency on com.google.android.gms:play-services-gcm. However if OneSignal were to depend on it's current minimum of 10.2.1 (as of version SDK 3.6.4) then it would conflict with developers that are using say com.google.android.gms:play-services-games:11.4.2. We would end up seeing compile errors or runtime errors about missing Java class files. A range of dependencies per module still won't solve the issue either. The root of the problem is not lining up the version across all modules under the group.

Group alignments versions doesn't seem to be something I could find Gradle working on. However even if this was coming soon there are other more specific edge cases to these libraries and Android SDK doesn't make sense for them to bake in. I'll cover these edges cases below.

Summary

What Gradle doesn't do that needs to be solved in some way.

  1. Version alignment of all modules under a group
    • Gradle doesn't do this either by default nor has a feature to do this.
      • They expect you to write a custom resolutionStrategy if you want this.
    • In the cases of gms, firebase, and support libraries there are exceptions to the rules.
      • com.android.support:multidex - This only has versions 1.0.X
      • com.google.firebase and com.google.android.gms must be aligned with each other.
  2. Android SDK version dependency
    • The version of CompileSdkVersion in your project can affect which libraries you can use.
    • Version 26 of the com.android.support can only be used if CompileSdkVersion 26 is set. Otherwise if the project has CompileSdkVersion 25 or older the project can only use the latest 25 of com.android.support. Otherwise it results in unhelp errors.
  3. Honor version ranges of group:module using the newest of the overlap
    • This is what I linked to before Gradle should honor version ranges constraints properly gradle/gradle#2869
      • This looks to be fixed in Gradle 4.3 which isn't released yet.
    • This in itself won't be an issue if rules are put into place at the group alignment versions anyway.
      • I put more weight on this in my last 2 comments before fully understanding the scope of this.
    • Side note, after Gradle 4.3 is live I can see ranges being used by more plugins slowly over time.

Gradle Plugin

Just released a beta Gradle plugin that address issues 1 and 2 above.
https://github.com/OneSignal/OneSignal-Gradle-Plugin

Note, the group alignment versions are defined staticly in the plugin in the current 0.5.0 version. I hope to correct this shortly, by using the dependencies' version directly from the project instead. Basically find and use the latest version of any com.google.android.gms used in the project or dependencies. Whether that ends up being OneSignal, another SDK, or a module directly defined in the project.

However until then the current state of the plugin provides an easy to implement and automatic solution to these immediate conflicts. Feedback can be gathered on any issues before this become part of the standard setup instructions. Would be interested in your feedback on this!

@jkasten2
Copy link
Member

jkasten2 commented Dec 6, 2017

@consp1racy Feel free to reopen this issue if you have any additional feedback on this. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants