You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by gorkem <gi...@git.apache.org> on 2015/02/12 17:52:55 UTC
[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms
GitHub user gorkem opened a pull request:
https://github.com/apache/cordova-lib/pull/166
Save/Restore for plugins and platforms
I think this is a good point to merge these features.. It provides most of the functionality and frankly I am tired of rebasing/merging.
Several features implemented.
1. Implements auto-restore for platforms and plugins on prepare. Platforms are only restore when a platforms is specifically requested.
2. Implements --save flag for cordova plugins add/rm
3. removes the old save and restore commands
4. removes the old tests for save and restore
What is missing:
1. Switch to plugin tag on config.xml. ( Another PR is coming for that shortly)
2. tests for --save flag
3. tests for autorestore
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gorkem/cordova-lib save_restore_final
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/166.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 #166
----
commit 037a5c2eae54d9ea69c71185ec2c7d78c500fb9a
Author: Gorkem Ercan <go...@gmail.com>
Date: 2015-02-10T00:48:48Z
remove save and restore commands
removes save.js and restore.js and references to those files
commit a70ae2d0c3927cbb6bdb1dc214db469ea74086dd
Author: Gorkem Ercan <go...@gmail.com>
Date: 2015-02-10T00:50:43Z
restore plugins and platforms on prepare
Collect the restore logic to restore-util.js and uses it from prepare to restore plugins and
platforms during prepare
commit 07e3862fd928248b7b8d63a9da2ca374d9e5c528
Author: Gorkem Ercan <go...@gmail.com>
Date: 2015-02-10T01:01:47Z
remove save and restore tests
commit ce976e93d4a5adb540af685cf0f171f14e54d73a
Author: Gorkem Ercan <go...@gmail.com>
Date: 2015-02-11T19:00:50Z
fix for test after prepare changes
restore util.cdProjectRoot which prepare depends on to generate correct error message.
commit d899bc23536449749586a00a6c1765f2950c03ef
Author: Gorkem Ercan <go...@gmail.com>
Date: 2015-02-12T16:26:25Z
--save flag for plugins
save implementation for adding or removing plugins
----
---
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: Save/Restore for plugins and platforms
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/166#discussion_r24875047
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -136,6 +135,35 @@ module.exports = function plugin(command, targets, opts) {
events.emit('verbose', 'Calling plugman.fetch on plugin "' + target + '"');
return plugman.raw.fetch(target, pluginsDir, { searchpath: searchPath, noregistry: opts.noregistry, link: opts.link, pluginInfoProvider: pluginInfoProvider});
})
+ .then(function(dir){
+ // save to config.xml
+
+ if(autosave || opts.save){
+ var pluginInfo = pluginInfoProvider.get(dir);
--- End diff --
Good chunk of code here. Would be appropriate for a helper method.
---
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: Save/Restore for plugins and platforms
Posted by gorkem <gi...@git.apache.org>.
Github user gorkem commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74294685
sure... as long as there plugin install logic uses variables from config.xml, which does not.
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74151061
Hey @gorkem, I notice that --save covers add and remove. Are you going to send a pull request to handle the update case ? : 'cordova plugin update org.apache.cordova.device@3.5.0 --save'
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-75640710
when --save/autosave is on, we should be overriding what's in config.xml, like we do on the platform side : https://github.com/apache/cordova-lib/blob/master/cordova-lib/src/cordova/platform.js#L161
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74143188
if no version is specified for (e.g: cordova plugin add org.apache.cordova.device --save), we end up saving the current edge version (org.apache.cordova.device@0.2.13) into config.xml. I think no version specified means always get the 'latest' edge version, not pin (and get) the 'current' edge version.
Also, this behavior is different from how 'cordova platform add android --save' currently behaves.
I think we should not save the current edge version in this case, and upon restoring we restore the edge version instead.
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74155732
When the flag --save/autosave on, we should not be retrieving the version from config.xml.
For example,
- config.xml contains org.apache.cordova.device@0.2.9
- running 'cordova plugin add org.apache.cordova.device --save' results in org.apache.cordova.device@0.2.9 being saved into config.xml instead of org.apache.cordova.device. (with no version).
This means that the check on this line should be updated: https://github.com/apache/cordova-lib/blob/master/cordova-lib/src/cordova/plugin.js#L124
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74293656
Also, we should probably save the plugin variables along with the version into config.xml, so that on prepare/restore, we don't fail.
---
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: Save/Restore for plugins and platforms
Posted by danielcompton <gi...@git.apache.org>.
Github user danielcompton commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-92569269
What is the command to run the restore command? I'm using cordova 4.3.0 and I get this message running restore:
```sh
$cordova restore
Cordova does not know restore; try `cordova help` for a list of all the available commands.
```
Looking at https://github.com/apache/cordova-cli/pull/209, the restore docs were removed. I can see code has been added in the Cordova lib repository but it doesn't seem to be working. Can we add documentation for running the restore? I assume it does work but I'm just calling it wrong.
Running `npm ls -g`
```sh
...
├─┬ cordova@4.3.0
│ ├─┬ cordova-lib@4.3.0
│ │ ├── bplist-parser@0.0.6
│ │ ├─┬ cordova-js@3.8.0
│ │ │ ├─┬ browserify@7.1.0
│ │ │ │ ├── assert@1.1.2
...
```
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74145216
I think the 'saving to config.xml' should be moved into a separate function and called from here. this function is getting way too long to easily reason about 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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74146324
'saving to config.xml' should happen after the plugin has been installed on the platforms.
That way, if any error is encountered during the install, the config.xml does not get updated.
What do you think ?
---
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: Save/Restore for plugins and platforms
Posted by gorkem <gi...@git.apache.org>.
Github user gorkem commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-75547919
I think I have addressed the concerns with the latest. If everyone is cool with it, we should go ahead and merge them 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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74313313
hmmm, it seems to me that during prepare, while restoring from config.xml, we do use variables from config.xml. so, we should save it during --save/autosave. Please, let me know if I'm missing something :
https://github.com/gorkem/cordova-lib/blob/a70ae2d0c3927cbb6bdb1dc214db469ea74086dd/cordova-lib/src/cordova/restore-util.js#L108
---
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: Save/Restore for plugins and platforms
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/166#discussion_r24875027
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -136,6 +135,35 @@ module.exports = function plugin(command, targets, opts) {
events.emit('verbose', 'Calling plugman.fetch on plugin "' + target + '"');
return plugman.raw.fetch(target, pluginsDir, { searchpath: searchPath, noregistry: opts.noregistry, link: opts.link, pluginInfoProvider: pluginInfoProvider});
})
+ .then(function(dir){
+ // save to config.xml
--- End diff --
please stick with a 4-space indent (used everywhere else)
---
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: Save/Restore for plugins and platforms
Posted by gorkem <gi...@git.apache.org>.
Github user gorkem commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74284189
Save logic is a bit disorganized because I was expecting @omefire to implement it for plugins as well, so I have whipped up something. I will probably make another pass with the syntax change for plugins on config.xml.
Here are some notes
* **no version on save:** The original save command implementation had a flag called -shrinkwrap that was used to indicate that you want to persist the version for the plugin to config.xml, without such a flag users need to know the plugin version beforehand to be able to pin. I am a bit conflicted about what to do without such a flag.
* ** retrieving the version from config.xml:** I have already added a check for this but I am failing to see how this feature is used. If an entry for a plugin already exists on config.xml, it will be restored (provided that this PR is applied) the moment that you try to do something useful with CLI (run,build, prepare, add another plugin) so the only way this feature will be executed is if you clone a project and try to add the plugin immediately before issuing any other commands.
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-75574968
Reviewing latest changes ...
---
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: Save/Restore for plugins and platforms
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/166
---
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: Save/Restore for plugins and platforms
Posted by gorkem <gi...@git.apache.org>.
Github user gorkem commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-92777120
correct. `restore` command is removed and restoration happens during `prepare`. You can call `cordova prepare` explicitly to get the similar results to `restore`.
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-75656828
what about the other options ? (opts).
shouldn't we pass them along to 'prepare' ?
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74128449
Cool feature list. reviewing 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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74151973
Hey @gorkem, I notice that --save covers add and remove. Are you going to send a pull request to handle the update case ? : 'cordova plugin update org.apache.cordova.device@3.5.0 --save'.
[IGNORE THIS COMMENT AS IT IS NOT APPLICABLE, there is no update for plugins]
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-75615699
you forgot to negate opts.save as well : if(!autosave && !opts.save && ...)
---
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: Save/Restore for plugins and platforms
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-74318530
What if a plugin install fails ? don't we want to continue with other plugins' install like we do for platforms ?
That way, we also handle platform-dependent plugins that are going to fail for certain platforms.
WDYT ?
---
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: Save/Restore for plugins and platforms
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:
https://github.com/apache/cordova-lib/pull/166#issuecomment-92610783
I believe the decision was made not to implement `restore`, since it happens automatically on `prepare`.
---
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