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

Add support for EnableGoogleServicesPlugin #438

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions bin/templates/project/app/build.gradle
Expand Up @@ -30,6 +30,7 @@ buildscript {

dependencies {
classpath 'com.android.tools.build:gradle:3.1.0'
classpath 'com.google.gms:google-services:3.2.1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to wrap this classpath in an if condition, like how the apply is done?
It would be nice to minimize the amount of downloading of build related libraries if they are not needed.

if (cdvHelpers.getConfigPreference('EnableGoogleServicesPlugin', 'false').toBoolean()) {
  classpath 'com.google.gms:google-services:3.2.1'
}

Copy link
Contributor Author

@chemerisuk chemerisuk Sep 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the object cdvHelpers is not available at the dependency scope. Project build failed with the error:

Could not get unknown property 'cdvHelpers' for object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

Happens when I apply the changes you suggest.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chemerisuk Yes, it seems cordova.gradle would need to be applied within the scope so it becomes available.

    dependencies {
        apply from: '../CordovaLib/cordova.gradle'

        classpath 'com.android.tools.build:gradle:3.1.0'

        if(cdvHelpers.getConfigPreference('EnableGoogleServicesPlugin', 'false').toBoolean()) {
            println 'Adding classpath "com.google.gms:google-services:3.2.1"'

            classpath 'com.google.gms:google-services:3.2.1'
        }
    }

@dpogue Would you know off-hand if something like this would be problematic? The cordova.gradle content seems to be only helper methods. Adding it seems to solve the desire to make the classpaths conditional.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about Gradle, and my experience consists mostly of swearing at it.

I believe you're correct that cordova.gradle is helper methods, but I don't know anything about scoping or when it's safe to import (apply?) stuff.

}
}

Expand Down Expand Up @@ -322,3 +323,7 @@ for (def func : cdvPluginPostBuildExtras) {
if (hasProperty('postBuildExtras')) {
postBuildExtras()
}

if (cdvHelpers.getConfigPreference('EnableGoogleServicesPlugin', 'false').toBoolean()) {
apply plugin: 'com.google.gms.google-services'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chemerisuk I've had to make this line:

apply plugin: 'com.google.gms.googleservices.GoogleServicesPlugin'

because the class must be used instead of id(string) to be able to apply plugin from non-root gradle file. I haven't tested the PR yet but I'm assuming the reason you are using the ID is because it is a root gradle file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because this is the root Gradle file. Worked fine on my side.

For testing I commented changes from cordova-support-google-services and inserted PR code manually. Then rebuilt the project and tried to start (btw google services v15 trigger error at runtime, if ‘google.services’ was not processed properly while previously it was build time error).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}