pnpm / supi

Fast, disk space efficient installation engine. Used by pnpm
MIT License
24 stars 5 forks source link

fix: run node-gyp rebuild when install is not specified #28

Closed etamponi closed 6 years ago

etamponi commented 6 years ago

The intent was there before, but the code assumed node-gyp was in the node_modules under supi, which might not be the case because of dependency deduping. Instead of doing path incantations, I just rewrite script['install'] to equal the default https://docs.npmjs.com/misc/scripts

zkochan commented 6 years ago

I think using require.resolve to find the node-gyp location would also work

zkochan commented 6 years ago

oh, I just recalled, I moved node-gyp to npm-lifecycle. https://github.com/npm/lifecycle/issues/4

The node-gyp probably can be removed from supi altogether.

zkochan commented 6 years ago

Seems like find-up can be removed as well. And try to install npm-lifecycle instead of @zkochan/npm-lifecycle

They have all the changes pnpm needs. Just rename the module declaration at the end of https://github.com/pnpm/supi/blob/master/typings/local.d.ts and it should work

etamponi commented 6 years ago

Awesome :+1:

Il 24 dic 2017 21:18, "Zoltan Kochan" notifications@github.com ha scritto:

Seems like find-up can be removed as well. And try to install npm-lifecycle instead of @zkochan/npm-lifecycle

They have all the changes pnpm needs. Just renamed module declaration at the end of https://github.com/pnpm/supi/blob/master/typings/local.d.ts and it should work

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/28#issuecomment-353801562, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjUNNyo80RBAuzbI1sMd0VSbLLY--HDks5tDrGjgaJpZM4RL_Hi .

etamponi commented 6 years ago

All done! Merry Christmas :)

zkochan commented 6 years ago

Thanks! Merry Christmas to you as well! :christmas_tree:

etamponi commented 6 years ago

squashed all commits into one

zkochan commented 6 years ago

ok, cool. I'll wait for the tests to pass and merge it