fix(server): checkpoints no longer rehash every file when one entry is skip-worktree or assume-unchanged#23624

open4
main
fix/issue-13741
opened2d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 2d ago
1 unresolved thread Β· Jump to file
0 / 2 viewed2 files changedLine totals unavailable

apps/server/src/vcs/GitVcsDriver.ts Β· Earlier revision 01daac62 Β· right 975

Original diff
@@ -898,57 +899,86 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
             });
             // read-tree can rewrite the index, so restore its racy timestamp afterward.
             yield* fileSystem.utimes(tempIndexPath, indexTime, indexTime);
-            let specialFlags = false;
+            // Manually flagged entries hide edits from add -A, so they are re-added below
+            // with no stat cache. Every other entry keeps its stat data and is not rehashed.
+            let fallback = false;
+            let flaggedBytes = 0;
             let recordStart = true;
-            let skipped = false;
-            let skippedRecord: number[] = [];
-            const skippedPaths: string[] = [];
+            let collect: "flagged" | "skipped" | undefined;
+            let record: number[] = [];
+            const flagged: string[] = [];
+            const skipped: string[] = [];
             yield* vcsProcess.run({
               operation,
               command: "git",
               cwd: input.cwd,
-              args: [...indexConfig, "ls-files", "--full-name", "--sparse", "-v", "-z"],
+              args: [...indexConfig, "ls-files", "--full-name", "--sparse", "--stage", "-v", "-z"],
               env: commitEnv,
               maxOutputBytes: 4_096,
               outputMode: "truncate",
               onStdoutChunk: (chunk) => {
                 for (const byte of chunk) {
-                  if (recordStart) skipped = byte === 83;
-                  if (skipped && sparseCheckout) {
-                    if (byte !== 0) skippedRecord.push(byte);
-                    else {
-                      if (skippedRecord.at(-1) !== 47) {
-                        const name = Buffer.from(skippedRecord).subarray(2);
-                        if (!NodeBuffer.isUtf8(name)) specialFlags = true;
-                        else skippedPaths.push(name.toString("utf8"));
-                      }
-                      skippedRecord = [];
-                    }
-                  }
-                  if (
-                    recordStart &&
-                    ((byte >= 97 && byte <= 122) || (!sparseCheckout && byte === 83))
-                  ) {
-                    specialFlags = true;
+                  if (recordStart) {
+                    record = [];
+                    // Sparse skip-worktree bits are expected; only in-cone ones are manual.
+                    collect =
+                      byte === 83 && sparseCheckout
+                        ? "skipped"
+                        : (byte >= 97 && byte <= 122) || byte === 83
+                          ? "flagged"
+                          : undefined;
+                    // index-info cannot keep skip-worktree while dropping assume-unchanged.
+                    if (byte === 115 && sparseCheckout) fallback = true;
                   }
                   recordStart = byte === 0;
+                  if (fallback || collect === undefined) continue;
+                  if (byte !== 0) {
+                    record.push(byte);
+                    if (collect === "flagged" && ++flaggedBytes > FLAGGED_INDEX_ENTRIES_MAX_BYTES)
+                      fallback = true;
+                    continue;
+                  }
+                  // Sparse directory entries stay collapsed.
+                  if (collect === "skipped" && record.at(-1) === 47) continue;
+                  // Drop the "<tag> " prefix, leaving index-info's "<mode> <oid> <stage>\t<path>".
+                  const entry = Buffer.from(record).subarray(2);
+                  if (!NodeBuffer.isUtf8(entry)) fallback = true;
+                  else (collect === "flagged" ? flagged : skipped).push(entry.toString("utf8"));
                 }
               },
             });
-            if (skippedPaths.length > 0 && !specialFlags) {
-              const selected = yield* execute({
+            if (fallback) return false;
+            if (skipped.length > 0) {
+              const paths = skipped.map((entry) => entry.slice(entry.indexOf("\t") + 1));
+              const stdin = paths.join("\0") + "\0";
+              // check-rules echoes a subset of its input, so the input size bounds its output.
+              const rules = yield* execute({
                 operation,
                 cwd: input.cwd,
                 args: [...indexConfig, "sparse-checkout", "check-rules", "-z"],
-                stdin: skippedPaths.join("\0") + "\0",
+                stdin,
+                env: commitEnv,
+                maxOutputBytes: Buffer.byteLength(stdin),
+                outputMode: "error",
+              });
+              const selected = new Set(rules.stdout.split("\0"));
+              skipped.forEach((entry, index) => {
+                if (selected.has(paths[index]!)) flagged.push(entry);
+              });
+            }
+            if (flagged.length > 0) {
+              // Flagged gitlinks keep the fresh-index behaviour rather than a guessed one.
+              if (flagged.some((entry) => entry.startsWith("160000 "))) return false;
+              yield* execute({
+                operation,
+                cwd: input.cwd,
+                args: [...indexConfig, "update-index", "-z", "--index-info"],
M
macroscopeappBot2d ago

🟠 High vcs/GitVcsDriver.ts:975

With repository-local core.ignorestat=true, this call re-adds flagged entries with CE_VALID, so the later git add -A skips their edits and the checkpoint contains stale HEAD contents. update-index --index-info sets this bit from core.ignorestat; override that setting for this call.

No longer relevant as of cfee52fc24f56169ed4c0ce48759ad1ff9a7f655

J

Confirmed with git 2.43: under core.ignorestat=true, update-index --index-info writes the re-added entry as h, and add -A then keeps HEAD's blob. Fixed in cfee52fc24 by passing -c core.ignorestat=false to that private-index write. The flagged modes of checkpoint index inspection handles entries beyond the output cap now set core.ignorestat true; they fail without the fix (expected 'original\n' to equal 'modified\n') and pass with it.