diff --git a/shape-doctor.py b/shape-doctor.py index 7c2dc8b2..c8f35d95 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: @@ -346,6 +348,7 @@ from __future__ import annotations import argparse +import contextlib import datetime as _dt import errno import importlib.util @@ -4466,6 +4469,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 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. @@ -4701,6 +4715,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)) + 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 @@ -5856,6 +5874,78 @@ def remotes_holding(shape: Path, remotes: list, commit: str) -> list: f"refs/remotes/{name}/"], shape)] +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 + 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 [] + 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 [] + return remotes + + def check_shape_currency(ctx: Context) -> Row: """`update-shape.py check`, summarised, against THIS checkout. @@ -11435,6 +11525,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..cc522c5b --- /dev/null +++ b/tests/test_doctor_no_lazy_fetch.py @@ -0,0 +1,279 @@ +# 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 re +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, + second_promisor, +) +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 + + +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 bare + + +#: 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, project, tmp_path, case): + """The pin's history is not in the partial clone of the root: the + 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): + """`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