You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by bso-intel <gi...@git.apache.org> on 2016/02/28 02:57:19 UTC
[GitHub] cordova-android pull request: CB-10673 fixed conflicting plugin in...
GitHub user bso-intel opened a pull request:
https://github.com/apache/cordova-android/pull/264
CB-10673 fixed conflicting plugin install issue with overlapped <sour…
…ce-file> tag
The problem is that copyNewFile is too strict for <source-file> tag.
You will never know which two plugins will write the library to the same target-dir of the source-file.
We should let it copy the library to the same location.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bso-intel/cordova-android CB-10673
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-android/pull/264.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 #264
----
commit 06f167efd91e17e1f3125056bb1da85efae165e2
Author: Byoungro So <by...@intel.com>
Date: 2016-02-27T06:32:37Z
CB-10673 fixed conflicting plugin install issue with overlapped <source-file> tag
----
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-191341584
@brodybits Would love to see some contributions in the area of plugin dev documentation - we know we have a lot of gaps there - since you have a bunch of experience with this - would love your help. We did a big documentation update recently and will talk more about it (compare [dev](http://cordova.apache.org/docs/en/dev/guide/platforms/android/plugin.html) to [latest](http://cordova.apache.org/docs/en/6.x/guide/platforms/android/plugin.html) - but more can be done. [This is where](http://cordova.apache.org/docs/en/dev/guide/platforms/android/plugin.html) we could perhaps add an example scenario for NDK integration with the corresponding plugin.xml. [CB-10758](https://issues.apache.org/jira/browse/CB-10758) to track this.
@bso-intel Can you provide details of the plugin that is using source-file to copy libraries instead of lib-file? Is it open source? We can work with the author to fix this. IMO having a flag for this is not ideal - but it might be the only way out to support the customer use case.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by brodybits <gi...@git.apache.org>.
Github user brodybits commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190951704
I would favor leaving it to the plugins to use the `lib-file` instead of `source-file` to install the libraries. A couple of things that could really help:
- very clear documentation on how to specify the native NDK Android libraries (I did not see it anywhere)
- clarify the error message in cases like 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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190947034
My concern with silently passing here is that there could be a runtime/build failure because potentially the wrong version of the file is added depending on the order of adding pluigns or even worse the same filename has completely different source code.
Fixing those runtime or gradle build failure might not be a pleasant experience either for the user. There is no good solution for this - but working with the conflicting plugins to change where they add those files. For commonly used libraries, they should just use the `framework` element as that handles scenarios around multiple plugins adding common libraries.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-android/pull/264
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by bso-intel <gi...@git.apache.org>.
Github user bso-intel commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-191616111
Since now I created the --force-copying-src option, the current behavior of throwing exception is preserved.
To get around this issue, the user can use this new command-line option.
I have created several related pull requests. Please review them together.
https://github.com/apache/cordova-cli/pull/236
https://github.com/apache/cordova-lib/pull/404
https://github.com/apache/cordova-ios/pull/199
https://github.com/apache/cordova-windows/pull/154
Thank you.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by bso-intel <gi...@git.apache.org>.
Github user bso-intel commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-191319864
I am working on adding a command-line option --force-copying-src to allow the Cordova user to try overwriting the conflicting file.
I hope this gives some breathing room for those users who need to use two conflicting plugins.
The default value of this option is false, so the existing behavior is unchanged.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190929927
I agree we should improve the error messages - especially the 'Uh oh!' :)
We have three tags:
- [lib-file](http://cordova.apache.org/docs/en/dev/plugin_ref/spec.html#lib-file)
- [source-file](http://cordova.apache.org/docs/en/dev/plugin_ref/spec.html#source-file)
- [resource-file](http://cordova.apache.org/docs/en/dev/plugin_ref/spec.html#resource-file)
@bso-intel - Should the plugin be using `lib-file` for libraries instead of `source-file`, which already follows the 'copyFile' semantics?
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by bso-intel <gi...@git.apache.org>.
Github user bso-intel commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190778940
@vladimir-kotikov,
This is just one example of the conflicting plugins.
There could be so many plugins out there that requires the same library files.
Usually, the library files are very commonly used in many plugins.
Those files usually come from the same cordova-<platform> templates.
The problem is that we disallow those plugins to be installed with an exception that says "Uh oh! stacktrace...".
This makes impossible to use two plugins that use the same library files.
This is not plugin writer's fault or app developer's fault.
We may have to print the warning log that says "the library file already exists. skipped copying them" or "overwrote it.".
Throwing an exception is too strict in my opinion.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by bso-intel <gi...@git.apache.org>.
Github user bso-intel commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190952289
Right. I agree that silently passing is not good.
So, I said that we should print the warning message to the user "Warning: <file_name> in the <path> already exists. {Skipped copying/Overwrote} that file".
My proposal leaves some options to make the plugins runnable without changing the Cordova source code.
I am not sure how the current behavior of throwing an exception is better than this proposal.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190719953
@bso-intel, IMO there is a reason for current approach - `<source-file>` element is intended to handle `.java`, `.m` and other types of source files - not binary packages or the whole directories. In case of source files silently overwrite conflicting files might be a bad idea which might cause build-time failures.
I think in your case it's more appropriate to use `<resource-file>` element which also have `target` attribute to specify exact target location of copied files and uses `copyFile` internally.
---
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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190564383
@vladimir-kotikov to review. Is there a test case that we can add? PR #265 has a good framework for testing something like 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-android pull request: CB-10673 fixed conflicting plugin in...
Posted by bso-intel <gi...@git.apache.org>.
Github user bso-intel commented on the pull request:
https://github.com/apache/cordova-android/pull/264#issuecomment-190937260
Hi nikhill,
I am not a plugin writer so I don't know whether the plugin write can use other tags for their purposes.
This issue was reported from one of the Intel xdk users who needed to use both plugins in his app.
It looks to me that <source-file> tag is quite frequently used for some library files (as Vladimir pointed out).
Whenever the user complains about this issue, we (Cordova) can't blame the plugin developers for not checking with possible conflicts with other plugins.
I think the users will understand the new warning message (once aded), and if any issue arise by skipping/overwriting the existing files, and they can try a different order to resolve the issue.
However, when Cordova CLI throws an exception, it just leaves no option for the user.
It is going to be a dead end for this use case.
---
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