fix(lockfile): read a CRLF or BOM-prefixed multi-document lockfile (#13609)
`extract_main_document` / `extract_env_document` split the combined `pnpm-lock.yaml` on literal `---\n` and `\n---\n`. A lockfile pacquet did not write may carry CRLF line endings — a `core.autocrlf` checkout on Windows — or a UTF-8 BOM, and neither marker then matches. The main extractor fell through to returning the whole file, which serde rejected as `multiple YAML documents detected`, so the lockfile was discarded and every dependency re-resolved from the registry. Normalize the whole file (strip BOM, CRLF to LF) inside both extractors, which covers every caller. `save_value_to_path` already normalized the same way inline and now shares the helper. The TypeScript stack needs no counterpart change: `pnpm11/lockfile/fs/src/yamlDocuments.ts` already normalizes CRLF in `extractMainDocument` / `extractEnvDocument` and strips the BOM at each read site. Closes https://github.com/pnpm/pnpm/issues/13606
This commit is contained in:
1 parent
a93748005a
commit
ee3bb4b586
8 files changed
+111
-19
No files matched your search
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pacquet": patch
|
||||
---
|
||||
|
||||
Fixed `pnpm install` ignoring a `pnpm-lock.yaml` that carries a leading env lockfile document when the file has CRLF line endings or a UTF-8 byte order mark, as a `core.autocrlf` checkout on Windows produces. The lockfile was reported as broken with `multiple YAML documents detected` and every dependency was re-resolved from the registry [#13606](https://github.com/pnpm/pnpm/issues/13606).
|
||||
@@ -116,7 +116,7 @@ impl EnvLockfile {
|
||||
let Some(env_doc) = extract_env_document(&content) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let mut env: EnvLockfile = serde_saphyr::from_str(env_doc)
|
||||
let mut env: EnvLockfile = serde_saphyr::from_str(&env_doc)
|
||||
.map_err(|source| LoadLockfileError::parse_yaml(&path, &source))?;
|
||||
env.root_importer_mut();
|
||||
Ok(Some(env))
|
||||
|
||||
@@ -69,6 +69,19 @@ fn reads_non_numeric_lockfile_version() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reads_a_crlf_combined_lockfile() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let env = sample_env_lockfile();
|
||||
env.write(dir.path()).unwrap();
|
||||
let path = dir.path().join(Lockfile::FILE_NAME);
|
||||
let crlf = std::fs::read_to_string(&path).unwrap().replace('\n', "\r\n");
|
||||
std::fs::write(&path, crlf).unwrap();
|
||||
|
||||
let read_back = EnvLockfile::read(dir.path()).unwrap().expect("env document present");
|
||||
assert_eq!(read_back, env);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_preserves_existing_main_document() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -118,7 +118,7 @@ impl Lockfile {
|
||||
return Ok(None);
|
||||
}
|
||||
serde_saphyr::from_str_with_options::<Self>(
|
||||
main,
|
||||
&main,
|
||||
serde_saphyr::options! {
|
||||
// Every size-proportional budget is raised to the document's
|
||||
// byte length: none of these dimensions can exceed the size of
|
||||
|
||||
@@ -87,6 +87,29 @@ fn parses_main_document_from_combined_yaml() {
|
||||
assert_eq!(combined_loaded, main_only_loaded);
|
||||
}
|
||||
|
||||
/// Regression test for <https://github.com/pnpm/pnpm/issues/13606>: a
|
||||
/// combined lockfile checked out with CRLF line endings was handed to
|
||||
/// serde whole, failing as "multiple YAML documents detected" and
|
||||
/// making every install re-resolve from the registry.
|
||||
#[test]
|
||||
fn parses_main_document_from_crlf_combined_yaml() {
|
||||
let combined = format!("---\n{ENV_DOC}\n---\n{MAIN_DOC}").replace('\n', "\r\n");
|
||||
let tmp = write_lockfile(&combined);
|
||||
let virtual_store_dir = tmp.path().join("node_modules").join(".pacquet");
|
||||
|
||||
let crlf_loaded = Lockfile::load_current_from_virtual_store_dir(&virtual_store_dir)
|
||||
.expect("load CRLF combined lockfile")
|
||||
.expect("CRLF combined lockfile should be present");
|
||||
|
||||
let tmp_main = write_lockfile(MAIN_DOC);
|
||||
let main_only_dir = tmp_main.path().join("node_modules").join(".pacquet");
|
||||
let main_only_loaded = Lockfile::load_current_from_virtual_store_dir(&main_only_dir)
|
||||
.expect("load main-only lockfile")
|
||||
.expect("main-only lockfile should be present");
|
||||
|
||||
assert_eq!(crlf_loaded, main_only_loaded);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn env_only_lockfile_loads_as_none() {
|
||||
let env_only = format!("---\n{ENV_DOC}\n");
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
use crate::{
|
||||
Lockfile, serialize_yaml,
|
||||
yaml_documents::{YAML_DOCUMENT_SEPARATOR, YAML_DOCUMENT_START, extract_env_document},
|
||||
yaml_documents::{
|
||||
YAML_DOCUMENT_SEPARATOR, YAML_DOCUMENT_START, extract_env_document,
|
||||
normalize_lockfile_content,
|
||||
},
|
||||
};
|
||||
use derive_more::{Display, Error};
|
||||
use pacquet_diagnostics::miette::{self, Diagnostic};
|
||||
@@ -79,9 +82,7 @@ pub fn save_value_to_path<Document: serde::Serialize>(
|
||||
) -> Result<(), SaveLockfileError> {
|
||||
let content = serialize_yaml::to_string(value).map_err(SaveLockfileError::SerializeYaml)?;
|
||||
let existing = match fs::read_to_string(path) {
|
||||
Ok(existing) => {
|
||||
Some(existing.strip_prefix('\u{feff}').unwrap_or(&existing).replace("\r\n", "\n"))
|
||||
}
|
||||
Ok(existing) => Some(normalize_lockfile_content(&existing).into_owned()),
|
||||
Err(error) if error.kind() == io::ErrorKind::NotFound => None,
|
||||
Err(error) => return Err(SaveLockfileError::WriteFile(error)),
|
||||
};
|
||||
|
||||
@@ -6,6 +6,13 @@
|
||||
//! and the second document is the regular project lockfile. Pacquet
|
||||
//! only consumes the second document, so this module strips the
|
||||
//! leading env document before handing the content to serde.
|
||||
//!
|
||||
//! Every entry point here takes the *whole* file and normalizes it
|
||||
//! first: a lockfile pacquet did not write may carry a UTF-8 BOM or
|
||||
//! CRLF line endings (a `core.autocrlf` checkout on Windows), and the
|
||||
//! document markers below would then match nothing.
|
||||
|
||||
use std::borrow::Cow;
|
||||
|
||||
/// Document-stream marker that ends one YAML document and starts the
|
||||
/// next.
|
||||
@@ -14,16 +21,25 @@ pub(crate) const YAML_DOCUMENT_SEPARATOR: &str = "\n---\n";
|
||||
/// Document-stream marker at the very start of a file.
|
||||
pub(crate) const YAML_DOCUMENT_START: &str = "---\n";
|
||||
|
||||
/// Strip a leading UTF-8 BOM and rewrite CRLF as LF, so the rest of the
|
||||
/// lockfile machinery only ever sees the byte shape pacquet writes.
|
||||
#[must_use]
|
||||
pub fn normalize_lockfile_content(content: &str) -> Cow<'_, str> {
|
||||
let content = content.strip_prefix('\u{feff}').unwrap_or(content);
|
||||
if content.contains("\r\n") {
|
||||
Cow::Owned(content.replace("\r\n", "\n"))
|
||||
} else {
|
||||
Cow::Borrowed(content)
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the main lockfile document (second YAML document) from a
|
||||
/// combined file.
|
||||
#[must_use]
|
||||
pub fn extract_main_document(content: &str) -> &str {
|
||||
let Some(rest) = content.strip_prefix(YAML_DOCUMENT_START) else {
|
||||
return content;
|
||||
};
|
||||
match rest.find(YAML_DOCUMENT_SEPARATOR) {
|
||||
Some(idx) => &rest[idx + YAML_DOCUMENT_SEPARATOR.len()..],
|
||||
None => "",
|
||||
pub fn extract_main_document(content: &str) -> Cow<'_, str> {
|
||||
match normalize_lockfile_content(content) {
|
||||
Cow::Borrowed(content) => Cow::Borrowed(main_document_of(content)),
|
||||
Cow::Owned(content) => Cow::Owned(main_document_of(&content).to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,12 +51,25 @@ pub fn extract_main_document(content: &str) -> &str {
|
||||
/// - Returns the slice between the leading `---\n` and the next
|
||||
/// `\n---\n` separator. A leading `---\n` with no following separator
|
||||
/// (an env-only file with no main document) also yields `None`.
|
||||
///
|
||||
/// pacquet reads the whole lockfile into memory rather than streaming,
|
||||
/// so this skips chunked BOM/CRLF handling — the only callers pass
|
||||
/// content pacquet itself wrote with LF line endings.
|
||||
#[must_use]
|
||||
pub fn extract_env_document(content: &str) -> Option<&str> {
|
||||
pub fn extract_env_document(content: &str) -> Option<Cow<'_, str>> {
|
||||
match normalize_lockfile_content(content) {
|
||||
Cow::Borrowed(content) => env_document_of(content).map(Cow::Borrowed),
|
||||
Cow::Owned(content) => env_document_of(&content).map(|doc| Cow::Owned(doc.to_string())),
|
||||
}
|
||||
}
|
||||
|
||||
fn main_document_of(content: &str) -> &str {
|
||||
let Some(rest) = content.strip_prefix(YAML_DOCUMENT_START) else {
|
||||
return content;
|
||||
};
|
||||
match rest.find(YAML_DOCUMENT_SEPARATOR) {
|
||||
Some(idx) => &rest[idx + YAML_DOCUMENT_SEPARATOR.len()..],
|
||||
None => "",
|
||||
}
|
||||
}
|
||||
|
||||
fn env_document_of(content: &str) -> Option<&str> {
|
||||
let rest = content.strip_prefix(YAML_DOCUMENT_START)?;
|
||||
rest.find(YAML_DOCUMENT_SEPARATOR).map(|idx| &rest[..idx])
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use super::extract_main_document;
|
||||
use super::{extract_env_document, extract_main_document};
|
||||
|
||||
#[test]
|
||||
fn returns_entire_content_when_it_does_not_start_with_separator() {
|
||||
@@ -18,3 +18,24 @@ fn returns_the_second_document_from_a_combined_file() {
|
||||
let combined = format!("---\nfoo: bar\n---\n{main}");
|
||||
assert_eq!(extract_main_document(&combined), main);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn splits_a_crlf_combined_file() {
|
||||
let combined = "---\r\nfoo: bar\r\n---\r\nlockfileVersion: 9.0\r\npackages: {}\r\n";
|
||||
assert_eq!(extract_main_document(combined), "lockfileVersion: 9.0\npackages: {}\n");
|
||||
assert_eq!(extract_env_document(combined).as_deref(), Some("foo: bar"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn splits_a_combined_file_behind_a_byte_order_mark() {
|
||||
let combined = "\u{feff}---\nfoo: bar\n---\nlockfileVersion: 9.0\n";
|
||||
assert_eq!(extract_main_document(combined), "lockfileVersion: 9.0\n");
|
||||
assert_eq!(extract_env_document(combined).as_deref(), Some("foo: bar"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalizes_a_single_document_file() {
|
||||
let content = "\u{feff}lockfileVersion: 9.0\r\npackages: {}\r\n";
|
||||
assert_eq!(extract_main_document(content), "lockfileVersion: 9.0\npackages: {}\n");
|
||||
assert_eq!(extract_env_document(content), None);
|
||||
}
|
||||
Reference in new issue
Block a user