Skip to content

fix(global): install packages into final prefix#1698

Draft
liangmiQwQ wants to merge 6 commits into
voidzero-dev:mainfrom
liangmiQwQ:codex/install-global-direct-prefix
Draft

fix(global): install packages into final prefix#1698
liangmiQwQ wants to merge 6 commits into
voidzero-dev:mainfrom
liangmiQwQ:codex/install-global-direct-prefix

Conversation

@liangmiQwQ
Copy link
Copy Markdown
Contributor

@liangmiQwQ liangmiQwQ commented May 27, 2026

As #664 (comment) said, #1685 did not completely fix #664.

The current global package install flow runs npm install -g in a temporary prefix and then moves the installed package prefix into VP_HOME/packages.

However, npm and some packages's postinstall script may create files that links an absolute path to this temporary dir, or a relative path use the temp dir as the base dir, they will be invalid after moving, causing bugs like that.

$ vp install -g .
VITE+ - The Unified Toolchain for the Web

info: Installing 1 global package with Node.js 25.0.0
✓ Installed just-a-normal-package 0.0.0
  Bins: just-a-normal-package

tip: Available short aliases: i = install, rm = remove, un = uninstall, up = update, ls = list, ln = link

$ just-a-normal-package
vp: Binary 'just-a-normal-package' not found: Package just-a-normal-package not found

(It is quite strange that it can't be reproduced on local snap tests. I am still figuring out the reason now.)
UPDATE: please refer to the comment below

So to fix this problem, this PR installs global packages directly into their final package prefix to avoid moving, to fix this problem.

There is another runtime behavior change worth mentioning: If one package in a multi-package install fails, this PR still finalizes shims for packages that already installed successfully instead of deleting them or leaving them without runnable bins. This is because if users are trying to reinstall a package or update, uninstall the installed packages can make problems. This problem only happens after the changes mentioned above, and it also follows npm's behavior.

🤖 Generated with Codex.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 27, 2026

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit bdc9308
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6a1a3700e2f6f30008659559

@liangmiQwQ liangmiQwQ changed the title fix: install global packages into final prefix fix(global): install packages into final prefix May 27, 2026
@liangmiQwQ liangmiQwQ force-pushed the codex/install-global-direct-prefix branch from 21941bc to 3e97d03 Compare May 27, 2026 22:33
@liangmiQwQ liangmiQwQ force-pushed the codex/install-global-direct-prefix branch from 3e97d03 to a37bd9f Compare May 28, 2026 13:32
@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

(It is quite strange that it can't be reproduced on local snap tests. I am still figuring out the reason now.)

I've found the causes with the help of Codex. It is a little bit complex and related to the current snaptest architecture.

Vite+ calls npm to install a package into a temp prefix, and move to the final directory.

# The example cwd: ~

# Vite+'s temp directory
~/.vite-plus/tmp/packages/just-a-normal-package

tree ~/.vite-plus/tmp/packages/just-a-normal-package/lib/node_modules/
#~/.vite-plus/tmp/packages/just-a-normal-package/lib/node_modules/
#└── just-a-normal-package -> ../../../../../../
# This is linked to ~, 6 layers of parent directory (`../`)

# and if it is moved to ~/.vite-plus/packages/just-a-normal-package (not ~/.vite-plus/tmp)
tree ~/.vite-plus/packages/just-a-normal-package/lib/node_modules/
#~/.vite-plus/packages/just-a-normal-package/lib/node_modules/
#└── just-a-normal-package -> ../../../../../../
# It is still  ../../../../../../ after moving (6 layers of `../`)
# So it will be linked to `~/../`, like `/Users`, instead of `~`. (We only need 5 layers of `../` there), make packages broken.

However, the snap test runner doesn't use ~ as the cwd. Instead, it copies the fixture to a random temp directory under /private/var, I will use /private as example.

# The example cwd: /private ⚠️

# Vite+'s temp directory
~/.vite-plus/tmp/packages/just-a-normal-package

tree ~/.vite-plus/tmp/packages/just-a-normal-package/lib/node_modules/
#~/.vite-plus/tmp/packages/just-a-normal-package/lib/node_modules/
#└── just-a-normal-package -> ../../../../../../../../private
# This is linked to ~, 8 layers of `../`

# and if it is moved to ~/.vite-plus/packages/just-a-normal-package (not ~/.vite-plus/tmp)
tree ~/.vite-plus/packages/just-a-normal-package/lib/node_modules/
#~/.vite-plus/packages/just-a-normal-package/lib/node_modules/
#└── just-a-normal-package -> ../../../../../../../../private
# It is still  ../../../../../../ after moving (still 8 layers of `../`)
# 👉 Different from the `~` case, one more `../` won't break the path because it hits the root `/` already.
# So it will be still linked to `/private`.

This logic makes snap tests invalid for this fixes.

@fengmk2, do you have any good idea to add validation tests for this fix?

@liangmiQwQ liangmiQwQ marked this pull request as ready for review May 30, 2026 00:43
@liangmiQwQ
Copy link
Copy Markdown
Contributor Author

@codex

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bdc9308081

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

tokio::fs::remove_dir_all(&staging_dir).await?;
) -> Result<InstalledPackage, Error> {
// 1. Prepare final package directory
cleanup_installed_package(package_name).await?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve installed package until replacement succeeds

When reinstalling or updating a package that is already installed, this cleanup runs before npm install has succeeded. If npm then fails (bad version, registry/network error, postinstall failure, etc.), the old package directory, shims, bin configs, and metadata have already been removed, so a failed replacement leaves the previously working global command uninstalled. The old install should remain in place until the new install is complete and ready to swap/finalize.

Useful? React with 👍 / 👎.

@liangmiQwQ liangmiQwQ marked this pull request as draft May 30, 2026 01:13
@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented May 30, 2026

@liangmiQwQ If it's too much trouble, let's skip adding a snap test for this special case for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vp install -g ./some-local-package not working

2 participants