feat: specify a custom install command

[etamponi]

I know this is a very Glitch-specific feature, but I would love to see this included in pnpm upstream (or at least something equivalent).

As I told you already, when an user runs pnpm install inside their project, it won't have access to the store. In order for it to work, pnpm install --ignore-scripts has to run outside of the container. We allow this in glitch by doing something like this:

curl ${HOST_ADDRESS}:${INSTALL_PORT}/install/${PROJECT_ID}

which basically triggers pnpm install --ignore-scripts on the user project. Then pnpm has still to rebuild packages and trigger the various install hooks (one of them, preinstall, should be run before we fetch/import the packages).

This is why I am suggesting to add this --custom-install option. I would then use pnpm inside a Glitch project in this way:

pnpm install --custom-install "curl ${HOST_ADDRESS}:${INSTALL_PORT}/install/${PROJECT_ID}"

It would be awesome because otherwise I've to run pnpm run several times, and every time I run it, there is at least 500ms of startup time to take into account...

[zkochan]

I don't really see how would it be used outside the glitch usecase. I'd recommend to use the pnpm's programmatic API instead. We can create a new package @pnpm/config that would load pnpm's configs and you'd be able to do something like

const pnpmConfigs = await pnpmConfig.load()
await supi.run('preinstall')
const {stdout, stderr} = await execPromise(opts.customInstall, {maxBuffer: 10 * 1024 * 1024})
logger.info(stdout)
await rebuild(opts)

[etamponi]

Unfortunately, it doesn't look like it is that easy. There is no such supi.run method, and I have no idea on how to extract the needed pnpmConfig. Isn't there something I can change to make this more likely to be included in the upstream supi? :(

[zkochan]

I will extract pnpmConfig into a separate package.

Instead of supi.run maybe npm-lifecycle can be used directly.

[etamponi]

Thank you :)

[etamponi]

@zkochan before you get too far, please notice that the pnpmConfig object might not be the main thing that is blocking me in this case. If you go through the install method in src/api/install.ts, you will see that it does tons of things before running the install hooks and actually installing stuff (it gets the "extended options", it fetches the context...). This is why I am not really sure your suggestion is going to work.

[zkochan]

ok, I will look into it.

I want to support things like this but I want to support it via hypermodularization. Right now I am getting rid of the PnpmOptions object (you might remember the inconvenience of changing this mega-object from @pnpm/types). Also, I am moving out storeController from supi. These changes are in PR #36 (in progress).

Once that is done, I'll check what can be divided further to allow you compose your thing.

[zkochan]

@etamponi I am a bit confused, wasn't the plan to do all manipulations with store via the pnpm server? pnpm install runs inside the container and posts messages to pnpm server, which runs outside of the container

[etamponi]

That's why I asked if the pnpm server was only responsible for downloading the packages, or if it also copied them over into node_modules. Since it only downloads the packages, copying/linking has to be done in a separate step because the containers can't access the store.

Il 08 gen 2018 20:55, "Zoltan Kochan" notifications@github.com ha scritto:

@etamponi https://github.com/etamponi I am a bit confused, wasn't the plan to do all manipulations with store via the pnpm server? pnpm install runs inside the container and posts messages to pnpm server, which runs outside of the container

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/34#issuecomment-356077789, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjUNG--gFfVZrIbyyG4p8ipXfj6VKrgks5tInLLgaJpZM4RP02- .

[zkochan]

I see... yes, it currently does not link the packages from store.

hm, it might make sense to link them by pnpm server. I am not 100% sure. I think it might make sense because sometimes we might want to unpack the packages directly to the project's node_modules. For instance, on CI servers like Travis we don't need the store, just fast installation. So abstracting this logic might be good

[etamponi]

Well, for sure it would remove the need for this --custom-install switch, and I can get rid of a lot of code in Glitch :) would it increase code complexity of pnpm significantly?

Il 08 gen 2018 21:10, "Zoltan Kochan" notifications@github.com ha scritto:

I see... yes, it currently does not link the packages from store.

hm, it might make sense to link them by pnpm server. I am not 100% sure. I think it might make sense because sometimes we might want to unpack the packages directly to the project's node_modules. For instance, on CI servers like Travis we don't need the store, just fast installation. So abstracting this logic might be good

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/34#issuecomment-356081757, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjUNEMO-0UEDL_RJavEC4IL0Ei5inf2ks5tInY6gaJpZM4RP02- .

[zkochan]

I think it won't increase complexity. We just need to move the linking logic to the store controller. Ideally, storeController.requestPackage() would do it but unfortunately, we don't know the location of the package in node_modules until we resolve all the peer dependencies. So there should be a new function for linking.

[etamponi]

Great :) is it something you can do in the following days? I'll be busy with some other things at work in the forthcoming weeks

Il 08 gen 2018 21:20, "Zoltan Kochan" notifications@github.com ha scritto:

I think it won't increase complexity. We just need to move the linking logic to the store controller. Ideally, storeController.requestPackage() would do it but unfortunately, we don't know the location of the package in node_modules until we resolve all the peer dependencies. So there should be a new function for linking.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/34#issuecomment-356084243, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjUNHcAZjGHC_zWHB8RAVqnpIJGMLjtks5tIniWgaJpZM4RP02- .

[zkochan]

ok. I'll look into it.

[zkochan]

Done, try pnpm@1.27.0-1

[etamponi]

As it is now, there is no way to specify a tcp connection instead of the socket... I'll open an issue

[zkochan]

So it doesn't work or false alarm? I've got an email notification but seems like you removed the message

[etamponi]

False alarm, I was doing something wrong :)

Il 09 gen 2018 23:05, "Zoltan Kochan" notifications@github.com ha scritto:

So it doesn't work or false alarm? I've got an email notification but seems like you removed the message

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pnpm/supi/pull/34#issuecomment-356429486, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjUNGAAB_wTYTAj_g3ecP84Vlg2770dks5tI-K2gaJpZM4RP02- .