use-node-version isn't respected if the first download fails

[gluxon]

pnpm version: 6.23.6

Code to reproduce the issue:

Suppose use-node-version is set:

# .npmrc
use-node-version = 16.13.0

If you were to exit pnpm while it downloads that version:

❯ pnpm node
^C

pnpm doesn't appear to re-attempt that download. It defaults to the system version for all future calls:

❯ pnpm node --version
v17.2.0

The ~/Library/pnpm/nodejs/16.13.0 dir stays empty in that case:

❯ ls -l ~/Library/pnpm/nodejs/16.13.0/

❯

Running rm -rf ~/Library/pnpm/nodejs/16.13.0 gets things to work as expected again.

❯ rm -rf ~/Library/pnpm/nodejs/16.13.0

❯ pnpm node --version
v16.13.0

Expected behavior:

Should pnpm re-attempt to download node if it didn't finish or failed in the past?

❯ pnpm node
# Interrupted download
^C

❯ pnpm node --version
# Redownloads node...
v16.13.0

Alternatively pnpm could set up it's Node.js download more atomically. So ~/Library/pnpm/nodejs/<version> only gets created after the tarball/zip download is complete.

Actual behavior:

pnpm detects that the empty ~/Library/pnpm/nodejs/16.13.0 dir exists and assumes node is in that path. Wrong node version is used from that point on.

Additional information:

• node -v prints: v17.2.0
• Windows, macOS, or Linux?: macOS

[gluxon]

Happy to help with this by the way. I think I'd mainly be curious which approach is preferred:

1. Try to make the download more atomic so the dir check can stay the same.
2. Extend the dir check by also seeing if node/node.exe exists.

[zkochan]

yes, we should do this

Try to make the download more atomic so the dir check can stay the same.

use the path-temp lib. Same as we do it here:

pnpm/packages/package-store/src/fs/importIndexedDir.ts

Line 19 in 5af305f

const stage = pathTemp(path.dirname(newDir))