Leave Shims on the PATH when executing a tool

[charlespierce]

Currently, when we build the PATH to execute a tool, we remove the ~/.volta/bin shim directory from the path, then prepend the relevant node and yarn directories. This allows a command like node to run and pick up the correct version of the node executable.

However, removing the shim directory means that any tool that is executed is not able to access other tools (except for node, npm, and yarn that are added to the path). This causes issues when two tools are designed to interoperate, such as bower and bower-away. The tools are both installed "globally", but when running one, you can't access the other.

Instead, we should leave the shim directory on the path, but make sure that the node and yarn directories are added first. This will allow a command like node index.js to still resolve to the correct version of Node (and not the shim), but will also let any tool see any other global tool that is installed (through the shim).

This has some risks: If we call node and for some reason the node executable isn't where we expect it, the call could come back to the shim, which will then try again, and again, until the OS kills the recursion. This shouldn't happen unless someone manually deletes the file, and if it does, it's a bug, but we might want to investigate if it's possible to detect a recursive call like that and break out early.

Another issue with tools calling each other is they'll each be adding their platform to the path, so it's possible that we wind up creating a merged platform that we didn't expect, or running into size limits on the length of the PATH, but I think those problems are fairly low-impact.

@dherman @chriskrycho @rwjblue Thoughts?

[dherman]

My initial reaction has been that this sounds promising, but I don't feel like I have my head fully around it yet. A couple things to make sure we think through:

Changing how we think about and explain execution contexts. My way of thinking about the current semantics is: when you're at the console, Volta is in control and you get automatic shims; once you're running in Node, you are running in a specific version of Node and it's as if Volta doesn't exist. In particular, this means if you're an API that shells out to Node or an npm binary, it will just be operating within a single Node installation (if you want to shell out to a shim, you could use something like volta exec once that exists). That wouldn't be how this works anymore. We'd need to figure out how to explain this more granular way that Volta changes the context.

Be careful about the risks of putting things early in the path. Any time you take untrusted directories and put them very early in the path, they can start overriding very core primitives (like ls or rm). We should be sure that the directories we're talking about putting early are either (1) trusted directories that only contain a fixed set of binaries known to us, (2) put only as early as necessary in the path but no earlier, e.g. right before the other Volta-controlled path entries but not at the very front (where they could shadow core system entries like /bin, /usr/bin, etc), or (3) both.

[charlespierce]

@dherman To your 2nd point: This proposal is not to add anything extra to the PATH, rather to avoid removing something that we were previously removing (i.e. the ~/.volta/bin directory). Your first point is still a concern, however, as leaving the shims on the PATH could result in a call from a tool using Node 8 to a tool using Node 12, meaning you have 2 different Node versions interacting in strange ways. There is also the recursive issue I mentioned with node.

One thought I just had towards resolving this: Instead of removing the shim directory from the PATH, we could set a purely-internal environment variable (VOLTA_SHIM_RUNNING or something like that), which lets future invocations of the shims know that we are already running in the context of a shim. We could then short-circuit the Tool behavior: Any call to a built-in shim (node, yarn, etc.) should cause an error, since that would mean that we have A) Already modified the PATH with the appropriate tool directories and B) Somehow have still called a shim instead of a tool, so continuing to call would result in a recursive loop.

At the same time, calls to 3rd-party Binary shims with VOLTA_SHIM_RUNNING set could short-circuit the platform finding logic and simply check for which executable needs to be run (user-level or project-local) and then call it, leaving the PATH alone since VOLTA_SHIM_RUNNING implies that we have already modified the PATH at the top-level.

[charlespierce]

One immediate issue I see with this: If we leave the shims on the PATH, then we lose the ability to pass-through to the rest of the system, as the shims will always be before other tools, and so will always be called up directly. For that, however, the pass-through logic is already separated from the rest of the code, so perhaps in those cases we can remove Volta completely, while in non-passthrough situations we use the above behavior.

[kellyselden]

Any reliance on https://www.npmjs.com/package/which is now unreliable with this PATH modifying logic. On a volta machine, all my which calls for global packages fail, where they work as expected on nvm or normal machines.

[jamesmacaulay]

This issue is currently preventing me from using Purescript's recommended package manager/build tool, spago, because the spago expects the Purescript compiler to be on the PATH.

Any ideas for workarounds? Volta seems to do a pretty good job of stripping ~/.volta/bin from the PATH however I try to add it, and of preventing me from installing these tools outside of volta.

[charlespierce]

Another use case for this behavior (from Discord): Allowing Node scripts to call volta run (for cases where you want a customized version of Node for a single command). Currently, the volta binary is removed from the PATH when we launch npm to run the script, so calling volta run inside the script results in a command not found error. With this change, that would work correctly.

We do need to be careful with something like this, however, to make sure that the short-circuit / recursion detection doesn't disallow valid use-cases through volta run.

[mfonsen]

Any ideas for workarounds?

$VOLTA_HOME/bin/{nameOfTheCommand} works at least in some environments