diff --git a/packages/git/src/diff.ts b/packages/git/src/diff.ts index b13a9f2..52177e1 100644 --- a/packages/git/src/diff.ts +++ b/packages/git/src/diff.ts @@ -1,7 +1,22 @@ import { exec, execLarge, execLines, execWithStdin } from './exec.js'; +/** + * Flags that neutralize user git config which would otherwise alter the diff + * format and break parsing: + * - `--no-color` guards against `color.ui=always` / `color.diff=always` + * - `--no-ext-diff` guards against a configured `diff.external` driver + * - `--src-prefix`/`--dst-prefix` force the standard `a/`/`b/` prefixes, + * overriding `diff.mnemonicPrefix`, `diff.noprefix` and custom prefixes + */ +const DIFF_FORMAT_ARGS = [ + '--no-color', + '--no-ext-diff', + '--src-prefix=a/', + '--dst-prefix=b/', +]; + export function getDiff(args: string[] = []): string { - const cmd = ['git', 'diff', ...args].join(' '); + const cmd = ['git', 'diff', ...DIFF_FORMAT_ARGS, ...args].join(' '); return execLarge(cmd); } @@ -14,7 +29,7 @@ export function getUntrackedDiff(files: string[]): string { for (const file of files) { try { - execLarge(`git diff --no-index -- /dev/null "${file}"`); + execLarge(`git diff ${DIFF_FORMAT_ARGS.join(' ')} --no-index -- /dev/null "${file}"`); } catch (err: unknown) { const error = err as { stdout?: string; status?: number }; if (error.status === 1 && error.stdout) { @@ -38,7 +53,11 @@ export function resolveDiffArgs(ref: string): RefDiffArgs { case 'work': return { type: 'args', args: ['HEAD'], includeUntracked: true }; default: - return { type: 'args', args: [normalizeRef(ref)], includeUntracked: true }; + // Bare refs (`diffity main`) diff against the working tree, so untracked + // files are part of the change set (#10). Ranges (`A..B`) pin both + // endpoints — the working tree isn't involved, so untracked files must + // be excluded or the diff won't match `git diff A..B`. + return { type: 'args', args: [normalizeRef(ref)], includeUntracked: !ref.includes('..') }; } } @@ -58,7 +77,7 @@ export function resolveRef(ref: string, extraArgs: string[] = []): string { export function getDiffFiles(ref: string): string[] { const resolved = resolveDiffArgs(ref); - const tracked = execLines(`git diff --name-only ${resolved.args.join(' ')}`.trim()); + const tracked = execLines(`git diff ${DIFF_FORMAT_ARGS.join(' ')} --name-only ${resolved.args.join(' ')}`.trim()); if (resolved.includeUntracked) { const untracked = getUntrackedFiles(); return [...new Set([...tracked, ...untracked])]; @@ -67,7 +86,7 @@ export function getDiffFiles(ref: string): string[] { } export function getDiffStat(args: string[] = []): string { - const cmd = ['git', 'diff', '--stat', ...args].join(' '); + const cmd = ['git', 'diff', ...DIFF_FORMAT_ARGS, '--stat', ...args].join(' '); try { return execLarge(cmd); } catch { diff --git a/packages/git/src/exec.ts b/packages/git/src/exec.ts index 623726e..68927ad 100644 --- a/packages/git/src/exec.ts +++ b/packages/git/src/exec.ts @@ -1,13 +1,24 @@ -import { execSync, type StdioOptions } from 'node:child_process'; +import { + execFileSync, + execSync, + type StdioOptions, +} from 'node:child_process'; const STDIO: StdioOptions = ['pipe', 'pipe', 'pipe']; +/** + * Node's default is 1 MB, which is smaller than a large repository's file + * listing: `git ls-files` in a ~29k-file monorepo emits over 2 MB and the child + * process dies with ENOBUFS. + */ +const MAX_BUFFER = 50 * 1024 * 1024; + export function execWithStdin(cmd: string, input: string): string { return execSync(cmd, { encoding: 'utf-8', stdio: STDIO, input, - maxBuffer: 50 * 1024 * 1024, + maxBuffer: MAX_BUFFER, }); } @@ -22,10 +33,23 @@ export function execLarge(cmd: string): string { return execSync(cmd, { encoding: 'utf-8', stdio: STDIO, - maxBuffer: 50 * 1024 * 1024, + maxBuffer: MAX_BUFFER, }); } +/** + * The argv form of `execLarge`, for commands whose arguments come from user + * input — a path with a space or a quote in it cannot be passed safely through + * a shell string. + */ +export function execFileLarge(command: string, args: string[]): string { + return execFileSync(command, args, { + encoding: 'utf-8', + stdio: STDIO, + maxBuffer: MAX_BUFFER, + }).trim(); +} + export function execLines(cmd: string): string[] { const output = exec(cmd); if (!output) { diff --git a/packages/git/src/tree.ts b/packages/git/src/tree.ts index b6a22ef..dec7055 100644 --- a/packages/git/src/tree.ts +++ b/packages/git/src/tree.ts @@ -1,7 +1,8 @@ -import { execFileSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; +import { execFileLarge } from './exec'; + export interface TreeEntry { type: 'blob' | 'tree'; path: string; @@ -11,19 +12,16 @@ export interface TreeEntry { function getWorkingTreeFiles(dirPath?: string): string[] { const pathArgs = dirPath ? [dirPath + '/'] : []; - const tracked = execFileSync('git', ['ls-files', ...pathArgs], { - encoding: 'utf-8', - }).trim(); + const tracked = execFileLarge('git', ['ls-files', ...pathArgs]); - const deleted = execFileSync('git', ['ls-files', '--deleted', ...pathArgs], { - encoding: 'utf-8', - }).trim(); + const deleted = execFileLarge('git', ['ls-files', '--deleted', ...pathArgs]); - const untracked = execFileSync( - 'git', - ['ls-files', '--others', '--exclude-standard', ...pathArgs], - { encoding: 'utf-8' }, - ).trim(); + const untracked = execFileLarge('git', [ + 'ls-files', + '--others', + '--exclude-standard', + ...pathArgs, + ]); const deletedSet = new Set(deleted ? deleted.split('\n') : []); const files = new Set(); @@ -71,30 +69,20 @@ export function getTreeEntries(_ref = 'HEAD', dirPath?: string): TreeEntry[] { } export function getTreeFingerprint(): string { - const tracked = execFileSync('git', ['ls-files'], { - encoding: 'utf-8', - }).trim(); + const tracked = execFileLarge('git', ['ls-files']); - const statOutput = execFileSync( - 'git', - ['status', '--porcelain', '-u'], - { encoding: 'utf-8' }, - ).trim(); + const statOutput = execFileLarge('git', ['status', '--porcelain', '-u']); return `${tracked.length}:${statOutput}`; } export function getWorkingTreeFileContent(filePath: string): string { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf-8', - }).trim(); + const root = execFileLarge('git', ['rev-parse', '--show-toplevel']); return readFileSync(join(root, filePath), 'utf-8'); } export function getWorkingTreeRawFile(filePath: string): { data: Buffer; fullPath: string } { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf-8', - }).trim(); + const root = execFileLarge('git', ['rev-parse', '--show-toplevel']); const fullPath = join(root, filePath); return { data: readFileSync(fullPath), fullPath }; } diff --git a/packages/git/tests/diff-format.test.ts b/packages/git/tests/diff-format.test.ts new file mode 100644 index 0000000..97617ff --- /dev/null +++ b/packages/git/tests/diff-format.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execSync } from 'node:child_process'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +let repoDir: string; +let origCwd: string; + +function git(cmd: string) { + execSync(`git ${cmd}`, { cwd: repoDir, stdio: 'pipe' }); +} + +function writeFile(name: string, content: string) { + writeFileSync(join(repoDir, name), content); +} + +beforeAll(() => { + origCwd = process.cwd(); + repoDir = mkdtempSync(join(tmpdir(), 'diffity-format-test-')); + + git('init -b main'); + git('config user.email "test@test.com"'); + git('config user.name "Test"'); + + // git config that alters the default diff format and previously broke parsing + git('config diff.mnemonicprefix true'); + git('config color.ui always'); + + writeFile('base.txt', 'a\nb\n'); + git('add .'); + git('commit -m "initial commit"'); + + process.chdir(repoDir); +}); + +afterAll(() => { + process.chdir(origCwd); + rmSync(repoDir, { recursive: true, force: true }); +}); + +describe('diff format neutralization', () => { + it('emits standard a/ b/ prefixes despite diff.mnemonicprefix', async () => { + const { resolveRef } = await import('../src/diff.js'); + writeFile('base.txt', 'a\nc\n'); + + const raw = resolveRef('unstaged'); + + expect(raw).toContain('diff --git a/base.txt b/base.txt'); + expect(raw).toContain('--- a/base.txt'); + expect(raw).toContain('+++ b/base.txt'); + // no ANSI color escapes even with color.ui=always + expect(raw).not.toMatch(/\x1b\[/); + + git('checkout -- base.txt'); + }); + + it('lists file names without color escapes despite color.ui', async () => { + const { getDiffFiles } = await import('../src/diff.js'); + writeFile('base.txt', 'a\nd\n'); + + const files = getDiffFiles('unstaged'); + + expect(files).toEqual(['base.txt']); + expect(files.join('')).not.toMatch(/\x1b\[/); + + git('checkout -- base.txt'); + }); + + it('emits a diffstat without color escapes despite color.ui', async () => { + const { getDiffStatForRef } = await import('../src/diff.js'); + writeFile('base.txt', 'a\ne\n'); + + const stat = getDiffStatForRef('unstaged'); + + expect(stat).toContain('base.txt'); + expect(stat).not.toMatch(/\x1b\[/); + + git('checkout -- base.txt'); + }); + + it('emits standard prefixes for untracked files', async () => { + const { resolveRef } = await import('../src/diff.js'); + writeFile('untracked.txt', 'x\ny\n'); + + const raw = resolveRef('work'); + + expect(raw).toContain('diff --git a/untracked.txt b/untracked.txt'); + expect(raw).toContain('+++ b/untracked.txt'); + expect(raw).not.toMatch(/\x1b\[/); + + execSync(`rm "${join(repoDir, 'untracked.txt')}"`, { stdio: 'pipe' }); + }); +}); diff --git a/packages/git/tests/get-diff-files.test.ts b/packages/git/tests/get-diff-files.test.ts index ec3684d..0b1ad05 100644 --- a/packages/git/tests/get-diff-files.test.ts +++ b/packages/git/tests/get-diff-files.test.ts @@ -86,6 +86,28 @@ describe('getDiffFiles', () => { git('checkout -- base.txt'); }); + it('includes untracked files for bare refs (diff against working tree)', async () => { + const { getDiffFiles } = await import('../src/diff.js'); + writeFile('untracked-file.txt', 'untracked\n'); + + const files = getDiffFiles('main'); + expect(files).toContain('untracked-file.txt'); + + execSync(`rm "${join(repoDir, 'untracked-file.txt')}"`, { stdio: 'pipe' }); + }); + + it('excludes untracked files for range refs (both endpoints pinned)', async () => { + const { getDiffFiles } = await import('../src/diff.js'); + writeFile('untracked-file.txt', 'untracked\n'); + + const files = getDiffFiles('main..feature'); + expect(files).toContain('feature.txt'); + expect(files).toContain('base.txt'); + expect(files).not.toContain('untracked-file.txt'); + + execSync(`rm "${join(repoDir, 'untracked-file.txt')}"`, { stdio: 'pipe' }); + }); + it('returns working tree files for work ref', async () => { const { getDiffFiles } = await import('../src/diff.js'); writeFile('untracked-file.txt', 'untracked\n'); diff --git a/packages/git/tests/get-tree-large-repo.test.ts b/packages/git/tests/get-tree-large-repo.test.ts new file mode 100644 index 0000000..6eca7b2 --- /dev/null +++ b/packages/git/tests/get-tree-large-repo.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +import { getTree, getTreeFingerprint } from '../src/tree'; + +/** + * Node's default maxBuffer is 1 MB. A repository whose file listing is larger + * than that used to kill `git ls-files` with ENOBUFS, which surfaced as + * "Failed to get tree" for every large repo. + */ +const DEFAULT_MAX_BUFFER = 1024 * 1024; + +let repoDir: string; +let origCwd: string; + +function git(cmd: string) { + // This fixture is deliberately larger than the default 1 MB buffer, and + // `git commit` names every file it creates — so the setup needs the same + // headroom the code under test does. + execSync(`git ${cmd}`, { + cwd: repoDir, + stdio: 'pipe', + maxBuffer: 50 * 1024 * 1024, + }); +} + +beforeAll(() => { + origCwd = process.cwd(); + repoDir = mkdtempSync(join(tmpdir(), 'diffity-large-tree-')); + + git('init -b main'); + git('config user.email "test@test.com"'); + git('config user.name "Test"'); + + // Enough path bytes to exceed the default buffer: names are padded so the + // listing crosses 1 MB without needing tens of thousands of files. + const padding = 'p'.repeat(180); + mkdirSync(join(repoDir, 'many')); + for (let i = 0; i < 6000; i += 1) { + writeFileSync(join(repoDir, 'many', `f${i}-${padding}.txt`), ''); + } + git('add .'); + git('commit -m "many files"'); + + process.chdir(repoDir); +}); + +afterAll(() => { + process.chdir(origCwd); + rmSync(repoDir, { recursive: true, force: true }); +}); + +describe('a repository whose listing exceeds the default buffer', () => { + it('lists every file rather than throwing ENOBUFS', () => { + const paths = getTree(); + + expect(paths).toHaveLength(6000); + expect(paths.join('\n').length).toBeGreaterThan(DEFAULT_MAX_BUFFER); + }); + + it('still fingerprints the tree', () => { + expect(getTreeFingerprint()).toMatch(/^\d+:/); + }); +});