6b84457247
`ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR` reports the OS error and nothing else, so
every entry under `node_modules` is a suspect. On 12.3.4, Windows 11:
```
Error: ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR
× installing dependencies
╰─▶ Failed to remove modules directory contents: 拒绝访问。 (os error 5)
```
(`拒绝访问` is "Access is denied" on a zh-CN Windows.)
With this change, the same failure on the same machine:
```
╰─▶ Failed to remove
C:\Users\me\AppData\Local\Temp\pxverify2\node_modules\is-odd from the
modules directory: 拒绝访问。 (os error 5)
```
## Where it came from
I hit this upgrading an existing project from pnpm 11 to pnpm 12 on Windows. From a clean
directory, an install with `pnpm@11` followed by an install with `pnpm@12` fails on the
first v12 run and succeeds on the second, three times out of three:
```
cycle 1: v12 run 1 exit=1 [ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR] run 2 exit=0
cycle 2: v12 run 1 exit=1 [ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR] run 2 exit=0
cycle 3: v12 run 1 exit=1 [ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR] run 2 exit=0
```
Narrowing down which entry was at fault took a bisect by hand, and I still got it wrong:
working from the outside I concluded the junctions pnpm 11 leaves *inside* `.pnpm` were
responsible. The patched binary answered it in one run, and the answer was the top-level
`node_modules/is-odd` junction instead.
**This PR does not fix that failure.** I have a reliable reproduction but not a confident
account of the mechanism, so I am not guessing at a fix. What I can fix is the part that
made the reproduction far harder to narrow down than it needed to be. If the underlying
upgrade failure is of interest, I am happy to open a separate issue with the full
reproduction.
It is a different failure from the transient Windows file-lock family in #14349, #14407
and #14549: those depend on another process holding a handle and clear on a retry, while
this one reproduces from a clean state every time and is specific to a pnpm 11 tree.
## The change
`RemoveModulesDir` becomes a struct variant carrying the path, the way
`PruneDirectDepsError::RemoveBin { path, error }` and
`InstallError::UnsafeFilteredModulesDir { modules_dir, workspace_root }` already do. It
names the entry the `remove_dir_all` and `remove_file` failures were on. The two
`read_dir` failures have the modules directory itself, so they build a sibling
`ReadModulesDir { path, error }` variant that says the directory could not be read; it
carries the same `ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR` code, so no new error code
reaches users. Neither variant has other consumers in the workspace.
It renders through `dunce::simplified` and `Path::display` rather than `{:?}`. The entries
come from a canonicalized modules directory, so `{:?}` prints the `\\?\` verbatim prefix
that #13990 removed from user-visible paths, and doubles every separator:
```
Failed to remove "\\\\?\\C:\\Users\\me\\...\\node_modules\\is-odd" from the modules directory
```
A path a user is meant to act on should be copy-pasteable. `dunce` is already a dependency
of this crate, and `path.display()` in an error is the shape used in `auth-commands` and
`cli`.
## Same shape, not included
Two variants in `InstallError` still wrap a bare `io::Error`:
- `CleanGitBranchLockfiles`, where a path would help for the same reason
- `ProjectLifecycleThreadPool`, where there is no path to name
I left them alone because I have not hit either. Happy to include
`CleanGitBranchLockfiles` here if you would rather it move in one go.
---------
Co-authored-by: xmhua <236586769+xmhuangzhijun-hue@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Zoltan Kochan <z@kochan.io>
246 B
246 B
pacquet
| pacquet |
|---|
| patch |
ERR_PNPM_PACKAGE_MANAGER_REMOVE_MODULES_DIR now names the file or directory in node_modules that pnpm could not clean up. It previously reported only the underlying OS error, such as "Access is denied (os error 5)".