cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian Clelland <iclell...@google.com>
Subject Re: Uninstallation of dependency-only plugins
Date Fri, 12 Jul 2013 03:16:53 GMT
Works perfectly now!

Agreed about looking into the 'right way' to do this; there are likely
still edge conditions that we just haven't encountered yet.

This is working great for now, though.



On Thu, Jul 11, 2013 at 6:24 PM, Filip Maj <fil@adobe.com> wrote:

> Hey Ian,
>
> I've patched this, pushed as plugman@0.9.3, and updated the CB-4077 branch
> of cordova-cli to use the new plugman.
>
> Assuming it works out for you (I tested with the file plugin), I will
> merge back into master and publish a new cli version.
>
> On 7/11/13 12:51 PM, "Ian Clelland" <iclelland@google.com> wrote:
>
> >It fails differently for me now... I think it's because of the second
> >level
> >dependency of file-transfer on file.
> >
> >On uninstall I can see all platform-level commands succeeding, and all
> >plugins are removed from both ios and android. Then uninstallPlugin starts
> >to delete the plugin dirs, deletes org.apache.cordova.core.file, and
> >breaks
> >when it tries to delete org.apache.cordova.core.file-transfer:
> >
> >Removing plugin org.cordova.mobile-spec-dependencies...
> >Dependencies detected, iterating through them and removing them first...
> >Removing plugin org.apache.cordova.core.battery-status...
> >org.apache.cordova.core.battery-status removed.
> >Removing plugin org.apache.cordova.core.camera...
> >org.apache.cordova.core.camera removed.
> >Removing plugin org.apache.cordova.core.console...
> >org.apache.cordova.core.console removed.
> >Removing plugin org.apache.cordova.core.contacts...
> >org.apache.cordova.core.contacts removed.
> >Removing plugin org.apache.cordova.core.device...
> >org.apache.cordova.core.device removed.
> >Removing plugin org.apache.cordova.core.device-motion...
> >org.apache.cordova.core.device-motion removed.
> >Removing plugin org.apache.cordova.core.device-orientation...
> >org.apache.cordova.core.device-orientation removed.
> >Removing plugin org.apache.cordova.core.dialogs...
> >org.apache.cordova.core.dialogs removed.
> >Removing plugin org.apache.cordova.core.file...
> >org.apache.cordova.core.file removed.
> >Removing plugin org.apache.cordova.core.file-transfer...
> >Dependencies detected, iterating through them and removing them first...
> >Error: ENOENT, no such file or directory
> >'/Users/iclelland/Code/Cordova3/mobilespec/plugins/org.apache.cordova.core
> >.file/plugin.xml'
> >
> >We could probably fix that by having uninstallPlugin ignore failures
> >caused
> >by missing dependencies (the point of the function is to ensure that
> >they're gone, and it's done that! :) )
> >
> >The other option is to sort the plugins, and dependencies, such that the
> >'deepest' dependents are always uninstalled first (including transitive
> >relationships like this), and work up to the top-level. I suspect that's a
> >lot more work, though.
> >
> >To test, I just wrapped uninstallPlugin in a simple try / catch block,
> >ignoring errors, and it was able to remove everything. Let me know if you
> >want me to send a patch.
> >
> >Ian
> >
> >
> >On Thu, Jul 11, 2013 at 3:17 PM, Filip Maj <fil@adobe.com> wrote:
> >
> >> Hey Ian,
> >>
> >> I think I've got it working. The issue was that I was invoking
> >> uninstallPlugin (which does plugin repo removal) when uninstallPlatform
> >> would recurse into dependencies.
> >>
> >> I've now tweaked the behavior: uninstallPlatform _NEVER_ invokes
> >> uninstallPlugin, and uninstallPlugin now recursed when it detects
> >> dependencies.
> >>
> >> I think it should be good to go now, I've published plugman 0.9.2 to npm
> >> with this fix. Can you give it one more shot on your end, just to be
> >>safe?
> >> I ran through your steps below with 0.9.2 and it worked for me.
> >>
> >> On 7/11/13 10:45 AM, "Ian Clelland" <iclelland@google.com> wrote:
> >>
> >> >I'm getting an error on uninstall -- haven't tracked it down yet, but
> >>you
> >> >might have some insight.
> >> >
> >> >My test sequence looks like this:
> >> >
> >> >cordova create cb4077test com.google.cb4077test
> >> >cd cb4077test
> >> >cordova platform add ios
> >> >cordova platform add android
> >> >cordova plugin install ../cordova-mobile-spec/dependencies-plugin
> >> >cordova plugin rm org.cordova.mobile-spec-dependencies
> >> >
> >> >On install, everything works correctly.
> >> >
> >> >The uninstall process starts like this:
> >> >Calling plugman.uninstall on plugin
> >>"org.cordova.mobile-spec-dependencies"
> >> >for platform "android"
> >> >Uninstalling 18 dangling dependent plugins...
> >> >Uninstalling org.apache.cordova.core.battery-status...
> >> >
> >> >and then proceeds to remove all 18 plugins. It then tries to do the
> >>same
> >> >for iOS, and fails immediately, because the plugins have already been
> >> >deleted. The last few lines of debug output are
> >> >
> >> >Calling plugman.uninstall on plugin
> >>"org.cordova.mobile-spec-dependencies"
> >> >for platform "ios"
> >> >Error: ENOENT, no such file or directory
> >>
> >>>'/Users/iclelland/Code/Cordova3/cb4077test/plugins/
> org.apache.cordova.co
> >>>re
> >> >.battery-status/plugin.xml'
> >> >    at Object.fs.openSync (fs.js:413:18)
> >> >    at Object.fs.readFileSync (fs.js:270:15)
> >> >    at Object.module.exports.parseElementtreeSync
> >>
> >>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/uti
> >>>l/
> >> >xml-helpers.js:107:27)
> >> >    at
> >>
> >>>/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/util
> >>>/d
> >> >ependencies.js:21:35
> >> >    at Array.forEach (native)
> >> >    at Object.module.exports.generate_dependency_info
> >>
> >>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/uti
> >>>l/
> >> >dependencies.js:20:45)
> >> >    at runUninstall
> >>
> >>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/uni
> >>>ns
> >> >tall.js:63:40)
> >> >    at Function.module.exports.uninstallPlatform
> >>
> >>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/uni
> >>>ns
> >> >tall.js:42:5)
> >> >    at /Users/iclelland/Code/Cordova3/cordova-cli/src/plugin.js:149:51
> >> >    at Array.forEach (native)
> >> >
> >> >The plugins directory is left with just android.json (correct, with
> >> >plugins
> >> >removed), ios.json (still full of plugins), and the
> >> >org.cordova.mobile-spec-dependencies plugin not removed.
> >> >
> >> >
> >> >On Thu, Jul 11, 2013 at 1:19 PM, Filip Maj <fil@adobe.com> wrote:
> >> >
> >> >> Thank you Ian!
> >> >>
> >> >> On 7/11/13 10:17 AM, "Ian Clelland" <iclelland@google.com> wrote:
> >> >>
> >> >> >Taking a look now (Sorry I missed this thread yesterday). I'll
have
> >>an
> >> >> >update for you shortly.
> >> >> >
> >> >> >
> >> >> >On Thu, Jul 11, 2013 at 1:04 PM, Filip Maj <fil@adobe.com>
wrote:
> >> >> >
> >> >> >> FYI plugman 0.9.0 is pushed up to npm and the cli is waiting
> >>review
> >> >>by
> >> >> >>Ian
> >> >> >> / Google folk. I've pushed up a branch CB-4077 to the cli
that
> >> >> >>integrates
> >> >> >> with the new plugman. Can you guys check that this branch
works
> >> >>properly
> >> >> >> for any of your flows?
> >> >> >>
> >> >> >> I'll assume everything works out if I don't hear back from
you
> >>guys
> >> >>and
> >> >> >> move forward with it later today.
> >> >> >>
> >> >> >> On 7/10/13 7:34 AM, "Ian Clelland" <iclelland@google.com>
wrote:
> >> >> >>
> >> >> >> >The new plugman works for me, when coupled with my CB-4077
> >>branch of
> >> >> >>cli.
> >> >> >> >
> >> >> >> >I noticed that the <project-root>/plugins/<plugin-dir>
> >>directories
> >> >> >>don't
> >> >> >> >get removed for dangling dependencies -- only the top-level
> >>plugin
> >> >>is
> >> >> >> >removed from there. However, the dependents are removed
from all
> >> >> >> >platforms,
> >> >> >> >so the uninstallation works correctly.
> >> >> >> >
> >> >> >> >I'll rebase my CLI against the newly-refreshed-master-branch
and
> >> >>force
> >> >> >> >push
> >> >> >> >it to github for you.
> >> >> >> >
> >> >> >> >Ian
> >> >> >> >
> >> >> >> >
> >> >> >> >On Tue, Jul 9, 2013 at 10:49 PM, Ian Clelland
> >><iclelland@google.com
> >> >
> >> >> >> >wrote:
> >> >> >> >
> >> >> >> >> Sounds good, Fil -- I'll take a look at the updates,
and run it
> >> >> >>through
> >> >> >> >> its paces here. I'll let you know right away if I
find anything
> >> >> >>unusual.
> >> >> >> >>
> >> >> >> >> Ian
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> On Tue, Jul 9, 2013 at 5:31 PM, Filip Maj <fil@adobe.com>
> >>wrote:
> >> >> >> >>
> >> >> >> >>> I've pushed up a CB-4077 branch of plugman up
to the apache
> >>git
> >> >> >>repo.
> >> >> >> >>>It
> >> >> >> >>> is a few extra commits on top of yours, Ian,
addressing some
> >> >>other
> >> >> >> >>>issues
> >> >> >> >>> I noticed during testing.
> >> >> >> >>>
> >> >> >> >>> It looks like it is safe to merge into master,
but I would
> >>like
> >> >>Ian
> >> >> >>and
> >> >> >> >>> Google co. to once-over it before we merge into
master.
> >> >> >> >>>
> >> >> >> >>> Once that¹s in plugman, we can publish a new
version of it to
> >> >>npm,
> >> >> >> >>>update
> >> >> >> >>> the dependency in cordova-cli, and make sure
it is in working
> >> >>order
> >> >> >> >>>with
> >> >> >> >>> the new plugman before we proceed with a cli
update.
> >> >> >> >>>
> >> >> >> >>> Sound good?
> >> >> >> >>>
> >> >> >> >>> On 7/5/13 12:01 PM, "Ian Clelland" <iclelland@google.com>
> >>wrote:
> >> >> >> >>>
> >> >> >> >>> >Oh, don't be sad, Brian ;)
> >> >> >> >>> >
> >> >> >> >>> >That's why I only pushed to my fork; looking
for constructive
> >> >> >>review.
> >> >> >> >>> >
> >> >> >> >>> >And now I know where the cli and plugman
tests are, and they
> >> >>shall
> >> >> >>be
> >> >> >> >>> made
> >> >> >> >>> >better before anything is pushed to a real
repo.
> >> >> >> >>> >
> >> >> >> >>> >
> >> >> >> >>> >
> >> >> >> >>> >On Fri, Jul 5, 2013 at 2:23 PM, Brian LeRoux
<b@brian.io>
> >> wrote:
> >> >> >> >>> >
> >> >> >> >>> >> =(
> >> >> >> >>> >>
> >> >> >> >>> >> Should go without saying but lets not
commit stuff without
> >> >>first
> >> >> >> >>> >> ensuring the tests pass, eh.
> >> >> >> >>> >>
> >> >> >> >>> >>
> >> >> >> >>> >> On Fri, Jul 5, 2013 at 10:05 AM, Filip
Maj <fil@adobe.com>
> >> >> wrote:
> >> >> >> >>> >> > Added comments to the issue thread.
The tests no longer
> >> >>pass +
> >> >> >> >>>we'll
> >> >> >> >>> >>need
> >> >> >> >>> >> > new tests to cover your changes.
> >> >> >> >>> >> >
> >> >> >> >>> >> > On 7/4/13 8:21 PM, "Ian Clelland"
<iclelland@google.com>
> >> >> wrote:
> >> >> >> >>> >> >
> >> >> >> >>> >> >>Thanks, Fil,
> >> >> >> >>> >> >>
> >> >> >> >>> >> >>Created CB-4077 to track this.
I'll start working on
> >> >>separating
> >> >> >> >>>those
> >> >> >> >>> >> >>functions.
> >> >> >> >>> >> >>
> >> >> >> >>> >> >>Ian
> >> >> >> >>> >> >>
> >> >> >> >>> >> >>On Thu, Jul 4, 2013 at 7:08
PM, Filip Maj <fil@adobe.com
> >
> >> >> >>wrote:
> >> >> >> >>> >> >>
> >> >> >> >>> >> >>> File an issue over at issues.cordova.io,
tag plugman,
> >>and
> >> >>we
> >> >> >> can
> >> >> >> >>> go
> >> >> >> >>> >> from
> >> >> >> >>> >> >>> there
> >> >> >> >>> >> >>>
> >> >> >> >>> >> >>> On 7/4/13 12:59 PM, "Ian
Clelland"
> >><iclelland@google.com>
> >> >> >> wrote:
> >> >> >> >>> >> >>>
> >> >> >> >>> >> >>> >This is the first time
I've tried to use the CLI tools
> >> >>with
> >> >> >>the
> >> >> >> >>> new
> >> >> >> >>> >> 3.0
> >> >> >> >>> >> >>> >project structure,
and I've discovered that I can't
> >> >> >>uninstall a
> >> >> >> >>> >>plugin
> >> >> >> >>> >> >>> >that
> >> >> >> >>> >> >>> >only has dependencies
(no source files, either JS or
> >> >>native)
> >> >> >> >>> >> >>> >
> >> >> >> >>> >> >>> >Specifically, I've
built a mobilespec app, installing
> >> >> >> >>> >> >>> >the mobile-spec-dependencies
plugin, which does
> >>nothing
> >> >>but
> >> >> >> >>>depend
> >> >> >> >>> >>on
> >> >> >> >>> >> >>> >every
> >> >> >> >>> >> >>> >Cordova core plugin.
I want to remove it, so that I
> >>can
> >> >> >>remove
> >> >> >> >>>and
> >> >> >> >>> >> >>> >reinstall one of the
dependencies, but the CLI tools
> >>will
> >> >> >>not
> >> >> >> >>> >>remove
> >> >> >> >>> >> >>>it.
> >> >> >> >>> >> >>> >
> >> >> >> >>> >> >>> >Digging through cordova-cli,
it looks like "cordova
> >> >>plugin
> >> >> >>rm"
> >> >> >> >>> >> >>>attempts to
> >> >> >> >>> >> >>> >invoke plugman.uninstall
once per platform, but
> >> >> >> >>> >> >>>mobile-spec-dependencies
> >> >> >> >>> >> >>> >doesn't declare any
platforms.
> >> >> >> >>> >> >>> >
> >> >> >> >>> >> >>> >plugman.uninstall seems
to do two things, which I
> >>think
> >> >> >>should
> >> >> >> >>>be
> >> >> >> >>> >> >>> >separated: It removes
the plugin from a specific
> >> >>platform,
> >> >> >>and
> >> >> >> >>>it
> >> >> >> >>> >> >>>removes
> >> >> >> >>> >> >>> >the plugin from the
project itself.
> >> >> >> >>> >> >>> >
> >> >> >> >>> >> >>> >In the case of a dependency-only
plugin, we only need
> >>to
> >> >>do
> >> >> >>the
> >> >> >> >>> >>second
> >> >> >> >>> >> >>> >task
> >> >> >> >>> >> >>> >(which currently doesn't
get done). For a regular
> >>plugin
> >> >> >>which
> >> >> >> >>>is
> >> >> >> >>> >> >>> >installed
> >> >> >> >>> >> >>> >in multiple platforms,
this also fails, since removing
> >> >>the
> >> >> >> >>>plugin
> >> >> >> >>> >>for
> >> >> >> >>> >> >>>the
> >> >> >> >>> >> >>> >first platform deletes
the plugin source directory,
> >>and
> >> >>then
> >> >> >> >>> >>removal
> >> >> >> >>> >> >>>for
> >> >> >> >>> >> >>> >subsequent platforms
fails with the error message
> >> >>"[Error:
> >> >> >> >>>Plugin
> >> >> >> >>> >> >>><plugin
> >> >> >> >>> >> >>> >id> not found. Already
uninstalled?]"
> >> >> >> >>> >> >>> >
> >> >> >> >>> >> >>> >Can anyone explain
the technical reasons behind this,
> >>or
> >> >> >> >>>should I
> >> >> >> >>> >>work
> >> >> >> >>> >> >>>on
> >> >> >> >>> >> >>> >separating those functions?
> >> >> >> >>> >> >>>
> >> >> >> >>> >> >>>
> >> >> >> >>> >> >
> >> >> >> >>> >>
> >> >> >> >>>
> >> >> >> >>>
> >> >> >> >>
> >> >> >>
> >> >> >>
> >> >>
> >> >>
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message