Skip to content

Commit 711e0da

Browse files
committed
Make the claim about other platforms testable
insideRoot compared a resolved native path against the forward slashes git reports on windows, so nothing was ever inside the root there and every symbol quietly vanished. Carriage returns rode into the plates from any repository with windows line endings. check.js builds a real repository and asserts what those bugs broke, so the matrix now runs it on linux, macos and windows rather than only proving the modules parse.
1 parent fd5be99 commit 711e0da

6 files changed

Lines changed: 143 additions & 10 deletions

File tree

‎.github/workflows/check.yml‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ on:
77

88
jobs:
99
check:
10-
runs-on: ubuntu-latest
10+
runs-on: ${{ matrix.os }}
1111
strategy:
12+
fail-fast: false
1213
matrix:
13-
node: ["18", "20", "22"]
14+
os: [ubuntu-latest, macos-latest, windows-latest]
15+
node: ["18", "22"]
1416
steps:
1517
- uses: actions/checkout@v5
1618
- uses: actions/setup-node@v5
1719
with:
1820
node-version: ${{ matrix.node }}
19-
- run: make check
21+
- run: node check.js

‎Makefile‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,4 @@ uninstall:
1515
@echo "removed $(TARGET)"
1616

1717
check:
18-
@node -e 'require("./src/parse-diff");require("./src/layout");require("./src/structure");require("./src/server");require("./src/git");console.log("modules load")'
19-
@node bin/gitdiff3d.js --help > /dev/null && echo "cli responds"
18+
@node check.js

