CB-13145: proposal for Play Services Version

[audreyso]

Proposal for Play Services

[stevengill]

Sorry to clarify this is one potential solution. The one from the issue. But the new proposal is actually below.

[goya]

could we allow it inside a platform tag as well for extra flexibility?

plugin variable || platform variable || global variable

[stevengill]

@goya that sounds reasonable. Lets say yes for now as we look at the implementation details.

[janpio]

Anything that gets rid of these gradle error messages has my full support.
Proposal looks good.

[filmaj]

I like @goya's suggestion. Also, why leverage <preference> in config.xml but <variable> in plugin.xml? Could we standardize on <variable> across the board?

[stevengill]

@filmaj sorry for the confusion. The first solution was to use <preference> but this document is proposing not to do that and instead to just use <variable>

[macdonst]

So just a heads up. That this may end up being more work than initially thought. Right now if we try to work around this by doing:

<preference name="FCM_VERSION" default="11.0.1"/>

and then trying to use it in the framework tag like so:

<framework src="com.google.firebase:firebase-messaging:$FCM_VERSION"/>

results in a build error:

* What went wrong:
A problem occurred evaluating root project 'android'.
> Could not get unknown property 'FCM_VERSION' for object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

Because the $FCM_VERSION variable does not get replaced in the framework tag.

[stevengill]

Okay update time!

So @audreyso and I have done some deep investigations into this work.

Firstly, I think our proposal here is too ambitious and overkill for now.

Intro to how plugin variables work now:

• plugin.xml defines a <preference name='varName' default='varValue'> tag.
• also in plugin.xml, inside of a <config-file> or <edit-config> tag, you define a place to use the variable. Something like <string>$varName</string>.
• When <config-file> or <edit-config> is getting executed, they do a Regex replace on plugin.xml replacing $varName with varValue. https://github.com/apache/cordova-common/blob/master/src/ConfigChanges/ConfigChanges.js#L300-L307
• Variables are only replaced inside of <config-file> or <edit-config>. We don't handle variables anywhere else inside plugin.xml
• Important to note that plugin.xml never gets changed during this process. The regex replace is only done on a in memory representation of plugin.xml and then the replaced values are used when <config-file> or <edit-config> writes out to their intended targets (AndroidManifest, plist, config.xml, etc)
• the value for the variable will either be the default (varValue) or a value passed in via the command line when doing plugin add. If doing a plugin restore, passing in plugin variable values via cli isn't possible, so we check config.xml for previously saved variables + values. These are only used during plugin restore (aka cordova prepare) and not during cordova plugin add.

Hopefully that explains our current plugin variable situation.

Now, back to the problem.

We need a way to define a variable, and have the <framework> tag consume it. Something like:

<preference name='varName' default='varValue'>

<framework src='someString:$varName'>

This would replace $varName with varValue.

I've sent a PR to cordova-common that adds support for the above solution. apache/cordova-common#4.

When the frameworks are being grabbed from plugin.xml, it will find and replace $varName with varValue. For Android, this then gets written out to the apps project.properties & build.gradle.

One caveat about this solution:

• this only looks at the <preference> tags. Variables passed in via CLI or saved in config.xml are not supported.

Both the push plugin and the google analytics plugin will have to add a preference for the playServices version. They can manually be kept in sync for now by plugin authors.

For the future, it would be nice if we added the ability to check the variable tags in config.xml with the ability to override the default value when frameworks are being handled. That would require the preferences to have the same name in both plugins.

[macdonst]

@stevengill that's some pretty awesome progress today. However, I think we should support using the variable tag to override the default preference in the plugin.xml file. This way if the preference in the push plugin.xml is to use 11.0.1 and in the google analytics plugin.xml it is 9.8.0 the user would be able to specify something like

    <plugin name="cordova-plugin-google-analytics" spec="^1.4.3">
        <variable name="PLAY_SERVICES_VERSION" value="11.0.1" />
    </plugin>

That would end up giving us the most flexibility as plugin authors would define their own variable/preference name i.e. there is no need to get agreement with other plugin authors on those names. Also, users could override the default version number without having to send a PR to the plugin authors.

[goya]

plugin variable || platform variable || global variable

not a preference element. the preference namespace is too unimaginative to NOT conflict with a random variable some plugin author chooses.

preference element is for core or platform only. IMHO.

[stevengill]

@macdonst I agree. I've now updated the PR to work with variable tags from config.xml. apache/cordova-common#4

I have only implemented it for the framework add usecase (both restore + cli passed in variable). For remove, it still uses getPreferences (aka default value in preference element in plugin.xml). I will have to add cli_variables to the uninstallOptions variable in cordova-lib.

To test this:

1. pull the PR above into your local cordova-common
2. npm link cordova-common to cordova-lib
3. go to local cordova-android repo and open package.json. Update cordova-common dependency to the path to your local cordova-common (npm link won't work)

ex: "cordova-common": "../cordova-common",

You can now run the following commands (on modified push plugin.xml from #74 (comment))

cordova plugin add ../../../phonegap/phonegap-plugin-push/  --variable FCM_VERSION='10.0.0

or to restore

Add to config.xml:
<plugin name="phonegap-plugin-push" spec="../../../phonegap/phonegap-plugin-push">
        <variable name="FCM_VERSION" value="9.0.0" />
</plugin>

Run:
cordova prepare

Whats left?

1. get remove to use passed in variables or config.xml variables
2. write tests

[stevengill]

@goya I agree that preference element seems wrong to use. But they are being used already for plugin variables (See #74 (comment)). They are tied toconfig-file and edit-config elements.

Are you suggesting we stop using preference even for edit-config and config-file and switch over to using a variable element instead? Or just not use preference for framework variable replacing.

One safety we do have is that the regex looks for $varName to replace instead of just varName.

The way it is setup currently, I don't think we would have clashes from different plugins using the same variable. In config.xml, the variable tag is still currently tied to a specific plugin. When we get the preferences from a plugin's plugin.xml, we can only use the variable tags associated with it. If we introduce global or platform variable tags in config.xml, I could see clashes arising if two plugins used same variable/preference name. I don't think we need to do global/platform variable tags anymore.

We can just expect the user to manually change versions if they need to.

In config.xml

<plugin name="phonegap-plugin-push" spec="../../../phonegap/phonegap-plugin-push">
        <variable name="FCM_VERSION" value="9.0.0" />
</plugin>
<plugin name="cordova-plugin-googleanalytics" spec="../../../phonegap/somePath">
        <variable name="FCM2_VERSION" value="11.0.0" />
</plugin>

In the above example, the user would have to manually make the versions (value) the same and restore. It doesn't matter if the variable name they use is the same or not between the two plugins.

[audreyso]

Hi all! Here are 2 PRs for cordova-lib and cordova-common
https://github.com/apache/cordova-lib/pull/590/files
https://github.com/apache/cordova-common/pull/6/files

If you could review or have any comments/advice/questions, that would be great! Thanks so much!

[audreyso]

Here are the main points of the solution we ended up using:

• The framework tag can now use variables.
• You can add and remove variables in the framework tag.
• We decided not to go with global variables for plugins.

Let us know if you have any questions!