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 <f...@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" <iclell...@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 <f...@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" <iclell...@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 <f...@adobe.com> wrote: > >> > > >> >> Thank you Ian! > >> >> > >> >> On 7/11/13 10:17 AM, "Ian Clelland" <iclell...@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 <f...@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" <iclell...@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 > >><iclell...@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 <f...@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" <iclell...@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 <f...@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" <iclell...@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 <f...@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" > >><iclell...@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? > >> >> >> >>> >> >>> > >> >> >> >>> >> >>> > >> >> >> >>> >> > > >> >> >> >>> >> > >> >> >> >>> > >> >> >> >>> > >> >> >> >> > >> >> >> > >> >> >> > >> >> > >> >> > >> > >> > >
