mirror of
https://github.com/pnpm/pnpm.git
synced 2026-06-28 01:45:30 -04:00
fix: avoid workspace state parse crashes (#12094)
* fix: avoid workspace state parse crashes * fix(workspace-state): write workspace state atomically Port the atomic-write half of the pnpm fix for #12020 to pacquet. pacquet's install/add/update/remove all call update_workspace_state, and the on-disk file is shared with the pnpm CLI, so a non-atomic fs::write could leave a torn file that a concurrent `pnpm run` reads and crashes on. Write to a temp file in the same directory and rename it into place, mirroring pnpm's switch to write-file-atomic. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
This commit is contained in:
6
.changeset/fix-workspace-state-race.md
Normal file
6
.changeset/fix-workspace-state-race.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/workspace.state": patch
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
Avoid crashing when the workspace state cache is partially written or malformed.
|
||||
@@ -17,10 +17,10 @@ derive_more = { workspace = true }
|
||||
indexmap = { workspace = true }
|
||||
serde = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
@@ -17,10 +17,12 @@ use pacquet_diagnostics::miette::{self, Diagnostic};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::{
|
||||
collections::BTreeMap,
|
||||
fs, io,
|
||||
fs,
|
||||
io::{self, Write},
|
||||
path::{Path, PathBuf},
|
||||
time::{SystemTime, UNIX_EPOCH},
|
||||
};
|
||||
use tempfile::NamedTempFile;
|
||||
|
||||
/// Basename of the workspace-state file, written inside `node_modules/`.
|
||||
///
|
||||
@@ -177,10 +179,17 @@ pub enum UpdateWorkspaceStateError {
|
||||
|
||||
/// Write `state` to `<workspace_dir>/node_modules/.pnpm-workspace-state-v1.json`.
|
||||
///
|
||||
/// Mirrors upstream's [`updateWorkspaceState`](https://github.com/pnpm/pnpm/blob/7ff112bac6/workspace/state/src/updateWorkspaceState.ts):
|
||||
/// `JSON.stringify(state, undefined, 2) + '\n'`. `serde_json`'s pretty
|
||||
/// printer uses the same 2-space indent and `": "` separator as JS, so
|
||||
/// the on-disk bytes round-trip cleanly between the two writers.
|
||||
/// Writes to a temporary file in the same directory, then atomically
|
||||
/// renames it into place, so a concurrent reader — pnpm or pacquet —
|
||||
/// never observes a half-written file. Mirrors upstream's
|
||||
/// [`updateWorkspaceState`](https://github.com/pnpm/pnpm/blob/7ff112bac6/workspace/state/src/updateWorkspaceState.ts),
|
||||
/// which writes through `write-file-atomic` for the same reason
|
||||
/// ([#12020](https://github.com/pnpm/pnpm/issues/12020)).
|
||||
///
|
||||
/// The serialized bytes are `JSON.stringify(state, undefined, 2) + '\n'`:
|
||||
/// `serde_json`'s pretty printer uses the same 2-space indent and `": "`
|
||||
/// separator as JS, so the on-disk bytes round-trip cleanly between the
|
||||
/// two writers.
|
||||
pub fn update_workspace_state(
|
||||
workspace_dir: &Path,
|
||||
state: &WorkspaceState,
|
||||
@@ -194,8 +203,17 @@ pub fn update_workspace_state(
|
||||
let mut serialized =
|
||||
serde_json::to_string_pretty(state).map_err(UpdateWorkspaceStateError::SerializeJson)?;
|
||||
serialized.push('\n');
|
||||
fs::write(&file_path, serialized.as_bytes())
|
||||
.map_err(|source| UpdateWorkspaceStateError::WriteFile { path: file_path, source })
|
||||
let mut temp = NamedTempFile::new_in(parent).map_err(|source| {
|
||||
UpdateWorkspaceStateError::WriteFile { path: file_path.clone(), source }
|
||||
})?;
|
||||
temp.write_all(serialized.as_bytes()).map_err(|source| {
|
||||
UpdateWorkspaceStateError::WriteFile { path: file_path.clone(), source }
|
||||
})?;
|
||||
temp.persist(&file_path).map_err(|error| UpdateWorkspaceStateError::WriteFile {
|
||||
path: file_path,
|
||||
source: error.error,
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Read the workspace state file at `<workspace_dir>/node_modules/.pnpm-workspace-state-v1.json`.
|
||||
|
||||
6
pnpm-lock.yaml
generated
6
pnpm-lock.yaml
generated
@@ -9957,6 +9957,9 @@ importers:
|
||||
ramda:
|
||||
specifier: 'catalog:'
|
||||
version: '@pnpm/ramda@0.28.1'
|
||||
write-file-atomic:
|
||||
specifier: 'catalog:'
|
||||
version: 7.0.1
|
||||
devDependencies:
|
||||
'@jest/globals':
|
||||
specifier: 'catalog:'
|
||||
@@ -9973,6 +9976,9 @@ importers:
|
||||
'@types/ramda':
|
||||
specifier: 'catalog:'
|
||||
version: 0.31.1
|
||||
'@types/write-file-atomic':
|
||||
specifier: 'catalog:'
|
||||
version: 4.0.3
|
||||
|
||||
workspace/workspace-manifest-reader:
|
||||
dependencies:
|
||||
|
||||
@@ -35,7 +35,8 @@
|
||||
"@pnpm/catalogs.types": "workspace:*",
|
||||
"@pnpm/config.reader": "workspace:*",
|
||||
"@pnpm/types": "workspace:*",
|
||||
"ramda": "catalog:"
|
||||
"ramda": "catalog:",
|
||||
"write-file-atomic": "catalog:"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@pnpm/logger": "catalog:"
|
||||
@@ -45,7 +46,8 @@
|
||||
"@pnpm/logger": "workspace:*",
|
||||
"@pnpm/prepare": "workspace:*",
|
||||
"@pnpm/workspace.state": "workspace:*",
|
||||
"@types/ramda": "catalog:"
|
||||
"@types/ramda": "catalog:",
|
||||
"@types/write-file-atomic": "catalog:"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=22.13"
|
||||
|
||||
@@ -18,5 +18,12 @@ export function loadWorkspaceState (workspaceDir: string): WorkspaceState | unde
|
||||
}
|
||||
throw error
|
||||
}
|
||||
return JSON.parse(cacheFileContent) as WorkspaceState
|
||||
try {
|
||||
return JSON.parse(cacheFileContent) as WorkspaceState
|
||||
} catch (error) {
|
||||
if (util.types.isNativeError(error) && error.name === 'SyntaxError') {
|
||||
return undefined
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import path from 'node:path'
|
||||
|
||||
import { logger } from '@pnpm/logger'
|
||||
import type { ConfigDependencies } from '@pnpm/types'
|
||||
import writeFileAtomic from 'write-file-atomic'
|
||||
|
||||
import { createWorkspaceState } from './createWorkspaceState.js'
|
||||
import { getFilePath } from './filePath.js'
|
||||
@@ -23,5 +24,5 @@ export async function updateWorkspaceState (opts: UpdateWorkspaceStateOptions):
|
||||
const workspaceStateJSON = JSON.stringify(workspaceState, undefined, 2) + '\n'
|
||||
const cacheFile = getFilePath(opts.workspaceDir)
|
||||
await fs.promises.mkdir(path.dirname(cacheFile), { recursive: true })
|
||||
await fs.promises.writeFile(cacheFile, workspaceStateJSON)
|
||||
await writeFileAtomic(cacheFile, workspaceStateJSON)
|
||||
}
|
||||
|
||||
@@ -71,3 +71,15 @@ test('loadWorkspaceState() when cache file exists and is correct', async () => {
|
||||
expect(loadWorkspaceState(workspaceDir)).toStrictEqual(workspaceState)
|
||||
expect(jest.mocked(logger.debug).mock.calls).toStrictEqual(expectedLoggerCalls)
|
||||
})
|
||||
|
||||
test('loadWorkspaceState() when cache file contains partial JSON', async () => {
|
||||
prepareEmpty()
|
||||
|
||||
const workspaceDir = process.cwd()
|
||||
const cacheFile = getFilePath(workspaceDir)
|
||||
fs.mkdirSync(path.dirname(cacheFile), { recursive: true })
|
||||
fs.writeFileSync(cacheFile, '{\n "settings": ')
|
||||
|
||||
expect(loadWorkspaceState(workspaceDir)).toBeUndefined()
|
||||
expect(jest.mocked(logger.debug).mock.calls).toStrictEqual(expectedLoggerCalls)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user