From 5b08d04302453951c0cef1b96267d366afcf7bdd Mon Sep 17 00:00:00 2001 From: KimChow Date: Sat, 17 Oct 2026 00:09:59 +0800 Subject: [PATCH] fix(package): parse tar listings without platform line endings (#4315) Windows ships two tar implementations. `C:\Windows\System32\tar.exe` (bsdtar) terminates every listed member with CRLF, so `verify-package-pack.mjs` splitting the listing on a bare "\n" left a trailing "\r" on each non-final member. `verifyPackedBridge` compares member identities exactly, so it reported `package omitted package/bridge/hopper_bridge.py` although the tarball ships that member (#4128). `parseTarListing` normalizes line endings once, at the listing owner, and preserves every member string beyond its line ending. `verifyPackagePack` is the only tar listing producer in the package verifiers, so its consumers now compare bare member names; the prefix checks in the same file keep their behavior because they never matched a line ending. Covered by `tests/conformance/package/packedTarListing.test.ts`: CRLF and LF listings both yield exact bridge identities, the trailing line ending no longer produces an empty member, member strings keep embedded spaces, and a regression guard pins the previous parse's failure. Real Windows 11 evidence (`package/bridge/hopper_bridge.py` in the listing): | tar | listing | split("\n") | parseTarListing | | ---------------------------------- | ------- | ----------- | --------------- | | Git for Windows GNU tar | LF | true | true | | C:\Windows\System32\tar.exe | CRLF | false | true | Fixes #4128 Co-authored-by: Chow Kim Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- scripts/lib/verify-package-listing.d.mts | 2 + scripts/lib/verify-package-listing.mjs | 15 +++++ scripts/verify-package-pack.mjs | 8 +-- .../package/packedTarListing.test.ts | 61 +++++++++++++++++++ 4 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 scripts/lib/verify-package-listing.d.mts create mode 100644 scripts/lib/verify-package-listing.mjs create mode 100644 tests/conformance/package/packedTarListing.test.ts diff --git a/scripts/lib/verify-package-listing.d.mts b/scripts/lib/verify-package-listing.d.mts new file mode 100644 index 0000000000..7ccbda45fc --- /dev/null +++ b/scripts/lib/verify-package-listing.d.mts @@ -0,0 +1,2 @@ +/** Split a `tar -tf` listing into member names without platform line endings. */ +export function parseTarListing(stdout: string): readonly string[]; diff --git a/scripts/lib/verify-package-listing.mjs b/scripts/lib/verify-package-listing.mjs new file mode 100644 index 0000000000..d670382dc8 --- /dev/null +++ b/scripts/lib/verify-package-listing.mjs @@ -0,0 +1,15 @@ +/** + * Parse a `tar -tf` listing into its member names. + * + * Windows `tar` terminates each listed member with CRLF, so splitting on a + * bare "\n" leaves a trailing "\r" on every non-final member. Consumers + * compare member identities exactly (`packedFiles.includes(path)`), so those + * carriers make members that ship in the package look omitted. One listing + * uses one separator: a bare LF identifies LF output, whose CR bytes then + * belong to filenames. Normalize here, once, at the listing owner, and + * preserve every member string beyond its line ending. + */ +export const parseTarListing = (stdout) => { + const lineEnding = /(^|[^\r])\n/u.test(stdout) ? "\n" : "\r\n"; + return stdout.split(lineEnding).filter((member) => member.length > 0); +}; diff --git a/scripts/verify-package-pack.mjs b/scripts/verify-package-pack.mjs index 891d3c6be0..bd33bdda11 100644 --- a/scripts/verify-package-pack.mjs +++ b/scripts/verify-package-pack.mjs @@ -3,6 +3,7 @@ import { join } from "node:path"; import { CATALOG_IDENTITY } from "../dist/catalogIdentity.js"; import { exec, execNpm } from "./lib/verify-package-core.mjs"; +import { parseTarListing } from "./lib/verify-package-listing.mjs"; import { verifyPackedBridge } from "./verify-packed-bridge.mjs"; /** Create a tarball and assert production packaging constraints. */ @@ -17,10 +18,9 @@ export async function verifyPackagePack({ root, workspace }) { }) ).stdout.trim(), ); - const listing = (await exec("tar", ["-tf", tarball])).stdout; - // A bare LF identifies LF output; preceding CRs then belong to filenames. - const lineEnding = /(^|[^\r])\n/u.test(listing) ? "\n" : "\r\n"; - const packedFiles = listing.split(lineEnding); + const packedFiles = parseTarListing( + (await exec("tar", ["-tf", tarball])).stdout, + ); const packedManifest = JSON.parse( (await exec("tar", ["-xOf", tarball, "package/package.json"])).stdout, ); diff --git a/tests/conformance/package/packedTarListing.test.ts b/tests/conformance/package/packedTarListing.test.ts new file mode 100644 index 0000000000..392d5a4951 --- /dev/null +++ b/tests/conformance/package/packedTarListing.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; + +import { parseTarListing } from "../../../scripts/lib/verify-package-listing.mjs"; + +/** + * `verify-packed-bridge.mjs` compares these members exactly + * (`packedFiles.includes(path)`), so a retained `\r` reports a shipped bridge + * as omitted. See issue #4128. + */ +const packedBridges = [ + "package/bridge/hopper_bridge.py", + "package/bridge/cutter/rea_bridge.py", + "package/bridge/ghidra/ReaGhidraBridge.java", + "package/bridge/ghidra/ReaGhidraBundleActivator.java", +]; + +const listing = (lineEnding: string) => + ["package/package.json", ...packedBridges, "package/skills/rea/SKILL.md"] + .map((member) => `${member}${lineEnding}`) + .join(""); + +describe("packed tar listing", () => { + it("keeps exact member identities when Windows tar emits CRLF", () => { + const packedFiles = parseTarListing(listing("\r\n")); + for (const path of packedBridges) expect(packedFiles).toContain(path); + expect(packedFiles.some((member) => member.includes("\r"))).toBe(false); + }); + + it("keeps exact member identities on POSIX listings", () => { + const packedFiles = parseTarListing(listing("\n")); + for (const path of packedBridges) expect(packedFiles).toContain(path); + }); + + it("drops the empty member left by the trailing line ending", () => { + expect(parseTarListing(listing("\r\n"))).toHaveLength(6); + expect(parseTarListing(listing("\n"))).toHaveLength(6); + expect(parseTarListing("")).toEqual([]); + }); + + it("preserves member strings beyond the line ending", () => { + expect(parseTarListing("package/a b/c d.py\r\npackage/e.py\r\n")).toEqual([ + "package/a b/c d.py", + "package/e.py", + ]); + }); + + it("keeps a literal CR at the end of a filename in an LF listing", () => { + // One listing has one separator; a bare LF makes CR bytes filename bytes. + expect(parseTarListing("package/a\r\npackage/b\n")).toEqual([ + "package/a\r", + "package/b", + ]); + }); + + it("splits a bare newline listing the way the Windows bug appeared", () => { + // Guard the reported regression: the previous parse kept `\r` on members. + const naive = listing("\r\n").split("\n"); + expect(naive).not.toContain(packedBridges[0]); + expect(parseTarListing(listing("\r\n"))).toContain(packedBridges[0]); + }); +});