mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix(cli): stop the migration guard misreading its own git history
`gitRecordedDatatableMigrationPaths` decides whether a data table migration the
working tree no longer has was ever tracked, and a push keeps the migration when
the answer is no. Two ways it answered wrongly:
`core.quotePath` is on by default, so a path holding a non-ASCII byte came back
C-quoted ("migrations/datatable/dt/1_caf\303\251.up.sql") and matched nothing the
caller held, reading as never recorded.
`git log HEAD` fails on a repository with no commits, which fell into "its
history could not be read" and told the user to check that git runs correctly.
Git runs fine; there is nothing committed yet, and that is what it now says.
Both are guarded by a temp-repo test, since the answer is a property of git's
output rather than of any logic testable without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0bbd559ac8
commit
da129416f1
+26
-1
@@ -156,9 +156,34 @@ export function gitRecordedDatatableMigrationPaths(): RecordedMigrationPaths {
|
||||
}
|
||||
const prefix = (prefixOut.stdout ?? "").trim();
|
||||
|
||||
// `git log HEAD` fails on a repository with no commits, which is not the same as
|
||||
// git being broken and must not be reported as such.
|
||||
const head = spawnSync("git", ["rev-parse", "--verify", "--quiet", "HEAD"], {
|
||||
encoding: "utf8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
if ((head.status ?? 1) !== 0) {
|
||||
return {
|
||||
kind: "unknown",
|
||||
reason: "this repository has no commits yet",
|
||||
remedy: "Commit the migrations you sync",
|
||||
};
|
||||
}
|
||||
|
||||
// `core.quotePath` (on by default) C-quotes any path with a non-ASCII byte, which
|
||||
// matches nothing the caller holds and reads as "never tracked".
|
||||
const r = spawnSync(
|
||||
"git",
|
||||
["log", "HEAD", "--format=", "--name-only", "--", "migrations/datatable"],
|
||||
[
|
||||
"-c",
|
||||
"core.quotePath=false",
|
||||
"log",
|
||||
"HEAD",
|
||||
"--format=",
|
||||
"--name-only",
|
||||
"--",
|
||||
"migrations/datatable",
|
||||
],
|
||||
{ encoding: "utf8", stdio: "pipe", maxBuffer: 64 * 1024 * 1024 },
|
||||
);
|
||||
if ((r.status ?? 1) !== 0) {
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
/**
|
||||
* `gitRecordedDatatableMigrationPaths` is what a push consults before deleting a data
|
||||
* table migration the working tree no longer has: a path in history is a real deletion,
|
||||
* a path never recorded is one this clone may simply never have synced. Both failures
|
||||
* pinned here made it answer "never recorded" or "git is broken" about a repository
|
||||
* that was neither, so a migration the user committed would have been kept and the
|
||||
* remedy offered would have been the wrong one.
|
||||
*
|
||||
* Runs real git in a temp repo — the answer is a property of git's output, not of any
|
||||
* logic that could be tested without it.
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { gitRecordedDatatableMigrationPaths } from "../src/utils/git.ts";
|
||||
|
||||
let dir: string;
|
||||
let cwd: string;
|
||||
|
||||
const git = (...args: string[]) =>
|
||||
execFileSync("git", args, { cwd: dir, encoding: "utf8", stdio: "pipe" });
|
||||
|
||||
const commitMigration = (name: string) => {
|
||||
const rel = `migrations/datatable/dt/${name}`;
|
||||
fs.mkdirSync(path.join(dir, path.dirname(rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, rel), "select 1;\n");
|
||||
git("add", "-A");
|
||||
git("commit", "-m", `add ${name}`);
|
||||
return rel;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
cwd = process.cwd();
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), "wmill-git-"));
|
||||
git("init", "-q", "-b", "main");
|
||||
git("config", "user.email", "t@t.dev");
|
||||
git("config", "user.name", "t");
|
||||
process.chdir(dir);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.chdir(cwd);
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe("gitRecordedDatatableMigrationPaths", () => {
|
||||
test("a repository with no commits is not a broken one", () => {
|
||||
const r = gitRecordedDatatableMigrationPaths();
|
||||
expect(r.kind).toBe("unknown");
|
||||
if (r.kind === "unknown") {
|
||||
expect(r.reason).toBe("this repository has no commits yet");
|
||||
}
|
||||
});
|
||||
|
||||
test("records a path with a non-ASCII byte unquoted", () => {
|
||||
// core.quotePath would return "migrations/datatable/dt/1_caf\303\251.up.sql",
|
||||
// which matches no path the caller holds and reads as never recorded.
|
||||
const rel = commitMigration("20260101000000_café.up.sql");
|
||||
const r = gitRecordedDatatableMigrationPaths();
|
||||
expect(r.kind).toBe("known");
|
||||
if (r.kind === "known") expect(r.paths.has(rel)).toBe(true);
|
||||
});
|
||||
|
||||
test("a committed migration stays recorded after its deletion is committed", () => {
|
||||
const rel = commitMigration("20260101000000_a.up.sql");
|
||||
fs.rmSync(path.join(dir, rel));
|
||||
git("add", "-A");
|
||||
git("commit", "-m", "delete it");
|
||||
const r = gitRecordedDatatableMigrationPaths();
|
||||
expect(r.kind).toBe("known");
|
||||
if (r.kind === "known") expect(r.paths.has(rel)).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user