dpa99c / cordova-android-play-services-gradle-release

Cordova/Phonegap plugin for Android to align versions of the Play Services library components specified by other plugins to a specific version.
111 stars 33 forks source link

Use a plugin variable to set the required library version #2

Closed christocracy closed 7 years ago

christocracy commented 7 years ago

Hi, I'm the author of a few cordova and react-native plugins. I've been struggling with the best way to solve play-services dependences myself for years.

While reading Gradle Dependency Resolution, I found myself here.

I've tweaked your script a bit:

I'm not sure what this does, I've left it out of my script.

// This will be important for google APIs 11.2.*, since they're now hosted at Maven
repositories{    
  maven {
    url 'https://maven.google.com'
  }
}

////
// Enter your desired google API version for all dependencies
// Would be nice to somehow make this configurable via <preference /> in plugin.xml
// Would require a <hook /> script to edit this file.
//
def GOOGLE_API_VERSION = '11.2.0';

////
// List of play-services libs to search for.
// Here's a list of all Google APIs:  https://developers.google.com/android/reference/packages
def GOOGLE_APIS = [
    'com.google.android.gms',
    'com.google.firebase'
]

configurations.all {
    resolutionStrategy.eachDependency { DependencyResolveDetails details ->
        if (details.requested.group in GOOGLE_APIS) {            
            details.useVersion GOOGLE_API_VERSION
        }
    }
}
christocracy commented 7 years ago

I've implemented ability to provide <variable name="GOOGLE_API_VERSION" value="11.2.0" /> in config.xml and have that re-write the .gradle file with a <hook type="before_prepare" />

I'm going to rename my version cordova-google-api-version

$ cordova plugin add cordova-google-api-version --variable GOOGLE_API_VERSION=11.2.0
dpa99c commented 7 years ago

Nice idea to make the version number configurable by a variable.

I'm not sure what this does, I've left it out of my script.

The "if not multidex" bit was present in the Gradle script I pilfered from elsewhere, but I think it's pretty redundant in Cordova applications which are not going to be using MultiDex

christocracy commented 7 years ago

Yea, multidex seems pretty hackish to me. I don't think it should be the business of a plugin to enable that.

I see that cordova-plugin-push now enables it.

christocracy commented 7 years ago

I see you're a plugin author as well. I sent you a donation, from one plugin author to another.

I'm the author of:

I've got the before_prepare script simply writing properties.gradle file which gets included from the main .gradle file

dpa99c commented 7 years ago

I sent you a donation, from one plugin author to another.

Thanks, really appreciate it. If only our plugin users appreciated us as much ;-)

Those are some really great plugins you've authored. I've been considering making the jump to React Native and porting some of my plugins as I go: though I love Cordova for what it is and has done for hybrid apps, I'm feeling the tide turn towards React Native. How much effort was it to convert the plugin interfaces for RN?

Some of my plugins I'm thinking of converting are:

christocracy commented 7 years ago

For background-geolocation, I spent a fair bit of time extracting all the core code into pure native libraries (a .framework for iOS and an .aar for Android).

It's the same with background-fetch. Both the cordova and react-native plugins share a common library transistor-background-fetch

The Cordova & React Native plugins are very similar, just a very thin facade in front of a pure native library shared by both. There's very little logic at all within the actual plugin code. Just proxies into the native library and implementing the semantics of the particular platform's callback / events mechanisms.

dpa99c commented 7 years ago

Great, that's what I wanted to hear: should merely be a case of changing the plugin interface to React Native vs Cordova.

I'll make a note to retrospectively add your variable version approach to this plugin and to cordova-android-support-gradle-release.

christocracy commented 7 years ago

I'm feeling the tide turn towards React Native

I see it objectively in sales of background-geolocation year-to-date stats:

Jan 1, 2016 - Aug 28, 2016 vs Jan 1, 2017 - Aug 28, 2017

dpa99c commented 7 years ago

Wow, those stats say it all.

With regard to your background geolocation plugin, have tested/fixed for Android 8 (O) Beta?

I have a walk navigation app that uses my own background geolocation solution and I think I need to be worried about the upcoming Android 8 policy towards background geolocation: https://developer.android.com/about/versions/oreo/background-location-limits.html

dpa99c commented 7 years ago

To be honest, if you've cracked the background geo problem for Android O, I might just re-invest your donation back to you and buy your plugin from you :-)

christocracy commented 7 years ago

I'm going to try O this week. I had a good read of the background execution policy and I think there'll be little effect on the lib since it allows for running as a foreground service.

If you want free keys for my lib, let me know.

christocracy commented 7 years ago

You can try Android free in debug builds.

License is enforced only in release builds

dpa99c commented 7 years ago

Ah, cool. I will give it go - just awaiting return of a Nexus 6P so I can real-world test my app with Android O (only tried Android O emulator so far). Will let you know the results.

christocracy commented 7 years ago

I've got O installed now. Everything is working normally. No changes needed so far.

It even auto-started after the first OS reboot (startOnBoot: true)

christocracy commented 7 years ago

Btw, gradle conflicts are much easier to solve in RN apps since the app is a standard Android app, no auto-generated magic like Cordova.

It's very easy to exclude/force groups from other plugins in the app's build.gradle

dpa99c commented 7 years ago

@christocracy I've updated both my plugins to use a variable for the version, based on your script. Again: nice solution!

FYI I'm still waiting on my Nexus 6P with Android 8.0 - some doofus posted it to the wrong address so now I'm waiting for the courier to recover it and send it to me. Once that's happened, I'm going to give your background plugin a good real-world test in Android 8.0 with my walking app: I'm expecting your plugin to be fine vs my current implementation to fail miserably - I'll let you know the results!

My app's here on Google Play if you're interested, though I'm planning to split it off from my company as an non-profit entity since it's more for the local community than for making money, but it gets a good amount of use from loyal users, so good for testing background geolocation to death!

dpa99c commented 7 years ago

@christocracy Results of my Android 8 testing are in: as expected, my current implementation based on Fused Background Location API fails miserably on Android 8: no background locations received once the app is backgrounded. However, using your plugin with the foreground service option, it performed flawlessy, both with screen off and with another app in the foreground.

If you want free keys for my lib, let me know.

If the offer still stands, I'll take you up on it! Would only need one for the iWalk Cornwall app.

christocracy commented 7 years ago

Email me: chris@transistorsoft.com