Fix per-file diff size gating
This commit is contained in:
@@ -1121,6 +1121,41 @@ const x = 1;
|
|||||||
expect(diff.diff).not.toContain("x".repeat(10_000));
|
expect(diff.diff).not.toContain("x".repeat(10_000));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("keeps small tracked files displayable when another tracked file has a massive diff", async () => {
|
||||||
|
writeFileSync(join(repoDir, "generated.js"), `const data = "old";\n`);
|
||||||
|
writeFileSync(join(repoDir, "small.ts"), `export const value = "old";\n`);
|
||||||
|
execFileSync("git", ["add", "generated.js", "small.ts"], { cwd: repoDir });
|
||||||
|
execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "add tracked files"], {
|
||||||
|
cwd: repoDir,
|
||||||
|
});
|
||||||
|
|
||||||
|
writeFileSync(join(repoDir, "generated.js"), `const data = "${"x".repeat(2_100_000)}";\n`);
|
||||||
|
writeFileSync(join(repoDir, "small.ts"), `export const value = "new";\n`);
|
||||||
|
|
||||||
|
startGitCommandMetrics();
|
||||||
|
const diff = await getCheckoutDiff(repoDir, { mode: "uncommitted", includeStructured: true });
|
||||||
|
const metrics = stopGitCommandMetrics();
|
||||||
|
const commands = metrics.commands.map((command) => command.args.join(" "));
|
||||||
|
|
||||||
|
expect(
|
||||||
|
diff.structured?.map((file) => ({
|
||||||
|
path: file.path,
|
||||||
|
status: file.status,
|
||||||
|
hunks: file.hunks.length,
|
||||||
|
})),
|
||||||
|
).toEqual([
|
||||||
|
{ path: "generated.js", status: "too_large", hunks: 0 },
|
||||||
|
{ path: "small.ts", status: "ok", hunks: 1 },
|
||||||
|
]);
|
||||||
|
expect(diff.diff).toContain("# generated.js: diff too large omitted");
|
||||||
|
expect(diff.diff).toContain(`-export const value = "old";`);
|
||||||
|
expect(diff.diff).toContain(`+export const value = "new";`);
|
||||||
|
expect(commands).toContain("diff --numstat HEAD");
|
||||||
|
expect(commands).toContain("diff HEAD -- generated.js");
|
||||||
|
expect(commands).toContain("diff HEAD -- small.ts");
|
||||||
|
expect(metrics.maxConcurrent).toBeLessThanOrEqual(8);
|
||||||
|
});
|
||||||
|
|
||||||
it("short-circuits tracked binary files", async () => {
|
it("short-circuits tracked binary files", async () => {
|
||||||
const trackedBinaryPath = join(repoDir, "tracked-blob.bin");
|
const trackedBinaryPath = join(repoDir, "tracked-blob.bin");
|
||||||
writeFileSync(trackedBinaryPath, Buffer.from([0x00, 0xff, 0x10, 0x80, 0x00]));
|
writeFileSync(trackedBinaryPath, Buffer.from([0x00, 0xff, 0x10, 0x80, 0x00]));
|
||||||
|
|||||||
@@ -538,7 +538,6 @@ function buildGitDiffArgs(args: { ignoreWhitespace?: boolean; extra: string[] })
|
|||||||
}
|
}
|
||||||
|
|
||||||
const TRACKED_DIFF_NUMSTAT_MAX_BYTES = 2 * 1024 * 1024; // 2MB
|
const TRACKED_DIFF_NUMSTAT_MAX_BYTES = 2 * 1024 * 1024; // 2MB
|
||||||
const TRACKED_DIFF_PER_FILE_MAX_CHARS = 1024 * 1024;
|
|
||||||
const EMPTY_TREE_OBJECT_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
|
const EMPTY_TREE_OBJECT_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
|
||||||
|
|
||||||
function isUnbornHeadDiffError(error: unknown): boolean {
|
function isUnbornHeadDiffError(error: unknown): boolean {
|
||||||
@@ -606,68 +605,29 @@ async function getTrackedNumstatByPath(
|
|||||||
return stats;
|
return stats;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface TrackedDiffSection {
|
async function getTrackedDiffTextForPath(input: {
|
||||||
|
cwd: string;
|
||||||
|
refsForDiff: CheckoutDiffRefs;
|
||||||
path: string;
|
path: string;
|
||||||
text: string;
|
ignoreWhitespace: boolean;
|
||||||
isTooLarge: boolean;
|
}): Promise<{ path: string; text: string; truncated: boolean }> {
|
||||||
}
|
const result = await runGitCommand(
|
||||||
|
buildGitDiffArgs({
|
||||||
|
ignoreWhitespace: input.ignoreWhitespace,
|
||||||
|
extra: [...getCheckoutDiffRefArgs(input.refsForDiff), "--", input.path],
|
||||||
|
}),
|
||||||
|
{
|
||||||
|
cwd: input.cwd,
|
||||||
|
envOverlay: READ_ONLY_GIT_ENV,
|
||||||
|
maxOutputBytes: PER_FILE_DIFF_MAX_BYTES,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
function extractTrackedDiffMetadataPath(section: string, prefix: "--- " | "+++ "): string | null {
|
return {
|
||||||
const line = section.split("\n").find((candidate) => candidate.startsWith(prefix));
|
path: input.path,
|
||||||
if (!line) {
|
text: result.stdout,
|
||||||
return null;
|
truncated: result.truncated,
|
||||||
}
|
};
|
||||||
const path = line.slice(prefix.length).replace(/\t.*$/, "").trimEnd();
|
|
||||||
if (path === "/dev/null") {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
return path.startsWith("a/") || path.startsWith("b/") ? path.slice(2) : path;
|
|
||||||
}
|
|
||||||
|
|
||||||
function extractTrackedDiffSectionPath(section: string): string | null {
|
|
||||||
const firstLineEnd = section.indexOf("\n");
|
|
||||||
const firstLine = firstLineEnd === -1 ? section : section.slice(0, firstLineEnd);
|
|
||||||
const header = firstLine.startsWith("diff --git ") ? firstLine.slice("diff --git ".length) : "";
|
|
||||||
const prefixedPathMatch = header.match(/^a\/(.+) b\/(.+)$/);
|
|
||||||
if (prefixedPathMatch) {
|
|
||||||
return prefixedPathMatch[2] ?? null;
|
|
||||||
}
|
|
||||||
|
|
||||||
const metadataPath =
|
|
||||||
extractTrackedDiffMetadataPath(section, "+++ ") ??
|
|
||||||
extractTrackedDiffMetadataPath(section, "--- ");
|
|
||||||
if (metadataPath) {
|
|
||||||
return metadataPath;
|
|
||||||
}
|
|
||||||
|
|
||||||
const pathMatch = header.match(/^(\S+)\s+(\S+)$/);
|
|
||||||
return pathMatch?.[2] ?? null;
|
|
||||||
}
|
|
||||||
|
|
||||||
function splitTrackedDiffSections(diffText: string): TrackedDiffSection[] {
|
|
||||||
const starts: number[] = [];
|
|
||||||
const diffHeaderPattern = /^diff --git /gm;
|
|
||||||
let match: RegExpExecArray | null;
|
|
||||||
while ((match = diffHeaderPattern.exec(diffText))) {
|
|
||||||
starts.push(match.index);
|
|
||||||
}
|
|
||||||
|
|
||||||
const sections: TrackedDiffSection[] = [];
|
|
||||||
for (let index = 0; index < starts.length; index += 1) {
|
|
||||||
const start = starts[index];
|
|
||||||
const end = starts[index + 1] ?? diffText.length;
|
|
||||||
const text = diffText.slice(start, end);
|
|
||||||
const path = extractTrackedDiffSectionPath(text);
|
|
||||||
if (!path) {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
sections.push({
|
|
||||||
path,
|
|
||||||
text,
|
|
||||||
isTooLarge: text.length > TRACKED_DIFF_PER_FILE_MAX_CHARS,
|
|
||||||
});
|
|
||||||
}
|
|
||||||
return sections;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
export class NotGitRepoError extends Error {
|
export class NotGitRepoError extends Error {
|
||||||
@@ -2016,7 +1976,6 @@ interface AppendStructuredTrackedDiffsInput {
|
|||||||
trackedNumstatByPath: Map<string, FileStat>;
|
trackedNumstatByPath: Map<string, FileStat>;
|
||||||
trackedPlaceholderByPath: Map<string, { status: "binary" | "too_large"; stat: FileStat }>;
|
trackedPlaceholderByPath: Map<string, { status: "binary" | "too_large"; stat: FileStat }>;
|
||||||
trackedDiffText: string;
|
trackedDiffText: string;
|
||||||
trackedDiffTruncated: boolean;
|
|
||||||
refsForDiff: CheckoutDiffRefs;
|
refsForDiff: CheckoutDiffRefs;
|
||||||
ignoreWhitespace: boolean;
|
ignoreWhitespace: boolean;
|
||||||
structured: ParsedDiffFile[];
|
structured: ParsedDiffFile[];
|
||||||
@@ -2037,7 +1996,6 @@ async function appendStructuredTrackedDiffs(
|
|||||||
trackedNumstatByPath,
|
trackedNumstatByPath,
|
||||||
trackedPlaceholderByPath,
|
trackedPlaceholderByPath,
|
||||||
trackedDiffText,
|
trackedDiffText,
|
||||||
trackedDiffTruncated,
|
|
||||||
refsForDiff,
|
refsForDiff,
|
||||||
ignoreWhitespace,
|
ignoreWhitespace,
|
||||||
structured,
|
structured,
|
||||||
@@ -2096,7 +2054,6 @@ async function appendStructuredTrackedDiffs(
|
|||||||
// structured placeholder in that case so whitespace-only edits truly disappear.
|
// structured placeholder in that case so whitespace-only edits truly disappear.
|
||||||
if (
|
if (
|
||||||
ignoreWhitespace &&
|
ignoreWhitespace &&
|
||||||
!trackedDiffTruncated &&
|
|
||||||
change.status.startsWith("M") &&
|
change.status.startsWith("M") &&
|
||||||
(!stat || (!stat.isBinary && stat.additions === 0 && stat.deletions === 0))
|
(!stat || (!stat.isBinary && stat.additions === 0 && stat.deletions === 0))
|
||||||
) {
|
) {
|
||||||
@@ -2110,7 +2067,7 @@ async function appendStructuredTrackedDiffs(
|
|||||||
additions: stat?.additions ?? 0,
|
additions: stat?.additions ?? 0,
|
||||||
deletions: stat?.deletions ?? 0,
|
deletions: stat?.deletions ?? 0,
|
||||||
hunks: [],
|
hunks: [],
|
||||||
status: trackedDiffTruncated ? "too_large" : "ok",
|
status: "ok",
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -2186,7 +2143,6 @@ interface ProcessTrackedChangesResult {
|
|||||||
trackedNumstatByPath: Map<string, FileStat>;
|
trackedNumstatByPath: Map<string, FileStat>;
|
||||||
trackedPlaceholderByPath: Map<string, { status: "binary" | "too_large"; stat: FileStat }>;
|
trackedPlaceholderByPath: Map<string, { status: "binary" | "too_large"; stat: FileStat }>;
|
||||||
trackedDiffText: string;
|
trackedDiffText: string;
|
||||||
trackedDiffTruncated: boolean;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async function processTrackedChanges(
|
async function processTrackedChanges(
|
||||||
@@ -2214,41 +2170,38 @@ async function processTrackedChanges(
|
|||||||
}
|
}
|
||||||
|
|
||||||
let trackedDiffText = "";
|
let trackedDiffText = "";
|
||||||
let trackedDiffTruncated = false;
|
let trackedDiffBytes = 0;
|
||||||
if (trackedDiffPaths.length > 0) {
|
if (trackedDiffPaths.length > 0) {
|
||||||
const trackedDiffResult = await runGitCommand(
|
const trackedDiffs = await Promise.all(
|
||||||
buildGitDiffArgs({
|
trackedDiffPaths.map((path) =>
|
||||||
ignoreWhitespace,
|
getTrackedDiffTextForPath({
|
||||||
extra: [...getCheckoutDiffRefArgs(refsForDiff), "--", ...trackedDiffPaths],
|
cwd,
|
||||||
}),
|
refsForDiff,
|
||||||
{
|
path,
|
||||||
cwd,
|
ignoreWhitespace,
|
||||||
envOverlay: READ_ONLY_GIT_ENV,
|
}),
|
||||||
maxOutputBytes: TOTAL_DIFF_MAX_BYTES,
|
),
|
||||||
},
|
|
||||||
);
|
);
|
||||||
trackedDiffTruncated = trackedDiffResult.truncated;
|
|
||||||
|
|
||||||
const visibleTrackedDiffs: string[] = [];
|
const visibleTrackedDiffs: string[] = [];
|
||||||
const sections = splitTrackedDiffSections(trackedDiffResult.stdout);
|
for (const fileDiff of trackedDiffs) {
|
||||||
for (let index = 0; index < sections.length; index += 1) {
|
if (fileDiff.truncated) {
|
||||||
const section = sections[index];
|
trackedPlaceholderByPath.set(fileDiff.path, {
|
||||||
const isTruncatedTail = trackedDiffTruncated && index === sections.length - 1;
|
|
||||||
if (section.isTooLarge || isTruncatedTail) {
|
|
||||||
trackedPlaceholderByPath.set(section.path, {
|
|
||||||
status: "too_large",
|
status: "too_large",
|
||||||
stat: trackedNumstatByPath.get(section.path) ?? null,
|
stat: trackedNumstatByPath.get(fileDiff.path) ?? null,
|
||||||
});
|
});
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
visibleTrackedDiffs.push(section.text);
|
const diffBytes = Buffer.byteLength(fileDiff.text, "utf8");
|
||||||
|
if (trackedDiffBytes + diffBytes > TOTAL_DIFF_MAX_BYTES) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
trackedDiffBytes += diffBytes;
|
||||||
|
visibleTrackedDiffs.push(fileDiff.text);
|
||||||
}
|
}
|
||||||
|
|
||||||
trackedDiffText = visibleTrackedDiffs.join("");
|
trackedDiffText = visibleTrackedDiffs.join("");
|
||||||
appendDiff(trackedDiffText);
|
appendDiff(trackedDiffText);
|
||||||
if (trackedDiffTruncated) {
|
|
||||||
appendDiff("# tracked diff truncated\n");
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@@ -2256,7 +2209,6 @@ async function processTrackedChanges(
|
|||||||
trackedNumstatByPath,
|
trackedNumstatByPath,
|
||||||
trackedPlaceholderByPath,
|
trackedPlaceholderByPath,
|
||||||
trackedDiffText,
|
trackedDiffText,
|
||||||
trackedDiffTruncated,
|
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2361,7 +2313,6 @@ export async function getCheckoutDiff(
|
|||||||
trackedNumstatByPath: trackedDiff.trackedNumstatByPath,
|
trackedNumstatByPath: trackedDiff.trackedNumstatByPath,
|
||||||
trackedPlaceholderByPath: trackedDiff.trackedPlaceholderByPath,
|
trackedPlaceholderByPath: trackedDiff.trackedPlaceholderByPath,
|
||||||
trackedDiffText: trackedDiff.trackedDiffText,
|
trackedDiffText: trackedDiff.trackedDiffText,
|
||||||
trackedDiffTruncated: trackedDiff.trackedDiffTruncated,
|
|
||||||
refsForDiff: effectiveRefsForDiff,
|
refsForDiff: effectiveRefsForDiff,
|
||||||
ignoreWhitespace,
|
ignoreWhitespace,
|
||||||
structured,
|
structured,
|
||||||
|
|||||||
Reference in New Issue
Block a user