Skip to content

Commit 2d8d2d3

Browse files
committed
fix: Add platform aware git repack with fallback strategy
1 parent 4de36f9 commit 2d8d2d3

File tree

1 file changed

+36
-33
lines changed

1 file changed

+36
-33
lines changed

src/services/checkpoints/ShadowCheckpointService.ts

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,13 @@ export abstract class ShadowCheckpointService extends EventEmitter {
2525
protected readonly dotGitDir: string
2626
protected git?: SimpleGit
2727
protected readonly log: (message: string) => void
28-
// --- ADDITION: Static logger for static methods ---
28+
// Static logger for static methods
2929
protected static log: (message: string) => void = console.log
3030

31-
// --- CHANGE START: Add cache for nested git repo paths ---
32-
// --- ADDITION: GC related properties ---
31+
// GC related properties
3332
private gcCounter: number = 0
3433
private readonly GC_CHECKPOINT_THRESHOLD: number = Number(process.env.GC_CHECKPOINT_THRESHOLD) || 20 // Run gc every 20 checkpoints
3534
private _nestedGitDirPaths: string[] | null = null // Cache for relative paths like "submodule/.git"
36-
// --- CHANGE END: Add cache for nested git repo paths ---
3735
protected shadowGitConfigWorktree?: string
3836

3937
public get baseHash() {
@@ -97,19 +95,10 @@ export abstract class ShadowCheckpointService extends EventEmitter {
9795

9896
// --- STAGE 1: Run GC on init for existing repo ---
9997
this.log(`[${this.constructor.name}#initShadowGit] Existing shadow repo found. Running garbage collection.`)
100-
try {
101-
const gcStartTime = Date.now()
102-
// Use the more thorough repack command
103-
this.log(`[${this.constructor.name}#initShadowGit] Running git repack -adf --path-walk --quiet...`)
104-
await git.raw(["repack", "-a", "-d", "-f", "--path-walk", "--quiet"])
105-
this.log(
106-
`[${this.constructor.name}#initShadowGit] Repository repack completed in ${Date.now() - gcStartTime}ms.`,
107-
)
108-
} catch (gcError) {
109-
this.log(
110-
`[${this.constructor.name}#initShadowGit] Repository repack failed: ${gcError instanceof Error ? gcError.message : String(gcError)}`,
111-
)
112-
}
98+
const gcStartTime = Date.now()
99+
100+
const isWindows = process.platform === 'win32'
101+
await this.tryRepack(git, isWindows, gcStartTime)
113102
} else {
114103
this.log(`[${this.constructor.name}#initShadowGit] creating shadow git repo at ${this.checkpointsDir}`)
115104
await git.init()
@@ -134,10 +123,9 @@ export abstract class ShadowCheckpointService extends EventEmitter {
134123

135124
await onInit?.()
136125

137-
// --- CHANGE START: Warm up the nested git paths cache ---
126+
// Warm up the nested git paths cache
138127
// This ensures the potentially slow scan happens once during initialization.
139128
await this.findAndCacheNestedGitRepoPaths()
140-
// --- CHANGE END: Warm up the nested git paths cache ---
141129

142130
this.emit("initialize", {
143131
type: "initialize",
@@ -161,7 +149,33 @@ export abstract class ShadowCheckpointService extends EventEmitter {
161149
await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n"))
162150
}
163151

164-
// --- CHANGE START: New method to find and cache nested .git directory paths ---
152+
private async tryRepack(git: SimpleGit, isWindows: boolean, gcStartTime: number) {
153+
// Define commands to try based on platform
154+
const commands = isWindows
155+
? [
156+
// Try the more thorough repack command on Git 2.49.0 for Windows
157+
{ args: ["repack", "-a", "-d", "-f", "--path-walk", "--quiet"], desc: "advanced repack with --path-walk" },
158+
{ args: ["repack", "-a", "-d", "-f", "--window=250", "--quiet"], desc: "repack with --window=250" }
159+
]
160+
: [
161+
{ args: ["repack", "-a", "-d", "-f", "--window=250", "--quiet"], desc: "repack with --window=250" }
162+
]
163+
164+
// Try each command until one succeeds
165+
for (const command of commands) {
166+
try {
167+
this.log(`[${this.constructor.name}#initShadowGit] Running ${command.desc}...`)
168+
await git.raw(command.args)
169+
this.log(`[${this.constructor.name}#initShadowGit] Repository repack completed in ${Date.now() - gcStartTime}ms.`)
170+
return // Success - exit early
171+
} catch (error) {
172+
this.log(`[${this.constructor.name}#initShadowGit] ${command.desc} failed: ${error instanceof Error ? error.message : String(error)}`)
173+
}
174+
}
175+
176+
this.log(`[${this.constructor.name}#initShadowGit] Repository repack failed, continuing without optimization.`)
177+
}
178+
165179
// This method scans for nested .git directories once and caches their paths
166180
// to avoid repeated expensive scans by ripgrep.
167181
private async findAndCacheNestedGitRepoPaths(): Promise<string[]> {
@@ -195,9 +209,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
195209
}
196210
return this._nestedGitDirPaths
197211
}
198-
// --- CHANGE END: New method to find and cache nested .git directory paths ---
199212

200-
// --- CHANGE START: Modify stageAll to be more performant ---
201213
// Instead of `git add .`, this uses `git status` to find specific changes
202214
// and then `git add <files>` and `git rm <files>`.
203215
private async stageAll(git: SimpleGit) {
@@ -255,10 +267,10 @@ export abstract class ShadowCheckpointService extends EventEmitter {
255267
await this.renameNestedGitRepos(false)
256268
}
257269
}
270+
258271
// Since we use git to track checkpoints, we need to temporarily disable
259272
// nested git repos to work around git's requirement of using submodules for
260273
// nested repos.
261-
262274
// This method now uses the pre-scanned and cached list of nested .git directories.
263275
private async renameNestedGitRepos(disable: boolean) {
264276
const nestedGitDirPaths = await this.findAndCacheNestedGitRepoPaths() // Uses cache after first scan
@@ -294,7 +306,6 @@ export abstract class ShadowCheckpointService extends EventEmitter {
294306
}
295307
}
296308
}
297-
// --- CHANGE END: Modify renameNestedGitRepos to use cached paths ---
298309

299310
private async getShadowGitConfigWorktree(git: SimpleGit) {
300311
if (!this.shadowGitConfigWorktree) {
@@ -381,20 +392,13 @@ export abstract class ShadowCheckpointService extends EventEmitter {
381392

382393
const start = Date.now()
383394

384-
// --- CHANGE START: Consider if renameNestedGitRepos is needed around clean/reset ---
385395
// If `git clean` or `git reset` could be affected by nested .git repos
386396
// (e.g., if they try to operate on them as submodules despite core.worktree),
387397
// you might need to wrap this section with renameNestedGitRepos(true/false).
388398
// However, `core.worktree` usually makes Git operate on the workspace files directly.
389399
// For now, assuming it's not strictly needed here for performance, but it's a thought.
390-
// await this.renameNestedGitRepos(true);
391-
// try {
392400
await this.git.clean("f", ["-d", "-f"])
393401
await this.git.reset(["--hard", commitHash])
394-
// } finally {
395-
// await this.renameNestedGitRepos(false);
396-
// }
397-
// --- CHANGE END: Consider if renameNestedGitRepos is needed around clean/reset ---
398402

399403
// Remove all checkpoints after the specified commitHash.
400404
const checkpointIndex = this._checkpoints.indexOf(commitHash)
@@ -532,7 +536,6 @@ export abstract class ShadowCheckpointService extends EventEmitter {
532536
}
533537
return result
534538
}
535-
// --- CHANGE END: Modify getDiff for performance and accuracy ---
536539

537540
/**
538541
* EventEmitter
@@ -693,4 +696,4 @@ export abstract class ShadowCheckpointService extends EventEmitter {
693696
return true
694697
}
695698
}
696-
}
699+
}

0 commit comments

Comments
 (0)