cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Maj <...@adobe.com>
Subject Re: Uninstallation of dependency-only plugins
Date Thu, 11 Jul 2013 19:17:55 GMT
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.core
>.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/util/
>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/util/
>dependencies.js:20:45)
>    at runUninstall
>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/unins
>tall.js:63:40)
>    at Function.module.exports.uninstallPlatform
>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/unins
>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
View raw message