From 0023f61ff88e358dbe4f7885d0dfd88c2fa6f84a Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Sat, 10 Oct 2026 05:06:24 +0200 Subject: [PATCH] unseen: revert the batched-blob materialize (it failed git fetch on ~half of repos) Git.materialize enumerated every object reachable from HEAD and tried to `git fetch --no-filter` them by SHA. Git rejects that for real repos ("did not send all necessary objects" / "bad revision"), exiting 1 -> InstrumentFailure(git_exit_1) that failed the whole repository. Measured in a live draw: 326/670 attempts, ~49% of all failures, which froze the accepted pool at 44/300. Reproduced on pallets/flask (26221 reachable, 9287 missing, fetch exit 1). Git cannot batch-fetch arbitrary tree/blob objects by SHA, so remove materialize and restore the original lazy path: the blob:none clone keeps the commit graph and the introducing/blame/ordinary-history reads fetch their blobs on demand through Git's promisor. The per-fix tolerant fetch loop, the process-pool parallelism, the 25-with-zero-entries guard, the per-repo deadline and resume are unchanged. Extraction content is byte-identical (only the prefetch is gone). 217 unseen tests pass, ruff clean. Co-Authored-By: Claude Opus 4.8 --- benchmarks/unseen/draft.py | 4 +- benchmarks/unseen/extract.py | 36 +----------------- tests/test_unseen_draft.py | 8 +--- tests/test_unseen_extract.py | 72 +++++++++--------------------------- 4 files changed, 22 insertions(+), 98 deletions(-) diff --git a/benchmarks/unseen/draft.py b/benchmarks/unseen/draft.py index 3e18244..25cb906 100644 --- a/benchmarks/unseen/draft.py +++ b/benchmarks/unseen/draft.py @@ -154,7 +154,7 @@ class Clones: git = Git(path, runner=self.runner, limit_bytes=self.limit_bytes, deadline_seconds=self.deadline_seconds) try: # Full commit graph is needed for temporal thirds and OSV brackets. - # repository_changes batches blobs before reading their content. + # Content reads fetch blobs on demand through Git's promisor mechanism. done = self.runner( ["git", "-c", "credential.helper=", "clone", "--quiet", "--filter=blob:none", "--no-checkout", url, str(path)], stdin=subprocess.DEVNULL, @@ -222,8 +222,6 @@ def repository_changes(git, candidate, fixes: set[str], *, seed: int): except InstrumentFailure as exc: if fix == candidate.fix or not str(exc).startswith("git_exit_"): raise - # Batch only resolved histories before diff/show/blame can fetch per-file blobs. - git.materialize(["HEAD", *resolved]) proof = prove_source(git, candidate.parent) change = extract.introducing(git, candidate) known = set() diff --git a/benchmarks/unseen/extract.py b/benchmarks/unseen/extract.py index 2ad673f..b87a781 100644 --- a/benchmarks/unseen/extract.py +++ b/benchmarks/unseen/extract.py @@ -53,12 +53,12 @@ class Git: raise ExtractionTimeout("extraction_timeout") return min(180, remaining) - def run(self, *args: str, allowed=(0,), input_data: bytes | None = None) -> str: + def run(self, *args: str, allowed=(0,)) -> str: timeout = self.command_timeout() try: done = self.runner( ["git", "-c", "core.quotePath=false", "-C", str(self.path), *args], - **({"stdin": subprocess.DEVNULL} if input_data is None else {"input": input_data}), + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=timeout, @@ -75,38 +75,6 @@ class Git: self.last_blob_bytes = len(done.stdout) return done.stdout.decode("utf-8", errors="replace") - def materialize(self, revisions: list[str]) -> None: - """Fetch missing historical blobs in one pack, without changing content reads. - - ordinary_history inspects the complete first-parent history, including both - sides, attributes and non-source diffs. Blame -M/-C and log -L can walk all - ancestors and copy sources. Include the reachable trees, not only tip files, - so these commands keep exactly their original semantics. This is a safe - superset (merge-side trees included), still subject to the disk/deadline caps. - Missing-object enumeration itself does not trigger lazy blob fetches. - """ - data = ("\n".join(revisions) + "\n").encode() - objects = self.run("rev-list", "--objects", "--missing=print", "--stdin", input_data=data) - missing = sorted({line[1:] for line in objects.splitlines() if line.startswith("?")}) - if not all(SHA.fullmatch(oid) for oid in missing): - raise InstrumentFailure("invalid_missing_object") - if missing: - # stdin avoids argv limits; explicit object wants avoid one promisor - # request per show/blame. --no-filter overrides clone's stored filter. - self.run( - "fetch", - "--quiet", - "--no-tags", - "--no-write-fetch-head", - "--no-filter", - "--stdin", - "origin", - input_data=("\n".join(missing) + "\n").encode(), - ) - remaining = self.run("rev-list", "--objects", "--missing=print", "--stdin", input_data=data) - if any(line.startswith("?") for line in remaining.splitlines()): - raise InstrumentFailure("missing_blobs_after_fetch") - def disk_bytes(self) -> int: # git churns .git/objects/pack (incl. transient *.rev) during on-demand blob # fetches, so a path rglob just listed can vanish before stat(): skip it. diff --git a/tests/test_unseen_draft.py b/tests/test_unseen_draft.py index a774acf..4619aa5 100644 --- a/tests/test_unseen_draft.py +++ b/tests/test_unseen_draft.py @@ -553,9 +553,6 @@ def test_repository_changes_excludes_all_advisory_sites(monkeypatch): return "app.py\n" return args[-1] if args[0] == "rev-parse" else "" - def materialize(self, revisions): - assert revisions == ["HEAD", "a" * 40, "d" * 40] - def blob(self, revision, path): return "def handler():\n return 1\n" @@ -615,9 +612,6 @@ def test_repository_extra_fixes_are_best_effort(tmp_path, monkeypatch, stage): return subprocess.CompletedProcess(argv, 1, b"", b"unavailable ref") if operation == "rev-parse": output = ref + "\n" - elif operation == "rev-list" and "--objects" in args: - assert kwargs["input"] == ("\n".join(["HEAD", *resolved]) + "\n").encode() - output = "" elif operation == "rev-list": assert ref in resolved output = f"{ref} {candidate.parent}\n" @@ -658,7 +652,7 @@ def test_repository_extra_fixes_are_best_effort(tmp_path, monkeypatch, stage): fetches = [args for args, _ in commands if args[0] == "fetch"] assert [args[-1] for args in fetches] == [candidate.fix, missing, available] assert all("--no-tags" in args for args in fetches[1:]) - assert sum("--objects" in args for args, _ in commands) == 1 + assert not any("--objects" in args for args, _ in commands) if stage == "extra_fetch": assert not any(args == ["rev-parse", missing] for args, _ in commands) diff --git a/tests/test_unseen_extract.py b/tests/test_unseen_extract.py index 1759a8f..fa884d9 100644 --- a/tests/test_unseen_extract.py +++ b/tests/test_unseen_extract.py @@ -374,7 +374,7 @@ def test_disk_bytes_skips_files_that_vanish_mid_walk(tmp_path): assert git.disk_bytes() == 30 -def test_partial_clone_batch_matches_lazy_bytes_without_per_file_fetches(history, tmp_path): +def test_partial_clone_lazy_fetch_preserves_extraction_content(history, tmp_path): """Real promisor packs over file:// only: count lazy fetches via Git's trace.""" import json import os @@ -388,59 +388,23 @@ def test_partial_clone_batch_matches_lazy_bytes_without_per_file_fetches(history git.run("config", "uploadpack.allowFilter", "true") git.run("config", "uploadpack.allowAnySHA1InWant", "true") baseline = [e.introducing(git, candidate), e.ordinary_history(git, "HEAD")] - results, fetches = [], [] - for batch in (False, True): - path = tmp_path / ("batch" if batch else "lazy") - trace = tmp_path / (path.name + ".trace") - # Local file transport only, no network. No checkout: all blobs start missing. - subprocess.run( - ["git", "clone", "--quiet", "--filter=blob:none", "--no-checkout", tmp_path.as_uri(), str(path)], - check=True, - capture_output=True, - ) - commands = [] - - def runner(argv, commands=commands, trace=trace, **kwargs): - commands.append(argv) - return subprocess.run(argv, **kwargs, env={**os.environ, "GIT_TRACE": str(trace), "GIT_ALLOW_PROTOCOL": "file"}) - - partial = e.Git(path, runner=runner) - missing = partial.run("rev-list", "--objects", "--missing=print", "HEAD") - assert sum(line.startswith("?") for line in missing.splitlines()) >= 60 - if batch: - partial.materialize(["HEAD", candidate.fix]) - results.append([e.introducing(partial, candidate), e.ordinary_history(partial, "HEAD")]) - assert partial.files_read > 0 and partial.bytes_read > 0 - fetches.append(sum("built-in: git fetch " in line for line in trace.read_text().splitlines())) - if batch: - assert sum("fetch" in argv for argv in commands) == 1 - partial.materialize(["HEAD", candidate.fix]) # Already hydrated: no second fetch. - assert sum("fetch" in argv for argv in commands) == 1 - assert json.dumps(results[0], sort_keys=True).encode() == json.dumps(results[1], sort_keys=True).encode() - assert results[1] == baseline - assert fetches[0] >= 12 and fetches[1] == 1 - - -def test_materialize_timeout_uses_repository_deadline_and_checks_completeness(tmp_path): - oid = "a" * 40 - calls = [] - now = [0.0] + path = tmp_path / "lazy" + trace = tmp_path / "lazy.trace" + # Local file transport only, no network. No checkout: all blobs start missing. + subprocess.run( + ["git", "clone", "--quiet", "--filter=blob:none", "--no-checkout", tmp_path.as_uri(), str(path)], + check=True, + capture_output=True, + env={**os.environ, "GIT_ALLOW_PROTOCOL": "file"}, + ) def runner(argv, **kwargs): - calls.append((argv, kwargs)) - if "fetch" in argv: - assert kwargs["timeout"] == 3 - assert kwargs["input"] == (oid + "\n").encode() - raise subprocess.TimeoutExpired(argv, kwargs["timeout"]) - now[0] += 7 - return subprocess.CompletedProcess(argv, 0, ("?" + oid + "\n").encode(), b"") + return subprocess.run(argv, **kwargs, env={**os.environ, "GIT_TRACE": str(trace), "GIT_ALLOW_PROTOCOL": "file"}) - with pytest.raises(e.ExtractionTimeout, match="extraction_timeout"): - e.Git(tmp_path, runner=runner, deadline_seconds=10, _now=lambda: now[0]).materialize(["HEAD"]) - assert len(calls) == 2 - - def incomplete(argv, **kwargs): - return subprocess.CompletedProcess(argv, 0, ("?" + oid + "\n").encode(), b"") - - with pytest.raises(e.InstrumentFailure, match="missing_blobs_after_fetch"): - e.Git(tmp_path, runner=incomplete).materialize(["HEAD"]) + partial = e.Git(path, runner=runner) + missing = partial.run("rev-list", "--objects", "--missing=print", "HEAD") + assert sum(line.startswith("?") for line in missing.splitlines()) >= 60 + result = [e.introducing(partial, candidate), e.ordinary_history(partial, "HEAD")] + assert partial.files_read > 0 and partial.bytes_read > 0 + assert json.dumps(result, sort_keys=True).encode() == json.dumps(baseline, sort_keys=True).encode() + assert sum("built-in: git fetch " in line for line in trace.read_text().splitlines()) >= 12