mirror of
https://github.com/stablyai/orca.git
synced 2026-10-02 00:02:05 +00:00
perf(git): normalize tracked discard paths once per operation
This commit is contained in:
@@ -7,7 +7,8 @@ import type { GitRuntimeOptions } from '../git-runtime-options'
|
||||
import { gitOptionsForWorktree } from '../git-runtime-options'
|
||||
import { gitExecFileAsync } from '../runner'
|
||||
import { invalidateGitReadCaches } from './git-read-cache-invalidation'
|
||||
import { bulkPathspecCommands, isTrackedPathSpec, literalPathspec } from './git-pathspec'
|
||||
import { partitionTrackedPathSpecs } from '../../../shared/git-tracked-pathspecs'
|
||||
import { bulkPathspecCommands, literalPathspec } from './git-pathspec'
|
||||
|
||||
/**
|
||||
* Discard working tree changes for a file.
|
||||
@@ -117,14 +118,7 @@ export async function bulkDiscardChanges(
|
||||
}
|
||||
|
||||
const trackedPathSpecs = await listTrackedPathSpecs(worktreePath, filePaths, options)
|
||||
const trackedPaths: string[] = []
|
||||
const untrackedPaths: string[] = []
|
||||
filePaths.forEach((filePath) => {
|
||||
const targetPaths = isTrackedPathSpec(filePath, trackedPathSpecs)
|
||||
? trackedPaths
|
||||
: untrackedPaths
|
||||
targetPaths.push(filePath)
|
||||
})
|
||||
const { trackedPaths, untrackedPaths } = partitionTrackedPathSpecs(filePaths, trackedPathSpecs)
|
||||
await removeSafeUntrackedDiscardTargets(
|
||||
worktreePath,
|
||||
untrackedPaths,
|
||||
|
||||
@@ -16,9 +16,7 @@ const BULK_CHUNK_SIZE = 100
|
||||
*/
|
||||
const POSIX_COMMAND_LINE_BUDGET = 128_000
|
||||
|
||||
function normalizeGitPathForCompare(filePath: string): string {
|
||||
return filePath.replace(/\\/g, '/').replace(/\/+$/, '')
|
||||
}
|
||||
export { isTrackedPathSpec } from '../../../shared/git-tracked-pathspecs'
|
||||
|
||||
export function literalPathspec(filePath: string, options: GitRuntimeOptions): string {
|
||||
// Why: Git inside WSL needs POSIX paths, but host paths must stay literal, so convert backslashes only for WSL.
|
||||
@@ -26,14 +24,6 @@ export function literalPathspec(filePath: string, options: GitRuntimeOptions): s
|
||||
return `:(literal)${runtimePath}`
|
||||
}
|
||||
|
||||
export function isTrackedPathSpec(filePath: string, trackedPaths: readonly string[]): boolean {
|
||||
const normalized = normalizeGitPathForCompare(filePath)
|
||||
return trackedPaths.some((trackedPath) => {
|
||||
const normalizedTracked = normalizeGitPathForCompare(trackedPath)
|
||||
return normalizedTracked === normalized || normalizedTracked.startsWith(`${normalized}/`)
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Length of the line the OS will actually be handed, wrapper included.
|
||||
*
|
||||
|
||||
@@ -222,6 +222,31 @@ describe('bulk git helpers', () => {
|
||||
expect(rmMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves bulk discard action selection and original path order for path edges', async () => {
|
||||
const filePaths = ['new', 'docs\\', '[ab].txt', 'docs///', 'new', 'src/file', 'docs\\']
|
||||
gitExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'docs/readme\0src/file-extra\0[ab].txt\0' })
|
||||
.mockResolvedValue({ stdout: '' })
|
||||
|
||||
await bulkDiscardChanges('/repo', filePaths)
|
||||
|
||||
expect(gitExecFileAsyncMock.mock.calls.map(([args]) => args)).toEqual([
|
||||
['ls-files', '-z', '--', ...filePaths.map((filePath) => `:(literal)${filePath}`)],
|
||||
[
|
||||
'restore',
|
||||
'--worktree',
|
||||
'--source=HEAD',
|
||||
'--',
|
||||
':(literal)docs\\',
|
||||
':(literal)[ab].txt',
|
||||
':(literal)docs///',
|
||||
':(literal)docs\\'
|
||||
],
|
||||
['clean', '-ffdx', '--', ':(literal)new', ':(literal)new', ':(literal)src/file']
|
||||
])
|
||||
expect(rmMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('handles large tracked path lists during bulk discard classification', async () => {
|
||||
const trackedStdout = Array.from({ length: 150_000 }, (_, index) => `docs/file-${index}.ts`)
|
||||
.join('\0')
|
||||
|
||||
@@ -4,23 +4,12 @@ import {
|
||||
removeSafeUntrackedDiscardTarget,
|
||||
removeSafeUntrackedDiscardTargets
|
||||
} from '../shared/git-discard-path-safety'
|
||||
import { partitionTrackedPathSpecs } from '../shared/git-tracked-pathspecs'
|
||||
import { detectConflictOperation } from './git-handler-status-ops'
|
||||
|
||||
const BULK_CHUNK_SIZE = GIT_BULK_CHUNK_SIZE
|
||||
|
||||
export class GitHandlerDiscardOperations extends GitHandlerOperationContext {
|
||||
private normalizeGitPathForCompare(filePath: string): string {
|
||||
return filePath.replace(/\\/g, '/').replace(/\/+$/, '')
|
||||
}
|
||||
|
||||
private isTrackedPathSpec(filePath: string, trackedPaths: readonly string[]): boolean {
|
||||
const normalized = this.normalizeGitPathForCompare(filePath)
|
||||
return trackedPaths.some((trackedPath) => {
|
||||
const normalizedTracked = this.normalizeGitPathForCompare(trackedPath)
|
||||
return normalizedTracked === normalized || normalizedTracked.startsWith(`${normalized}/`)
|
||||
})
|
||||
}
|
||||
|
||||
private assertInWorktree(worktreePath: string, filePath: string): string {
|
||||
const resolved = path.resolve(worktreePath, filePath)
|
||||
const rel = path.relative(path.resolve(worktreePath), resolved)
|
||||
@@ -98,11 +87,9 @@ export class GitHandlerDiscardOperations extends GitHandlerOperationContext {
|
||||
}
|
||||
}
|
||||
|
||||
const trackedPaths = filePaths.filter((filePath) =>
|
||||
this.isTrackedPathSpec(filePath, trackedPathSpecs)
|
||||
)
|
||||
const untrackedPaths = filePaths.filter(
|
||||
(filePath) => !this.isTrackedPathSpec(filePath, trackedPathSpecs)
|
||||
const { trackedPaths, untrackedPaths } = partitionTrackedPathSpecs(
|
||||
filePaths,
|
||||
trackedPathSpecs
|
||||
)
|
||||
await removeSafeUntrackedDiscardTargets(
|
||||
worktreePath,
|
||||
|
||||
@@ -303,6 +303,36 @@ describe('GitHandler', () => {
|
||||
await expect(fs.access(path.join(tmpDir, 'new.txt'))).rejects.toThrow()
|
||||
})
|
||||
|
||||
it('preserves bulk discard action selection and original path order for path edges', async () => {
|
||||
const filePaths = ['new', 'docs\\', '[ab].txt', 'docs///', 'new', 'src/file', 'docs\\']
|
||||
const gitMock = vi
|
||||
.spyOn(
|
||||
handler as unknown as {
|
||||
git: (args: string[], cwd: string) => Promise<{ stdout: string; stderr: string }>
|
||||
},
|
||||
'git'
|
||||
)
|
||||
.mockResolvedValueOnce({ stdout: 'docs/readme\0src/file-extra\0[ab].txt\0', stderr: '' })
|
||||
.mockResolvedValue({ stdout: '', stderr: '' })
|
||||
|
||||
await dispatcher.callRequest('git.bulkDiscard', { worktreePath: tmpDir, filePaths })
|
||||
|
||||
expect(gitMock.mock.calls.map(([args]) => args)).toEqual([
|
||||
['ls-files', '-z', '--', ...filePaths.map((filePath) => `:(literal)${filePath}`)],
|
||||
[
|
||||
'restore',
|
||||
'--worktree',
|
||||
'--source=HEAD',
|
||||
'--',
|
||||
':(literal)docs\\',
|
||||
':(literal)[ab].txt',
|
||||
':(literal)docs///',
|
||||
':(literal)docs\\'
|
||||
],
|
||||
['clean', '-ffdx', '--', ':(literal)new', ':(literal)new', ':(literal)src/file']
|
||||
])
|
||||
})
|
||||
|
||||
it('handles large tracked path lists during bulk discard classification', async () => {
|
||||
const trackedStdout = Array.from({ length: 150_000 }, (_, index) => `docs/file-${index}.ts`)
|
||||
.join('\0')
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { isTrackedPathSpec, partitionTrackedPathSpecs } from './git-tracked-pathspecs'
|
||||
|
||||
function previousPartition(filePaths: readonly string[], trackedPaths: readonly string[]) {
|
||||
const normalize = (value: string) => value.replace(/\\/g, '/').replace(/\/+$/, '')
|
||||
const isTracked = (filePath: string) => {
|
||||
const normalized = normalize(filePath)
|
||||
return trackedPaths.some((trackedPath) => {
|
||||
const normalizedTracked = normalize(trackedPath)
|
||||
return normalizedTracked === normalized || normalizedTracked.startsWith(`${normalized}/`)
|
||||
})
|
||||
}
|
||||
return {
|
||||
trackedPaths: filePaths.filter(isTracked),
|
||||
untrackedPaths: filePaths.filter((filePath) => !isTracked(filePath))
|
||||
}
|
||||
}
|
||||
|
||||
function countNormalizations(run: () => unknown): number {
|
||||
const replace = vi.spyOn(String.prototype, 'replace')
|
||||
try {
|
||||
run()
|
||||
return replace.mock.calls.filter(
|
||||
([pattern]) => pattern instanceof RegExp && pattern.source === '\\\\'
|
||||
).length
|
||||
} finally {
|
||||
replace.mockRestore()
|
||||
}
|
||||
}
|
||||
|
||||
describe('tracked pathspec partition', () => {
|
||||
it.each([
|
||||
['docs', ['docs/readme.md'], true],
|
||||
['doc', ['docs/readme.md'], false],
|
||||
['docs/file', ['docs/file-extra'], false],
|
||||
['docs///', ['docs\\readme.md'], true],
|
||||
['src\\file.ts\\', ['src/file.ts///'], true],
|
||||
['DOCS', ['docs/readme.md'], false],
|
||||
['./docs', ['docs/readme.md'], false],
|
||||
['docs//file', ['docs/file'], false],
|
||||
['docs/../file', ['file'], false],
|
||||
['[ab].txt', ['a.txt'], false],
|
||||
['[ab].txt', ['[ab].txt'], true],
|
||||
[':(glob)*', ['a.txt'], false],
|
||||
['a b/é', ['a b/é/file\nname'], true],
|
||||
['é', ['e\u0301'], false],
|
||||
['C:\\repo\\docs', ['C:/repo/docs/file'], true],
|
||||
['\\\\host\\share', ['//host/share/file'], true],
|
||||
['', [], false],
|
||||
['', ['relative'], false],
|
||||
['', ['/absolute'], true],
|
||||
['/', [''], true]
|
||||
] as const)('keeps matching semantics for %j against %j', (request, tracked, expected) => {
|
||||
expect(isTrackedPathSpec(request, tracked)).toBe(expected)
|
||||
expect(partitionTrackedPathSpecs([request], tracked)).toEqual(
|
||||
previousPartition([request], tracked)
|
||||
)
|
||||
})
|
||||
|
||||
it('preserves original spelling, duplicates and relative order in both action lists', () => {
|
||||
const requests = ['new', 'docs\\', '[ab].txt', 'docs///', 'new', 'src/file', 'docs\\']
|
||||
const tracked = ['docs/readme', 'src/file-extra', '[ab].txt', 'docs/readme']
|
||||
expect(partitionTrackedPathSpecs(requests, tracked)).toEqual({
|
||||
trackedPaths: ['docs\\', '[ab].txt', 'docs///', 'docs\\'],
|
||||
untrackedPaths: ['new', 'new', 'src/file']
|
||||
})
|
||||
})
|
||||
|
||||
it('matches the previous implementation across combinations of path edges', () => {
|
||||
const paths = [
|
||||
'',
|
||||
'/',
|
||||
'.',
|
||||
'./a',
|
||||
'a',
|
||||
'a/',
|
||||
'a//',
|
||||
'a/b',
|
||||
'a\\b',
|
||||
'a//b',
|
||||
'ab',
|
||||
'A',
|
||||
'../a',
|
||||
'a/../b',
|
||||
'[a]',
|
||||
'*',
|
||||
'a b',
|
||||
'é',
|
||||
'e\u0301',
|
||||
'/a',
|
||||
'//host/share',
|
||||
'C:\\a'
|
||||
]
|
||||
for (const tracked of [[], paths, ...paths.map((entry) => [entry])]) {
|
||||
expect(partitionTrackedPathSpecs(paths, tracked)).toEqual(previousPartition(paths, tracked))
|
||||
}
|
||||
})
|
||||
|
||||
it('normalizes each visited tracked entry once and each request once per operation', () => {
|
||||
const requests = Array.from({ length: 64 }, (_, index) => `missing/${index}`)
|
||||
const tracked = Array.from({ length: 256 }, (_, index) => `docs\\file-${index}///`)
|
||||
expect(countNormalizations(() => previousPartition(requests, tracked))).toBe(32_896)
|
||||
expect(countNormalizations(() => partitionTrackedPathSpecs(requests, tracked))).toBe(320)
|
||||
expect(countNormalizations(() => partitionTrackedPathSpecs(requests, tracked))).toBe(320)
|
||||
})
|
||||
|
||||
it('retains early exit for a selected directory with many tracked descendants', () => {
|
||||
const tracked = Array.from({ length: 150_000 }, (_, index) => `docs/file-${index}`)
|
||||
expect(countNormalizations(() => previousPartition(['docs'], tracked))).toBe(4)
|
||||
expect(countNormalizations(() => partitionTrackedPathSpecs(['docs'], tracked))).toBe(2)
|
||||
expect(countNormalizations(() => partitionTrackedPathSpecs([], tracked))).toBe(0)
|
||||
})
|
||||
|
||||
it('does not reuse tracked evidence across operations', () => {
|
||||
expect(partitionTrackedPathSpecs(['docs'], ['docs/file']).trackedPaths).toEqual(['docs'])
|
||||
expect(partitionTrackedPathSpecs(['docs'], []).untrackedPaths).toEqual(['docs'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,37 @@
|
||||
function normalizeGitPathForCompare(filePath: string): string {
|
||||
return filePath.replace(/\\/g, '/').replace(/\/+$/, '')
|
||||
}
|
||||
|
||||
function createTrackedPathSpecMatcher(
|
||||
trackedPaths: readonly string[]
|
||||
): (filePath: string) => boolean {
|
||||
// Normalize lazily so selecting a directory still stops at its first tracked descendant.
|
||||
const normalizedTrackedPaths: string[] = []
|
||||
return (filePath) => {
|
||||
const normalized = normalizeGitPathForCompare(filePath)
|
||||
const descendantPrefix = `${normalized}/`
|
||||
return trackedPaths.some((trackedPath, index) => {
|
||||
const normalizedTracked = (normalizedTrackedPaths[index] ??=
|
||||
normalizeGitPathForCompare(trackedPath))
|
||||
return normalizedTracked === normalized || normalizedTracked.startsWith(descendantPrefix)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
export function isTrackedPathSpec(filePath: string, trackedPaths: readonly string[]): boolean {
|
||||
return createTrackedPathSpecMatcher(trackedPaths)(filePath)
|
||||
}
|
||||
|
||||
export function partitionTrackedPathSpecs(
|
||||
filePaths: readonly string[],
|
||||
trackedPathSpecs: readonly string[]
|
||||
): { trackedPaths: string[]; untrackedPaths: string[] } {
|
||||
const isTracked = createTrackedPathSpecMatcher(trackedPathSpecs)
|
||||
const trackedPaths: string[] = []
|
||||
const untrackedPaths: string[] = []
|
||||
// Keep original spellings, duplicates and order: these arrays select restore versus clean.
|
||||
for (const filePath of filePaths) {
|
||||
;(isTracked(filePath) ? trackedPaths : untrackedPaths).push(filePath)
|
||||
}
|
||||
return { trackedPaths, untrackedPaths }
|
||||
}
|
||||
Reference in New Issue
Block a user