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 22:24:22 GMT
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
View raw message