feat: Initial pnpm support #1273

chawyehsu · 2022-07-30T13:02:54Z

This PR is going to implement the first-class support for the pnpm package manager.

Thanks to @mikrostew for the well-written volta-cli/rfcs#46, I've been able to implement this feature initially. Most of the critical functionalities have been implemented, except for the global package management. Due to a lack of further details on how to intercept pnpm to relocate its global package installs to fit Volta's current layout.

Implemented

pnpm shim
pnpm fetch / install / pin
pnpm hooks
per project pnpm manipulation
volta run with --pnpm and --no-pnpm

Not Implemented

global package manipulation/management via pnpm

In fact the argument parser for_pnpm() has been introduced to try to intercept pnpm args, but I found that there is no proper way to make pnpm's global installs whose original location is <path to pnpm home>/global/5/node_modules fit Volta's current global package image layout <path to volta home>/tools/image/packages. Hence I intercepted all --global pnpm commands and marked them as unimplemented.

Trackers

RFC: volta-cli/rfcs#46
Tracking issue: #737

This is currently a draft PR, please leave your review or comment if you have any ideas to improve the PR to push forward.

Demo

❯ .\target\debug\volta.exe list
⚡️ Currently active tools:

    Node: v16.16.0 (default)
    npm: v8.15.1 (default)
    pnpm: v7.7.1 (default)
    Yarn: v1.22.19 (default)
    Tool binaries available:
        cleancss (default)
        yarn-deduplicate (default)

See options for more detailed reports by running `volta list --help`.
DESKTOP in volta on  feature/pnpm [?] is 📦 v1.0.8 via 🦀 v1.62.0
❯ .\target\debug\pnpm.exe -h
Version 7.7.1
Usage: pnpm [command] [flags]
       pnpm [ -h | --help | -v | --version ]

Manage your dependencies:
      add                  Installs a package and any packages that it depends on. By default, any new package is installed as a prod dependency
      import               Generates a pnpm-lock.yaml from an npm package-lock.json (or npm-shrinkwrap.json) file
   i, install              Install all dependencies for a project
  it, install-test         Runs a pnpm install followed immediately by a pnpm test
  ln, link                 Connect the local project to another one
      prune                Removes extraneous packages
  rb, rebuild              Rebuild a package
  rm, remove               Removes packages from node_modules and from the project's package.json
      unlink               Unlinks a package. Like yarn unlink but pnpm re-installs the dependency after removing the external link
  up, update               Updates packages to their latest version based on the specified range

Review your dependencies:
      audit                Checks for known security issues with the installed packages
  ls, list                 Print all the versions of packages that are installed, as well as their dependencies, in a tree-structure
      outdated             Check for outdated packages

Run your scripts:
      exec                 Executes a shell command in scope of a project
      run                  Runs a defined package script
      start                Runs an arbitrary command specified in the package's "start" property of its "scripts" object
   t, test                 Runs a package's "test" script, if one was provided

Other:
      pack
      publish              Publishes a package to the registry
      root

Manage your store:
      store add            Adds new packages to the pnpm store directly. Does not modify any projects or files outside the store
      store prune          Removes unreferenced (extraneous, orphan) packages from the store
      store status         Checks for modified packages in the store

Options:
  -r, --recursive          Run the command for each project in the workspace.
DESKTOP in volta on  feature/pnpm [!] is 📦 v1.1.0-alpha.1 via 🦀 v1.62.0
❯ .\target\debug\pnpm.exe dlx create-vite vite-project --template vanilla
.../../../.pnpm-store/v3/tmp/dlx-31720   |   +6 +
Packages are hard linked from the content-addressable store to the virtual store.
  Content-addressable store is at: D:\.pnpm-store\v3
  Virtual store is at:             ../../../../../.pnpm-store/v3/tmp/dlx-31720/node_modules/.pnpm
.../../../.pnpm-store/v3/tmp/dlx-31720   | Progress: resolved 6, reused 6, downloaded 0, added 6, done

