Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pnpm Support #46

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Pnpm Support #46

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c5c20f
squash pnpm-support branch
mikrostew Jul 27, 2021
76344cd
final editing pass
mikrostew Jul 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
270 changes: 270 additions & 0 deletions text/0000-pnpm-support.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
- Feature Name: pnpm-support
- Start Date: 2021-07-26
- RFC PR:
- Volta Issue:

# Summary
[summary]: #summary

Add first-class support for the [`pnpm`](https://pnpm.io) package manager.

This will support everything we currently support for `npm` and `yarn`: pinning in `package.json`, auto-download, seamless version switching, global install interception, etc.
This should be a feature-add, not a breaking change (we will not change `package.json` in an incompatible way).


# Motivation
[motivation]: #motivation

pnpm is a popular Node package manager. At the time of this RFC, it has [11.9k stars on github](https://github.com/pnpm/pnpm), compared to [4.7k for npm](https://github.com/npm/cli) and [39.9k for yarn](https://github.com/yarnpkg/yarn).

There are outstanding issues on the [pnpm project](https://github.com/pnpm/pnpm/issues/3146) and in [Volta](https://github.com/volta-cli/volta/issues/737) requesting that Volta support pnpm, and at least a few people have inquired about pnpm support on our Discord channel.

Given that it's popular, and users want support for it, adding support for pnpm should further increase adoption of Volta, and additionally provide an example of how to onboard any new package managers we wish to support in the future.


# Pedagogy
[pedagogy]: #pedagogy

We already support `yarn` as a 3rd-party package manager, so things should work the same way for `pnpm` (we shouldn't require any new terminology or concepts):

- global installs (capturing commands and reworking them to install to our location)
- cache expiry
- hooks
- autodownload
- fetch / install / pin

Using `volta run` should work the same way, with the addition of `--pnpm` and `--no-pnpm` options.

## Documentation and Examples

We will need to add examples and documentation on the website - this feature is not done until that happens.


# Details
[details]: #details

## Fetch, Install, Pin

We will need to duplicate the existing Yarn functionality (in `crates/volta-core/src/tool/yarn/`) for pnpm (into `crates/volta-core/src/tool/pnpm/`). This will likely be copy-paste with some modifications, and may be a chance for us to refactor some of this duplication that also exists for Npm.

## Settings

We will need to add support for reading and writing `pnpm` settings, mainly in the `platform`, `project`, and `toolchain` modules in `crates/volta-core/src/`. This should mostly involve adding the `pnpm` key alongside `npm` and `yarn`.

This will modify the `package.json` format to add an additional optional `pnpm` field:

```
{
"volta": {
"node": 1.2.3,
"npm": 2.3.4,
"yarn": 3.4.5,
"pnpm": 4.5.6
}
}
```

And we will need to add `--pnpm` and `--no-pnpm` to the options for the `volta run` command (in `src/command/run`).

## Shims

pnpm includes two executables, `pnpm` and `pnpx`. These will need to be supported in `crates/volta-core/src/run/`, the same way that we support `yarn` and `npx`. There is probably some opportunity for refactoring duplicated code in this section as well.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are deprecating pnpx in v7. I don't think support for pnpx is needed.

pnpx is changed to pnpm dlx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know, thanks!


We will also need to intercept these global install commands:

* `pnpm add --global <package>`
* `pnpm add -g <package>`

The same way we intercept `yarn` and `npm` global commands, in `crates/volta-core/src/run/parser`.

## Installing Local Packages

Package installation with pnpm works differently than Npm and Yarn. Instead of storing all files local to the project, it hard-links the files in `node_modules` to a shared content-addressable store. This is typically at `$HOME/.pnpm-store/`. Since this is a versioned store that links to individual files, based on their contents, this should be safe to leave as-is, even if there are multiple different versions of pnpm used on the same machine at the same time.

## Installing Global Packages

Currently, pnpm installs global packages based on the `PATH` (see [the discussion in this issue](https://github.com/pnpm/pnpm/issues/1360)). The first directory that has `node`, `npm`, or some other node-related binaries is chosen, or failing that, the first directory that is writeable.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changed in pnpm v7. Now we only write global bin files into the directory which specified through the PNPM_HOME env variable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh very cool and good to know @zkochan! Maybe the way forward here for us would be to start with support for pnpm v7, and hold off on the special-casing for previous versions until / unless we get significant user feedback about needing them.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be super interested in this approach as well!

Copy link

@chawyehsu chawyehsu Jul 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zkochan

This is changed in pnpm v7. Now we only write global bin files into the directory which specified through the PNPM_HOME env variable.

I could change this for v7 by setting the PNPM_HOME env. But is there any way to modify global bin files writing for pnpm version 6.x down to version 4.2?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

global-bin-dir https://pnpm.io/6.x/npmrc#global-bin-dir

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

global-bin-dir https://pnpm.io/6.x/npmrc#global-bin-dir

Thanks, noticed it's added in v6.15.0, what about older versions below v6.15.0?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember whether there was a way to do it in prior versions. I also don't see any value in researching it. Just don't support older versions of pnpm.


```
$ node --version
v14.16.1

$ pnpm add --global cowsay
Packages: +33
+++++++++++++++++++++++++++++++++
Progress: resolved 33, reused 33, downloaded 0, added 0, done

/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other small complication here, which I believe is fairly easy to work around, but worth noting: The pnpm global directory (wherever you tell it to install) appends a versioned directory (the 5 in this path), then puts all of the files under that directory. The version isn't tied to the pnpm version, but rather their internal directory layout version. In our case, I don't think it's a big issue, because we can find the single directory and step into it, but it's something to be aware of.

It's also nice because if their directory layout changes in the future, we'll get a direct indication of it via the number changing, so we can likely have a better integration moving forward.

+ cowsay 1.5.0
```

In that case, the `cowsay` package has been installed under `pnpm-global/`, which is a peer directory to the `bin/` directory that appears in the `PATH`:

```
$ ll ~/.volta/tools/image/node/14.16.1/bin/
total 161328
drwxr-xr-x 7 mikrostew staff 224 Jul 26 15:52 ./
drwxr-xr-x 10 mikrostew staff 320 Jul 26 15:52 ../
-rwxr-xr-x 1 mikrostew staff 2194 Jul 26 15:52 cowsay*
-rwxr-xr-x 1 mikrostew staff 2194 Jul 26 15:52 cowthink*
-rwxr-xr-x 1 mikrostew staff 73883024 Apr 6 11:22 node*
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npm@ -> ../lib/node_modules/npm/bin/npm-cli.js
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npx@ -> ../lib/node_modules/npm/bin/npx-cli.js
```

The executable `cowsay` in the `bin/` directory is a shell script that sets up the `NODE_PATH`, and tries to execute the `cowsay` file using the `node` that it was installed with.

```
$ cat cowsay
#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
*CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -z "$NODE_PATH" ]; then
export NODE_PATH="/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/[email protected]/node_modules/cowsay/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/[email protected]/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/node_modules:/Users/mikrostew/.volta/tools/image/node/node_modules:/Users/mikrostew/.volta/tools/image/node_modules:/Users/mikrostew/.volta/tools/node_modules:/Users/mikrostew/.volta/node_modules:/Users/mikrostew/node_modules:/Users/node_modules:/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/cowsay/node_modules"
else
export NODE_PATH="$NODE_PATH:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/[email protected]/node_modules/cowsay/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/[email protected]/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/.pnpm/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/node_modules:/Users/mikrostew/.volta/tools/image/node/node_modules:/Users/mikrostew/.volta/tools/image/node_modules:/Users/mikrostew/.volta/tools/node_modules:/Users/mikrostew/.volta/node_modules:/Users/mikrostew/node_modules:/Users/node_modules:/node_modules:/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5/node_modules/cowsay/node_modules"
fi
if [ -x "$basedir/node" ]; then
exec "$basedir/node" "$basedir/../pnpm-global/5/node_modules/cowsay/cli.js" "$@"
else
exec node "$basedir/../pnpm-global/5/node_modules/cowsay/cli.js" "$@"
fi
```

We need to change where these global packages are installed (to `$HOME/.volta/tools/image/packages/` in our current layout), to provide the same benefits that we give to global installs using Npm and Yarn.

There are a couple options that we can use for that:

- the `NPM_CONFIG_GLOBAL_DIR` environment variable
- the `--global-dir` command-line option
Comment on lines +142 to +143
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would lean towards setting the environment variable. That's what we've done for npm and yarn to set the similar settings in those global install interceptions. See https://github.com/volta-cli/volta/blob/main/crates/volta-core/src/tool/package/manager.rs#L84


Both of those were introduced in pnpm 4.2, and function the same way.

Using `NPM_CONFIG_GLOBAL_DIR`:

```
$ NPM_CONFIG_GLOBAL_DIR=/Users/mikrostew/src/test-pnpm pnpm add --global cowsay
Packages: +33
+++++++++++++++++++++++++++++++++
Packages are hard linked from the content-addressable store to the virtual store.
Content-addressable store is at: /Users/mikrostew/.pnpm-store/v3
Virtual store is at: node_modules/.pnpm

/Users/mikrostew/src/test-pnpm/5:
+ cowsay 1.5.0

Progress: resolved 33, reused 33, downloaded 0, added 33, done
```

It installs to the provided directory, but still adds shell scripts to the bin dir where `node` is located:

```
$ ll /Users/mikrostew/.volta/tools/image/node/14.16.1/bin/
total 161328
drwxr-xr-x 7 mikrostew staff 224 Jul 27 10:58 ./
drwxr-xr-x 10 mikrostew staff 320 Jul 27 10:57 ../
-rwxr-xr-x 1 mikrostew staff 1812 Jul 27 10:58 cowsay*
-rwxr-xr-x 1 mikrostew staff 1812 Jul 27 10:58 cowthink*
-rwxr-xr-x 1 mikrostew staff 73883024 Apr 6 11:22 node*
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npm@ -> ../lib/node_modules/npm/bin/npm-cli.js
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npx@ -> ../lib/node_modules/npm/bin/npx-cli.js
```

Using the `--global-dir` command-line option does the same thing:

```
$ pnpm add --global-dir /Users/mikrostew/src/test-pnpm --global cowsay
Packages: +33
+++++++++++++++++++++++++++++++++

/Users/mikrostew/src/test-pnpm/5:
+ cowsay 1.5.0

Progress: resolved 33, reused 33, downloaded 0, added 0, done
```

Again, this adds the binaries to the same bin directory as `node`:

```
$ ll /Users/mikrostew/.volta/tools/image/node/14.16.1/bin/
total 161328
drwxr-xr-x 7 mikrostew staff 224 Jul 27 11:22 ./
drwxr-xr-x 10 mikrostew staff 320 Jul 27 11:21 ../
-rwxr-xr-x 1 mikrostew staff 1812 Jul 27 11:22 cowsay*
-rwxr-xr-x 1 mikrostew staff 1812 Jul 27 11:22 cowthink*
-rwxr-xr-x 1 mikrostew staff 73883024 Apr 6 11:22 node*
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npm@ -> ../lib/node_modules/npm/bin/npm-cli.js
lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npx@ -> ../lib/node_modules/npm/bin/npx-cli.js
```

I don't see an easy way to change where those schell scripts are written. We need to have `node` on the `PATH` to run pnpm.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we may be able to work around this: Currently, all global packages are installed into the $VOLTA_HOME/tmp directory first, then moved to their final location. We already have some logic for creating sub-paths so that relative symlinks line up. What we can try is to make one of those sub-directories named node, and then add the target directory we want to use for the scripts to the PATH before executing the command. That shouldn't break anything (because there won't be anything there yet), but should fit into pnpm's logic for locating the bin directory.


Having those shell scripts written to the Node image directory interferes with the isolation that we provide for global packages. We will probably have to clean those up, or move them somehow, after the install completes.


## Linking

Using `pnpm link` creates a symlink directly from one package to another, which should work without a problem.

But, `pnpm link --global` stores the links globally, in the same directory as the `node` version that was used to install it:

```
$ pnpm link --global

/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5:
+ yaml-fromat 0.2.0 <- ../../../../../../../src/yaml-fromat
```

That means that if two projects are using different Node versions, they will not be able to link to each other globally using this workflow.
This is basically the problem we had with `npm link`, so this will need functionality similar to that: <https://github.com/volta-cli/volta/pull/888>


# Critique
[critique]: #critique


## Global package install using PATH

For a global package installation alternative, I tried to setup the `PATH` to force it to use a specific directory for everything.

If there is `node` somewhere on the `PATH`, it will install where it finds `node`:

```
$ PATH=/Users/mikrostew/.volta/bin:/Users/mikrostew/src/test-pnpm pnpm add --global cowsay
Packages: +33
+++++++++++++++++++++++++++++++++
Progress: resolved 33, reused 33, downloaded 0, added 0, done

/Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5:
+ cowsay 1.5.0
```

If there is no directory containing `node` (or similar command), it will fail to install:

```
$ PATH=/Users/mikrostew/src/test-pnpm /Users/mikrostew/.volta/tools/image/node/14.16.1/bin/node /Users/mikrostew/.volta/tools/image/packages/pnpm/bin/pnpm add --global cowsay
 ERROR  Couldn't find a suitable global executables directory.
There should be a node, nodejs, npm, or pnpm directory in the "PATH" environment variable
```

This approach seems brittle, because manipulating the `PATH` relies on internal behavior that may change in the future.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that any solution will have some amount of brittleness, since they ultimately will have to work with the internal details of where pnpm decides to put those files. One way we might be able to mitigate that would be to engage with the pnpm maintainers (they've been pretty open from what I've seen, even participating in the issue here on our repo) to add a flag / config setting of some kind to allow us to directly set the bin directory.

Then we could limit our support to e.g. pnpm 4.2 and higher, use the hacky PATH approach for existing versions and the more stable approach for versions that support it. Alternatively, we could even work to add that config setting and then only support pnpm at that version or higher. The (admittedly anecdotal) indication I got from the Volta issue was that back support wouldn't be a major blocker.

We could also do it in stages: Add support for the recent versions once we get a flag added, then expand to include the older ones with the hack later, if there's a community need for it. That way we could avoid any hacky workarounds until they're definitely needed.


For those reasons I did not include this as a workable option.


# Unresolved questions
[unresolved]: #unresolved-questions

## Global package shell scripts

How do we handle the shell scripts that are auto-installed for global packages? This is probably something that will need to be worked out during the implementation of this feature.

## pnpm Version Support

What versions of pnpm should we support? Should we only support certain versions?
- versions of pnpm prior to v3 don't support the current Node LTS versions (see <https://pnpm.io/installation#compatibility>)
- versions of pnpm prior to v4.2 don't support `NPM_CONFIG_GLOBAL_DIR` and `--global-dir`, for global package installation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4.2 was released in Nov 2019, which is relatively old as these things go. That said, it looks like there was a similar setting prior to version 4.2: pnpm/pnpm#2121 --global-dir was renamed from --pnpm-prefix, so if we set both to the same value, we can probably push that support back even further.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm has fairly regular updates. I would de-prioritize backwards compatibility.