You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by marcuspridham <gi...@git.apache.org> on 2015/08/26 20:44:25 UTC
[GitHub] cordova-lib pull request: CB-9560 Issue using plugin restore for p...
GitHub user marcuspridham opened a pull request:
https://github.com/apache/cordova-lib/pull/288
CB-9560 Issue using plugin restore for plugins with common dependencies
Similar change to CB-9278. This change makes it so only one plugin is restored at a time.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/marcuspridham/cordova-lib CB-9560
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/288.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 #288
----
commit e282bc64cd07fa2b49d200d1ad627a0af8532536
Author: Marcus Pridham <ma...@sap.com>
Date: 2015-08-26T18:37:32Z
CB-9560 Issue using plugin restore for plugins with common dependencies
----
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135899844
LGTM ! @marcuspridham, I'll pull these changes in.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135899446
@dpogue , I see what's going on there. This might call for us to be able to handle dependent plugin's versioning in Cordova.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135883628
This issue arises because [common.copyFileNew] (https://github.com/apache/cordova-lib/blob/dff78b0779444d06e259140694e07d5e8c5f5308/cordova-lib/src/plugman/platforms/common.js#L64) throws an exception when it tries to copy a file that already exists, having been placed there by a previously installed plugin.
So, I'm curious why does this not show up when we do the copying in serial, but shows up when we do it in parallel ?
Seems fishy to me.
---
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-9560 Issue using plugin restore for p...
Posted by marcuspridham <gi...@git.apache.org>.
Github user marcuspridham commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135566988
Added comments. I'll try to put together a sample that shows the issue. The ones I am using are not available on NPM.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135879399
Cool @marcuspridham . thanks for the confirmation.
---
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-9560 Issue using plugin restore for p...
Posted by dpogue <gi...@git.apache.org>.
Github user dpogue commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135884862
A related issue about dependency versions from config.xml not being respected when they are dependencies of another plugin is [CB-9525](https://issues.apache.org/jira/browse/CB-9525)
---
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-9560 Issue using plugin restore for p...
Posted by marcuspridham <gi...@git.apache.org>.
Github user marcuspridham commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135886715
In serial it seems to be able to detect the file plugin is already installed and does not try to install it again. The parallel way you can see in the logs that it does not see the plugin is installed yet so it runs the install again. There is a race condition.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135557411
@marcuspridham, could you please add a comment like the one added for CB-9278 ?
That way, we could signal other devs not to try and optimize this piece of code by parallelizing it ( switching back to 'map' function).
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135564467
Also, does this happen when you use the following two plugins : cordova-plugin-file and cordova-plugin-file-transfer ?
Otherwise, could you please provide some example plugins which elicit this behavior ?
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135556631
@marcuspridham, could you please add a comment like this one ?
That way, we could signal other devs not to try and optimize this piece of code by parallelizing it ( switching back to 'map' function).
---
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-9560 Issue using plugin restore for p...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/288
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-137900148
Merged !
---
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-9560 Issue using plugin restore for p...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135300595
@omefire to review
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135883534
This issue arises because [common.copyFileNew] (https://github.com/apache/cordova-lib/blob/dff78b0779444d06e259140694e07d5e8c5f5308/cordova-lib/src/plugman/platforms/common.js#L64) throws an exception when it tries to copy a file that already exists, having been placed there by a previously installed plugin.
So, I'm curious why does this not show up when we do the copy in serial, but shows up when we do it in parallel ?
Seems fishy to me.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135565178
Also, does this happen when you use the following two plugins : cordova-plugin-file and cordova-plugin-file-transfer ?
Otherwise, could you please provide some example plugins which elicit this behavior ?
I'd like to test using them.
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135884296
This issue arises because [common.copyFileNew] (https://github.com/apache/cordova-lib/blob/dff78b0779444d06e259140694e07d5e8c5f5308/cordova-lib/src/plugman/platforms/common.js#L64) throws an exception when it tries to copy a file that already exists, having been placed there by a previously installed plugin.
So, I'm curious why does this not show up when we do the copying in serial, but shows up when we do it in parallel ? [the way the code is structured, it should happen no matter what.].
Seems fishy to me.
---
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-9560 Issue using plugin restore for p...
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-137920543
@omefire These pull requests should just close themselves if you merge->push
Are you rebasing?
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-138107803
@purplecabbage This one did close itself. (asfgit closed it)
---
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-9560 Issue using plugin restore for p...
Posted by marcuspridham <gi...@git.apache.org>.
Github user marcuspridham commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135579250
Yep it does seem to happen with cordova-plugin-file-transfer and cordova-plugin-file. I added the following to the default cordova project config.xml and added a platform
<plugin name="cordova-plugin-file-transfer" spec="~1.2.1" />
<plugin name="cordova-plugin-file" spec="~3.0.0" />
You can see in the output that it is trying to install the file plugin twice. With my change the error does not happen...
Cyar:cdvtest marcuspridham$ cordova platform add android
Adding android project...
Creating Cordova project for the Android platform:
Path: platforms/android
Package: io.cordova.hellocordova
Name: HelloCordova
Activity: MainActivity
Android target: android-22
Copying template files...
Android project created with cordova-android@4.1.1
Discovered plugin "cordova-plugin-whitelist" in config.xml. Installing to the project
Discovered plugin "cordova-plugin-file-transfer" in config.xml. Installing to the project
Discovered plugin "cordova-plugin-file" in config.xml. Installing to the project
Fetching plugin "cordova-plugin-whitelist@1" via npm
Fetching plugin "cordova-plugin-file-transfer@~1.2.1" via npm
Fetching plugin "cordova-plugin-file@~3.0.0" via npm
Installing "cordova-plugin-whitelist" for android
Installing "cordova-plugin-file-transfer" for android
Fetching plugin "cordova-plugin-file" via npm
Installing "cordova-plugin-file" for android
Installing "cordova-plugin-file" for android
The Android Persistent storage location now defaults to "Internal". Please check this plugins README to see if you application needs any changes in its config.xml.
If this is a new application no changes are required.
If this is an update to an existing application that did not specify an "AndroidPersistentFileLocation" you may need to add:
"<preference name="AndroidPersistentFileLocation" value="Compatibility" />"
to config.xml in order for the application to find previously stored files.
Error during processing of action! Attempting to revert...
Failed to install 'cordova-plugin-file':Error: Uh oh!
"/Users/marcuspridham/cdvtest/platforms/android/src/org/apache/cordova/file/EncodingException.java" already exists!
---
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-9560 Issue using plugin restore for p...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/288#issuecomment-135892414
I see. The parallel mode exhibits a race condition with regards to this check : [platformJson.isPluginInstalled](https://github.com/apache/cordova-lib/blob/bed6d3a201e0193b2ffdb60fa6de74de0c870cec/cordova-lib/src/plugman/install.js#L275).
---
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