Excellent, I'll merge into master shortly and bump the version. On 7/11/13 8:16 PM, "Ian Clelland" <iclell...@google.com> wrote:
>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.co >>>re >> >.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/u >>>>>ti >> >>>l/ >> >> >xml-helpers.js:107:27) >> >> > at >> >> >> >>>>>/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/ut >>>>>il >> >>>/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/u >>>>>ti >> >>>l/ >> >> >dependencies.js:20:45) >> >> > at runUninstall >> >> >> >>>>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/u >>>>>ni >> >>>ns >> >> >tall.js:63:40) >> >> > at Function.module.exports.uninstallPlatform >> >> >> >>>>>(/Users/iclelland/Code/Cordova3/cordova-cli/node_modules/plugman/src/u >>>>>ni >> >>>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? >> >> >> >> >>> >> >>> >> >> >> >> >>> >> >>> >> >> >> >> >>> >> > >> >> >> >> >>> >> >> >> >> >> >>> >> >> >> >> >>> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >>
