You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by david-barth-canonical <gi...@git.apache.org> on 2015/09/01 19:48:36 UTC
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
GitHub user david-barth-canonical opened a pull request:
https://github.com/apache/cordova-lib/pull/294
CB-9590 - Ubuntu support for the new plugin naming convention
This patch allows ubuntu users to add plugins using the new naming convention and build properly. This also leaves support in place for the old style plugin names, during the transition.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cordova-ubuntu/cordova-lib CB-9590
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/294.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #294
----
commit 187711dd7fb7cd21e9ea276239b2e8b87a9e1ee6
Author: David Barth <da...@canonical.com>
Date: 2015-09-01T17:40:35Z
Ubuntu support for the new plugin naming convention
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by david-barth-canonical <gi...@git.apache.org>.
Github user david-barth-canonical commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-148743166
ok, i understand the problem now. In particular, the code to guess the
classname is nice, but other platforms resort to encapsulating this
information into the plugin.xml file which is safer.
We have some new code in cordova-ubuntu to handle dependencies, so I wonder
how much of that logic needs to be updated in cordova-lib, rather than
directly in cordova-ubuntu? See mardy's new branch
https://github.com/mardy/cordova-ubuntu/commit/d3a674f2a07ab269bfb2e2acaa60dc2ab25c4d50
On Thu, Oct 15, 2015 at 6:26 PM, Tim Barham <no...@github.com>
wrote:
> @david-barth-canonical <https://github.com/david-barth-canonical> - I
> think @stevengill <https://github.com/stevengill>'s point is that the
> intent of your change is to strip the cordova-plugin from the start of
> the plugin name to generate the class name, to get the equivalent of the
> old functionality. That logic won't work with plugins that don't start with
> cordova-plugin-. Perhaps an alternative would be to look for -plugin- in
> the name, and take everything after that (?otherwise just use the whole
> name).
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cordova-lib/pull/294#issuecomment-148444797>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-148109218
I want to merge this in, but it doesn't seem to support plugins that don't start with cordova-plugin-*. Example, phonegap-plugin-push. Plugins can be named anything now. See http://cordova.apache.org/plugins/
Thoughts on how to handle this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by david-barth-canonical <gi...@git.apache.org>.
Github user david-barth-canonical commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/294#discussion_r46471716
--- Diff: cordova-lib/src/plugman/platforms/ubuntu.js ---
@@ -29,6 +29,51 @@ function toCamelCase(str) {
}).join('');
}
+function getPluginXml(plugin_dir) {
+ var et = require('elementtree'),
+ fs = require('fs'),
+ path = require('path');
+
+ var pluginxml;
+ var config_path = path.join(plugin_dir, 'plugin.xml');
+
+ if (fs.existsSync(config_path)) {
+ // Get the current plugin.xml file
+ pluginxml = et.parse(fs.readFileSync(config_path, 'utf-8'));
+ }
+
+ return pluginxml;
+}
+
+function findClassName(pluginxml, plugin_id) {
+ var class_name;
+
+ // first check if we have a class-name parameter in the plugin config
+ if (pluginxml) {
+ var platform = pluginxml.find("./platform/[@name='ubuntu']/");
+ if (platform) {
+ var param = platform.find("./config-file/[@target='config.xml']/feature/param/[@name='ubuntu-package']");
+ if (param && param.attrib) {
+ class_name = param.attrib.value;
+ return class_name;
+ }
+ }
+ }
+
+ // fallback to guess work, based on the plugin package name
+
+ if (plugin_id.match(/\.[^.]+$/)) {
+ // old-style plugin name
+ class_name = plugin_id.match(/\.[^.]+$/)[0].substr(1);
+ class_name = toCamelCase(class_name);
+ } else {
+ class_name = plugin_id.match(/cordova\-plugin\-([\w\-]+)$/)[0].substr(15);
--- End diff --
Thanks! I updated the branch to take that into account. I was forcing plugins to upgrade to adding their class name to the XML file, but there is a more gradual way ;)
If that PR can't be re-opened I opened https://github.com/apache/cordova-lib/pull/349 . Otherwise feel free to close this other one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by david-barth-canonical <gi...@git.apache.org>.
Github user david-barth-canonical commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-138310314
It does work both with old style plugin names, like "org.apache.cordova.battery-status" and new style names "cordova-plugin-media-capture". I can add and build a demo application with the 2 of those here on my system.
The facebookconnect plugin failed compilation in my tests, but it seems more due to the absence of ubuntu platform support code.
If there are still issues, can you give an example or a log of where it fails?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by david-barth-canonical <gi...@git.apache.org>.
Github user david-barth-canonical commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-148342742
It does work on my system at least. I run this branch along with my other 3 pull requests, but I don't think they have an influence on the plugin installation process.
dbarth@thinkmac:~/devel/wily/cordova/demo$ cordova -d plugin add phonegap-plugin-push
no version specified, retrieving version from config.xml
Calling plugman.fetch on plugin "phonegap-plugin-push"
Fetching plugin "phonegap-plugin-push" via npm
Copying plugin "/home/dbarth/.npm/phonegap-plugin-push/1.3.0/package" => "/home/dbarth/devel/wily/cordova/demo/plugins/phonegap-plugin-push"
Calling plugman.install on plugin "/home/dbarth/devel/wily/cordova/demo/plugins/phonegap-plugin-push" for platform "ubuntu
Installing "phonegap-plugin-push" for ubuntu
Running command: /home/dbarth/devel/wily/cordova/demo/platforms/ubuntu/cordova/version
Command finished with error code 0: /home/dbarth/devel/wily/cordova/demo/platforms/ubuntu/cordova/version
cordova has been detected as using a development branch. Attemping to install anyways.
Install start for "phonegap-plugin-push" on ubuntu.
Beginning processing of action stack for ubuntu project...
Action stack processing complete.
Install complete for phonegap-plugin-push on ubuntu.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by david-barth-canonical <gi...@git.apache.org>.
Github user david-barth-canonical commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-153422772
I have update the pull request to add support for taking the class name from the plugin.xml file. For plugins using one of the 2 existing naming conventions, we can fallback to "guess work", otherwise new plugins using arbitrary names can just specify the 'package-ubuntu' parameter (like iOS or Android) to specify the class name directly.
I decided to go ahead and keep on adding code to the cordova-lib for now, but will start adding support for that preparation step in cordova-ubuntu direclty. In 2 steps, I imagine we can migrate core plugins to using the new parameter and deprecate the specific code from the ubuntu/install stage in cordova-lib eventually.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/294
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-148444797
@david-barth-canonical - I think @stevengill's point is that the intent of your change is to strip the `cordova-plugin` from the start of the plugin name to generate the class name, to get the equivalent of the old functionality. That logic won't work with plugins that don't start with `cordova-plugin-`. Perhaps an alternative would be to look for `-plugin-` in the name, and take everything after that (?otherwise just use the whole name).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/294#discussion_r46228904
--- Diff: cordova-lib/src/plugman/platforms/ubuntu.js ---
@@ -29,6 +29,51 @@ function toCamelCase(str) {
}).join('');
}
+function getPluginXml(plugin_dir) {
+ var et = require('elementtree'),
+ fs = require('fs'),
+ path = require('path');
+
+ var pluginxml;
+ var config_path = path.join(plugin_dir, 'plugin.xml');
+
+ if (fs.existsSync(config_path)) {
+ // Get the current plugin.xml file
+ pluginxml = et.parse(fs.readFileSync(config_path, 'utf-8'));
+ }
+
+ return pluginxml;
+}
+
+function findClassName(pluginxml, plugin_id) {
+ var class_name;
+
+ // first check if we have a class-name parameter in the plugin config
+ if (pluginxml) {
+ var platform = pluginxml.find("./platform/[@name='ubuntu']/");
+ if (platform) {
+ var param = platform.find("./config-file/[@target='config.xml']/feature/param/[@name='ubuntu-package']");
+ if (param && param.attrib) {
+ class_name = param.attrib.value;
+ return class_name;
+ }
+ }
+ }
+
+ // fallback to guess work, based on the plugin package name
+
+ if (plugin_id.match(/\.[^.]+$/)) {
+ // old-style plugin name
+ class_name = plugin_id.match(/\.[^.]+$/)[0].substr(1);
+ class_name = toCamelCase(class_name);
+ } else {
+ class_name = plugin_id.match(/cordova\-plugin\-([\w\-]+)$/)[0].substr(15);
--- End diff --
This will fail with a JavaScript error if `plugin_id` does not start with `cordova-plugin`, since `match()` will return `null`. Instead, should do the `match()` call separately, then only try to get the first array element if the return value is not `null`. If the return value *is* `null`, then should use some fallback (like camel-case the entire `plugin_id`, for example).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9590 - Ubuntu support for the new plu...
Posted by Zaspire <gi...@git.apache.org>.
Github user Zaspire commented on the pull request:
https://github.com/apache/cordova-lib/pull/294#issuecomment-137925691
it will not work for third_party plugins (e.g. com.phonegap.plugins.facebookconnect)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org