‎check.js‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// What has to be true on every platform we claim to run on: the modules load,
2+
// a real repository produces a scene with symbols in it, a file with windows
3+
// line endings does not smuggle a carriage return into the plates, and the
4+
// viewer is actually served.
5+
const { execFileSync } = require("node:child_process");
6+
const fs = require("node:fs");
7+
const http = require("node:http");
8+
const os = require("node:os");
9+
const path = require("node:path");
10+
11+
const CLI = path.join(__dirname, "bin", "gitdiff3d.js");
12+
13+
const fail = (what) => {
14+
console.error("FAIL " + what);
15+
process.exit(1);
16+
};
17+
18+
const ok = (condition, what) => (condition ? console.log("ok " + what) : fail(what));
19+
20+
const git = (args, cwd) =>
21+
execFileSync("git", ["-c", "user.email=check@example.com", "-c", "user.name=check", ...args], {
22+
cwd,
23+
encoding: "utf8",
24+
stdio: ["ignore", "pipe", "pipe"],
25+
});
26+
27+
const sceneOf = (cwd, args = []) =>
28+
JSON.parse(execFileSync(process.execPath, [CLI, ...args, "--json"], {
29+
cwd,
30+
encoding: "utf8",
31+
maxBuffer: 64 * 1024 * 1024,
32+
}));
33+
34+
const get = (port, route) =>
35+
new Promise((resolve) =>
36+
http.get({ host: "127.0.0.1", port, path: route }, (res) => {
37+
let body = "";
38+
res.on("data", (chunk) => { body += chunk; });
39+
res.on("end", () => resolve({ status: res.statusCode, body }));
40+
}),
41+
);
42+
43+
function repoWith(files) {
44+
const root = fs.mkdtempSync(path.join(os.tmpdir(), "gitdiff3d-"));
45+
git(["init", "-q", "."], root);
46+
git(["config", "core.autocrlf", "false"], root);
47+
Object.entries(files.before).forEach(([name, text]) =>
48+
fs.writeFileSync(path.join(root, name), text));
49+
git(["add", "-A"], root);
50+
git(["commit", "-qm", "base"], root);
51+
Object.entries(files.after).forEach(([name, text]) =>
52+
fs.writeFileSync(path.join(root, name), text));
53+
return root;
54+
}
55+
56+
function modulesLoad() {
57+
["parse-diff", "layout", "structure", "server", "git", "order"]
58+
.forEach((name) => require("./src/" + name));
59+
ok(true, "modules load");
60+
execFileSync(process.execPath, [CLI, "--help"], { stdio: "ignore" });
61+
ok(true, "cli responds");
62+
}
63+
64+
// The symbol only appears if the new side of the file could be read, which is
65+
// the whole of the path handling: this is the check that fails on windows when
66+
// the root is compared with the wrong separator.
67+
function scenesHaveSymbols() {
68+
const root = repoWith({
69+
before: { "a.js": "function alpha() {\n return 1;\n}\n" },
70+
after: { "a.js": "function alpha() {\n return 2;\n}\n" },
71+
});
72+
try {
73+
const scene = sceneOf(root);
74+
ok(scene.totals.files === 1, "one changed file");
75+
ok(scene.hunks.length === 1, "one hunk");
76+
ok(scene.files[0].language === "c-like", "language recognised");
77+
ok(scene.hunks[0].symbol === "alpha", "hunk named by its enclosing function");
78+
ok(!JSON.stringify(scene).includes(root), "scene carries no path from this machine");
79+
} finally {
80+
fs.rmSync(root, { recursive: true, force: true, maxRetries: 5 });
81+
}
82+
}
83+
84+
function carriageReturnsAreDropped() {
85+
const root = repoWith({
86+
before: { "b.js": "const a = 1;\r\nconst b = 2;\r\n" },
87+
after: { "b.js": "const a = 1;\r\nconst b = 3;\r\n" },
88+
});
89+
try {
90+
const scene = sceneOf(root);
91+
const texts = scene.hunks.flatMap((hunk) => hunk.lines.map((line) => line.text));
92+
ok(texts.length > 0, "crlf file still produces lines");
93+
ok(texts.every((text) => !text.includes("\r")), "no carriage return survives into a plate");
94+
} finally {
95+
fs.rmSync(root, { recursive: true, force: true, maxRetries: 5 });
96+
}
97+
}
98+
99+
async function viewerIsServed() {
100+
const { serve } = require("./src/server");
101+
const root = repoWith({
102+
before: { "c.js": "const x = 1;\n" },
103+
after: { "c.js": "const x = 2;\n" },
104+
});
105+
try {
106+
const scene = sceneOf(root);
107+
const port = await serve(() => scene, 0);
108+
const page = await get(port, "/");
109+
const json = await get(port, "/scene.json");
110+
const three = await get(port, "/vendor/three.min.js");
111+
const elsewhere = await get(port, "/etc/passwd");
112+
ok(page.status === 200 && page.body.includes("<title>gitdiff3d"), "viewer served");
113+
ok(json.status === 200 && JSON.parse(json.body).hunks.length === 1, "scene served");
114+
ok(three.status === 200 && three.body.length > 100000, "three served");
115+
ok(elsewhere.status === 404, "unknown routes refused");
116+
} finally {
117+
fs.rmSync(root, { recursive: true, force: true, maxRetries: 5 });
118+
}
119+
}
120+
121+
async function main() {
122+
modulesLoad();
123+
scenesHaveSymbols();
124+
carriageReturnsAreDropped();
125+
await viewerIsServed();
126+
console.log("all checks passed on " + process.platform + " node " + process.versions.node);
127+
process.exit(0);
128+
}
129+
130+
main();

‎src/git.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,12 @@ const newSideOf = (revisions, cwd) => {
4949
};
5050

5151
// A diff header is text, and text can say "../../elsewhere"; the working tree
52-
// is the only place this is allowed to read from.
52+
// is the only place this is allowed to read from. Both sides are resolved
53+
// because git reports the root with forward slashes even on Windows.
5354
const insideRoot = (root, filePath) => {
54-
const full = path.resolve(root, filePath);
55-
return full === root || full.startsWith(root + path.sep);
55+
const base = path.resolve(root);
56+
const full = path.resolve(base, filePath);
57+
return full === base || full.startsWith(base + path.sep);
5658
};
5759

5860
const sourceOf = (rev, filePath, root) => {

‎src/parse-diff.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function parseDiff(text) {
2626
flags = null;
2727
};
2828

29-
for (const raw of text.split("\n")) {
29+
for (const raw of text.split(/\r?\n/)) {
3030
const header = FILE_HEADER.exec(raw);
3131
if (header) {
3232
closeFile();

‎src/structure.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ const indentUnitOf = (lines) => {
9797
};
9898

9999
function analyze(language, text) {
100-
const lines = text.split("\n");
100+
const lines = text.split(/\r?\n/);
101101
const endOf = language.scope === "braces" ? braceEnd : indentEnd;
102102
const found = [];
103103
lines.forEach((line, i) => {

0 commit comments

Comments
 (0)