Tighten sha256convert: scanner, recursion, dead field, fsck match, close, memoization · Entire

Tighten sha256convert: scanner, recursion, dead field, fsck match, close, memoization

d63f3ab→main·

nodo·1mo ago·2 files·+178 added/-47 removed

  1. fsckHasError used bufio.Scanner with the default 64 KiB buffer, so a single long line (paths in repository-scale fsck output) would silently truncate and we'd miss whatever followed it. Switch to bytes.Split on raw newlines.

  2. discoverReachable was recursive, which on long linear chains (kernel-scale: ~70k commits on the deepest single-parent path) grew the goroutine stack into the tens of MiB. Iterativize with an explicit work stack — memory now scales with the in-flight frontier, not the longest chain. translate() stays recursive: its edges are dynamic (tree entries + commit parents

  1. translator.dst was set in newTranslator but never read — a landmine inviting a future contributor to call go-git's SHA1-hardcoded SetEncodedObject. Field removed; the type assertion stays (with __) so a memory-backed dst is still refused with a clear error.

  2. fsckHasError's error-line matcher was prefix-only and case- sensitive ("error:" / "fatal:"). Broaden to any line whose first token starts with "error" or "fatal" (case-insensitive), keep the "missing "/"broken link"/"bad " object reports for older git. Closer to the previous substring match's coverage without the path-substring false positive.

  3. writeMappingFile dropped Close errors via a deferred Close. On NFS or quota-bound filesystems write failures surface at close time, not flush time, so the caller would think the mapping landed when it hadn't. Explicit Close on the success path, with the deferred Close kept as a best-effort net for the failure path.

  4. resolveMessageRef ran a full O(len(reachable)) scan for every short hash prefix, hit twice per token (once by extractMessageReferences, once by rewriteHashesInMessage). reachable is frozen before translation starts, so the (prefix → matchResult) mapping is stable; memoize it on the translator. Halves the documented quadratic ceiling on message-token resolution.

Tests cover the long-line + case-insensitive fsck path and the resolveMessageRef cache (second call returns the cached answer even after reachable is mutated).

Sessions

Transcript data is unavailable for this checkpoint.

Changes

2

664 unmodified lines

return checks

// fsckHasError reports whether git-fsck output contains a line that signals // a real problem (an "error:" or "fatal:" prefix, or a "missing"/"bad" // object report). Dangling and warning lines are ignored. // fsckHasError reports whether git-fsck output contains a line that // signals a real problem. We match (case-insensitively) any line whose // first token starts with "error" or "fatal" — covering "error:", // "fatal:", and the rare "errorInX:" variants — plus the // "missing " / "broken link" / "bad " object reports // emitted by older git. Dangling and warning lines are intentionally // ignored. // // Splits on raw newlines rather than using bufio.Scanner so a single // very long line (some fsck reports include long paths) is not // silently truncated at the scanner's 64 KiB default. func fsckHasError(out []byte) bool { scanner := bufio.NewScanner(bytes.NewReader(out)) for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if strings.HasPrefix(line, "error:") || strings.HasPrefix(line, "fatal:") { for _, raw := range bytes.Split(out, []byte("\n")) { line := strings.TrimSpace(string(raw)) if line == "" { continue } lower := strings.ToLower(line) if strings.HasPrefix(lower, "error") || strings.HasPrefix(lower, "fatal") { return true } if strings.HasPrefix(line, "missing ") || strings.HasPrefix(line, "broken link") || strings.HasPrefix(line, "bad ") { if strings.HasPrefix(lower, "missing ") || strings.HasPrefix(lower, "broken link") || strings.HasPrefix(lower, "bad ") { return true } } } } } return false }