From eb0568acda9a595ce7abc93057d5e574386474dd Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:57:01 +0000 Subject: [PATCH 1/6] shape-doctor.py: the partial-clone row's NEXT fetches each commit it lacks objects at, by id, from the promisor remote whose branches hold it, so it no longer loops where the pinned commit sits only on another remote or another branch, and its `why` says where the clone does not hold the commit at all (#754) RV-f's review of #742, measured with git 2.43: - S2: the NEXT was `fetch --refetch --no-filter `, the remote whole. With the pinned commit on a second promisor remote's branches only, it exited 0 and the row came back the same; in a `--single-branch` clone whose pin is on another branch, the clone still did not hold the commit afterwards. The NEXT now names each lacking commit by id, and `remote_holding` picks the promisor remote whose tracking refs hold them all (`for-each-ref --contains`, which reads commits alone and fetches nothing). Where none does -- always for a commit the clone does not hold -- it asks the first, and the `why` names every promisor remote. - S1: where `objects_lacking` is None (the clone does not hold the commit), the `why` says so. - N1: the reason says "at least", since a missing tree hides what is under it from `rev-list`. - N2: the `why` says `--refetch` needs git 2.36 or later. tests/test_doctor_partial_clone.py: a `--single-branch` partial clone of a standard whose `main` lacks C1 (exit 3, `lacking == {C1: None}`, C1 still not held), its NEXT run as printed, two promisor remotes with C1 on the second's branches only, and the NEXT and "at least" of the existing case. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 97 +++++++++++++++++++------ tests/test_doctor_partial_clone.py | 110 +++++++++++++++++++++++++++-- 2 files changed, 182 insertions(+), 25 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index befe2808..57e0333b 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -4522,6 +4522,12 @@ def objects_lacking(shape: Path, commit: str) -> int | None: `bad object` there, exit 128, and None, which `partial_clone_row` counts as lacking: `update-shape.py`'s first read of that commit, its `rev-parse`, fetches it (measured with git 2.43). + + A FLOOR, NOT A COUNT, WHERE A TREE IS MISSING (#754). `rev-list` cannot + walk into a tree it does not hold, so a `--filter=tree:0` clone's + missing root tree is one `?` with everything under it uncounted, + and the id alone does not say it was a tree. So the row says "at + least" of every count. """ listed = git_answer(["rev-list", "--objects", "--no-walk", "--missing=print", commit], shape, binary=True) @@ -4567,12 +4573,22 @@ def partial_clone_row(ctx: Context, pinned: str) -> Row | None: THAT ROW'S, MADE TO FIT: there, the fetch a person runs to put in this checkout what it lacks. Here `fetch --all` would put in no blob, since a promisor remote's fetch keeps its `partialclonefilter`, so the NEXT - is `fetch --refetch --no-filter` from the remote a read would fetch - from: measured with git 2.43, it leaves the clone lacking nothing and - its config as it was, and the doctor run again answers. It is the - person's fetch, never this doctor's, and the `why` names the other way, - a full clone of the standard, as the precedent's names a clone that - has the commit. + is `fetch --refetch --no-filter`: measured with git 2.43, it leaves the + clone lacking nothing at what it fetches and its config as it was, and + the doctor run again answers. It is the person's fetch, never this + doctor's, and the `why` names the other way, a full clone of the + standard, as the precedent's names a clone that has the commit. + `--refetch` needs git 2.36 or later, and the `why` says so. + + FROM THE REMOTE THAT HOLDS IT, AND OF THE COMMITS IT LACKS (#754). The + NEXT fetched the first promisor remote whole, and looped where that + remote's branches do not hold the pinned commit: with two promisor + remotes and the commit on the second's, and in a `--single-branch` + clone whose pin is on another branch, which then still does not hold + it. So it names each commit lacked, by id, and fetches it from the + remote `remote_holding` finds holding them; where none does -- always + for a commit the clone does not hold at all, which the `why` says -- + from the first promisor remote, and the `why` names every one. """ remotes = promisor_remotes(ctx.shape) if not remotes: @@ -4584,29 +4600,70 @@ def partial_clone_row(ctx: Context, pinned: str) -> Row | None: if count != 0} if not lacking: return None - said = [] - for commit, count in lacking.items(): - which = ("the pinned commit" if commit == pinned - else "this standard's commit") - said.append(f"git cannot list {which} {commit[:12]} there" - if count is None else - f"it lacks {count} object{'' if count == 1 else 's'} " - f"at {which} {commit[:12]}") - remote = remotes[0] + def which(commit: str) -> str: + return (f"the pinned commit {commit[:12]}" if commit == pinned + else f"this standard's commit {commit[:12]}") + + said = [f"it does not hold {which(commit)} at all" if count is None + else f"it lacks at least {count} object" + f"{'' if count == 1 else 's'} at {which(commit)}" + for commit, count in lacking.items()] + absent = [commit for commit, count in lacking.items() if count is None] + held = remote_holding(ctx.shape, remotes, + [commit for commit in lacking + if commit not in absent]) + remote = held or remotes[0] + them = "it" if len(lacking) == 1 else "them" + whence = ([f"{' and '.join(which(commit) for commit in absent)} " + + ("is" if len(absent) == 1 else "are") + + " not in this clone at all"] if absent else []) + if held: + whence.append(f"{held}'s branches hold {them}, so fetch every " + f"object at {them} from {held}") + else: + others = ("" if len(remotes) == 1 else + f", or from whichever of {', '.join(remotes)} has {them}") + whence.append(f"no branch this clone tracks holds {them}, so fetch " + f"{them} by id from {remote}{others}") return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, "the copies cannot be compared without fetching: this " "checkout of the standard is a partial clone, and " + " and ".join(said), f"git -C {quote_arg(ctx.shape)} fetch --refetch --no-filter " - f"{quote_arg(remote)}", + f"{quote_arg(remote)} " + " ".join(lacking), {"error": PARTIAL_CLONE_LACKS, "environment": True, "pinned": pinned, "standard": standard, "remote": remote, "lacking": lacking}, why="this checkout of the standard is a partial clone, and " - "a read of an object it lacks fetches it from " - f"{remote}, which this doctor never does; fetch every " - f"object {remote} holds, then run the doctor again, or " - "run it from a full clone of the standard") + "a read of an object it lacks fetches it from a promisor " + "remote, which this doctor never does; " + + "; ".join(whence) + + " (--refetch needs git 2.36 or later), then run the " + "doctor again, or run it from a full clone of the " + "standard") + + +def remote_holding(shape: Path, remotes: list, commits: list) -> str | None: + """The first of `remotes` whose branches here -- its tracking refs, + `refs/remotes//` -- hold every one of `commits`, or None where + none does or `commits` is empty (#754). + + `for-each-ref --contains` reads commits alone, and a `--filter` clone + holds every commit a fetch brought, so it fetches nothing: measured with + git 2.43, no fetch in a trace2 log. It is never asked of a commit the + clone does not hold, which no branch here can hold and whose read would + fetch it: `partial_clone_row` passes only the commits `objects_lacking` + could list. + """ + if not commits: + return None + for name in remotes: + if all(git_answer(["for-each-ref", "--count=1", + "--format=%(refname)", f"--contains={commit}", + f"refs/remotes/{name}/"], shape) + for commit in commits): + return name + return None def check_shape_currency(ctx: Context) -> Row: diff --git a/tests/test_doctor_partial_clone.py b/tests/test_doctor_partial_clone.py index a24b9f82..c2499153 100644 --- a/tests/test_doctor_partial_clone.py +++ b/tests/test_doctor_partial_clone.py @@ -41,7 +41,8 @@ from conftest import FILE_PROTOCOL, git, load_without_bytecode, run_script from test_shape_doctor import ( # noqa: F401 UPSTREAM_TARGET, doctor, doctor_module, pasted, project, rows_of, - run_pasted, scaffolded_here, standard, standard_ahead, verdict_line, + run_pasted, scaffolded_here, standard, standard_ahead, + standard_of_another_history, verdict_line, ) #: The doctor's `error` for a partial clone that lacks what the comparison @@ -140,18 +141,22 @@ def test_a_partial_clone_behind_its_pin_cannot_answer_and_fetches_nothing( def test_the_row_names_the_commit_short_and_the_fetch_that_ends_it( moved, project, tmp_path): """What the row says of the clone it could not read: the pinned commit, - how many objects it lacks there, the promisor remote, and a NEXT that - is that remote's whole fetch, never `fetch --all`, which keeps the - clone's filter and brings in no blob.""" + how many objects it lacks there at least -- a floor, since a missing + tree hides what is under it (#754) -- the promisor remote, and a NEXT + that fetches every object at that commit from it, never `fetch --all`, + which keeps the clone's filter and brings in no blob.""" clone = partial_clone_of(moved["ahead"], tmp_path / "partial") row = currency_of(doctor(clone, project, "--json")) assert row["detail"] == { "error": PARTIAL, "environment": True, "pinned": moved["c1"], "standard": moved["c2"], "remote": "origin", "lacking": {moved["c1"]: 1}}, row + assert row["reason"].endswith( + f"it lacks at least 1 object at the pinned commit " + f"{moved['c1'][:12]}"), row["reason"] assert pasted(row["next"]) == [ "git", "-C", str(clone.resolve()), "fetch", "--refetch", - "--no-filter", "origin"] + "--no-filter", "origin", moved["c1"]] assert row["why"].startswith("this checkout of the standard is a " "partial clone"), row["why"] @@ -206,6 +211,101 @@ def test_the_commit_head_names_is_asked_too(moved, project, tmp_path): assert after == before, "the doctor fetched" +def holds(clone: Path, commit: str) -> bool: + """Whether `clone` holds `commit`, asked of git without fetching: + `rev-list --missing=print` refuses a commit it does not hold.""" + return subprocess.run( + ["git", "rev-list", "--objects", "--no-walk", "--missing=print", + commit], cwd=str(clone), capture_output=True, + check=False).returncode == 0 + + +@pytest.fixture +def elsewhere(standard, moved, tmp_path) -> dict: + """A standard whose `main` is a history of its own, `S`, that holds C1, + the commit the project is pinned at, on another branch only, `pinned`. + Its `main` holds the same tools and templates, so the doctor runs from + a clone of it as from the standard.""" + stranger = standard_of_another_history(standard, tmp_path) + git(*FILE_PROTOCOL, "fetch", "-q", str(moved["ahead"]), + f"{moved['c1']}:refs/heads/pinned", cwd=stranger) + git("config", "uploadpack.allowFilter", "true", cwd=stranger) + return {"stranger": stranger, "c1": moved["c1"]} + + +@pytest.fixture +def single_branch(elsewhere, tmp_path) -> Path: + """A `--single-branch` partial clone of `elsewhere`'s `main`: it does + not hold C1 at all, which only another branch has (#754).""" + clone = partial_clone_of(elsewhere["stranger"], tmp_path / "partial", + "blob:none", "--single-branch", "--branch", + "main") + assert not holds(clone, elsewhere["c1"]), "the clone holds C1" + return clone + + +def test_a_partial_clone_without_the_pinned_commit_cannot_answer( + elsewhere, single_branch, project): + """A partial clone that does not hold the pinned commit at all (#754 + S1): `objects_lacking` is None there, which no test reached, and a + mutant reading it as "lacks nothing" fetched six times and answered. + CANNOT ANSWER, exit 3, `lacking == {C1: None}`, the `why` says the + commit is not in the clone, and the clone still does not hold it.""" + result = doctor(single_branch, project, "--json") + assert result.returncode == 3, result.stdout + result.stderr + row = currency_of(result) + assert row["detail"]["lacking"] == {elsewhere["c1"]: None}, row + assert (f"the pinned commit {elsewhere['c1'][:12]} is not in this " + "clone at all") in row["why"], row["why"] + assert not holds(single_branch, elsewhere["c1"]), "the doctor fetched" + + +def run_the_next_and_ask_again(clone: Path, project: Path) -> dict: + """The row's NEXT run as a person pastes it, then the `shape currency` + row the doctor gives after it.""" + run_pasted(currency_of(doctor(clone, project, "--json"))["next"]) + return currency_of(doctor(clone, project, "--json")) + + +def test_the_next_fetches_a_pinned_commit_only_another_branch_holds( + elsewhere, single_branch, project): + """The NEXT names the commit by id (#754 S2): fetching the clone's one + branch again left it without C1, and the row printed the same NEXT. Run + as printed, the clone holds C1 and the doctor answers.""" + again = run_the_next_and_ask_again(single_branch, project) + assert again["detail"].get("error") != PARTIAL, again + + +@pytest.fixture +def two_promisors(elsewhere, single_branch, moved) -> Path: + """`single_branch`'s partial clone of `elsewhere`'s `main`, `origin`, + with the standard one commit on as a second promisor remote, + `second`, fetched with the same filter: it holds C1's commit, on + `second`'s branches only, and not C1's blobs that `main` does not + share.""" + clone = single_branch + git("remote", "add", "second", moved["ahead"].as_uri(), cwd=clone) + git("config", "remote.second.promisor", "true", cwd=clone) + git("config", "remote.second.partialclonefilter", "blob:none", + cwd=clone) + git(*FILE_PROTOCOL, "fetch", "-q", "second", cwd=clone) + assert holds(clone, elsewhere["c1"]), "the clone lacks C1's commit" + return clone + + +def test_the_next_names_the_promisor_remote_that_holds_the_commit( + elsewhere, two_promisors, project): + """Two promisor remotes, the pinned commit on the second's branches + only (#754 S2): the NEXT named the first, `origin`, which exits 0 and + brings in nothing at C1, so the row came back the same. It names + `second`, and run as printed, the doctor answers.""" + row = currency_of(doctor(two_promisors, project, "--json")) + assert pasted(row["next"])[-2:] == ["second", elsewhere["c1"]], \ + row["next"] + again = run_the_next_and_ask_again(two_promisors, project) + assert again["detail"].get("error") != PARTIAL, again + + def test_promisor_false_alone_is_a_read_that_fails_never_a_fetch_or_both( moved, project, tmp_path): """`remote.origin.promisor = false` with no `partialclonefilter` beside From 155db31fb87a12cf533ebb2b8b51fe1f899e8377 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 03:57:37 +0000 Subject: [PATCH 2/6] shape-doctor.py: `objects_lacking` reads a commit the clone does not hold as absent on every git, so from git 2.45 on it is no longer counted as one object lacked and handed to `for-each-ref --contains`, which fetches it (#754, round 2) PR #773's CI, on git 2.55.0, failed three tests at setup with "the clone holds C1". The clone did not hold it. From git 2.45 on, `rev-list --missing=print` takes a missing tip as missing too: it prints `?` and exits 0 (git's rev-list documentation since v2.45.0, "If some tips passed to the traversal are missing"), where git 2.43, this machine's, refuses it, `bad object`, exit 128. The tests' `holds` read exit 0 as "held". The doctor read the same command. On git 2.45 and later `objects_lacking` counted the absent commit as one object lacked, not None, so the row said "lacks at least 1 object" where the clone does not hold the commit at all, and `remote_holding` asked `for-each-ref --contains` of a commit the clone lacks, a read that fetches it. A `?` line for the commit itself is now None, as the refusal is. tests/test_doctor_partial_clone.py: `holds` takes both answers as "not held"; `elsewhere` puts C1 on its `pinned` branch only once `single_branch`'s clone exists, so no clone flag of any git can bring it in, and the precondition stays; `objects_lacking` is held to all four answers it can get, the two for a missing commit included, through the module, since one machine has one git. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 21 +++++--- tests/test_doctor_partial_clone.py | 78 ++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 21 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 57e0333b..dec8254b 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -4518,10 +4518,17 @@ def objects_lacking(shape: Path, commit: str) -> int | None: `--missing=` turns git's lazy fetch off for its run: measured with git 2.43, no fetch in a trace2 log, and the same objects missing after it. It is asked before any `ls-tree`, which fetches a tree a - `--filter=tree:0` clone lacks. A commit the clone does not hold is - `bad object` there, exit 128, and None, which `partial_clone_row` - counts as lacking: `update-shape.py`'s first read of that commit, its - `rev-parse`, fetches it (measured with git 2.43). + `--filter=tree:0` clone lacks. + + A COMMIT THE CLONE DOES NOT HOLD IS NONE, ON EVERY GIT (#754). Before + git 2.45 `rev-list` refuses it, `bad object`, exit 128; from 2.45 on it + takes a missing tip as missing too and prints it, `?`, exit 0 + (git's `rev-list` documentation, "If some tips passed to the traversal + are missing"). Counted as one object there, the commit was "lacked" + rather than absent, and `remote_holding` asked `--contains` of a commit + the clone does not hold, a read that fetches it. `partial_clone_row` + counts None as lacking: `update-shape.py`'s first read of that commit, + its `rev-parse`, fetches it (measured with git 2.43). A FLOOR, NOT A COUNT, WHERE A TREE IS MISSING (#754). `rev-list` cannot walk into a tree it does not hold, so a `--filter=tree:0` clone's @@ -4531,10 +4538,10 @@ def objects_lacking(shape: Path, commit: str) -> int | None: """ listed = git_answer(["rev-list", "--objects", "--no-walk", "--missing=print", commit], shape, binary=True) - if listed is None: + lines = (listed or b"").split(b"\n") + if listed is None or b"?" + commit.lower().encode("ascii") in lines: return None - return sum(1 for line in listed.split(b"\n") - if MISSING_OBJECT.fullmatch(line)) + return sum(1 for line in lines if MISSING_OBJECT.fullmatch(line)) #: `partial_clone_row`'s `error` (#683). diff --git a/tests/test_doctor_partial_clone.py b/tests/test_doctor_partial_clone.py index c2499153..22925d84 100644 --- a/tests/test_doctor_partial_clone.py +++ b/tests/test_doctor_partial_clone.py @@ -212,34 +212,49 @@ def test_the_commit_head_names_is_asked_too(moved, project, tmp_path): def holds(clone: Path, commit: str) -> bool: - """Whether `clone` holds `commit`, asked of git without fetching: - `rev-list --missing=print` refuses a commit it does not hold.""" - return subprocess.run( + """Whether `clone` holds `commit`, asked of git without fetching. + + `rev-list --missing=print` answers a commit the clone does not hold in + one of two ways, by git version: before 2.45 it refuses it, `bad + object`, exit 128; from 2.45 on it prints it, `?`, and exits 0 + (#754: CI's git 2.55 read this as held). Either is "not held", and + neither fetches. `cat-file -e` would read the commit, which in a + partial clone fetches it wherever git does not honour + `GIT_NO_LAZY_FETCH`.""" + listed = subprocess.run( ["git", "rev-list", "--objects", "--no-walk", "--missing=print", - commit], cwd=str(clone), capture_output=True, - check=False).returncode == 0 + commit], cwd=str(clone), capture_output=True, check=False) + return (listed.returncode == 0 + and f"?{commit}".encode() not in listed.stdout.splitlines()) @pytest.fixture def elsewhere(standard, moved, tmp_path) -> dict: - """A standard whose `main` is a history of its own, `S`, that holds C1, - the commit the project is pinned at, on another branch only, `pinned`. - Its `main` holds the same tools and templates, so the doctor runs from - a clone of it as from the standard.""" + """A standard whose `main` is a history of its own, `S`, with the same + tools and templates as the standard, so the doctor runs from a clone of + it as from the standard; and `pin`, which puts C1, the commit the + project is pinned at, on another branch of it, `pinned`, and nowhere + else. Called only once a clone of `main` exists, so no flag of any + git's `clone` can bring C1 in (#754).""" stranger = standard_of_another_history(standard, tmp_path) - git(*FILE_PROTOCOL, "fetch", "-q", str(moved["ahead"]), - f"{moved['c1']}:refs/heads/pinned", cwd=stranger) git("config", "uploadpack.allowFilter", "true", cwd=stranger) - return {"stranger": stranger, "c1": moved["c1"]} + + def pin() -> None: + git(*FILE_PROTOCOL, "fetch", "-q", str(moved["ahead"]), + f"{moved['c1']}:refs/heads/pinned", cwd=stranger) + + return {"stranger": stranger, "c1": moved["c1"], "pin": pin} @pytest.fixture def single_branch(elsewhere, tmp_path) -> Path: - """A `--single-branch` partial clone of `elsewhere`'s `main`: it does - not hold C1 at all, which only another branch has (#754).""" + """A `--single-branch` partial clone of `elsewhere`'s `main`, made + before C1 was on any branch of it: it does not hold C1 at all, which + only another branch of its remote has (#754).""" clone = partial_clone_of(elsewhere["stranger"], tmp_path / "partial", "blob:none", "--single-branch", "--branch", "main") + elsewhere["pin"]() assert not holds(clone, elsewhere["c1"]), "the clone holds C1" return clone @@ -399,3 +414,38 @@ def test_upstream_blob_is_a_files_bytes_or_none_where_there_is_no_file( f"{head(standard)}:{path}"], cwd=str(standard), capture_output=True, check=False).stdout assert upstream.blob(head(standard), path) == (held if found else None) + + +#: A commit id, and two of the objects its tree reaches, for the outputs +#: below. +SOME_COMMIT = "c" * 40 +SOME_OBJECTS = ("a" * 40, "b" * 40) + +#: `rev-list --objects --no-walk --missing=print `'s answer, +#: as `git_answer` hands it on, and what `objects_lacking` makes of it. +REV_LIST_ANSWERS = { + "a commit not held, before git 2.45 (bad object, exit 128)": (None, None), + "a commit not held, from git 2.45 on (printed missing, exit 0)": ( + f"?{SOME_COMMIT}\n".encode(), None), + "a commit held, with two objects missing": ( + f"{SOME_COMMIT}\n{'d' * 40} \n?{SOME_OBJECTS[0]}\n" + f"?{SOME_OBJECTS[1]}\n".encode(), 2), + "a commit held, with nothing missing": ( + f"{SOME_COMMIT}\n{'d' * 40} \n{'e' * 40} README.md\n".encode(), 0), +} + + +@pytest.mark.parametrize("case", list(REV_LIST_ANSWERS)) +def test_objects_lacking_reads_a_missing_commit_on_every_git( + standard, tmp_path, monkeypatch, case): + """`objects_lacking` on each answer `rev-list --missing=print` gives + (#754). From git 2.45 on, a commit the clone does not hold is printed + `?`, exit 0, where git 2.43 refused it; read as one object + lacked, it was not "absent", and `remote_holding` asked `--contains` + of it, a read that fetches. Both are None. Handed to the module, not + run, because one machine has one git.""" + answer, expected = REV_LIST_ANSWERS[case] + module = doctor_module(standard) + monkeypatch.setattr(module, "git_answer", + lambda *_args, **_kwargs: answer) + assert module.objects_lacking(tmp_path, SOME_COMMIT) == expected From 296a6dd6a9bb9b0f566297007698f4cba44b7c72 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 05:29:46 +0000 Subject: [PATCH 3/6] shape-doctor.py: where the commits a partial clone lacks objects at are on different remotes' branches, the NEXT fetches from the remote holding the pinned commit only the commits that remote holds, and the `why` says which remote holds which, so the fetch no longer fails `not our ref` and comes back unchanged (#754, round 3) RV-f's review of #773 rounds 1 and 2 (SHOULD S2): a partial clone whose HEAD lacked objects on `origin`'s branches alone, with the pinned commit on `second`'s alone, got `fetch --refetch --no-filter origin `. git refuses a fetch by id whole where the remote lacks one id, so the row came back the same, its `why` saying no branch held them. - `partial_clone_fetch` picks the remote and the commits: the first remote whose branches hold every lacking commit some remote holds, else the first holding the pinned commit, and only the commits that remote holds. The doctor run again names the rest. Where no remote's branches hold any, it is the first promisor remote and every commit, as before. - The `why` says where each commit is (`where_lacked`). Where the remote is a guess, it spells out the same fetch from each other promisor remote as a line to paste (N1). - `remotes_holding` (was `remote_holding`) lists every remote whose branches hold one commit, and is still never asked of a commit the clone does not hold. - SonarCloud S3776 and S3358 (S1): the reason, the `why` and the fetch are built by helpers, with no nested conditional, and the nested `def which` is gone (N2). Tests: `test_commits_on_two_remotes_are_fetched_from_each_in_turn` runs RV-f's case's NEXT twice to an answer, and `test_the_why_spells_out_the_fetch_from_each_other_promisor_remote` runs the `why`'s command for a never-fetched `second`. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 206 +++++++++++++++++++++-------- tests/test_doctor_partial_clone.py | 100 +++++++++++++- 2 files changed, 247 insertions(+), 59 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index dec8254b..00aa5128 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -4525,7 +4525,7 @@ def objects_lacking(shape: Path, commit: str) -> int | None: takes a missing tip as missing too and prints it, `?`, exit 0 (git's `rev-list` documentation, "If some tips passed to the traversal are missing"). Counted as one object there, the commit was "lacked" - rather than absent, and `remote_holding` asked `--contains` of a commit + rather than absent, and `remotes_holding` asked `--contains` of a commit the clone does not hold, a read that fetches it. `partial_clone_row` counts None as lacking: `update-shape.py`'s first read of that commit, its `rev-parse`, fetches it (measured with git 2.43). @@ -4592,10 +4592,8 @@ def partial_clone_row(ctx: Context, pinned: str) -> Row | None: remote's branches do not hold the pinned commit: with two promisor remotes and the commit on the second's, and in a `--single-branch` clone whose pin is on another branch, which then still does not hold - it. So it names each commit lacked, by id, and fetches it from the - remote `remote_holding` finds holding them; where none does -- always - for a commit the clone does not hold at all, which the `why` says -- - from the first promisor remote, and the `why` names every one. + it. So it names each commit lacked by id, from the remote + `partial_clone_fetch` picks, and the `why` says where each commit is. """ remotes = promisor_remotes(ctx.shape) if not remotes: @@ -4607,70 +4605,174 @@ def partial_clone_row(ctx: Context, pinned: str) -> Row | None: if count != 0} if not lacking: return None - def which(commit: str) -> str: - return (f"the pinned commit {commit[:12]}" if commit == pinned - else f"this standard's commit {commit[:12]}") - - said = [f"it does not hold {which(commit)} at all" if count is None - else f"it lacks at least {count} object" - f"{'' if count == 1 else 's'} at {which(commit)}" - for commit, count in lacking.items()] - absent = [commit for commit, count in lacking.items() if count is None] - held = remote_holding(ctx.shape, remotes, - [commit for commit in lacking - if commit not in absent]) - remote = held or remotes[0] - them = "it" if len(lacking) == 1 else "them" - whence = ([f"{' and '.join(which(commit) for commit in absent)} " - + ("is" if len(absent) == 1 else "are") - + " not in this clone at all"] if absent else []) - if held: - whence.append(f"{held}'s branches hold {them}, so fetch every " - f"object at {them} from {held}") - else: - others = ("" if len(remotes) == 1 else - f", or from whichever of {', '.join(remotes)} has {them}") - whence.append(f"no branch this clone tracks holds {them}, so fetch " - f"{them} by id from {remote}{others}") + remote, commits, whence = partial_clone_fetch(ctx.shape, remotes, + pinned, lacking) return Row("shape-currency", LABEL_SHAPE_CURRENCY, FINDING, "the copies cannot be compared without fetching: this " "checkout of the standard is a partial clone, and " - + " and ".join(said), - f"git -C {quote_arg(ctx.shape)} fetch --refetch --no-filter " - f"{quote_arg(remote)} " + " ".join(lacking), + + lacking_said(lacking, pinned), + refetch_command(ctx.shape, remote, commits), {"error": PARTIAL_CLONE_LACKS, "environment": True, "pinned": pinned, "standard": standard, "remote": remote, "lacking": lacking}, why="this checkout of the standard is a partial clone, and " "a read of an object it lacks fetches it from a promisor " - "remote, which this doctor never does; " - + "; ".join(whence) - + " (--refetch needs git 2.36 or later), then run the " - "doctor again, or run it from a full clone of the " - "standard") + "remote, which this doctor never does; " + whence + + ", or run it from a full clone of the standard") + + +def lacked_commit(commit: str, pinned: str) -> str: + """How `partial_clone_row` names a commit the clone lacks objects at: + the pinned commit or this standard's, by its id's first twelve + digits.""" + if commit == pinned: + return f"the pinned commit {commit[:12]}" + return f"this standard's commit {commit[:12]}" + + +def lacking_said(lacking: dict, pinned: str) -> str: + """The reason's account of `lacking`: each commit the clone does not + hold at all, and how many objects at least it lacks at each it does + hold.""" + said = [] + for commit, count in lacking.items(): + if count is None: + said.append(f"it does not hold {lacked_commit(commit, pinned)} " + "at all") + else: + said.append(f"it lacks at least {count} object" + f"{'' if count == 1 else 's'} at " + f"{lacked_commit(commit, pinned)}") + return " and ".join(said) + + +def refetch_command(shape: Path, remote: str, commits: list) -> str: + """The fetch that puts in `shape` every object at each of `commits`, + from `remote`, whatever its `partialclonefilter` (#683, #754).""" + return (f"git -C {quote_arg(shape)} fetch --refetch --no-filter " + f"{quote_arg(remote)} " + " ".join(commits)) + + +def partial_clone_fetch(shape: Path, remotes: list, pinned: str, + lacking: dict) -> tuple: + """`(remote, commits, whence)` for a partial clone at `shape` that + lacks objects at `lacking`'s commits: the promisor remote the NEXT + fetches from, the commits it names, and the `why`'s account of where + each commit is and what to run (#754). + + ONLY THE COMMITS THE REMOTE IS KNOWN TO HOLD, WHERE ONE IS. A fetch by + id from a remote that does not have one of the ids fails whole, exit + 128, `not our ref`, and puts in nothing. Measured by RV-f on #773: in + a sparse clone whose HEAD lacked objects on `origin`'s branches alone, + with the pinned commit on `second`'s alone, the NEXT fetched both from + `origin`, git refused it, and the row came back the same, its `why` + saying no branch held them. So where some remote's branches hold a + lacking commit, the NEXT fetches from the first remote whose branches + hold every such commit, else from the first that holds the pinned + commit, and names only the commits that remote's branches hold. The + doctor run again names the rest: two runs at most, for two commits. + + THE FIRST PROMISOR REMOTE, AND EVERY COMMIT, WHERE NONE IS. A commit + the clone does not hold, or holds only on a tag or on no ref, is on no + branch it tracks, and no read that fetches nothing says which remote + has it. The NEXT asks the first promisor remote, the one git's own + lazy fetch asks first, and the `why` spells out the same fetch from + each other promisor remote, so that each is a line to paste. + """ + holding = {commit: remotes_holding(shape, remotes, commit) + for commit, count in lacking.items() if count is not None} + known = {commit: names for commit, names in holding.items() if names} + remote = fetch_source(remotes, pinned, known) + commits = [commit for commit, names in known.items() + if remote in names] or list(lacking) + said = [where_lacked(commit, holding.get(commit), pinned) + for commit in lacking] + said.append(fetch_said(shape, remotes, remote, commits, + fetched_named(commits, lacking, pinned), + bool(known))) + return (remote, commits, + "; ".join(said) + then_said(commits, lacking, pinned)) + + +def fetch_source(remotes: list, pinned: str, known: dict) -> str: + """The promisor remote the NEXT fetches from, of `known`, each lacking + commit some remote's branches hold and the remotes that hold it: the + first remote that holds every one -- the first promisor remote, where + `known` is empty -- else the first that holds the pinned commit + (#754).""" + for name in remotes: + if all(name in names for names in known.values()): + return name + return (known.get(pinned) or next(iter(known.values())))[0] + + +def where_lacked(commit: str, holders: list | None, pinned: str) -> str: + """The `why`'s word on where `commit` is: not in this clone at all, + where `holders` is None, on no branch this clone tracks, where it is + empty, or on the branches of each of `holders` (#754).""" + named = lacked_commit(commit, pinned) + if holders is None: + return f"{named} is not in this clone at all" + if not holders: + return f"no branch this clone tracks holds {named}" + return (f"{named} is on " + + " and ".join(f"{name}'s" for name in holders) + " branches") + + +def fetch_said(shape: Path, remotes: list, remote: str, commits: list, + them: str, known: bool) -> str: + """The `why`'s word on the fetch: every object at `them` from + `remote`, whose branches hold them, or, where none is `known` to, by + id from `remote` and else from each other promisor remote, that fetch + spelled out as a line to paste (#754).""" + if known: + return f"so fetch every object at {them} from {remote}" + said = f"so fetch {them} by id from {remote}" + others = [f"from {name}: `{refetch_command(shape, name, commits)}`" + for name in remotes if name != remote] + if others: + said += (f", or, where {remote} does not have {them}, " + + ", or ".join(others)) + return said + + +def fetched_named(commits: list, lacking: dict, pinned: str) -> str: + """How the `why` names the commits the NEXT fetches: `it` or `them`, + where they are every commit `lacking` holds, else each by name + (#754).""" + if len(commits) < len(lacking): + return " and ".join(lacked_commit(commit, pinned) + for commit in commits) + return "it" if len(commits) == 1 else "them" + + +def then_said(commits: list, lacking: dict, pinned: str) -> str: + """The end of the `why`'s word on the fetch: the git it needs, the + doctor run again, and the commits of `lacking` that run names a fetch + for, those the NEXT leaves out (#754).""" + said = " (--refetch needs git 2.36 or later), then run the doctor again" + rest = [lacked_commit(commit, pinned) for commit in lacking + if commit not in commits] + if rest: + said += ", which names the fetch for " + " and ".join(rest) + return said -def remote_holding(shape: Path, remotes: list, commits: list) -> str | None: - """The first of `remotes` whose branches here -- its tracking refs, - `refs/remotes//` -- hold every one of `commits`, or None where - none does or `commits` is empty (#754). +def remotes_holding(shape: Path, remotes: list, commit: str) -> list: + """Those of `remotes` whose branches here -- their tracking refs, + `refs/remotes//` -- hold `commit`, in `remotes`' order (#754). `for-each-ref --contains` reads commits alone, and a `--filter` clone holds every commit a fetch brought, so it fetches nothing: measured with git 2.43, no fetch in a trace2 log. It is never asked of a commit the clone does not hold, which no branch here can hold and whose read would - fetch it: `partial_clone_row` passes only the commits `objects_lacking` - could list. + fetch it: `partial_clone_fetch` asks only of the commits + `objects_lacking` could list. """ - if not commits: - return None - for name in remotes: - if all(git_answer(["for-each-ref", "--count=1", + return [name for name in remotes + if git_answer(["for-each-ref", "--count=1", "--format=%(refname)", f"--contains={commit}", - f"refs/remotes/{name}/"], shape) - for commit in commits): - return name - return None + f"refs/remotes/{name}/"], shape)] def check_shape_currency(ctx: Context) -> Row: diff --git a/tests/test_doctor_partial_clone.py b/tests/test_doctor_partial_clone.py index 22925d84..18340303 100644 --- a/tests/test_doctor_partial_clone.py +++ b/tests/test_doctor_partial_clone.py @@ -33,6 +33,7 @@ from __future__ import annotations import json +import re import subprocess from pathlib import Path @@ -291,6 +292,18 @@ def test_the_next_fetches_a_pinned_commit_only_another_branch_holds( assert again["detail"].get("error") != PARTIAL, again +def second_promisor(clone: Path, source: Path, *, fetched: bool) -> Path: + """`clone` with `source` as a second promisor remote, `second`, under + the same filter as `origin`, and fetched where `fetched` says so.""" + git("remote", "add", "second", source.as_uri(), cwd=clone) + git("config", "remote.second.promisor", "true", cwd=clone) + git("config", "remote.second.partialclonefilter", "blob:none", + cwd=clone) + if fetched: + git(*FILE_PROTOCOL, "fetch", "-q", "second", cwd=clone) + return clone + + @pytest.fixture def two_promisors(elsewhere, single_branch, moved) -> Path: """`single_branch`'s partial clone of `elsewhere`'s `main`, `origin`, @@ -298,12 +311,7 @@ def two_promisors(elsewhere, single_branch, moved) -> Path: `second`, fetched with the same filter: it holds C1's commit, on `second`'s branches only, and not C1's blobs that `main` does not share.""" - clone = single_branch - git("remote", "add", "second", moved["ahead"].as_uri(), cwd=clone) - git("config", "remote.second.promisor", "true", cwd=clone) - git("config", "remote.second.partialclonefilter", "blob:none", - cwd=clone) - git(*FILE_PROTOCOL, "fetch", "-q", "second", cwd=clone) + clone = second_promisor(single_branch, moved["ahead"], fetched=True) assert holds(clone, elsewhere["c1"]), "the clone lacks C1's commit" return clone @@ -321,6 +329,84 @@ def test_the_next_names_the_promisor_remote_that_holds_the_commit( assert again["detail"].get("error") != PARTIAL, again +@pytest.fixture +def split_across_remotes(elsewhere, moved, tmp_path) -> dict: + """A partial clone of `elsewhere`'s `main`, `origin`, whose HEAD is a + commit made on `origin` after the clone and fetched under its filter, + so that HEAD lacks a blob only `origin`'s branches hold; and the + standard one commit on as a second promisor remote, `second`, whose + branches alone hold C1. RV-f's case on #773 (S2) left HEAD lacking with + a sparse checkout, which here would leave out the doctor's own files. + The clone and the commit HEAD names, as `clone` and `head`.""" + stranger = elsewhere["stranger"] + clone = partial_clone_of(stranger, tmp_path / "partial") + (stranger / "NOTES.md").write_bytes(b"made after the clone\n") + git("add", "NOTES.md", cwd=stranger) + git("commit", "-q", "-m", "A commit the clone never checked out", + cwd=stranger) + git(*FILE_PROTOCOL, "fetch", "-q", "origin", cwd=clone) + later = head(stranger) + git("update-ref", "--no-deref", "HEAD", later, cwd=clone) + second_promisor(clone, moved["ahead"], fetched=True) + blob = git("rev-parse", f"{later}:NOTES.md", cwd=stranger).stdout.strip() + assert f"?{blob}".encode() in missing(clone), "HEAD lacks nothing" + assert holds(clone, elsewhere["c1"]), "the clone lacks C1's commit" + return {"clone": clone, "head": later} + + +def test_commits_on_two_remotes_are_fetched_from_each_in_turn( + elsewhere, split_across_remotes, project): + """HEAD lacks objects that `origin`'s branches alone hold, and C1 is on + `second`'s alone (RV-f on #773, S2). The NEXT fetched both from + `origin`, which has no C1, so git refused it whole, `not our ref`, and + the row came back the same, its `why` saying no branch held them. It + fetches C1 from `second`, the `why` says which remote holds which, and + run as printed it is followed by HEAD's fetch from `origin`, after + which the doctor answers.""" + clone, later = split_across_remotes["clone"], split_across_remotes["head"] + first = currency_of(doctor(clone, project, "--json")) + assert pasted(first["next"])[-2:] == ["second", elsewhere["c1"]], \ + first["next"] + assert (f"the pinned commit {elsewhere['c1'][:12]} is on second's " + "branches") in first["why"], first["why"] + assert (f"this standard's commit {later[:12]} is on origin's " + "branches") in first["why"], first["why"] + then = run_the_next_and_ask_again(clone, project) + assert pasted(then["next"])[-2:] == ["origin", later], then["next"] + again = run_the_next_and_ask_again(clone, project) + assert again["detail"].get("error") != PARTIAL, again + + +@pytest.fixture +def never_fetched(elsewhere, moved, tmp_path) -> Path: + """A partial clone of `elsewhere`'s `main`, `origin`, which has no C1, + and the standard one commit on as a second promisor remote, `second`, + never fetched: the clone does not hold C1, and no branch it tracks + says which remote has it (RV-f on #773, N1).""" + clone = second_promisor(partial_clone_of(elsewhere["stranger"], + tmp_path / "partial"), + moved["ahead"], fetched=False) + assert not holds(clone, elsewhere["c1"]), "the clone holds C1" + return clone + + +def test_the_why_spells_out_the_fetch_from_each_other_promisor_remote( + elsewhere, never_fetched, project): + """C1 is not in the clone, and only `second`, never fetched, has it + (RV-f on #773, N1). The NEXT asks `origin`, the first promisor remote, + which has no C1, and the `why` said "or from whichever of origin, + second has it", a name and no line to paste. It spells out the fetch + from `second`, and run as printed, the doctor answers.""" + row = currency_of(doctor(never_fetched, project, "--json")) + assert pasted(row["next"])[-2:] == ["origin", elsewhere["c1"]], \ + row["next"] + offered = re.search(r"from second: `([^`]+)`", row["why"]) + assert offered, row["why"] + run_pasted(offered.group(1)) + again = currency_of(doctor(never_fetched, project, "--json")) + assert again["detail"].get("error") != PARTIAL, again + + def test_promisor_false_alone_is_a_read_that_fails_never_a_fetch_or_both( moved, project, tmp_path): """`remote.origin.promisor = false` with no `partialclonefilter` beside @@ -441,7 +527,7 @@ def test_objects_lacking_reads_a_missing_commit_on_every_git( """`objects_lacking` on each answer `rev-list --missing=print` gives (#754). From git 2.45 on, a commit the clone does not hold is printed `?`, exit 0, where git 2.43 refused it; read as one object - lacked, it was not "absent", and `remote_holding` asked `--contains` + lacked, it was not "absent", and `remotes_holding` asked `--contains` of it, a read that fetches. Both are None. Handed to the module, not run, because one machine has one git.""" answer, expected = REV_LIST_ANSWERS[case] From 152e9c39059a8bde7f54c2802c926fbeb0a19726 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 09:19:45 +0000 Subject: [PATCH 4/6] shape-doctor.py: every git the doctor starts runs with `GIT_NO_LAZY_FETCH=1`, so a read of an object a partial clone lacks fails rather than fetches, and `pin_restore` answers `partial` and names the fetch where a root's own partial clone lacks its pin's history (#787) "It never fetches" held read by read: #683 and #754 ask a partial clone of the standard with `rev-list --missing=print` before anything reads it. A read nothing asked about first still fetched. Measured with git 2.43: a partial clone of a ROOT whose HEAD commit broke its pin made one lazy fetch (`blob:none`) or two (`tree:0`), when `pin_restore` walked the pin's history. - `lazy_fetch_off` sets `GIT_NO_LAZY_FETCH=1` in this process's environment for the run of `main`, and puts it back after. Every git inherits it: the doctor's own reads, `update-shape.py`'s `Upstream` as the doctor drives it in-process, the validators and their gits. No printed line carries it, so a NEXT `fetch --refetch` still fetches. git 2.45.0 and later honour it, and the 2024-05-14 security releases back to 2.39.4 (CVE-2024-32465); an older git ignores it, harmlessly. - With it, `pin_restore`'s walk read a failed read as "no pin there" and said `none`: no commit holds a pin to put back, which is not so. `history_left_out` asks, without fetching, whether a partial clone lacks the pin's history. Where it does, `restore` is `partial`, and the NEXT is `git -C fetch --refetch --no-filter `. Run as printed, the doctor run again names the commit (`committed`). Tests (`tests/test_doctor_no_lazy_fetch.py`): - no row starts a fetch from a partial clone of the standard (trace2); - the report is byte-identical with the variable ignored by every git, on a full clone and on two partial clones of the standard; - a partial root whose pin history is not here answers `partial`; - `lazy_fetch_off` sets the variable and puts it back. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 110 +++++++++++++++- tests/test_doctor_no_lazy_fetch.py | 195 +++++++++++++++++++++++++++++ 2 files changed, 304 insertions(+), 1 deletion(-) create mode 100644 tests/test_doctor_no_lazy_fetch.py diff --git a/shape-doctor.py b/shape-doctor.py index 00aa5128..29f7c82a 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -27,7 +27,9 @@ answer on a machine that has never spoken to github.com. A partial clone (`git clone --filter`) fetches what it lacks on a read, so it is asked first, without fetching, whether it holds what the comparison reads, and the verdict -is CANNOT ANSWER where it does not (#683). It runs the project's +is CANNOT ANSWER where it does not (#683). Every git it starts runs with +`GIT_NO_LAZY_FETCH=1`, so a read nothing asked about first fails rather than +fetches (`lazy_fetch_off`, #787). It runs the project's own validators OFFLINE: `run_validator` starts each child with no token, no `gh` configuration and a proxy that goes nowhere (#297), so they are offline whatever their own contract says. ONE ROW IS THE EXCEPTION AND IT IS NAMED: @@ -341,6 +343,7 @@ from __future__ import annotations import argparse +import contextlib import datetime as _dt import importlib.util import json @@ -3844,6 +3847,14 @@ def pin_restore(ctx: Context, detail: dict | None = None, whose parent holds the OLD pin -- is still the one to put back, and the `why` says what comes after it: `gitlink_left_behind` (#547). + partial the walk above found no pin to put back, and the root is + a partial clone (`git clone --filter`) that lacks a pin + or a tree the walk reads (`history_left_out`, #787). + This doctor fetches nothing, so the read failed and it + cannot say which commit broke the pin, or that none + did. `fetch --refetch --no-filter` from the first + promisor remote puts in every object on its branches, + and the doctor run again walks the whole history. none nothing above: no history, a root commit, no git, a walk that meets a parent holding no pin, or goes `PIN_HISTORY_DEPTH` commits back, before a pin that reads. @@ -4045,9 +4056,53 @@ def pin_restore(ctx: Context, detail: dict | None = None, f"broke it, and the newest {what} in its history that reads. " f"Commit that rather than editing the {what}" + gitlink_left_behind(ctx, before, pin)) + remote = history_left_out(root, pin) + if remote is not None: + return "partial", ( + f"{git_c} fetch --refetch --no-filter {quote_arg(remote)}" + ), (f"this root is a partial clone, and the {what}'s history is not " + "all in it. git fetches what a partial clone lacks on a read, " + "which this doctor never does, so which commit broke the " + f"{what} cannot be read here. This fetches every object on " + f"{remote}'s branches (--refetch needs git 2.36 or later); then " + f"run this doctor again, which names the {what} to put back") return "none", None, None +def history_left_out(root: Path, pin: str) -> str | None: + """The first promisor remote of `root`, where `root` is a partial clone + that lacks an object `pin_restore`'s walk of `pin`'s history reads, + asked without fetching; None where it lacks none, or is no partial + clone (#787). + + THE WALK'S READS FAIL THERE, AND THE DOCTOR FETCHED THEM BEFORE. + Measured on #787 with git 2.43: a `--filter=blob:none` clone of a root + whose HEAD commit broke its pin made one lazy fetch, of the pin before + it, and a `--filter=tree:0` one two. With `GIT_NO_LAZY_FETCH` set + (`lazy_fetch_off`) each read fails instead, and the walk, which reads a + failure as "no pin there", said "none": no commit in this root holds a + pin to put back, which is not so. + + `rev-list --objects --missing=print` over the commits that touched + `pin`, one more than the walk reads, lists the pin at each and the + trees down to it, and prints `?` for each the clone does not hold, + without fetching (`objects_lacking` says why). Where the clone lacks a + tree, as a `tree:0` one does, it cannot tell which commits touched the + pin and fails, as the walk's own `rev-list` does: that is lacking too. + A root with no HEAD has no history to walk, and is not asked. + """ + remotes = promisor_remotes(root) + if not remotes or head_of(root) is None: + return None + listed = git_answer(["rev-list", "--objects", "--missing=print", "-n", + str(PIN_HISTORY_DEPTH + 1), "HEAD", "--", pin], + root, binary=True) + if listed is not None and not any( + MISSING_OBJECT.fullmatch(line) for line in listed.split(b"\n")): + return None + return remotes[0] + + def taken_with_the_pin(ctx: Context, kind, copies: list, stages: dict) -> list: """Which of `copies`, conflicted beside the shape pin, `pin_restore` @@ -10036,6 +10091,59 @@ def build_parser() -> argparse.ArgumentParser: return parser +#: The variable git reads to leave out the fetch a partial clone makes for +#: an object it lacks (#787). +NO_LAZY_FETCH = "GIT_NO_LAZY_FETCH" + + +@contextlib.contextmanager +def lazy_fetch_off(): + """This process's environment with `GIT_NO_LAZY_FETCH=1` for as long as + it runs, and as it was afterwards (#787). + + "IT NEVER FETCHES", BY CONSTRUCTION. A partial clone (`git clone + --filter`) fetches an object it lacks from a promisor remote on any read + that needs one, so the promise held read by read: #683 and #754 ask + `rev-list --missing=print`, which never fetches, before anything reads + a partial clone of the standard, and never ask `for-each-ref --contains` + of a commit the clone does not hold. A read added to a row later, or one + in a project's own partial clone, need not ask first. With the variable + set, git fails that read instead of fetching: `warning: lazy fetching + disabled; some objects may not be available`, then the read's own error + (measured with git 2.43: exit 128, and no `fetch` in a trace2 log). That + is a read that failed, as one does where the clone's remote is out of + reach, and every reader here already answers one: `git_answer`'s None, + `git_out`'s `git-failed`, `update-shape.py`'s + `update-upstream-unreadable`. + + EVERY GIT THIS PROCESS STARTS, AND NOTHING IT PRINTS. The doctor's own + reads (`git_out`, `git_answer` and its few `subprocess.run` of git), + `update-shape.py`'s `Upstream` as `check_shape_currency` drives it in + this process, the validators `run_validator` starts with a copy of this + environment, and every git each of those starts, inherit it. A NEXT the + doctor prints is text for a person's own shell, which does not have it, + so a `fetch --refetch` printed there fetches. The environment is put + back on the way out, for a caller that runs `main` in its own process. + + WHICH GIT HONOURS IT. git 2.45.0 and later, and the security releases of + 2024-05-14 back to 2.39.4 (2.39.4, 2.40.2, 2.41.1, 2.42.2, 2.43.4, + 2.44.1), whose fix for CVE-2024-32465 made `promisor-remote.c` read it; + distributions carry that fix too (Ubuntu's 2.43.0-1ubuntu7.1 and later). + An older git ignores the variable, harmlessly, and there the guards + above are what keep the doctor from fetching, as before. + """ + before = os.environ.get(NO_LAZY_FETCH) + os.environ[NO_LAZY_FETCH] = "1" + try: + yield + finally: + if before is None: + os.environ.pop(NO_LAZY_FETCH, None) + else: + os.environ[NO_LAZY_FETCH] = before + + +@lazy_fetch_off() def main(argv: list[str] | None = None) -> int: # FIRST, so that every `print` after it -- `--help` included -- escapes # a name stdout cannot encode rather than raising on it (#214). diff --git a/tests/test_doctor_no_lazy_fetch.py b/tests/test_doctor_no_lazy_fetch.py new file mode 100644 index 00000000..0fbe0dde --- /dev/null +++ b/tests/test_doctor_no_lazy_fetch.py @@ -0,0 +1,195 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`shape-doctor.py` starts every git with `GIT_NO_LAZY_FETCH=1` (#787). + +THE PROMISE WAS KEPT READ BY READ. The doctor "never fetches", and a +partial clone (`git clone --filter`) fetches an object it lacks on any read +that needs one. #683 and #754 ask a partial clone of the standard, with +`rev-list --missing=print`, before anything reads it; a read nothing asked +about first fetched. Measured on #787 with git 2.43: a partial clone of a +ROOT whose HEAD commit broke its pin made one lazy fetch (`blob:none`) or +two (`tree:0`), when `pin_restore` walked the pin's history. + +NOW `main` runs under `lazy_fetch_off`, so every git the doctor starts, +and every git its validators start, fails a read that would fetch. Such a +read was a fetch that failed where the remote is out of reach, and the +doctor already answers those; `pin_restore`'s walk now says, of a partial +clone that lacks the pin's history, `partial`, and names the fetch. + +"FETCHED NOTHING" IS ASKED OF GIT. `GIT_TRACE2_EVENT` names a folder, and +each git the run starts writes its events there: a lazy fetch is a +`child_start` of `git ... fetch`, logged even where the fetch then fails, +so a remote out of reach hides nothing. `rev-list --objects --all +--missing=print` before and after says the same of a remote in reach. +""" + +from __future__ import annotations + +import json +import os +import shlex +import shutil +from pathlib import Path + +import pytest + +from conftest import FILE_PROTOCOL, git, run_script +from test_doctor_partial_clone import ( # noqa: F401 + currency_of, missing, moved, out_of_reach, partial_clone_of, +) +from test_shape_doctor import ( # noqa: F401 + SHAPE_PIN, doctor_module, pasted, project, run_pasted, scaffolded_here, + standard, +) + +#: The variable `lazy_fetch_off` sets for the doctor's run. +NO_LAZY_FETCH = "GIT_NO_LAZY_FETCH" + + +def traced(shape: Path, root: Path, trace: Path, *args: str, + path: str | None = None): + """`shape`'s doctor on `root`, every git it starts logging its trace2 + events into the folder `trace`; `path`, where given, is the `PATH` it + runs with.""" + trace.mkdir() + env = {"GIT_TRACE2_EVENT": str(trace)} + if path is not None: + env["PATH"] = path + return run_script(shape / "shape-doctor.py", "--root", str(root), *args, + env=env) + + +def events(trace: Path) -> list: + """Every trace2 event the run's gits wrote into `trace`.""" + found = [] + for log in sorted(trace.iterdir()): + for line in log.read_text(encoding="utf-8", + errors="replace").splitlines(): + found.append(json.loads(line)) + return found + + +def fetches(trace: Path) -> list: + """Each `fetch` a git of the run started, as its argv: a lazy fetch is + one, `git -c fetch.negotiationAlgorithm=noop fetch ...`.""" + return [event["argv"] for event in events(trace) + if event.get("event") == "child_start" + and "fetch" in event.get("argv", [])] + + +def gits(trace: Path) -> int: + """How many git processes the run started: one trace2 log each.""" + return len(list(trace.iterdir())) + + +@pytest.mark.parametrize("reachable", [False, True]) +@pytest.mark.parametrize("filter_spec", ["blob:none", "tree:0"]) +def test_every_row_from_a_partial_clone_of_the_standard_starts_no_fetch( + moved, project, tmp_path, filter_spec, reachable): + """Every row, asked of a partial clone of C2 behind the pin at C1: + no git of the run started a fetch, and the clone lacks what it lacked. + The trace saw the run's gits, so "none" is an answer.""" + clone = partial_clone_of(moved["ahead"], tmp_path / "partial", + filter_spec) + if not reachable: + out_of_reach(clone, tmp_path) + before = missing(clone) + result = traced(clone, project, tmp_path / "trace", "--json") + assert result.returncode == 3, result.stdout + result.stderr + assert gits(tmp_path / "trace") > 1, "the trace saw no git" + assert fetches(tmp_path / "trace") == [], "a git of the run fetched" + assert missing(clone) == before, "the doctor fetched" + + +def without_the_variable(tmp_path: Path) -> str: + """A `PATH` whose first `git` unsets `GIT_NO_LAZY_FETCH` and runs the + real one: every git the doctor starts runs as before #787.""" + shim = tmp_path / "shim" + shim.mkdir() + script = shim / "git" + script.write_text("#!/bin/sh\nunset GIT_NO_LAZY_FETCH\nexec " + f"{shlex.quote(shutil.which('git'))} \"$@\"\n", + encoding="utf-8") + script.chmod(0o755) + return f"{shim}{os.pathsep}{os.environ['PATH']}" + + +def clone_of_the_standard(kind: str, standard: Path, moved: dict, + tmp_path: Path) -> Path: + """The standard itself, a full clone, or a partial clone of C2 behind + the pin with its remote out of reach, by `kind`.""" + if kind == "full": + return standard + return out_of_reach(partial_clone_of(moved["ahead"], tmp_path / "partial", + kind), tmp_path) + + +@pytest.mark.skipif(os.name == "nt", reason="the git shim is a sh script") +@pytest.mark.parametrize("args", [["--json"], []]) +@pytest.mark.parametrize("kind", ["full", "blob:none", "tree:0"]) +def test_the_report_is_the_same_byte_for_byte_with_the_variable_ignored( + standard, moved, project, tmp_path, kind, args): + """The doctor's report, `--json` and the table, as it is and with every + git it starts run without `GIT_NO_LAZY_FETCH`: the same bytes, the + same exit. A full clone lacks nothing, and every read of a partial + clone of the standard is asked first, so the variable changes no row + there.""" + shape = clone_of_the_standard(kind, standard, moved, tmp_path) + now = traced(shape, project, tmp_path / "now", *args) + before = traced(shape, project, tmp_path / "before", *args, + path=without_the_variable(tmp_path)) + assert now.stdout == before.stdout + assert now.stderr == before.stderr + assert now.returncode == before.returncode + + +@pytest.fixture +def broken_pin_partial_root(project, tmp_path, request) -> Path: + """A partial clone, under `request.param`'s filter, of a root whose + HEAD commit broke its pin: the pin before it, which `pin_restore` puts + back, is a blob (and in `tree:0`, under trees) the clone lacks.""" + (project / SHAPE_PIN).write_bytes(b"files: 5\n") + git("commit", "-q", "-am", "A pin that does not read", cwd=project) + bare = tmp_path / "project.git" + git(*FILE_PROTOCOL, "clone", "-q", "--bare", "--no-local", + project.as_uri(), str(bare), cwd=tmp_path) + git("config", "uploadpack.allowFilter", "true", cwd=bare) + return partial_clone_of(bare, tmp_path / "partial-root", request.param) + + +@pytest.mark.parametrize("broken_pin_partial_root", ["blob:none", "tree:0"], + indirect=True) +def test_a_partial_root_whose_pin_history_is_not_here_names_the_fetch( + standard, broken_pin_partial_root, tmp_path): + """The pin's history is not in the partial clone of the root: the + doctor fetched it before #787, to name the commit that broke the pin. + It starts no fetch, `restore` is `partial`, and the NEXT is the fetch + of the root's promisor remote; run as printed, the doctor run again + names the pin to put back, `committed`.""" + root = broken_pin_partial_root + result = traced(standard, root, tmp_path / "trace", "--json") + row = currency_of(result) + assert fetches(tmp_path / "trace") == [], "a git of the run fetched" + assert row["detail"]["restore"] == "partial", row + assert pasted(row["next"]) == [ + "git", "-C", str(root.resolve()), "fetch", "--refetch", + "--no-filter", "origin"], row["next"] + run_pasted(row["next"]) + again = currency_of(traced(standard, root, tmp_path / "again", "--json")) + assert again["detail"]["restore"] == "committed", again + + +@pytest.mark.parametrize("before", [None, "0"]) +def test_lazy_fetch_off_sets_the_variable_and_puts_it_back( + standard, monkeypatch, before): + """`lazy_fetch_off`, which `main` runs under: `GIT_NO_LAZY_FETCH=1` + inside, and outside the environment as it was, unset or not, for a + caller that runs `main` in its own process.""" + module = doctor_module(standard) + if before is None: + monkeypatch.delenv(NO_LAZY_FETCH, raising=False) + else: + monkeypatch.setenv(NO_LAZY_FETCH, before) + with module.lazy_fetch_off(): + inside = os.environ.get(NO_LAZY_FETCH) + assert inside == "1" + assert os.environ.get(NO_LAZY_FETCH) == before From 196654207d1125bf3de3649ce56f414bb386070c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 09:30:02 +0000 Subject: [PATCH 5/6] shape-doctor.py: `history_left_out` sits with the partial-clone readers, after `remotes_holding`, so #791 merges with main, whose `other_sides_copies` (bf1f099b) took the place after `pin_restore` (#787) A move, and nothing else: every line removed is added back unchanged. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 68 ++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 29f7c82a..efc0f100 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -4069,40 +4069,6 @@ def pin_restore(ctx: Context, detail: dict | None = None, return "none", None, None -def history_left_out(root: Path, pin: str) -> str | None: - """The first promisor remote of `root`, where `root` is a partial clone - that lacks an object `pin_restore`'s walk of `pin`'s history reads, - asked without fetching; None where it lacks none, or is no partial - clone (#787). - - THE WALK'S READS FAIL THERE, AND THE DOCTOR FETCHED THEM BEFORE. - Measured on #787 with git 2.43: a `--filter=blob:none` clone of a root - whose HEAD commit broke its pin made one lazy fetch, of the pin before - it, and a `--filter=tree:0` one two. With `GIT_NO_LAZY_FETCH` set - (`lazy_fetch_off`) each read fails instead, and the walk, which reads a - failure as "no pin there", said "none": no commit in this root holds a - pin to put back, which is not so. - - `rev-list --objects --missing=print` over the commits that touched - `pin`, one more than the walk reads, lists the pin at each and the - trees down to it, and prints `?` for each the clone does not hold, - without fetching (`objects_lacking` says why). Where the clone lacks a - tree, as a `tree:0` one does, it cannot tell which commits touched the - pin and fails, as the walk's own `rev-list` does: that is lacking too. - A root with no HEAD has no history to walk, and is not asked. - """ - remotes = promisor_remotes(root) - if not remotes or head_of(root) is None: - return None - listed = git_answer(["rev-list", "--objects", "--missing=print", "-n", - str(PIN_HISTORY_DEPTH + 1), "HEAD", "--", pin], - root, binary=True) - if listed is not None and not any( - MISSING_OBJECT.fullmatch(line) for line in listed.split(b"\n")): - return None - return remotes[0] - - def taken_with_the_pin(ctx: Context, kind, copies: list, stages: dict) -> list: """Which of `copies`, conflicted beside the shape pin, `pin_restore` @@ -4830,6 +4796,40 @@ def remotes_holding(shape: Path, remotes: list, commit: str) -> list: f"refs/remotes/{name}/"], shape)] +def history_left_out(root: Path, pin: str) -> str | None: + """The first promisor remote of `root`, where `root` is a partial clone + that lacks an object `pin_restore`'s walk of `pin`'s history reads, + asked without fetching; None where it lacks none, or is no partial + clone (#787). + + THE WALK'S READS FAIL THERE, AND THE DOCTOR FETCHED THEM BEFORE. + Measured on #787 with git 2.43: a `--filter=blob:none` clone of a root + whose HEAD commit broke its pin made one lazy fetch, of the pin before + it, and a `--filter=tree:0` one two. With `GIT_NO_LAZY_FETCH` set + (`lazy_fetch_off`) each read fails instead, and the walk, which reads a + failure as "no pin there", said "none": no commit in this root holds a + pin to put back, which is not so. + + `rev-list --objects --missing=print` over the commits that touched + `pin`, one more than the walk reads, lists the pin at each and the + trees down to it, and prints `?` for each the clone does not hold, + without fetching (`objects_lacking` says why). Where the clone lacks a + tree, as a `tree:0` one does, it cannot tell which commits touched the + pin and fails, as the walk's own `rev-list` does: that is lacking too. + A root with no HEAD has no history to walk, and is not asked. + """ + remotes = promisor_remotes(root) + if not remotes or head_of(root) is None: + return None + listed = git_answer(["rev-list", "--objects", "--missing=print", "-n", + str(PIN_HISTORY_DEPTH + 1), "HEAD", "--", pin], + root, binary=True) + if listed is not None and not any( + MISSING_OBJECT.fullmatch(line) for line in listed.split(b"\n")): + return None + return remotes[0] + + def check_shape_currency(ctx: Context) -> Row: """`update-shape.py check`, summarised, against THIS checkout. From 1d774ab353197f65c5debce0675d3e581c78f810 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 10:47:04 +0000 Subject: [PATCH 6/6] shape-doctor.py: the `partial` `why` says the pin's history, HEAD's included, is not all in the clone, and spells out the fetch from each other promisor remote; a test records that every git a doctor run starts, its validators' and `Upstream`'s included, has `GIT_NO_LAZY_FETCH=1` (#787, RV-f's NITs on #791) - N1: "which commit broke the pin cannot be read here" was not so where HEAD was moved back without a checkout. There the doctor run after the fetch answers `uncommitted`. The `why` now says "the pin's history, HEAD's included, is not all in this clone" (`partial_history_why`), and a `reset --soft` case is tested. - N2: `test_every_git_the_run_starts_has_the_variable` puts a recording `git` first on `PATH`. Every git the run starts has the variable: `validate-pins.py`'s, started by `run_validator`, and `Upstream.blob`'s `cat-file blob` as the doctor drives it in-process. - N3: `history_left_out` returns every promisor remote. The `why` spells out the same fetch from each one after the first, where the history is on another remote's branches alone. Tested with `origin` naming another repository: the NEXT leaves the row the same, and the `why`'s `second` line ends it. Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 87 +++++++++++++++------- tests/test_doctor_no_lazy_fetch.py | 112 +++++++++++++++++++++++++---- 2 files changed, 159 insertions(+), 40 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index efc0f100..8d945611 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3847,14 +3847,17 @@ def pin_restore(ctx: Context, detail: dict | None = None, whose parent holds the OLD pin -- is still the one to put back, and the `why` says what comes after it: `gitlink_left_behind` (#547). - partial the walk above found no pin to put back, and the root is - a partial clone (`git clone --filter`) that lacks a pin - or a tree the walk reads (`history_left_out`, #787). - This doctor fetches nothing, so the read failed and it - cannot say which commit broke the pin, or that none - did. `fetch --refetch --no-filter` from the first - promisor remote puts in every object on its branches, - and the doctor run again walks the whole history. + partial the reads above found no pin to put back, and the root + is a partial clone (`git clone --filter`) that lacks a + pin or a tree they read, HEAD's included + (`history_left_out`, #787). This doctor fetches nothing, + so those reads failed, and what puts the pin back, a + commit or HEAD's own pin, cannot be read here. `fetch + --refetch --no-filter` from the first promisor remote + puts in every object on its branches, and the doctor run + again reads the whole history; the `why` spells out that + fetch from each other promisor remote + (`partial_history_why`). none nothing above: no history, a root commit, no git, a walk that meets a parent holding no pin, or goes `PIN_HISTORY_DEPTH` commits back, before a pin that reads. @@ -4056,16 +4059,10 @@ def pin_restore(ctx: Context, detail: dict | None = None, f"broke it, and the newest {what} in its history that reads. " f"Commit that rather than editing the {what}" + gitlink_left_behind(ctx, before, pin)) - remote = history_left_out(root, pin) - if remote is not None: - return "partial", ( - f"{git_c} fetch --refetch --no-filter {quote_arg(remote)}" - ), (f"this root is a partial clone, and the {what}'s history is not " - "all in it. git fetches what a partial clone lacks on a read, " - "which this doctor never does, so which commit broke the " - f"{what} cannot be read here. This fetches every object on " - f"{remote}'s branches (--refetch needs git 2.36 or later); then " - f"run this doctor again, which names the {what} to put back") + remotes = history_left_out(root, pin) + if remotes: + return "partial", refetch_of(git_c, remotes[0]), partial_history_why( + git_c, what, remotes) return "none", None, None @@ -4796,11 +4793,49 @@ def remotes_holding(shape: Path, remotes: list, commit: str) -> list: f"refs/remotes/{name}/"], shape)] -def history_left_out(root: Path, pin: str) -> str | None: - """The first promisor remote of `root`, where `root` is a partial clone - that lacks an object `pin_restore`'s walk of `pin`'s history reads, - asked without fetching; None where it lacks none, or is no partial - clone (#787). +def refetch_of(git_c: str, remote: str) -> str: + """The fetch of every object on `remote`'s branches into the root + `git_c` names, whatever its `partialclonefilter` (#787).""" + return f"{git_c} fetch --refetch --no-filter {quote_arg(remote)}" + + +def partial_history_why(git_c: str, what: str, remotes: list) -> str: + """`pin_restore`'s `why` for `partial` (#787): the root's partial clone + lacks the history of its `what`, the NEXT fetches the first of + `remotes`, and the same fetch from each other one is spelled out. + + HISTORY, HEAD'S INCLUDED, AND NOT "THE COMMIT THAT BROKE IT" (RV-f on + #791). Where HEAD was moved without a checkout (`reset --soft`), the + pin HEAD holds is a blob the clone lacks too, and the doctor run after + the fetch answers `uncommitted`, which no commit broke. + + EVERY PROMISOR REMOTE, EACH A LINE TO PASTE. The NEXT fetches the first + one's branches, the one git's own lazy fetch asks first. Where the + history lacked is on another remote's alone, or on a branch forced + away from the first, that fetch puts in nothing it needs and the row + comes back the same, so the `why` names the fetch from each other one. + """ + first, *others = remotes + said = (f"this root is a partial clone, and the {what}'s history, " + "HEAD's included, is not all in this clone: git fetches what a " + "partial clone lacks on a read, which this doctor never does, " + f"so what puts the {what} back cannot be read here. This " + f"fetches every object on {first}'s branches (--refetch needs " + "git 2.36 or later)") + if others: + said += (f"; where {first}'s branches do not hold it, the same " + "from " + ", or from ".join( + f"{name}: `{refetch_of(git_c, name)}`" + for name in others)) + return (said + "; then run this doctor again, which names what puts " + f"the {what} back") + + +def history_left_out(root: Path, pin: str) -> list: + """The promisor remotes of `root`, in the order git asks them, where + `root` is a partial clone that lacks an object `pin_restore`'s walk of + `pin`'s history reads, asked without fetching; empty where it lacks + none, or is no partial clone (#787). THE WALK'S READS FAIL THERE, AND THE DOCTOR FETCHED THEM BEFORE. Measured on #787 with git 2.43: a `--filter=blob:none` clone of a root @@ -4820,14 +4855,14 @@ def history_left_out(root: Path, pin: str) -> str | None: """ remotes = promisor_remotes(root) if not remotes or head_of(root) is None: - return None + return [] listed = git_answer(["rev-list", "--objects", "--missing=print", "-n", str(PIN_HISTORY_DEPTH + 1), "HEAD", "--", pin], root, binary=True) if listed is not None and not any( MISSING_OBJECT.fullmatch(line) for line in listed.split(b"\n")): - return None - return remotes[0] + return [] + return remotes def check_shape_currency(ctx: Context) -> Row: diff --git a/tests/test_doctor_no_lazy_fetch.py b/tests/test_doctor_no_lazy_fetch.py index 0fbe0dde..cc522c5b 100644 --- a/tests/test_doctor_no_lazy_fetch.py +++ b/tests/test_doctor_no_lazy_fetch.py @@ -26,6 +26,7 @@ import json import os +import re import shlex import shutil from pathlib import Path @@ -35,6 +36,7 @@ from conftest import FILE_PROTOCOL, git, run_script from test_doctor_partial_clone import ( # noqa: F401 currency_of, missing, moved, out_of_reach, partial_clone_of, + second_promisor, ) from test_shape_doctor import ( # noqa: F401 SHAPE_PIN, doctor_module, pasted, project, run_pasted, scaffolded_here, @@ -142,42 +144,124 @@ def test_the_report_is_the_same_byte_for_byte_with_the_variable_ignored( assert now.returncode == before.returncode -@pytest.fixture -def broken_pin_partial_root(project, tmp_path, request) -> Path: - """A partial clone, under `request.param`'s filter, of a root whose - HEAD commit broke its pin: the pin before it, which `pin_restore` puts - back, is a blob (and in `tree:0`, under trees) the clone lacks.""" +def broken_pin_remote(project: Path, tmp_path: Path) -> Path: + """A bare copy of `project`, filters allowed, whose HEAD commit broke + its pin: a partial clone of it lacks the pin before that commit, which + `pin_restore` puts back, and in `tree:0` the trees down to it.""" (project / SHAPE_PIN).write_bytes(b"files: 5\n") git("commit", "-q", "-am", "A pin that does not read", cwd=project) bare = tmp_path / "project.git" git(*FILE_PROTOCOL, "clone", "-q", "--bare", "--no-local", project.as_uri(), str(bare), cwd=tmp_path) git("config", "uploadpack.allowFilter", "true", cwd=bare) - return partial_clone_of(bare, tmp_path / "partial-root", request.param) + return bare -@pytest.mark.parametrize("broken_pin_partial_root", ["blob:none", "tree:0"], - indirect=True) +#: Each partial clone of `broken_pin_remote` asked about: its filter, +#: whether its HEAD is then moved back one commit without a checkout +#: (`reset --soft`, RV-f on #791), and what the doctor run after the +#: fetch says puts the pin back. +PARTIAL_ROOTS = { + "blob:none": ("blob:none", False, "committed"), + "tree:0": ("tree:0", False, "committed"), + "HEAD moved back by reset --soft": ("blob:none", True, "uncommitted"), +} + + +@pytest.mark.parametrize("case", list(PARTIAL_ROOTS)) def test_a_partial_root_whose_pin_history_is_not_here_names_the_fetch( - standard, broken_pin_partial_root, tmp_path): + standard, project, tmp_path, case): """The pin's history is not in the partial clone of the root: the - doctor fetched it before #787, to name the commit that broke the pin. - It starts no fetch, `restore` is `partial`, and the NEXT is the fetch - of the root's promisor remote; run as printed, the doctor run again - names the pin to put back, `committed`.""" - root = broken_pin_partial_root + doctor fetched it before #787. It starts no fetch, `restore` is + `partial`, the `why` says the history, HEAD's included, is not all + here, and the NEXT is the fetch of the root's promisor remote; run as + printed, the doctor run again names what puts the pin back: the + commit before the one that broke it, or, where HEAD was moved back + without a checkout, HEAD's own pin (`uncommitted`).""" + filter_spec, moved_back, answer = PARTIAL_ROOTS[case] + root = partial_clone_of(broken_pin_remote(project, tmp_path), + tmp_path / "partial-root", filter_spec) + if moved_back: + git("reset", "-q", "--soft", "HEAD~1", cwd=root) result = traced(standard, root, tmp_path / "trace", "--json") row = currency_of(result) assert fetches(tmp_path / "trace") == [], "a git of the run fetched" assert row["detail"]["restore"] == "partial", row + assert ("the pin's history, HEAD's included, is not all in this " + "clone") in row["why"], row["why"] assert pasted(row["next"]) == [ "git", "-C", str(root.resolve()), "fetch", "--refetch", "--no-filter", "origin"], row["next"] run_pasted(row["next"]) again = currency_of(traced(standard, root, tmp_path / "again", "--json")) + assert again["detail"]["restore"] == answer, again + + +def test_the_why_spells_out_the_fetch_from_each_other_promisor_remote( + standard, moved, project, tmp_path): + """The pin's history is on the second promisor remote alone: `origin` + now names another repository (RV-f on #791, N3). The NEXT fetches + `origin`, and run as printed the row comes back the same; the `why` + spells out that fetch from `second`, and run as printed, the doctor + names the commit that puts the pin back.""" + bare = broken_pin_remote(project, tmp_path) + root = partial_clone_of(bare, tmp_path / "partial-root") + git("remote", "set-url", "origin", moved["ahead"].as_uri(), cwd=root) + second_promisor(root, bare, fetched=False) + row = currency_of(traced(standard, root, tmp_path / "trace", "--json")) + run_pasted(row["next"]) + same = currency_of(traced(standard, root, tmp_path / "same", "--json")) + assert same["detail"]["restore"] == "partial", same + offered = re.search(r"from second: `([^`]+)`", same["why"]) + assert offered, same["why"] + run_pasted(offered.group(1)) + again = currency_of(traced(standard, root, tmp_path / "again", "--json")) assert again["detail"]["restore"] == "committed", again +def recording(tmp_path: Path, log: Path) -> str: + """A `PATH` whose first `git` appends to `log`, for each git started, + its `GIT_NO_LAZY_FETCH` (`unset` where it has none), the command line + of the process that started it and its own arguments, a tab between + each, then runs the real one.""" + shim = tmp_path / "recording" + shim.mkdir() + script = shim / "git" + script.write_text( + "#!/bin/sh\n" + "printf '%s\\t%s\\t%s\\n' \"${GIT_NO_LAZY_FETCH-unset}\" " + "\"$(ps -o args= -p \"$PPID\")\" \"$*\" >> " + + shlex.quote(str(log)) + "\n" + "exec " + shlex.quote(shutil.which("git")) + " \"$@\"\n", + encoding="utf-8") + script.chmod(0o755) + return f"{shim}{os.pathsep}{os.environ['PATH']}" + + +@pytest.mark.skipif(os.name == "nt", reason="the git shim is a sh script") +def test_every_git_the_run_starts_has_the_variable( + standard, project, tmp_path): + """Every git a doctor run starts, logged by a `git` first on `PATH` + (RV-f on #791, N2), has `GIT_NO_LAZY_FETCH=1`: the doctor's own, those + of `update-shape.py`'s `Upstream` as the doctor drives it in its own + process (`cat-file blob :` of the standard's copies), + and those of a validator `run_validator` starts with a copy of its + environment (`validate-pins.py`).""" + log = tmp_path / "gits.log" + result = run_script(standard / "shape-doctor.py", "--root", str(project), + "--json", env={"PATH": recording(tmp_path, log)}) + assert result.returncode == 0, result.stdout + result.stderr + gits_started = [line.split("\t", 2) for line in + log.read_text(encoding="utf-8").splitlines()] + assert {value for value, _parent, _args in gits_started} == {"1"}, \ + gits_started + assert any("validate-pins.py" in parent + for _value, parent, _args in gits_started), gits_started + assert any("shape-doctor.py" in parent and args.startswith("cat-file blob") + and ":templates/" in args + for _value, parent, args in gits_started), gits_started + + @pytest.mark.parametrize("before", [None, "0"]) def test_lazy_fetch_off_sets_the_variable_and_puts_it_back( standard, monkeypatch, before):