Scaffolding project in D:\workspace\repos\chawyehsu\volta\vite-project...

Done. Now run:

  cd vite-project
  pnpm install
  pnpm run dev

DESKTOP in volta on  feature/pnpm [!?] is 📦 v1.1.0-alpha.1 via 🦀 v1.62.0 took 5s
❯ cd vite-project
DESKTOP in volta\vite-project on  feature/pnpm [!?] via ⬢ v16.16.0
❯ ..\target\debug\pnpm.exe i
Packages: +14
++++++++++++++
Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: D:\.pnpm-store\v3
  Virtual store is at:             node_modules/.pnpm
node_modules/.pnpm/[email protected]/node_modules/esbuild: Running postinstall script, done in 318ms
Progress: resolved 34, reused 0, downloaded 14, added 14, done

devDependencies:
+ vite 3.0.4
DESKTOP in volta\vite-project on  feature/pnpm [!?] via ⬢ v16.16.0
❯ ..\target\debug\volta.exe pin node pnpm
success: pinned [email protected] (with [email protected]) in package.json
success: pinned [email protected] in package.json
DESKTOP in volta\vite-project on  feature/pnpm [!?] via ⬢ v16.16.0 took 3s
❯ cat .\package.json
{
  "name": "vite-project",
  "private": true,
  "version": "0.0.0",
  "type": "module",
  "scripts": {
    "dev": "vite",
    "build": "vite build",
    "preview": "vite preview"
  },
  "devDependencies": {
    "vite": "^3.0.0"
  },
  "volta": {
    "node": "16.16.0",
    "pnpm": "7.7.1"
  }
}
DESKTOP in volta\vite-project on  feature/pnpm [!?] via ⬢ v16.16.0
❯ ..\target\debug\pnpm.exe --version
7.7.1
DESKTOP in volta\vite-project on  feature/pnpm [!?] via ⬢ v16.16.0
❯

Signed-off-by: Chawye Hsu <[email protected]>

hustcer · 2022-08-14T10:06:26Z

Thanks for the PR

hustcer · 2022-08-16T07:23:18Z

Any update on this?

chriskrycho · 2022-08-16T15:24:57Z

@chawyehsu this is excellent to see! 👏🏼 I approved CI to run. @charlespierce or @mikrostew if either of you has time to look and see what needs to be done for the global intercept, that would be fantastic. 🤩

Signed-off-by: Chawye Hsu <[email protected]>

chawyehsu · 2022-08-16T17:01:46Z

@chriskrycho Thanks! I fixed tests on Unix according to the CI result.

chriskrycho · 2022-08-16T17:23:18Z

The GH setting is such that I’ll have to reapprove every time, so I'll try to keep an eye on this to keep rerunning it – sorry about the hassle on that front!

chawyehsu · 2022-08-17T05:09:54Z

I didn't notice there is a --features mock-network flag to turn on all tests including sandboxed smoke tests. I was only using cargo test --all until I saw the CI arguments.

I'll try to fix them today when I have a couple of hours to figure out.

charlespierce

Thanks for this! Apologies for taking a bit longer to get to it than I expected, it's been a very busy couple of weeks at work 😅

I haven't finished reading through the meatiest parts (resolving, parsing, etc.), but left a few minor style comments on the surrounding infrastructure. Will follow-up later in the week with a review on the core parts, as well as some potential thoughts on the FIXME comments.

Super excited for this to land!

crates/volta-core/src/hook/serial.rs

crates/volta-core/src/tool/package/manager.rs

crates/volta-core/src/tool/package/metadata.rs

crates/volta-layout/src/v3.rs

crates/volta-migrate/src/v3/config.rs

crates/volta-core/src/project/mod.rs

src/command/run.rs

This allows rust-analyzer to recognize tests modules. For development convenience. Signed-off-by: Chawye Hsu <[email protected]>

Signed-off-by: Chawye Hsu <[email protected]>

