Skip to content

Commit 3e83581

Browse files
zkochanzkochan
authored
fix: drop patchPnpmEnv so standalone+self-update works on Windows (#258)
`patchPnpmEnv` prepended `dest/node_modules/.bin` to PATH before spawning `pnpm install` / `pnpm store prune`. On Windows in standalone mode, `.bin/pnpm.cmd` is an npm-created shim that always points at the BOOTSTRAP pnpm (currently 11.0.4) — the binary npm linked when it installed `@pnpm/exe` into `node_modules`. The self-updated pnpm written by `pnpm self-update` lives at `$PNPM_HOME/bin`, which is separately added to PATH via `addPath()` in install-pnpm. When the user requested a pnpm version different from the bootstrap under `standalone: true` on Windows, patchPnpmEnv's `.bin` entry shadowed the self-updated `$PNPM_HOME/bin` and the action's internal `pnpm install` ran on the bootstrap. On a pnpm 11.0.x bootstrap this broke any 11.1+ install flag (e.g. `--no-runtime`), reporting: ERROR Unknown option: 'runtime' POSIX standalone got lucky because `.bin` and `$PNPM_HOME` resolve to the same directory there. Non-standalone never tripped on this since the `.bin/pnpm` symlink for a regular `pnpm` package keeps working across self-updates. Removed `patchPnpmEnv` and the now-empty `src/utils/` module. `spawnSync` now inherits `process.env`, whose PATH is already correctly fronted by `$PNPM_HOME/bin` and `$PNPM_HOME` via the `addPath` calls in install-pnpm. Added `standalone_windows_self_update` to test.yaml as a regression guard: standalone on Windows + target 11.1.0 + `run_install` with `--no-runtime`. With the previous code, the install would have run under the bootstrap (11.0.4) and errored on the unknown flag. Originally found while building pnpm/setup (the new combined pnpm + runtime action).
1 parent 551b42e commit 3e83581

5 files changed

Lines changed: 190 additions & 150 deletions

File tree

‎.github/workflows/test.yaml‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,54 @@ jobs:
194194
pnpm add is-odd
195195
shell: bash
196196

197+
standalone_windows_self_update:
198+
# Regression guard for the patchPnpmEnv PATH-shadow bug. When
199+
# standalone: true on Windows AND the requested pnpm differs from the
200+
# bootstrap, the previous patchPnpmEnv prepended node_modules/.bin to
201+
# PATH; that directory contains an npm-created pnpm.cmd shim pointing
202+
# at the BOOTSTRAP pnpm, which shadowed the self-updated pnpm at
203+
# $PNPM_HOME/bin and caused `pnpm install` inside the action to run
204+
# under the bootstrap version. Exercising a newer-pnpm-only flag
205+
# (`--no-runtime`, added in 11.1.0) makes the regression assertable:
206+
# if the bootstrap (11.0.4) handles the install, it errors with
207+
# "Unknown option: 'runtime'".
208+
name: 'Standalone Windows self-update (PATH regression)'
209+
runs-on: windows-latest
210+
steps:
211+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
212+
213+
- name: Set up package.json with a minimal manifest
214+
# run_install needs a manifest to install against. Removing the
215+
# repo's existing pnpm-lock.yaml avoids frozen-lockfile mismatch.
216+
run: |
217+
rm -f pnpm-lock.yaml
218+
echo '{"name":"sw","private":true,"packageManager":"pnpm@11.1.0"}' > package.json
219+
shell: bash
220+
221+
- name: Run the action
222+
uses: ./
223+
with:
224+
version: 11.1.0
225+
standalone: true
226+
run_install: |
227+
args: ['--no-runtime']
228+
229+
- name: 'Test: pnpm install completed under the self-updated pnpm'
230+
# If the bug recurs, the previous step's run_install will have failed
231+
# the job with "Unknown option: 'runtime'", so reaching this step
232+
# implies success. Still verify the version on PATH matches request.
233+
env:
234+
REQUIRED: '11.1.0'
235+
run: |
236+
set -e
237+
actual="$(pnpm --version)"
238+
echo "pnpm --version: ${actual}"
239+
if [ "${actual}" != "${REQUIRED}" ]; then
240+
echo "Expected pnpm ${REQUIRED}, got ${actual}"
241+
exit 1
242+
fi
243+
shell: bash
244+
197245
cache_store_path:
198246
# Regression guard for #233. When package.json pins a pnpm major that
199247
# differs from the bootstrap pnpm's major, the bootstrap reports its

‎dist/index.js‎

Lines changed: 134 additions & 134 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/pnpm-install/index.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
import { setFailed, startGroup, endGroup } from '@actions/core'
22
import { spawnSync } from 'child_process'
33
import { Inputs } from '../inputs'
4-
import { patchPnpmEnv } from '../utils'
54

65
export function runPnpmInstall(inputs: Inputs) {
7-
const env = patchPnpmEnv(inputs)
8-
96
for (const options of inputs.runInstall) {
107
const args = ['install']
118
if (options.recursive) args.unshift('recursive')
@@ -14,11 +11,16 @@ export function runPnpmInstall(inputs: Inputs) {
1411
const cmdStr = ['pnpm', ...args].join(' ')
1512
startGroup(`Running ${cmdStr}...`)
1613

14+
// spawnSync inherits process.env, which already has $PNPM_HOME/bin and
15+
// $PNPM_HOME prepended via addPath() in install-pnpm. Do NOT pass a
16+
// hand-patched env that adds node_modules/.bin to the front — on
17+
// Windows standalone, .bin/pnpm.cmd is an npm shim pointing at the
18+
// BOOTSTRAP pnpm, which would shadow the self-updated one and break
19+
// newer-pnpm-only behavior.
1720
const { error, status } = spawnSync('pnpm', args, {
1821
stdio: 'inherit',
1922
cwd: options.cwd,
2023
shell: true,
21-
env,
2224
})
2325

2426
endGroup()

‎src/pnpm-store-prune/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { warning, startGroup, endGroup } from '@actions/core'
22
import { spawnSync } from 'child_process'
33
import { Inputs } from '../inputs'
4-
import { patchPnpmEnv } from '../utils'
54

65
export function pruneStore(inputs: Inputs) {
76
if (inputs.runInstall.length === 0) {
@@ -10,10 +9,11 @@ export function pruneStore(inputs: Inputs) {
109
}
1110

1211
startGroup('Running pnpm store prune...')
12+
// spawnSync inherits process.env (which has the right PATH from addPath
13+
// in install-pnpm). See pnpm-install/index.ts for the rationale.
1314
const { error, status } = spawnSync('pnpm', ['store', 'prune'], {
1415
stdio: 'inherit',
1516
shell: true,
16-
env: patchPnpmEnv(inputs),
1717
})
1818
endGroup()
1919

‎src/utils/index.ts‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

0 commit comments

Comments
 (0)