mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
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 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
40bfdb8e18
commit
0023f61ff8
@@ -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()
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user