…g for pnpm Signed-off-by: Chawye Hsu <[email protected]>

Signed-off-by: Chawye Hsu <[email protected]>

ScreamZ · 2022-08-23T17:17:09Z

Any news on this boyz ? Need it

chawyehsu · 2022-08-25T03:43:26Z

@ScreamZ Sorry no ETA, let's wait for the team back from their main work.

charlespierce

Thank you again! Took a look through the meat of the code and it's all reasonable. Also points to a lot of things it would be good for us to clean up re: Adding package support (lots of boilerplate).

Left a few (mostly minor) suggestions. For the ones in parser.rs, I think it's totally reasonable to resolve them with FIXME comments, as I see the overall global interception isn't totally wired up yet anyway.

.vscode/settings.json

crates/volta-core/src/platform/tests.rs

crates/volta-core/src/run/parser.rs

charlespierce · 2022-09-16T23:50:53Z

crates/volta-core/src/tool/package/manager.rs

@@ -87,6 +88,10 @@ impl PackageManager {
        if let PackageManager::Yarn = self {
            command.env("npm_config_global_folder", self.source_root(package_root));
        }
+
+        // FIXME: Find out if there is a good way to redirect pnpm global installs


Thanks for leaving this comment! Totally reasonable to leave this as a follow-up, I know we've done some research and it's possible, so we should be able to add that on later.

From my research, I think there will be much work to implement global commands manipulation for pnpm. pnpm relies on absolute path symlinks heavily (binaries generation, symlinked node_modules structure), this makes it very difficult for volta to install and update global packages via pnpm without a layout change. I don't know if there is a proper way to update those generated symlinks after moving packages from the temp directory to the image directory to ensure they won't be broken. Related annotation in e830d3d#diff-80dfff59af9a37069dea094c900c30eafd02dd51c2dea21d150a017d1a86c9d3R90.

src/cli.rs

charlespierce · 2022-09-17T00:03:13Z

tests/acceptance/support/sandbox.rs

@@ -622,6 +674,7 @@ impl SandboxBuilder {
        ok_or_panic! { fs::create_dir_all(node_cache_dir()) };
        ok_or_panic! { fs::create_dir_all(node_inventory_dir()) };
        ok_or_panic! { fs::create_dir_all(package_inventory_dir()) };
+        ok_or_panic! { fs::create_dir_all(pnpm_inventory_dir()) };


Random question: Since we aren't doing a migration (and all users will be without this directory to start out once this is released), do these tests work if this is left out? Ideally we would leave this directory uncreated and allow it to be automatically created as needed. Totally reasonable if the tests don't support that at the moment, however.

This is just for the sandbox tests. The inventory directory is ensured to exist by the fetch function in production, https://github.com/volta-cli/volta/pull/1273/files#diff-cecf323599e6ac86e26655f65f1935e8d8435f00e05003ad7e228a9dc29d056cR92-R94

It looks like this is not needed - I removed this line and the tests still pass (for me locally on OSX).

crates/volta-core/src/tool/pnpm/resolve.rs

crates/volta-core/src/tool/pnpm/fetch.rs

crates/volta-layout/src/v3.rs

Signed-off-by: Chawye Hsu <[email protected]>

mikrostew

This will also need to add the pnpm and pnpx shims to the get_shim_list_deduped() fn in crates/volta-core/src/shim.rs, for Unix systems to get the new shims (looks like Windows is fine though).

Otherwise I agree with @charlespierce, it seems reasonable not to block on pnpm globals and leave that unimplemented for now.

Sorry there is so much boilerplate needed to onboard a new package manager.

crates/volta-core/src/run/parser.rs

Signed-off-by: Chawye Hsu <[email protected]>

chawyehsu · 2022-10-05T15:50:00Z

pnpx has been deprecated, hence I didn't implement the shimming for it.

it seems reasonable not to block on pnpm globals and leave that unimplemented for now.

It's easy to unblock global commands by simply commenting out the global commands guard in crates/volta-core/src/run/pnpm.rs, this will leave pnpm global packages out of volta's control, however. Blocking global commands may bring inconvenience while allowing the unimplemented to be executed outside volta's layout may get package migration issues in the future. In fact, for me, I may choose to not block global commands but this causes the UX issue that volta can not list global packages installed by pnpm.

avaly · 2022-11-02T11:34:20Z

Could we get this PR merged as-is with a disclaimer that pnpm support is experimental for now, until the global commands can be properly implemented?

We'd really like to use this in our workflows for local development and Docker builds, so the missing global commands support is not really impacting us.

runspired · 2022-11-02T16:45:23Z

I second that: pnpm ships often enough that running volta install pnpm@<version-project-uses> many times a day has become annoying :D

chawyehsu · 2022-11-03T02:51:06Z

Hi folks, thanks for all your interest, response, and emoji reactions to this. Undoubtedly, I hope we could make this happen as possible, Volta and pnpm are both great tools for me to be involved in the ecosystem, and I'm happy with them.

Therefore I made the PR to try to improve these everyday tooling things a little bit more productive and efficient, with a micro goal of hunting some bounties because of my state of out-of-work. The waiting time was a little longer than I expected, expected two months at most for rolling this out. But it's definitely up to the Volta team, I'm not one of its members hence I ain't able to push it to go further. The team is so busy I guess, let's wait a bit more time and we will see.

If you have a strong demand for it right now, you may build a nightly version based on my branch. It's also the base of the local build that I'm currently using.

charlespierce

Exciting! A couple of small changes to make sure we're not impacting the performance of the existing flows with unused code or extra data copying.

To the release strategy, I agree with the plan of merging this and then releasing experimental support. Then we can take another look at the global command interception and look to get that working to remove the experimental label.

crates/volta-core/src/tool/package/manager.rs

crates/volta-core/src/run/pnpm.rs

crates/volta-core/src/tool/package/manager.rs

Signed-off-by: Chawye Hsu <[email protected]>

chawyehsu · 2022-11-03T07:15:36Z

Changes are applied.

A side note here: When a user updates to a version of Volta that has first-class pnpm support, if they have installed pnpm (as a package) before they might need to uninstall it manually by npm uninstall -g pnpm in order to make the new pnpm shim be available.

charlespierce

That's a great callout re: existing pnpm installed as a package! We'll probably want to think a bit about how we want to message that before publishing the release as the latest version. I can make the build however, which will allow people to try it out.

charlespierce · 2022-11-03T16:45:17Z

Thank you @chawyehsu!

chawyehsu · 2022-11-04T04:27:49Z

Glad that! CI artifacts are now available to try https://github.com/volta-cli/volta/actions/runs/3383641697

ScreamZ · 2022-11-07T10:10:40Z

Just tried, seems to work, now we need a way to auto-update volta ahah v1.1.0 released

smblee · 2022-11-09T17:34:38Z

Also tested the artifact above and it works 🥳 What does the official release cycle look like for this binary to be available?

PindaPixel · 2022-11-10T12:55:06Z

Does this take pnpm/node version compatibility into account?

chawyehsu · 2022-11-10T14:59:13Z

Does this take pnpm/node version compatibility into account?

@PindaPixel Didn't have any extra implementation for the node version compatibility, the compatibility check will be passed to pnpm.

smblee · 2022-11-14T21:47:23Z

Hey @charlespierce just wanted to check on when this PR will be officially released

michaelhays · 2022-11-16T02:38:59Z

@smblee See this comment; there's no ETA as of now.

feat: Initial pnpm support

402e177

Signed-off-by: Chawye Hsu <[email protected]>

chawyehsu mentioned this pull request Jul 30, 2022

Support for pnpm #737

Open

update tests

814cdc5

Signed-off-by: Chawye Hsu <[email protected]>

chawyehsu marked this pull request as ready for review August 16, 2022 16:57

charlespierce mentioned this pull request Aug 17, 2022

Volta error: Could not locate executable pnpm in your project #1293

Closed

charlespierce reviewed Aug 17, 2022

View reviewed changes

chawyehsu added 6 commits August 17, 2022 18:24

chore(vscode): Enable all cargo features by default

68099bb

This allows rust-analyzer to recognize tests modules. For development convenience. Signed-off-by: Chawye Hsu <[email protected]>

fix(volta-migrate): LegacyPlatform does not have pnpm field

cfb216b

Signed-off-by: Chawye Hsu <[email protected]>

chore(style): module import cleanup, consistent all-lowercase brandin…

ee257d6

…g for pnpm Signed-off-by: Chawye Hsu <[email protected]>

fix: add pnpm platforms merge and fallback

6f8a57f

Signed-off-by: Chawye Hsu <[email protected]>

fix: add seamless migration for platform spec

1ef45ec

Signed-off-by: Chawye Hsu <[email protected]>

test: Add acceptance tests for pnpm

de495b6

Signed-off-by: Chawye Hsu <[email protected]>

charlespierce reviewed Sep 17, 2022

View reviewed changes

chawyehsu added 9 commits September 19, 2022 10:15

fix: correct pnpm resolver naming

6770d25

Signed-off-by: Chawye Hsu <[email protected]>

chore: rename overwrite[_cmd]_launcher to write[_cmd]_launcher

73ac17e

Signed-off-by: Chawye Hsu <[email protected]>

chore: annotation tweaks

a70c00f

Signed-off-by: Chawye Hsu <[email protected]>

test: add hook test fixtures for pnpm

464de06

Signed-off-by: Chawye Hsu <[email protected]>

chore: revert 68099bb

3b00570

Signed-off-by: Chawye Hsu <[email protected]>

fix: pnpm parser improvement

e830d3d

Signed-off-by: Chawye Hsu <[email protected]>

tests: add hooks tests for pnpm

fb5c002

Signed-off-by: Chawye Hsu <[email protected]>

fix: fix pnpm archive filename

4732b0d

Signed-off-by: Chawye Hsu <[email protected]>

test: fix pnpm archive filename tests

36643a1

Signed-off-by: Chawye Hsu <[email protected]>

ran-dall mentioned this pull request Sep 28, 2022

Add devcontainer configuration withastro/compiler#468

Merged

mikrostew suggested changes Oct 4, 2022

View reviewed changes

crates/volta-core/src/run/parser.rs Outdated Show resolved Hide resolved

crates/volta-core/src/run/parser.rs Outdated Show resolved Hide resolved

chawyehsu added 2 commits October 5, 2022 23:36

style: fix as per linter

e987d1a

Signed-off-by: Chawye Hsu <[email protected]>

fix: add pnpm shim for unix

9006eaf

Signed-off-by: Chawye Hsu <[email protected]>

charlespierce requested changes Nov 3, 2022

View reviewed changes

crates/volta-core/src/tool/package/manager.rs Outdated Show resolved Hide resolved

crates/volta-core/src/run/pnpm.rs Outdated Show resolved Hide resolved

crates/volta-core/src/tool/package/manager.rs Outdated Show resolved Hide resolved

chawyehsu added 2 commits November 3, 2022 14:21

chore: Use #[allow(dead_code)] for for_pnpm

de8fe43

Signed-off-by: Chawye Hsu <[email protected]>

perf: revert to PathBuf

79db5a3

Signed-off-by: Chawye Hsu <[email protected]>

charlespierce approved these changes Nov 3, 2022

View reviewed changes

charlespierce merged commit 7827ade into volta-cli:main Nov 3, 2022

avaly mentioned this pull request Nov 10, 2022

Add multiplatform linux build to CI #1372

Closed

mastastealth mentioned this pull request Nov 11, 2022

Migrate from yarn to pnpm embroider-build/embroider#1216

Closed

charlespierce mentioned this pull request Dec 3, 2022

Gate pnpm support behind a feature flag to avoid compatibility issues #1394

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Initial pnpm support #1273

feat: Initial pnpm support #1273

chawyehsu commented Jul 30, 2022 •

edited

Loading

hustcer commented Aug 14, 2022

hustcer commented Aug 16, 2022

chriskrycho commented Aug 16, 2022

chawyehsu commented Aug 16, 2022

chriskrycho commented Aug 16, 2022

chawyehsu commented Aug 17, 2022 •

edited

Loading

charlespierce left a comment

ScreamZ commented Aug 23, 2022

chawyehsu commented Aug 25, 2022

charlespierce left a comment

charlespierce Sep 16, 2022

chawyehsu Sep 19, 2022 •

edited

Loading

charlespierce Sep 17, 2022

chawyehsu Sep 19, 2022

mikrostew Oct 4, 2022

mikrostew left a comment

chawyehsu commented Oct 5, 2022 •

edited

Loading

avaly commented Nov 2, 2022 •

edited

Loading

runspired commented Nov 2, 2022

chawyehsu commented Nov 3, 2022

charlespierce left a comment

chawyehsu commented Nov 3, 2022

charlespierce left a comment

charlespierce commented Nov 3, 2022

chawyehsu commented Nov 4, 2022 •

edited

Loading

ScreamZ commented Nov 7, 2022

smblee commented Nov 9, 2022 •

edited

Loading

PindaPixel commented Nov 10, 2022

chawyehsu commented Nov 10, 2022

smblee commented Nov 14, 2022

michaelhays commented Nov 16, 2022

feat: Initial pnpm support #1273

feat: Initial pnpm support #1273

Conversation

chawyehsu commented Jul 30, 2022 • edited Loading

Implemented

Not Implemented

Trackers

Demo

hustcer commented Aug 14, 2022

hustcer commented Aug 16, 2022

chriskrycho commented Aug 16, 2022

chawyehsu commented Aug 16, 2022

chriskrycho commented Aug 16, 2022

chawyehsu commented Aug 17, 2022 • edited Loading

charlespierce left a comment

Choose a reason for hiding this comment

ScreamZ commented Aug 23, 2022

chawyehsu commented Aug 25, 2022

charlespierce left a comment

Choose a reason for hiding this comment

charlespierce Sep 16, 2022

Choose a reason for hiding this comment

chawyehsu Sep 19, 2022 • edited Loading

Choose a reason for hiding this comment

charlespierce Sep 17, 2022

Choose a reason for hiding this comment

chawyehsu Sep 19, 2022

Choose a reason for hiding this comment

mikrostew Oct 4, 2022

Choose a reason for hiding this comment

mikrostew left a comment

Choose a reason for hiding this comment

chawyehsu commented Oct 5, 2022 • edited Loading

avaly commented Nov 2, 2022 • edited Loading

runspired commented Nov 2, 2022

chawyehsu commented Nov 3, 2022

charlespierce left a comment

Choose a reason for hiding this comment

chawyehsu commented Nov 3, 2022

charlespierce left a comment

Choose a reason for hiding this comment

charlespierce commented Nov 3, 2022

chawyehsu commented Nov 4, 2022 • edited Loading

ScreamZ commented Nov 7, 2022

smblee commented Nov 9, 2022 • edited Loading

PindaPixel commented Nov 10, 2022

chawyehsu commented Nov 10, 2022

smblee commented Nov 14, 2022

michaelhays commented Nov 16, 2022

chawyehsu commented Jul 30, 2022 •

edited

Loading

chawyehsu commented Aug 17, 2022 •

edited

Loading

chawyehsu Sep 19, 2022 •

edited

Loading

chawyehsu commented Oct 5, 2022 •

edited

Loading

avaly commented Nov 2, 2022 •

edited

Loading

chawyehsu commented Nov 4, 2022 •

edited

Loading

smblee commented Nov 9, 2022 •

edited

Loading