Skip to content

[PR] fix(dsh-ledger): ignore pnpm's peer suffix when comparing lockfile versions, preserve it when rewriting | fix(dsh-ledger): 比较 lockfile 版本时忽略 pnpm 的 peer 后缀，改写时保留它 #6314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BBleae wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[PR] fix(dsh-ledger): ignore pnpm's peer suffix when comparing lockfile versions, preserve it when rewriting | fix(dsh-ledger): 比较 lockfile 版本时忽略 pnpm 的 peer 后缀，改写时保留它 #6314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions src/dsh-ledger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ function unquote(v: string): string {
return v.replace(/^["']|["']$/g, "");
}

/** Split a pnpm lockfile `version:` value into the resolved version and the
* peer-resolution suffix pnpm appends when the package (or something it
* depends on) has peers — `1.2.3(@types/node@16.9.1)` → ["1.2.3",
* "(@types/node@16.9.1)"]. The suffix names which peer set this install was
* resolved against; it is NOT part of the version, so every comparison with
* the on-disk copy must ignore it, and a rewrite must carry it over (the
* `snapshots:` key it points at carries the same suffix). */
function splitPeerSuffix(v: string): [version: string, peerSuffix: string] {
const at = v.indexOf("(");
return at > 0 ? [v.slice(0, at), v.slice(at)] : [v, ""];
}

/** Plain-YAML-safe scalar: quote anything that could be misread (leading
* indicators like `>` or the alias indicator `*`, whitespace, …). Versions
* and ^/~ specs stay bare; a leading `*` is quoted because YAML would parse
Expand Down Expand Up @@ -125,6 +137,8 @@ interface LockResolution {
* - flat (lockfile ≤5.4): same-line values — under the top-level
* `specifiers:` section the value is the declared spec, under the flat
* `dependencies:` map it is the RESOLVED version.
* The resolved version is returned WITHOUT pnpm's peer suffix
* (`1.2.3(@types/node@16.9.1)` reads as `1.2.3`) — see splitPeerSuffix.
* Returns undefined when no lockfile exists or records no entry. */
function readPnpmLockResolution(profileDir: string): LockResolution | undefined {
let text: string;
Expand Down Expand Up @@ -158,7 +172,7 @@ function readPnpmLockResolution(profileDir: string): LockResolution | undefined
const spec = inner.match(/^\s*specifier:\s*(\S+)/);
if (spec && out.specifier === undefined) out.specifier = unquote(spec[1]);
const ver = inner.match(/^\s*version:\s*(\S+)/);
if (ver && out.version === undefined) out.version = unquote(ver[1]);
if (ver && out.version === undefined) out.version = splitPeerSuffix(unquote(ver[1]))[0];
}
}
return out.specifier !== undefined || out.version !== undefined ? out : undefined;
Expand Down Expand Up @@ -295,9 +309,18 @@ function patchNestedEntries(lines: string[], from: number, to: number, spec: str
const vm = inner.match(/^([ \t]*)version:[ \t]*(\S.*)?$/);
if (vm !== null) {
sawVersion = true;
if (unquote(vm[2] ?? "") !== version) {
lines[j] = `${vm[1]}version: ${yamlScalar(version)}`;
details.push({ file: "pnpm-lock.yaml", field: "version", before: unquote(vm[2] ?? ""), after: version });
// Compare and swap the version part only: a peer suffix
// (`1.2.3(@types/node@16.9.1)`) is kept verbatim, in the
// quoting style the line already has (pnpm writes it bare;
// yamlScalar would quote it and churn the line).
const raw = (vm[2] ?? "").trimEnd();
const cur = unquote(raw);
const [curVersion, peerSuffix] = splitPeerSuffix(cur);
if (curVersion !== version) {
const quote = /^["']/.test(raw) ? raw[0] : "";
const next = peerSuffix === "" ? yamlScalar(version) : `${quote}${version}${peerSuffix}${quote}`;
lines[j] = `${vm[1]}version: ${next}`;
details.push({ file: "pnpm-lock.yaml", field: "version", before: cur, after: `${version}${peerSuffix}` });
}
}
}
Expand Down
133 changes: 133 additions & 0 deletions tests/issue3496-dsh-ledger-convergence.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,51 @@ function pnpmLockV9(spec: string, version: string): string {
].join("\n");
}

/** pnpm v9 lockfile where the importer's resolution carries a PEER SUFFIX —
* what pnpm writes when billion-context (or a dependency of it) has a peer
* that the profile satisfies: `version: 0.1.197(@types/node@16.9.1)`, with
* the suffixed key under `snapshots:` and the bare key under `packages:`. */
function pnpmLockV9Peer(spec: string, version: string, peerSuffix = "(@types/node@16.9.1)"): string {
return [
"lockfileVersion: '9.0'",
"",
"settings:",
" autoInstallPeers: false",
" excludeLinksFromLockfile: false",
"",
"importers:",
"",
" .:",
" dependencies:",
" '@types/node':",
" specifier: 16.9.1",
" version: 16.9.1",
" billion-context:",
` specifier: ${spec}`,
` version: ${version}${peerSuffix}`,
" other-dep:",
" specifier: ^1.2.3",
" version: 1.2.3(@types/node@16.9.1)",
"",
"packages:",
"",
" '@types/node@16.9.1':",
" resolution: {integrity: sha512-fake==}",
"",
` billion-context@${version}:`,
" resolution: {integrity: sha512-fake==}",
"",
"snapshots:",
"",
" '@types/node@16.9.1': {}",
"",
` billion-context@${version}${peerSuffix}:`,
" dependencies:",
" '@types/node': 16.9.1",
"",
].join("\n");
}

/** Flat pnpm ≤5.4 lockfile (same-line values). */
function pnpmLockFlat(spec: string, version: string): string {
return [
Expand Down Expand Up @@ -278,6 +323,94 @@ test("#3496: healthy triple — ZERO writes, byte-identical files", () => {
}
});

test("#3496: healthy triple with a pnpm peer suffix on the lock version — no finding, ZERO writes", () => {
const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-peer-a-"));
try {
// `version: 0.1.197(@types/node@16.9.1)` IS v0.1.197 — the suffix names
// the peer set, not a different version. Reading it as one made every
// such profile a permanent copy-vs-lock finding, and made convergence
// strip the suffix (pointing the importer at a snapshots key that does
// not exist).
const dir = makeProfile(root, "peer", { spec: "^0.1.197", copyVersion: "0.1.197", pnpmLock: pnpmLockV9Peer("^0.1.197", "0.1.197") });
assert.equal(readDshLedgerState(dir).lockVersion, "0.1.197", "the peer suffix is not part of the resolved version");
assert.deepEqual(dshProfileLedgerFindings(dir), []);
const before = profileFiles(dir);
const beforeMt = profileMtimes(dir);
const res = convergeDshLedger(dir, "0.1.197");
assert.equal(res.error, undefined);
assert.deepEqual(res.changed, []);
assert.deepEqual(res.details, []);
assertZeroWrites(dir, before, beforeMt);
} finally {
rmrf(root);
rmrf(__cacheRoot);
}
});

test("#3496: peer-suffixed lock version — real drift swaps the version part only, the suffix survives", () => {
const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-peer-b-"));
try {
const dir = makeProfile(root, "peerdrift", { spec: "^0.1.197", copyVersion: "0.1.197", pnpmLock: pnpmLockV9Peer("^0.1.196", "0.1.196") });
const findings = dshProfileLedgerFindings(dir);
assert.deepEqual(findings.map((f) => f.kind), ["copy-vs-lock", "manifest-vs-lock"]);
assert.equal(findings[0]!.lockVersion, "0.1.196", "findings name the version, not the peer set");
const before = readFileSync(path.join(dir, "pnpm-lock.yaml"), "utf8");
const res = convergeDshLedger(dir, "0.1.197");
assert.equal(res.error, undefined);
assert.deepEqual(res.changed, ["pnpm-lock.yaml"]);
assert.deepEqual(res.details.map((d) => [d.field, d.before, d.after]), [
["specifier", "^0.1.196", "^0.1.197"],
["version", "0.1.196(@types/node@16.9.1)", "0.1.197(@types/node@16.9.1)"],
]);
const after = readFileSync(path.join(dir, "pnpm-lock.yaml"), "utf8");
// Exactly the two billion-context importer lines move; the version line
// stays bare (pnpm's own style — no quoting churn) and keeps its suffix.
const expected = before
.replace(" specifier: ^0.1.196", " specifier: ^0.1.197")
.replace(" version: 0.1.196(@types/node@16.9.1)", " version: 0.1.197(@types/node@16.9.1)");
assert.equal(after, expected);
assert.ok(after.includes(" version: 1.2.3(@types/node@16.9.1)"), "another dependency's suffixed version is not ours to touch");
assert.ok(after.includes(" billion-context@0.1.196(@types/node@16.9.1):"), "store bookkeeping sections stay untouched (#3570)");
assert.deepEqual(dshProfileLedgerFindings(dir), [], "re-audit after convergence must be clean");
// Second pass is a no-op: the suffixed record is already in step.
const mt = profileMtimes(dir);
const again = convergeDshLedger(dir, "0.1.197");
assert.deepEqual(again.changed, []);
assertZeroWrites(dir, profileFiles(dir), mt);
} finally {
rmrf(root);
rmrf(__cacheRoot);
}
});

test("#3496: patchPnpmLock peer suffix — multi-peer and quoted values keep their shape; v6–v8 nested entries too", () => {
// several peers chain their parentheses; the whole tail is the suffix
const multi = pnpmLockV9Peer("^0.1.196", "0.1.196", "(@types/node@16.9.1)(zod@4.1.8)");
const inStep = patchPnpmLock(multi, "^0.1.196", "0.1.196");
assert.equal(inStep.kind, "ok");
assert.deepEqual(inStep.kind === "ok" ? inStep.details : undefined, [], "same version + suffix is in step — no details, so no write");
const moved = patchPnpmLock(multi, "^0.1.196", "0.1.197");
assert.equal(moved.kind, "ok");
if (moved.kind === "ok") {
assert.ok(moved.next.includes(" version: 0.1.197(@types/node@16.9.1)(zod@4.1.8)\n"));
assert.deepEqual(moved.details.map((d) => [d.field, d.before, d.after]), [
["version", "0.1.196(@types/node@16.9.1)(zod@4.1.8)", "0.1.197(@types/node@16.9.1)(zod@4.1.8)"],
]);
}
// a quoted value keeps its quotes
const quoted = pnpmLockV9Peer("^0.1.196", "0.1.196").replace("version: 0.1.196(@types/node@16.9.1)", "version: '0.1.196(@types/node@16.9.1)'");
const q = patchPnpmLock(quoted, "^0.1.196", "0.1.197");
assert.equal(q.kind, "ok");
if (q.kind === "ok") assert.ok(q.next.includes(" version: '0.1.197(@types/node@16.9.1)'\n"));
// v6–v8: nested entries under the top-level dependencies map carry the same suffix form
const v6 = pnpmLockV6("^0.1.196", "0.1.196").replace(" version: 0.1.196", " version: 0.1.196(@types/node@16.9.1)");
const v6Same = patchPnpmLock(v6, "^0.1.196", "0.1.196");
assert.deepEqual(v6Same.kind === "ok" ? v6Same.details : undefined, []);
const v6Moved = patchPnpmLock(v6, "^0.1.196", "0.1.197");
assert.equal(v6Moved.kind, "ok");
if (v6Moved.kind === "ok") assert.ok(v6Moved.next.includes(" version: 0.1.197(@types/node@16.9.1)\n"));
});

test("#3496: flat pnpm ≤5.4 lockfile converges both same-line sections", () => {
const root = mkdtempSync(path.join(tmpdir(), "bc-i3496-e-"));
try {
Expand Down
Loading