From fe74d018fc98518113c953b63433bdc77142d1d1 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 12:58:31 +0000 Subject: [PATCH 01/17] checkpoint: adopt-project.py: `check` asks the split's own `git clone` of the source in the work directory, and `_resolved` and `_answers` make their repositories as #910's throwaways are made (#1083) Work in progress for #1083, stopped at a usage checkpoint. Built: `_split_clone_asked` (asked first in `_entry_findings`; a failure is `adopt-git-unasked`, which `execute` refuses before any leg); `_resolved` and `_answers` made with `_throwaway`/`_throwaway_made` in the work directory, no hooks, `--template=`, `-b`, the source's format, under `_inherited_env()`; `_throwaway_git`; `_submodule_findings` skips the url questions once one throwaway could not be asked. 16 rows added or changed in tests/test_adopt_check_throwaway_repos.py (45 rows: all pass on this head; 14 fail and 31 pass over 2a119f28's adopt-project.py). Left: the TMPDIR docstring in tests/test_adopt_reencoded_files.py, the gate, and the PR body. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 290 +++++++++---- tests/test_adopt_check_throwaway_repos.py | 477 ++++++++++++++++++++-- 2 files changed, 664 insertions(+), 103 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 169ad39f..98114b0e 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -107,7 +107,9 @@ a topic they are. A path of the source commit git refuses as a path, `..` or `.Git/x`, is `plan-path-git-refuses`. Its questions to git are asked in throwaway repositories in the work - directory. Each but #183's symlink probe is made with the + directory, the first the split's own `git clone` of the source, + and the submodule urls' among them (#1083). Each but #183's + symlink probe is made with the source's object format and runs no hook of this machine's, and one git cannot make or ask there is `adopt-git-unasked`, which says why (#910). The symlink probe keeps its plain `git init` @@ -7653,61 +7655,88 @@ def _default_remote(source: Source) -> tuple[str, str, Path | None]: return url, f"its remote {_spelled(name)}", None -def _config_value(repository: Path, key: str) -> str | None: +def _config_value(repository: Path, key: str, + env: dict | None = None) -> str | None: """`key` in `repository`'s git config, as git reads it -- the last value, when there are several; None when unset. The key as the bytes it spells (`_path_argument`): a remote's name in it is one git printed, and as text `remote.\\u00dcrsprung.url` raised `UnicodeEncodeError` - under `LC_ALL=C PYTHONUTF8=0` (#653).""" + under `LC_ALL=C PYTHONUTF8=0` (#653). In `env`, given one, else in this + process's environment.""" got = subprocess.run(["git", GIT_CONFIG, "-z", "--get", _path_argument(key)], - cwd=repository, capture_output=True, check=False) + cwd=repository, env=env, capture_output=True, + check=False) return None if got.returncode else _git_path(got.stdout.rstrip(b"\x00")) def _resolved(base: str, urls: list[str], - working_tree: Path | None = None) -> list[_Resolution]: + working_tree: Path | None = None, + source: Source | None = None) -> list[_Resolution]: """Each of `urls` as `git submodule init` resolves it in a repository whose `origin` is `base`, in a scratch repository made for the purpose and removed after it; and, given a `working_tree`, git's relative answer joined to it, which is what the clone opens (`_default_remote`). A url git cannot resolve is what git said: that is where the clone fails. - `base` and each url reach git as the bytes they spell (`_run_exact`): + `base` and each url reach git as the bytes they spell (`_path_argument`): as text, `../\\u00dcnterlagen` raised `UnicodeEncodeError` under - `LC_ALL=C PYTHONUTF8=0`, and `check` and `execute` died (#653).""" - scratch = Path(tempfile.mkdtemp(prefix="openreposhape-url-")) + `LC_ALL=C PYTHONUTF8=0`, and `check` and `execute` died (#653). + + THE SCRATCH REPOSITORY IS ONE OF `check`'S THROWAWAYS (#1083), made as + #910 makes the others (`_throwaway_made`): in the work directory `source` + was opened in, with its object format, on `THROWAWAY_BRANCH`, from an + empty template, under `_inherited_env()`, and with no hook of this + machine's, whose `post-index-change` its `update-index` ran. Its plain + `git init` in `TMPDIR` stopped `check` with "a git or gh command + failed", exit 2, under an `init.defaultBranch` git will not take or a + `GIT_DEFAULT_HASH` that names no hash. `_Unasked` where git cannot make + or ask it there.""" + scratch = _throwaway("openreposhape-url-", source) + repository = scratch / THROWAWAY_REPOSITORY + env = _inherited_env() try: - run(["git", "init", "-q", str(scratch)]) - _run_exact(["git", GIT_CONFIG, "remote.origin.url", base], scratch) - for index, url in enumerate(urls): - for key, value in (("path", f"u{index}"), ("url", url)): - _run_exact(["git", GIT_CONFIG, "-f", GITMODULES, - f"submodule.u{index}.{key}", value], scratch) - # A gitlink names a commit nobody fetches here; any object id of the - # repository's own format will do, and this one is at hand. - oid = run(["git", "hash-object", GITMODULES], cwd=scratch) - for index in range(len(urls)): - run(["git", "update-index", "--add", "--cacheinfo", - f"{GITLINK_MODE},{oid},u{index}"], cwd=scratch) - # One `init` per url, because git stops at the first it cannot - # resolve and each finding names its own. - return [_resolution_of(scratch, f"u{index}", working_tree) - for index in range(len(urls))] + hooks_off = _throwaway_made(scratch, repository, source, env) + with _asked_there(scratch.parent): + _throwaway_git(scratch, hooks_off, [ + GIT_CONFIG, "remote.origin.url", _path_argument(base)], env) + for index, url in enumerate(urls): + for key, value in (("path", f"u{index}"), ("url", url)): + _throwaway_git(scratch, hooks_off, [ + GIT_CONFIG, "-f", GITMODULES, + f"submodule.u{index}.{key}", _path_argument(value)], + env) + # A gitlink names a commit nobody fetches here; any object id of + # the repository's own format will do, and this one is at hand. + oid = _throwaway_git(scratch, hooks_off, + ["hash-object", GITMODULES], env) + for index in range(len(urls)): + _throwaway_git(scratch, hooks_off, [ + "update-index", "--add", "--cacheinfo", + f"{GITLINK_MODE},{oid},u{index}"], env) + # One `init` per url, because git stops at the first it cannot + # resolve and each finding names its own. + return [_resolution_of(repository, f"u{index}", working_tree, + hooks_off, env) + for index in range(len(urls))] finally: - shutil.rmtree(scratch, ignore_errors=True) + _throwaway_removed(scratch) -def _resolution_of(scratch: Path, name: str, - working_tree: Path | None) -> _Resolution: +def _resolution_of(scratch: Path, name: str, working_tree: Path | None, + hooks_off: list[str] | tuple = (), + env: dict | None = None) -> _Resolution: """`name`'s url, as `git submodule init` copies it into the config, and - joined to `working_tree` when there is one.""" - init = subprocess.run(["git", "submodule", "init", "-q", "--", name], - cwd=scratch, capture_output=True, check=False) + joined to `working_tree` when there is one. `hooks_off` and `env` are + `_resolved`'s, for its throwaway repository `scratch` (#1083).""" + init = subprocess.run(["git", *hooks_off, "submodule", "init", "-q", + "--", name], + cwd=scratch, env=env, capture_output=True, + check=False) if init.returncode: return _Resolution(None, _one_line(init.stderr.decode( errors="replace"))) - url = _config_value(scratch, f"submodule.{name}.url") + url = _config_value(scratch, f"submodule.{name}.url", env) if working_tree is None or url is None: return _Resolution(url) # Not normalised: `..` after a symlink is the disk's to resolve, and @@ -7750,8 +7779,8 @@ def _relative_url_findings(plan: Plan, source: Source, tree: list, urls = [url for _, _, url in kept] found, alike, from_the_leg = [], [], {} for (path, entry, url), here, there in zip( - kept, _resolved(base, urls, working_tree), - _resolved(remote, urls)): + kept, _resolved(base, urls, working_tree, source), + _resolved(remote, urls, None, source)): if here.same_as(there): alike.append(f"{_spelled(url)} of {_spelled(path)}, {there}") from_the_leg[path] = (remote, there.url) @@ -7854,7 +7883,8 @@ def _on_this_disk(url: str) -> bool: os.name == "nt" or DRIVE_RE.match(path) is None) -def _answers(urls: list[str]) -> list[tuple[str, str | None]]: +def _answers(urls: list[str], source: Source | None = None + ) -> list[tuple[str, str | None]]: """`(expanded, said)` for each of `urls`: the url as git here expands it (`git ls-remote --get-url`, which contacts nothing) and, where that is on this machine's disk, None when `git ls-remote` reaches a repository @@ -7865,31 +7895,38 @@ def _answers(urls: list[str]) -> list[tuple[str, str | None]]: its questions, so that no repository's own config around this process rewrites a url as the assembly's clone would not; and under ON_THIS_DISK_ONLY, so that nothing a config could do sends git to a - network. Each url as the bytes it spells (`_path_argument`, #653).""" - scratch = Path(tempfile.mkdtemp(prefix="openreposhape-ask-")) - env = {**os.environ, **ON_THIS_DISK_ONLY} + network. Each url as the bytes it spells (`_path_argument`, #653). + That repository is one of `check`'s throwaways, made as `_resolved` + makes its own (#1083): in the work directory `source` was opened in, + with no hook and under `_inherited_env()`, which leaves out a `GIT_DIR` + or a `GIT_CONFIG_PARAMETERS` around this process; `_Unasked` where git + cannot make it there.""" + scratch = _throwaway("openreposhape-ask-", source) + repository = scratch / THROWAWAY_REPOSITORY + env = {**_inherited_env(), **ON_THIS_DISK_ONLY} try: - run(["git", "init", "-q", str(scratch)]) + hooks_off = _throwaway_made(scratch, repository, source, env) out = [] - for url in urls: - got = subprocess.run(["git", "ls-remote", "--get-url", "--", - _path_argument(url)], - cwd=scratch, env=env, capture_output=True, - check=False) - expanded = ("" if got.returncode else - _git_path(got.stdout.rstrip(b"\r\n"))) or url - if not _on_this_disk(expanded): - out.append((expanded, None)) - continue - listed = subprocess.run(["git", "ls-remote", "--", - _path_argument(url)], - cwd=scratch, env=env, capture_output=True, - check=False) - out.append((expanded, None if not listed.returncode - else _first_words(listed))) + with _asked_there(scratch.parent): + for url in urls: + got = subprocess.run(["git", *hooks_off, "ls-remote", + "--get-url", "--", _path_argument(url)], + cwd=repository, env=env, + capture_output=True, check=False) + expanded = ("" if got.returncode else + _git_path(got.stdout.rstrip(b"\r\n"))) or url + if not _on_this_disk(expanded): + out.append((expanded, None)) + continue + listed = subprocess.run(["git", *hooks_off, "ls-remote", "--", + _path_argument(url)], + cwd=repository, env=env, + capture_output=True, check=False) + out.append((expanded, None if not listed.returncode + else _first_words(listed))) return out finally: - shutil.rmtree(scratch, ignore_errors=True) + _throwaway_removed(scratch) def _first_words(failed: subprocess.CompletedProcess) -> str: @@ -7906,7 +7943,8 @@ def _first_words(failed: subprocess.CompletedProcess) -> str: def _unanswered_url_findings(plan: Plan, tree: list, registered: Registrations | str, - from_the_leg: dict[str, tuple[str, str]] + from_the_leg: dict[str, tuple[str, str]], + source: Source | None = None ) -> tuple[list[str], list[str]]: """`(findings, notes)` for the url of each submodule the plan keeps in the leg its `.gitmodules` goes to (`_kept_urls`), as that leg resolves @@ -7917,7 +7955,9 @@ def _unanswered_url_findings(plan: Plan, tree: list, is its own resolution, and a relative one not in it is that function's finding and is not asked. The NOTE is the same whichever remotes a relative url was resolved from: a url on a network is not asked under - `--local-remote-dir` either, and `check` given one names it (#417).""" + `--local-remote-dir` either, and `check` given one names it (#417). + `source` is the one `check` opened, in whose work directory `_answers` + asks (#1083).""" leg, kept = _kept_urls(plan.entries, tree, registered) candidates = [] for path, entry, url in kept: @@ -7927,7 +7967,7 @@ def _unanswered_url_findings(plan: Plan, tree: list, candidates.append((path, entry, url, *from_the_leg[path])) on_disk = [each for each in candidates if _on_this_disk(each[4])] answers = dict(zip([each[0] for each in on_disk], - _answers([each[4] for each in on_disk]))) \ + _answers([each[4] for each in on_disk], source))) \ if on_disk else {} found, unasked = [], [] for path, entry, url, remote, there in candidates: @@ -7989,9 +8029,11 @@ def _answers_nothing(path: str, entry: str, leg: str, url: str, def _submodule_findings(plan: Plan, source: Source, tree: list, - local_remote_dir: Path | None = None + local_remote_dir: Path | None = None, + unasked: _Unasked | None = None ) -> tuple[tuple[list[str], list[str]], - tuple[list[str], list[str]], list[str]]: + tuple[list[str], list[str]], list[str], + _Unasked | None]: """`(findings, remediation)` for `submodule_plan_problems`, one FINDING each and SUBMODULE_REMEDIATION when there is one; the same for the urls of the submodules a leg keeps, each `_relative_url_findings` one and @@ -8001,19 +8043,32 @@ def _submodule_findings(plan: Plan, source: Source, tree: list, after every finding. The two come back apart because `_entry_findings` lists the urls last, after the values #255 finds. `check` is where the person is told to fix the plan, so it says how, in the words `execute`'s - refusal would use one step later.""" + refusal would use one step later. + + THE URLS ARE ASKED IN THROWAWAY REPOSITORIES (#1083), as `check`'s other + questions to git are (#910), so they are asked as those are: none where + `unasked`, `_entry_findings`' `_Unasked`, says one could not be made or + asked in the work directory; and where one cannot be, those asked + before it, and none after it. The last value is the `_Unasked` that + stands for them, `unasked` where there was one already: one cause, one + finding.""" registered = source.registered_submodules(tree) found = [f"FINDING {code}: {detail}" for code, detail in submodule_plan_problems(plan.entries, tree, registered)] - moved, notes, from_the_leg = _relative_url_findings( - plan, source, tree, registered, local_remote_dir) - unanswered, unasked = _unanswered_url_findings( - plan, tree, registered, from_the_leg) + moved, notes, unanswered, not_asked = [], [], [], [] + if unasked is None: + try: + moved, notes, from_the_leg = _relative_url_findings( + plan, source, tree, registered, local_remote_dir) + unanswered, not_asked = _unanswered_url_findings( + plan, tree, registered, from_the_leg, source) + except _Unasked as exc: + unasked = exc return ((found, [SUBMODULE_REMEDIATION] if found else []), (moved + unanswered, ([RELATIVE_URL_REMEDIATION] if moved else []) + ([URL_ANSWERS_NOTHING_REMEDIATION] if unanswered else [])), - notes + unasked) + notes + not_asked, unasked) #: The answers that never extract an entry into a leg. A path answered @@ -9290,6 +9345,10 @@ def _entry_findings(plan: Plan, source: Source, asked of git first, in a throwaway repository of its own. `check`'S THROWAWAY REPOSITORIES (#910) are made in the work directory. + The first is the split's own `git clone` of the source (#1083), asked as + `execute` makes it, so that one this git will not make is said before + any leg exists, and `_resolved` and `_answers` make the submodule urls' + own there (#1083). Each but #183's symlink probe is made with the source's object format and runs no hook of this machine's; where git cannot make or ask one, `adopt-git-unasked` says why, once, in the place of the questions asked @@ -9342,12 +9401,15 @@ def _entry_findings(plan: Plan, source: Source, """ tree = source.tree() targets = symlink_targets(source, tree) - # Which of the source's paths git holds at all, asked of git first, in a - # throwaway repository of its own: one it refuses is a fact of the - # source, said with the others (#910). Where that repository cannot be - # made or asked, neither can the others `check` makes in the same work - # directory, and `_Unasked.said` says why, once, in their place below. + # The split's own `git clone` of the source, asked first, in the work + # directory, as `execute` makes it after both legs exist (#1083); then + # which of the source's paths git holds at all, in a throwaway + # repository of its own: one it refuses is a fact of the source, said + # with the others (#910). Where either cannot be made or asked, none of + # the others `check` makes in the same work directory is asked, and + # `_Unasked.said` says why, once, in their place below. try: + _split_clone_asked(source) refused, unasked = _paths_git_refuses(source, tree), None except _Unasked as exc: refused, unasked = [], exc @@ -9362,8 +9424,8 @@ def _entry_findings(plan: Plan, source: Source, # digest the clone `plan` read it from recomputes otherwise (#449). digest_findings, pin_notes = _pin_digest_findings(plan) findings += digest_findings - (split, split_remediation), (moved, moved_remediation), notes = \ - _submodule_findings(plan, source, tree, local_remote_dir) + (split, split_remediation), (moved, moved_remediation), notes, unasked = \ + _submodule_findings(plan, source, tree, local_remote_dir, unasked) unwritable_findings, unwritable_remediation = _value_findings(plan) value_findings, value_remediation = _manifest_findings(plan) field_findings, field_remediation = _plan_field_findings(plan) @@ -10897,8 +10959,23 @@ def _refuse_symlinks_checked_out_as_files(plan: Plan, source: Source, # git will not take there (`a..b`) stops no question. Read, it stopped # them all as `adopt-git-unasked`, which `execute` refused, though the # split's `git clone` of the source does not stop at it on git 2.55, -# which CI runs (CI on #1031). On git 2.43 that clone stops at it, a -# clone `check` does not make. +# which CI runs (CI on #1031). On git 2.43 that clone stops at it. +# * THE SPLIT'S OWN CLONE (#1083). So `check` makes that clone too, first +# (`_split_clone_asked`): what this git's `git clone` of the source makes +# of this machine's settings is that clone's to say, and no `git init`'s. +# Measured on 2a119f28, with git 2.43 and `init.defaultBranch = a..b`: +# `check` said `plan ok`, and `execute` made both legs and stopped +# extracting the spec leg, `fatal: invalid branch name`, with nothing +# rolled back; `GIT_DEFAULT_HASH=bogus` stops every clone the same way. +# Asked here, either is `adopt-git-unasked`, which `execute` refuses +# before either leg, and on a git whose clone takes the name, as 2.55's +# does, it is no finding, and every question is asked. +# * THE SUBMODULE URLS' OWN (#1083). `_resolved` and `_answers` ask git how +# a kept submodule's url resolves and whether it answers, each in a +# repository of its own, made the same way here: their plain `git init` +# in `TMPDIR` stopped `check` with "a git or gh command failed" under +# either setting, and `_resolved`'s `update-index` ran this machine's +# `post-index-change` hook. #: `check`'s finding, and the code of `execute`'s refusal, for the paths of #: the source commit that this machine's git will not hold in an index, and @@ -10924,6 +11001,9 @@ def _refuse_symlinks_checked_out_as_files(plan: Plan, source: Source, #: The branch each throwaway repository's `git init` names (`-b`), so that it #: reads no `init.defaultBranch`, as the split's own `git init`s read none. THROWAWAY_BRANCH = "throwaway" +#: The repository in each throwaway directory of `_resolved`'s, `_answers`' +#: and `_split_clone_asked`'s, beside `THROWAWAY_EMPTY` (#1083). +THROWAWAY_REPOSITORY = "repository" class _Unasked(Refusal): @@ -11043,6 +11123,66 @@ def removed(remove, target, error) -> None: shutil.rmtree(scratch, **{REMOVAL_HANDLER: removed}) +def _throwaway_git(scratch: Path, hooks_off: list[str], args: list, + env: dict) -> str: + """`git ` in the throwaway repository in `scratch`, with + `hooks_off` (`_throwaway_made`'s) and in `env`: its stdout, stripped, or, + where git fails, the `_Unasked` that says so in git's words, as + `git_out` spells them (#1083). A value git spelled is handed to it as + the bytes it spells (`_path_argument`).""" + repository = scratch / THROWAWAY_REPOSITORY + done = subprocess.run(["git", *hooks_off, *args], cwd=str(repository), + env=env, capture_output=True, check=False) + if done.returncode: + raise _Unasked(scratch.parent, ( + f"`git {args[0]}` in {shown_path(repository)} exited " + f"{done.returncode}: " + spelled_as_git_out( + done.stderr.decode("utf-8", "replace").strip()))) + return done.stdout.decode("utf-8", "replace").strip() + + +def _split_clone_asked(source: Source) -> None: + """The split's own `git clone` of the source, made in a throwaway + directory in the work directory and removed again (#1083): nothing + where git makes it, and the `_Unasked` that says why, in git's words, + where it does not. + + `execute`'s first act after it makes both legs is `_extract_leg`'s `git + clone` of the source, and no question `check` asked was that clone. + git 2.43's reads `init.defaultBranch` and stops at a name it will not + take, `a..b`, and every clone stops at a `GIT_DEFAULT_HASH` naming no + hash, where each throwaway `git init`, which names its branch and the + source's format, makes its repository. So `check` said `plan ok`, and + `execute` stopped after both legs existed. Asked as that clone is made, + the answer is the split's on every version of git: with `FILE_PROTOCOL`, + as `execute` gives each of its clones, and the template this machine's + git copies in, as `_extract_leg`'s clone takes it. Left out is only what + the other questions ask, or what would run here: `--no-checkout`, as the + rehearsal asks what a checkout makes of each file and + `_paths_git_refuses` which paths git holds at all; `--shared`, so no + object is copied; and, as in every throwaway, any hook of this + machine's (`core.hooksPath` an empty directory) and the environment + `_inherited_env()` leaves out.""" + scratch = _throwaway("openreposhape-clone-", source) + try: + empty = scratch / THROWAWAY_EMPTY + with _asked_there(scratch.parent): + empty.mkdir() + cloned = subprocess.run( + ["git", *FILE_PROTOCOL, "-c", f"core.hooksPath={empty}", + "clone", "-q", "--no-checkout", "--shared", + str(source.path), str(scratch / THROWAWAY_REPOSITORY)], + cwd=str(scratch), env=_inherited_env(), capture_output=True, + check=False) + if cloned.returncode: + raise _Unasked(scratch.parent, ( + f"the split's `git clone` of the source exited " + f"{cloned.returncode} there: " + spelled_as_git_out( + cloned.stderr.decode("utf-8", "replace").strip()))) + finally: + _throwaway_removed(scratch) + + def _paths_git_refuses(source: Source, tree: list) -> list[str]: """Each path of `tree` (`Source.tree()`'s rows) that this machine's git will not hold in an index, in the tree's order (#910), asked in a diff --git a/tests/test_adopt_check_throwaway_repos.py b/tests/test_adopt_check_throwaway_repos.py index e105895b..890b174a 100644 --- a/tests/test_adopt_check_throwaway_repos.py +++ b/tests/test_adopt_check_throwaway_repos.py @@ -29,6 +29,20 @@ `plan-path-git-refuses`, a repository git cannot make or ask there is `adopt-git-unasked`, which says why, and the probe's failure stands for its finding, with a NOTE. + +#1083 followed, measured on 2a119f28 with git 2.43: + +* `check` made no clone of the source, and the split's first act after it + makes both legs is one. git 2.43's `git clone` stops at an + `init.defaultBranch` it will not take (`a..b`), and every one at a + `GIT_DEFAULT_HASH` naming no hash, so `check` said `plan ok` and + `execute` stopped after the legs existed. `check` now makes the split's + own clone first, and one this git will not make is `adopt-git-unasked`. +* `_resolved` and `_answers`, which ask how a kept submodule's url resolves + and whether it answers, made their repositories in `TMPDIR` with a plain + `git init`, so either setting stopped `check` with "a git or gh command + failed", and `_resolved`'s `update-index` ran this machine's + `post-index-change` hook. They are made as the others are now. """ from __future__ import annotations @@ -50,6 +64,9 @@ from test_adopt_nested_attributes import (KEEP_GITHUB, ROOT, ROOT_ATTRIBUTES, SOURCE, YML_CRLF, adoption_plan, committed_raw, github) +from test_adopt_relative_submodule_url import (BOTH_IN_CODE, EXTRA, SIBLING, + source_with) +from test_adopt_relative_submodule_url import planned as planned_with_urls from test_adopt_root_symlinks import (GITHUB, OUTSIDE_CI, WORKFLOW, climbs, finding, symlink_findings) from test_adopt_root_symlinks import links_as_links # noqa: F401 @@ -68,9 +85,13 @@ #: How `plan-file-reencoded` begins for the one CRLF file the plan keeps. KEPT_CRLF = ("the plan keeps 1 file in the assembly root, and this machine's " "`git add` would store it as other bytes than its blob") -#: The prefixes of the throwaway repositories of #530 and #654. +#: The prefixes of the throwaway repositories of #530 and #654, and of the +#: split's own clone (#1083). THROWAWAY_PREFIXES = ("openreposhape-attributes-", - "openreposhape-reencoded-") + "openreposhape-reencoded-", "openreposhape-clone-") +#: The prefixes of `_resolved`'s and `_answers`' (#1083). +URL_PREFIXES = ("openreposhape-url-", "openreposhape-ask-") +RELATIVE_URL = "plan-submodule-relative-url" #: The hooks the counting `core.hooksPath` holds: every one git runs where #: an index, a checkout or a ref is written, or a commit made. HOOKS = ("post-index-change", "post-checkout", "reference-transaction", @@ -299,17 +320,9 @@ def test_the_rehearsal_asks_nothing_more_of_a_tree_git_refuses(adopter, # L1-92: in the work directory # --------------------------------------------------------------------------- -def test_each_throwaway_repository_is_made_in_the_work_directory( - adopter, monkeypatch, tmp_path): - """The source keeps a nested `.gitattributes` above a pinned copy and - its own root one, so all three questions are asked: each throwaway - repository is made in the work directory the source was opened in, not - in `TMPDIR`, and none is left there.""" - plan = adopter.Plan.load(adoption_plan(tmp_path, github(YML_CRLF), - KEEP_GITHUB)) - work = tmp_path / "work" - work.mkdir() - opened = plan.open_source(None, work) +def recording_where_made(adopter, monkeypatch) -> list: + """`(prefix, parent)` of each directory `adopt-project.py` makes with + `tempfile.mkdtemp` from here on, appended as it is made.""" made = [] def mkdtemp(suffix=None, prefix=None, dir=None): @@ -320,11 +333,30 @@ def mkdtemp(suffix=None, prefix=None, dir=None): monkeypatch.setattr(adopter, "tempfile", SimpleNamespace( mkdtemp=mkdtemp, gettempdir=tempfile.gettempdir, TemporaryDirectory=tempfile.TemporaryDirectory)) + return made + + +def asked_where(made: list, prefixes: tuple) -> list: + return [(prefix, parent) for prefix, parent in made if prefix in prefixes] + + +def test_each_throwaway_repository_is_made_in_the_work_directory( + adopter, monkeypatch, tmp_path): + """The source keeps a nested `.gitattributes` above a pinned copy and + its own root one, so all three questions are asked, after the split's + own clone of the source (#1083): each throwaway repository is made in + the work directory the source was opened in, not in `TMPDIR`, and none + is left there.""" + plan = adopter.Plan.load(adoption_plan(tmp_path, github(YML_CRLF), + KEEP_GITHUB)) + work = tmp_path / "work" + work.mkdir() + opened = plan.open_source(None, work) + made = recording_where_made(adopter, monkeypatch) found, _, _ = adopter._entry_findings(plan, opened) assert any(line.startswith("FINDING plan-nested-attributes: ") for line in found), found - asked = [(prefix, parent) for prefix, parent in made - if prefix in THROWAWAY_PREFIXES] + asked = asked_where(made, THROWAWAY_PREFIXES) assert {prefix for prefix, _ in asked} == set(THROWAWAY_PREFIXES) assert {parent for _, parent in asked} == {work.resolve()} assert list(work.iterdir()) == [] @@ -411,10 +443,12 @@ def linked_github_plan(base: Path) -> Path: def test_a_probe_git_will_not_make_stands_for_its_finding( links_as_links, tmp_path): # noqa: F811 - """`GIT_DEFAULT_HASH=bogus` makes the probe's `git init` fail, and only - it: each throwaway repository names its format. `check` prints the - finding the probe stands for, as it does with no such variable, and - says why in a NOTE.""" + """`GIT_DEFAULT_HASH=bogus` makes the probe's `git init` fail, and of + the `git init`s only it: each throwaway repository names its format. + `check` prints the finding the probe stands for, as it does with no + such variable, and says why in a NOTE. (The split's own `git clone` + stops at that variable too, on git 2.43, which `adopt-git-unasked` + says beside them: the rows of #1083 below.)""" result = checked(linked_github_plan(tmp_path), {"GIT_DEFAULT_HASH": "bogus"}) assert "REFUSED" not in result.stderr, result.stderr @@ -520,21 +554,70 @@ def test_a_repository_git_cannot_make_is_one_finding_that_says_why( assert list(work.iterdir()) == [] -def test_a_default_branch_git_will_not_take_stops_no_question(tmp_path): - """Each throwaway `git init` names its branch, as the split's own `git - init -b` does, so `init.defaultBranch = a..b` stops none of `check`'s - questions, on any git: the CRLF file the plan keeps is found. Read, it - was `adopt-git-unasked`, which `execute` refused, though git 2.55's - `git clone`, which CI runs, does not stop at it (CI on #1031).""" +def this_gits_clone(base: Path, env: dict) -> subprocess.CompletedProcess: + """This machine's `git clone` of a repository of its own, under `env`: + whether it makes one there is its version's to say (CI on #1031).""" + source = base / "clone-asked-of" + git("init", "-q", "-b", "main", str(source), cwd=base) + return subprocess.run(["git", "clone", "-q", "--no-checkout", "--shared", + str(source), str(base / "clone-asked")], + env={**os.environ, **env}, capture_output=True, + check=False) + + +def unasked_clone(work: Path, code: int, said: str) -> str: + """`adopt-git-unasked` for a split's clone git ended with `code` and + `said` in `work`.""" + return (f"FINDING {UNASKED}: " + + UNASKED_OPENS.format(os.path.realpath(work)) + + f"the split's `git clone` of the source exited {code} there: " + + said + UNASKED_ENDS) + + +def test_a_default_branch_is_asked_of_the_splits_own_clone(tmp_path): + """`init.defaultBranch = a..b`: `check` answers as the split's own `git + clone` of the source does on this git (#1083). git 2.43's stops at it, + `fatal: invalid branch name`, and `check` says so as one + `adopt-git-unasked`, where 2a119f28 said `plan ok` and `execute` then + stopped after both legs existed. git 2.55's takes it (CI on #1031), and + there no `git init` stops a question: each names its branch, and the + CRLF file the plan keeps is found.""" plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, NOTES: CRLF}), tmp_path, {NOTES: ROOT}) work = tmp_path / "work" - result = checked(plan, bad_default_branch(tmp_path), "--work-dir", - str(work)) + env = bad_default_branch(tmp_path) + clone = this_gits_clone(tmp_path, env) + result = checked(plan, env, "--work-dir", str(work)) assert "REFUSED" not in result.stderr, result.stderr assert result.returncode == 1, result.stderr - assert codes(result) == [REENCODED_FILES], result.stderr + if clone.returncode == 0: + assert codes(result) == [REENCODED_FILES], result.stderr + else: + assert finding_lines(result) == [unasked_clone( + work, clone.returncode, clone.stderr.decode().strip())] + assert list(work.iterdir()) == [] + + +def test_a_default_branch_git_will_not_take_stops_no_throwaway_init( + adopter, monkeypatch, tmp_path): + """Each throwaway `git init` names its branch, as the split's own `git + init -b` does, so `init.defaultBranch = a..b` stops none of the + questions asked in them, on any git: asked of the rehearsal, which + clones nothing, the CRLF file the plan keeps is found. Read, it was + `adopt-git-unasked`, which `execute` refused, though git 2.55's `git + clone`, which CI runs, does not stop at it (CI on #1031).""" + plan = adopter.Plan.load(planned( + a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, NOTES: CRLF}), + tmp_path, {NOTES: ROOT})) + work = tmp_path / "work" + work.mkdir() + opened = plan.open_source(None, work) + for name, value in bad_default_branch(tmp_path).items(): + monkeypatch.setenv(name, value) + found = adopter._reencoded_files_findings(plan, opened, opened.tree()) + assert len(found) == 1, found + assert KEPT_CRLF in found[0], found[0] assert list(work.iterdir()) == [] @@ -614,3 +697,341 @@ def test_a_work_directory_nothing_can_be_made_in_says_so_once( assert codes(result) == [WORK_DIR_UNWRITABLE], result.stderr assert UNMADE_NOTE.format(os.path.realpath(unwritable_work)) in \ result.stdout.splitlines(), result.stdout + + +# --------------------------------------------------------------------------- +# #1083 (a): the split's own clone of the source +# --------------------------------------------------------------------------- + +#: What the stub `git` of `refusing_clone` says for every `git clone`. +REFUSED_CLONE = "fatal: w1083: no clone can be made here" + + +def refusing_clone(base: Path) -> dict: + """The environment of a git that clones nothing: a stub first on PATH + refuses each `git clone` and hands any other command to the real git. + Which setting stops a clone is git's version's to say (CI on #1031), so + the rows below do not lean on one.""" + stubs = base / "clone-stubs" + stubs.mkdir() + stub = stubs / "git" + stub.write_bytes("\n".join([ + "#!/bin/sh", + 'for arg in "$@"; do', + ' [ "$arg" = clone ] || continue', + f' echo "{REFUSED_CLONE}" >&2', + " exit 128", + "done", + 'exec "$REAL_GIT" "$@"', ""]).encode("ascii")) + stub.chmod(0o755) + return {"PATH": str(stubs) + os.pathsep + os.environ["PATH"], + "REAL_GIT": shutil.which("git")} + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH +def test_a_clone_git_will_not_make_is_one_finding_that_says_why(tmp_path): + """The split's first act after it makes both legs is its `git clone` of + the source, and `check` makes that clone first, in the work directory: + where git will not make it, that is one `adopt-git-unasked` in git's + words, the questions after it are not asked, and nothing is left there. + On 2a119f28 `check` cloned nothing, and said `plan ok`.""" + plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, + NOTES: CRLF}), + tmp_path, {NOTES: ROOT}) + work = tmp_path / "work" + result = checked(plan, refusing_clone(tmp_path), "--work-dir", str(work)) + assert "REFUSED" not in result.stderr, result.stderr + assert result.returncode == 1, result.stderr + assert finding_lines(result) == [unasked_clone(work, 128, REFUSED_CLONE)] + assert list(work.iterdir()) == [] + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH +def test_execute_refuses_a_clone_git_will_not_make_before_any_leg( + adopter, monkeypatch, tmp_path): + """On 2a119f28 `execute` made both legs, and its first clone of the + source then stopped it, with nothing rolled back.""" + plan = planned(a_source(tmp_path), tmp_path, {}) + for name, value in refusing_clone(tmp_path).items(): + monkeypatch.setenv(name, value) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + remotes = tmp_path / "remotes" + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan), source=None, local_remote_dir=remotes, + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work")) + assert caught.value.code == UNASKED + assert REFUSED_CLONE in caught.value.detail + assert "no leg repository was created" in caught.value.remediation + assert not remotes.exists() + + +def test_the_clone_is_asked_with_the_protocol_setting_execute_gives_it( + tmp_path): + """`protocol.file.allow = never` in this machine's config refuses a + `git clone` of a path, and `execute` gives each of its clones + `-c protocol.file.allow=always` (`FILE_PROTOCOL`), so `check` asks the + split's clone with it too, and finds nothing: a control, `plan ok` on + 2a119f28 as here.""" + plan = planned(a_source(tmp_path), tmp_path, {}) + work = tmp_path / "work" + result = checked(plan, config_with(tmp_path, '[protocol "file"]', + "\tallow = never"), + "--work-dir", str(work)) + assert result.returncode == 0, result.stderr + assert "plan ok" in result.stdout + assert list(work.iterdir()) == [] + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH +def test_no_url_is_asked_once_the_clone_cannot_be_made(tmp_path): + """Where the split's clone cannot be made, that is the one finding, and + no url question is asked after it, in a repository of the same work + directory, as no other is (#910): one cause, one finding. On 2a119f28 + `check` cloned nothing and named the relative url alone.""" + work = tmp_path / "work" + result = checked(url_plan(tmp_path), refusing_clone(tmp_path), + "--work-dir", str(work)) + assert "REFUSED" not in result.stderr, result.stderr + assert finding_lines(result) == [unasked_clone(work, 128, REFUSED_CLONE)] + assert list(work.iterdir()) == [] + + +def test_execute_refuses_a_default_branch_its_clone_will_not_take( + adopter, monkeypatch, tmp_path): + """`init.defaultBranch = a..b`, on a git whose `git clone` stops at it, + as 2.43's does: `execute` refuses it before any leg exists, where on + 2a119f28 it made both and stopped extracting the spec leg (#1083).""" + plan = planned(a_source(tmp_path), tmp_path, {}) + env = bad_default_branch(tmp_path) + if this_gits_clone(tmp_path, env).returncode == 0: + pytest.skip("this git's `git clone` takes `init.defaultBranch = " + "a..b`, as 2.55's does: the split clones there") + for name, value in env.items(): + monkeypatch.setenv(name, value) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + remotes = tmp_path / "remotes" + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan), source=None, local_remote_dir=remotes, + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work")) + assert caught.value.code == UNASKED + assert "invalid branch name" in caught.value.detail + assert not remotes.exists() + + +# --------------------------------------------------------------------------- +# #1083 (b): the submodule urls' own throwaway repositories +# --------------------------------------------------------------------------- + +def url_plan(base: Path) -> Path: + """A plan that keeps in the code leg a submodule whose url is relative, + which `_resolved` resolves from the source and from the leg, and one + whose url is an absolute path on this disk, which `_answers` asks.""" + dependency = base / "dependency" + source = source_with(base, SIBLING, second=dependency.as_posix(), + dependency=dependency) + return planned_with_urls(source, base, + answers={**BOTH_IN_CODE, EXTRA: "code"}) + + +def asked_with_urls(adopter, base: Path): + """`(plan, opened, work)`: `url_plan`, opened in a work directory.""" + plan = adopter.Plan.load(url_plan(base)) + work = base / "work" + work.mkdir() + return plan, plan.open_source(None, work), work + + +def the_clone_as_2_55_makes_it(adopter, monkeypatch) -> None: + """The split's own clone made, as git 2.55's `git clone` makes it under + an `init.defaultBranch` 2.43's stops at (CI on #1031), so that the rows + below ask what the urls' own repositories make of the setting on any + git. (`raising=False`: 2a119f28 makes no such clone.)""" + monkeypatch.setattr(adopter, "_split_clone_asked", lambda _source: None, + raising=False) + + +#: `(what this machine's git is given, how the row is named)`: each stopped +#: `check` with "a git or gh command failed" at `_resolved`'s `git init -q +#: /openreposhape-url-*` on 2a119f28. +URL_SETTINGS = [ + pytest.param(bad_default_branch, id="init-default-branch-a..b"), + pytest.param(lambda _base: {"GIT_DEFAULT_HASH": "bogus"}, + id="git-default-hash-bogus"), +] + + +@pytest.mark.parametrize("setting", URL_SETTINGS) +def test_the_submodule_urls_are_asked_whatever_the_setting( + adopter, monkeypatch, tmp_path, setting): + """Each of `_resolved`'s and `_answers`' repositories names its branch + and the source's format, as every throwaway does: what `check` says of + the urls under the setting is what it says without it.""" + plan, opened, work = asked_with_urls(adopter, tmp_path) + wanted = adopter._entry_findings(plan, opened) + assert [line.split(" ", 2)[1].rstrip(":") for line in wanted[0]] == [ + RELATIVE_URL], wanted[0] + the_clone_as_2_55_makes_it(adopter, monkeypatch) + for name, value in setting(tmp_path).items(): + monkeypatch.setenv(name, value) + assert adopter._entry_findings(plan, opened) == wanted + assert list(work.iterdir()) == [] + + +def test_the_submodule_urls_are_asked_in_the_work_directory( + adopter, monkeypatch, tmp_path): + """`_resolved`'s and `_answers`' repositories are made in the work + directory the source was opened in, where 2a119f28 made them in + `TMPDIR`, and none is left there.""" + plan, opened, work = asked_with_urls(adopter, tmp_path) + made = recording_where_made(adopter, monkeypatch) + adopter._entry_findings(plan, opened) + asked = asked_where(made, URL_PREFIXES) + assert {prefix for prefix, _ in asked} == set(URL_PREFIXES), made + assert {parent for _, parent in asked} == {work.resolve()} + assert list(work.iterdir()) == [] + + +@pytest.mark.parametrize("configured", [ + pytest.param(in_the_global_config, id="global-config"), + pytest.param(in_the_command_scope, id="git-config-count")]) +def test_no_hook_of_this_machines_runs_where_the_urls_are_asked( + tmp_path, configured): + """On 2a119f28 `_resolved`'s `update-index` ran the counting + `post-index-change` hook twice in this `check`, once for each side a + relative url is resolved from.""" + plan = url_plan(tmp_path) + hooks, log = counting_hooks(tmp_path) + result = checked(plan, configured(tmp_path, hooks)) + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [RELATIVE_URL], result.stderr + assert runs(log) == [] + + +def test_a_url_question_git_cannot_ask_is_one_finding_after_the_rest( + adopter, monkeypatch, tmp_path): + """Where `_answers`' repository cannot be made or asked, that is + `adopt-git-unasked`, once, as for every throwaway: the relative url + `_resolved` answered before it is still said, and no question after it + is asked, nor the NOTE of urls `_answers` would not ask. On 2a119f28 it + was a `Refusal` out of `check`.""" + plan, opened, work = asked_with_urls(adopter, tmp_path) + + def unasked(_urls, _source=None): + raise adopter._Unasked(work, "w1083: not here") + + monkeypatch.setattr(adopter, "_answers", unasked) + asked = [] + monkeypatch.setattr(adopter, "_asked_in_throwaways", + lambda *args: asked.append(args) or ([], [])) + try: + found, _, notes = adopter._entry_findings(plan, opened) + except adopter.Refusal as refusal: + pytest.fail(f"REFUSED {refusal.code}: {refusal.detail}") + assert [line.split(" ", 2)[1].rstrip(":") for line in found] == [ + UNASKED, RELATIVE_URL], found + assert "w1083: not here" in found[0] + assert asked == [] + assert not any("did not ask whether the url" in note for note in notes) + + +def test_a_step_git_fails_in_a_url_repository_is_unasked_in_its_words( + adopter, tmp_path): + """A step git fails in `_resolved`'s or `_answers`' repository once it + is made is `adopt-git-unasked` in git's words, as one in the rehearsal + is, and never "a git or gh command failed" (`_throwaway_git`).""" + work = tmp_path / "work" + work.mkdir() + scratch = work / "scratch" + scratch.mkdir() + env = adopter._inherited_env() + hooks_off = adopter._throwaway_made( + scratch, scratch / adopter.THROWAWAY_REPOSITORY, None, env) + assert adopter._throwaway_git(scratch, hooks_off, + ["rev-parse", "--is-inside-work-tree"], + env) == "true" + with pytest.raises(adopter._Unasked) as caught: + adopter._throwaway_git(scratch, hooks_off, ["hash-object", "absent"], + env) + assert caught.value.code == UNASKED + assert caught.value.where == work + assert caught.value.why.startswith("`git hash-object` in "), \ + caught.value.why + assert "exited 128: fatal: " in caught.value.why, caught.value.why + assert "absent" in caught.value.why, caught.value.why + + +def test_a_work_directory_nothing_can_be_made_in_asks_no_url( + tmp_path, unwritable_work): # noqa: F811 + """No url question is asked in a work directory nothing can be made in: + `adopt-work-dir-unwritable` is the one finding, as for every other + throwaway (#524, #910), where 2a119f28 asked the urls in `TMPDIR`, and + one `_Unasked` more would have stopped `check`.""" + plan = url_plan(tmp_path) + result = checked(plan, None, "--work-dir", str(unwritable_work)) + assert "Traceback" not in result.stderr, result.stderr + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [WORK_DIR_UNWRITABLE], result.stderr + assert UNMADE_NOTE.format(os.path.realpath(unwritable_work)) in \ + result.stdout.splitlines(), result.stdout + + +# --------------------------------------------------------------------------- +# #1083 (c): a work directory not made yet, and case twins +# --------------------------------------------------------------------------- + +def test_a_work_directory_not_made_yet_is_made_and_asked_in(adopter, + tmp_path): + """r1031's NIT-R2-2: a `Source` opened outside a command, on a work + directory that is not there, is asked as one `check --work-dir` makes + (`_throwaway`'s `_made_0755`): the CRLF file is found, there is no NOTE + of a directory that could not be made, and the directory is left there, + empty. Before #910 its questions were asked in `TMPDIR`.""" + plan = adopter.Plan.load(planned( + a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, NOTES: CRLF}), + tmp_path, {NOTES: ROOT})) + work = tmp_path / "not-made" / "yet" + found, _, notes = adopter._entry_findings( + plan, plan.open_source(None, work)) + assert [line.split(" ", 2)[1].rstrip(":") for line in found] == [ + REENCODED_FILES], found + assert not any(note.startswith( + "NOTE `check` could not make a throwaway repository") + for note in notes), notes + assert list(work.iterdir()) == [] + + +#: Two paths one case-insensitive disk holds as one file. +TWINS = ("Notes.md", "notes.md") + + +def test_case_twins_are_both_held_under_core_ignorecase(adopter, monkeypatch, + tmp_path): + """`_paths_git_refuses` hands every path of the source commit to `git + update-index --index-info`, and names `plan-path-git-refuses` for one + the index then lacks. On a case-insensitive disk `git init` sets + `core.ignorecase=true`, and `git add` will not hold case twins. That + question reads no file of the disk, only the setting, which this row + sets on a disk that holds both twins: the control shows git honours it + (`git add` holds one twin at most), and `--index-info` still holds + both, so neither is named.""" + source = a_source(tmp_path) + with_paths(source, TWINS) + plan = adopter.Plan.load(planned(source, tmp_path, {})) + work = tmp_path / "work" + work.mkdir() + opened = plan.open_source(None, work) + for name, value in config_with(tmp_path, "[core]", + "\tignorecase = true").items(): + monkeypatch.setenv(name, value) + control = tmp_path / "control" + git("init", "-q", "-b", "main", str(control), cwd=tmp_path) + for twin, data in zip(TWINS, (b"a\n", b"b\n")): + (control / twin).write_bytes(data) + subprocess.run(["git", "add", "--", *TWINS], cwd=str(control), + capture_output=True, check=False) + held = git("ls-files", cwd=control).stdout.split() + assert len(held) < 2, held + assert adopter._paths_git_refuses(opened, opened.tree()) == [] + assert list(work.iterdir()) == [] From bdcf99528c08beb3fc486173d2c2da9085fb553f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 13:42:06 +0000 Subject: [PATCH 02/17] tests/test_adopt_reencoded_files.py: the docstring of `test_a_link_the_checkout_writes_is_never_written_through` says the throwaway repository is made in the work directory, as #910 makes it, and no longer in `TMPDIR` (#1083) The last item of #1083's (c): the docstring still said the throwaway repository the link is asked in is in `TMPDIR`. Since #910 it is made in the work directory, which is as deep below `tmp_path` as the `TMPDIR` the row sets, so the row asks the same question and only its words change. (The issue's `adopt-project.py:466` names `_pin_row_problem` at 74c17705; this docstring is the one PR #1031's body named.) Gate on the merged tree (main 6da757cb): see the pull request body. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_reencoded_files.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_adopt_reencoded_files.py b/tests/test_adopt_reencoded_files.py index 8735534b..14bfdd2d 100644 --- a/tests/test_adopt_reencoded_files.py +++ b/tests/test_adopt_reencoded_files.py @@ -470,7 +470,9 @@ def test_a_link_the_checkout_writes_is_never_written_through(monkeypatch, tmp_path): """The source keeps a root `.gitattributes` that is a symlink out of it, `../../../victim.txt`, which leads to the same file from the source and - from the throwaway repository in `TMPDIR`. `plan-root-symlink` names it + from the throwaway repository `check` makes in the work directory + (#910), as from one in `TMPDIR`, where it was made before: both are as + deep below `tmp_path`. `plan-root-symlink` names it alone: the question writes the shape's `.gitattributes` in the link's place, never through it (at 0224f96a `check` overwrote that file), and does not ask about the link it replaced.""" From 9b99893667f178c832ff47d4632240c1fa832140 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 15:26:56 +0000 Subject: [PATCH 03/17] adopt-project.py: check's clone of the source takes the -c settings a parent git hands it, as execute's does, and no variable naming another repository; _resolved and _answers read their environment before their throwaway (#1083) r1119's fix round on #1119. SHOULD-2: `_split_clone_asked` makes the clone in `_split_clone_env()`: `_inherited_env()` plus `GIT_CONFIG_PARAMETERS` where it is set. Measured on bdcf9952 with git 2.43, `GIT_CONFIG_PARAMETERS="'init.defaultbranch'='a..b'"` gave `plan ok` from `check`, and `execute` made both legs and stopped extracting the spec leg. Now `check` says `adopt-git-unasked` and `execute` refuses before any leg, as for the setting in a config file or in `GIT_CONFIG_COUNT`. A parent's `-c core.hooksPath` runs no hook: the clone's own `-c` comes after it. SHOULD-1: a row runs `check` under `GIT_OBJECT_DIRECTORY` naming the source's own objects and holds the source's object directory unchanged. It kills M6: in the whole environment the clone wrote `objects/info/alternates` into the source. A row under `GIT_DIR` naming the source, whose own config rewrites the on-disk url, kills M14 (`_answers` in the whole environment) and holds the source's config unchanged. NIT-1: the docstring says git ignores `--shared` for a shallow source, so that clone is a full copy, a cost `execute`'s own clones pay too. NIT-2: the unmade-work-directory NOTE says it asked nothing about the urls of the submodules the plan keeps either. NIT-3: `_resolved`, `_answers` and the clone read their environment before they make their throwaway, so a raise there leaves none behind. tests/test_adopt_check_throwaway_repos.py: 63 rows (45 before), 18 new. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 84 ++++-- tests/test_adopt_check_throwaway_repos.py | 317 +++++++++++++++++++++- 2 files changed, 378 insertions(+), 23 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index a1e54c88..f580ed94 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -7704,10 +7704,11 @@ def _resolved(base: str, urls: list[str], `git init` in `TMPDIR` stopped `check` with "a git or gh command failed", exit 2, under an `init.defaultBranch` git will not take or a `GIT_DEFAULT_HASH` that names no hash. `_Unasked` where git cannot make - or ask it there.""" + or ask it there. The environment is read before the throwaway is made, + so that nothing raised reading it leaves one behind (r1119's NIT-3).""" + env = _inherited_env() scratch = _throwaway("openreposhape-url-", source) repository = scratch / THROWAWAY_REPOSITORY - env = _inherited_env() try: hooks_off = _throwaway_made(scratch, repository, source, env) with _asked_there(scratch.parent): @@ -7913,10 +7914,11 @@ def _answers(urls: list[str], source: Source | None = None makes its own (#1083): in the work directory `source` was opened in, with no hook and under `_inherited_env()`, which leaves out a `GIT_DIR` or a `GIT_CONFIG_PARAMETERS` around this process; `_Unasked` where git - cannot make it there.""" + cannot make it there. The environment is read before the throwaway is + made, as `_resolved` reads it.""" + env = {**_inherited_env(), **ON_THIS_DISK_ONLY} scratch = _throwaway("openreposhape-ask-", source) repository = scratch / THROWAWAY_REPOSITORY - env = {**_inherited_env(), **ON_THIS_DISK_ONLY} try: hooks_off = _throwaway_made(scratch, repository, source, env) out = [] @@ -10982,7 +10984,10 @@ def _refuse_symlinks_checked_out_as_files(plan: Plan, source: Source, # rolled back; `GIT_DEFAULT_HASH=bogus` stops every clone the same way. # Asked here, either is `adopt-git-unasked`, which `execute` refuses # before either leg, and on a git whose clone takes the name, as 2.55's -# does, it is no finding, and every question is asked. +# does, it is no finding, and every question is asked. It takes the `-c` +# settings a parent git hands this process, as `execute`'s clone does, +# and no variable naming another repository, which it would write into +# (`_split_clone_env`, r1119's SHOULD-1 and SHOULD-2). # * THE SUBMODULE URLS' OWN (#1083). `_resolved` and `_answers` ask git how # a kept submodule's url resolves and whether it answers, each in a # repository of its own, made the same way here: their plain `git init` @@ -11051,7 +11056,8 @@ def said(self) -> tuple[list[str], list[str]]: f"in {_spelled(str(self.where))}, the work directory, " f"to ask this machine's git what the split will make " f"of the source: {self.why}. It asked nothing about " - "the source's paths and files there; run `check` " + "the source's paths and files there, nor about the " + "urls of the submodules the plan keeps; run `check` " "again with a --work-dir this user can write in."] return [f"FINDING {self.code}: {self.detail}"], [] @@ -11154,6 +11160,44 @@ def _throwaway_git(scratch: Path, hooks_off: list[str], args: list, return done.stdout.decode("utf-8", "replace").strip() +#: The `-c` settings a parent git hands each command it runs, in git's own +#: quoted form: `git -c =` running an alias, or a hook, that +#: runs `adopt-project.py`. `_inherited_env()` leaves it out, and only the +#: split's own clone takes it back (`_split_clone_env`). +CONFIG_PARAMETERS = "GIT_CONFIG_PARAMETERS" + + +def _split_clone_env() -> dict: + """The environment `_split_clone_asked` makes the split's own clone in: + `_inherited_env()`, and the `-c` settings a parent git hands this + process (`CONFIG_PARAMETERS`) where it hands some, as `execute`'s own + clone of the source takes them, in the whole environment (r1119's + SHOULD-2). + + THOSE SETTINGS, SO THAT THE CLONE ANSWERS AS THE SPLIT'S. Measured on + bdcf9952 with git 2.43: under `GIT_CONFIG_PARAMETERS` holding + `'init.defaultbranch'='a..b'`, `check` said `plan ok`, and `execute` + made both legs and stopped extracting the spec leg, `fatal: invalid + branch name`, with nothing rolled back, where the same setting in a + config file or in `GIT_CONFIG_COUNT` was `adopt-git-unasked` already. A + parent's `-c core.hooksPath` runs no hook in the clone: git reads the + command's own `-c` after the parent's, and takes a key's last value. + An `attr.tree` among them, which is why `_inherited_env()` leaves them + out of the questions a checkout answers, changes nothing in a clone + that checks nothing out (`attr.tree=bogus` measured: it clones). + + NOTHING ELSE `_inherited_env()` LEAVES OUT. Each of those names another + repository, its objects, an index or a work tree, and the clone would + write there: in the whole environment, under a `GIT_OBJECT_DIRECTORY` + naming the source's own objects, it wrote `objects/info/alternates` + into the source, which `check` never writes to (r1119's SHOULD-1).""" + env = _inherited_env() + parameters = os.environ.get(CONFIG_PARAMETERS) + if parameters is not None: + env[CONFIG_PARAMETERS] = parameters + return env + + def _split_clone_asked(source: Source) -> None: """The split's own `git clone` of the source, made in a throwaway directory in the work directory and removed again (#1083): nothing @@ -11168,14 +11212,23 @@ def _split_clone_asked(source: Source) -> None: source's format, makes its repository. So `check` said `plan ok`, and `execute` stopped after both legs existed. Asked as that clone is made, the answer is the split's on every version of git: with `FILE_PROTOCOL`, - as `execute` gives each of its clones, and the template this machine's - git copies in, as `_extract_leg`'s clone takes it. Left out is only what - the other questions ask, or what would run here: `--no-checkout`, as the - rehearsal asks what a checkout makes of each file and - `_paths_git_refuses` which paths git holds at all; `--shared`, so no - object is copied; and, as in every throwaway, any hook of this - machine's (`core.hooksPath` an empty directory) and the environment - `_inherited_env()` leaves out.""" + as `execute` gives each of its clones, the template this machine's git + copies in, as `_extract_leg`'s clone takes it, and the `-c` settings a + parent git hands this process (`_split_clone_env`). Left out is only + what the other questions ask, or what would run or write here: + `--no-checkout`, as the rehearsal asks what a checkout makes of each + file and `_paths_git_refuses` which paths git holds at all; any hook of + this machine's (`core.hooksPath` an empty directory), as in every + throwaway; and each variable that names another repository, which + `_split_clone_env()` leaves out. + + `--shared` copies no object of a source that is not shallow: the clone + borrows the source's. git ignores it for a shallow source, and says + nothing, so that clone is a full copy of the source's objects (r1119's + NIT-1, measured: 117 MiB for a 115 MiB pack). That is a cost, not a + fault: each of `execute`'s own clones of such a source is a full copy + too.""" + env = _split_clone_env() scratch = _throwaway("openreposhape-clone-", source) try: empty = scratch / THROWAWAY_EMPTY @@ -11185,8 +11238,7 @@ def _split_clone_asked(source: Source) -> None: ["git", *FILE_PROTOCOL, "-c", f"core.hooksPath={empty}", "clone", "-q", "--no-checkout", "--shared", str(source.path), str(scratch / THROWAWAY_REPOSITORY)], - cwd=str(scratch), env=_inherited_env(), capture_output=True, - check=False) + cwd=str(scratch), env=env, capture_output=True, check=False) if cloned.returncode: raise _Unasked(scratch.parent, ( f"the split's `git clone` of the source exited " diff --git a/tests/test_adopt_check_throwaway_repos.py b/tests/test_adopt_check_throwaway_repos.py index 890b174a..d693c314 100644 --- a/tests/test_adopt_check_throwaway_repos.py +++ b/tests/test_adopt_check_throwaway_repos.py @@ -394,14 +394,38 @@ def in_the_command_scope(_base: Path, hooks: Path) -> dict: [("core.hooksPath", hooks.as_posix())]) -@pytest.mark.parametrize("configured", [ +def appended_to_parameters(entries) -> dict: + """`GIT_CONFIG_PARAMETERS` as a parent `git -c =` hands it + to each command it runs, an alias or a hook: `entries` after any the + environment holds, each in git's own quoted form, `''=''` + (git 2.43 writes the key in the case it was given).""" + held = os.environ.get("GIT_CONFIG_PARAMETERS", "") + added = " ".join(f"'{key}'='{value}'" for key, value in entries) + return {"GIT_CONFIG_PARAMETERS": f"{held} {added}".strip()} + + +def from_a_parent_git(_base: Path, hooks: Path) -> dict: + return appended_to_parameters([("core.hooksPath", hooks.as_posix())]) + + +#: Where a hook's directory is configured: the second and third reach the +#: split's own clone as a parent git hands them (r1119's SHOULD-2), where +#: that clone's own `-c core.hooksPath` comes after them. +HOOKS_CONFIGURED = [ pytest.param(in_the_global_config, id="global-config"), - pytest.param(in_the_command_scope, id="git-config-count")]) + pytest.param(in_the_command_scope, id="git-config-count"), + pytest.param(from_a_parent_git, id="git-config-parameters")] + + +@pytest.mark.parametrize("configured", HOOKS_CONFIGURED) def test_no_hook_of_this_machines_runs_in_a_throwaway_repository( tmp_path, configured): """The source keeps its own root `.gitattributes`, so #619's question of `git add` is asked as well as the rehearsal's: on c7a3ff16 the - global config's hook ran 12 times in this `check`.""" + global config's hook ran 12 times in this `check`. The split's own + clone writes refs, which runs `reference-transaction`, and takes the + settings a parent git hands it too (`git-config-parameters`), so its + own `-c core.hooksPath` must come after them.""" plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: b"*.md text\n"}), tmp_path, {}) hooks, log = counting_hooks(tmp_path) @@ -410,9 +434,7 @@ def test_no_hook_of_this_machines_runs_in_a_throwaway_repository( assert runs(log) == [] -@pytest.mark.parametrize("configured", [ - pytest.param(in_the_global_config, id="global-config"), - pytest.param(in_the_command_scope, id="git-config-count")]) +@pytest.mark.parametrize("configured", HOOKS_CONFIGURED) def test_the_counting_hook_runs_where_this_machines_config_reaches( tmp_path, configured): """The control: the same config runs the hook in a repository of its @@ -680,7 +702,8 @@ def git(self, args, index, data=b""): "the work directory, to ask this machine's git what the split " "will make of the source: a directory or a file could not be " "made there (Permission denied). It asked nothing about the " - "source's paths and files there; run `check` again with a " + "source's paths and files there, nor about the urls of the " + "submodules the plan keeps; run `check` again with a " "--work-dir this user can write in.") @@ -1035,3 +1058,283 @@ def test_case_twins_are_both_held_under_core_ignorecase(adopter, monkeypatch, assert len(held) < 2, held assert adopter._paths_git_refuses(opened, opened.tree()) == [] assert list(work.iterdir()) == [] + + +# --------------------------------------------------------------------------- +# r1119: what the split's own clone inherits, and what it must not +# --------------------------------------------------------------------------- + +def with_parameters(_base: Path, entries) -> dict: + """`entries` as a parent git hands them: `git -c =` + running an alias or a hook that runs `adopt-project.py`.""" + return appended_to_parameters(entries) + + +def with_command_scope(_base: Path, entries) -> dict: + """`entries` in git's command scope (`GIT_CONFIG_COUNT`).""" + return appended_to_command_scope(os.environ, entries) + + +def with_global_config(base: Path, entries) -> dict: + """`entries` in a global config file.""" + lines = [] + for key, value in entries: + section, _, name = key.rpartition(".") + lines += [f"[{section}]", f"\t{name} = {value}"] + return config_with(base, *lines) + + +#: The two forms in which a parent git hands this process its settings. On +#: bdcf9952 `_inherited_env()` gave the split's own clone the second and +#: left out the first, which `execute`'s clone takes (r1119's SHOULD-2). +FROM_A_PARENT = [ + pytest.param(with_parameters, id="git-config-parameters"), + pytest.param(with_command_scope, id="git-config-count")] +BAD_DEFAULT_BRANCH = [("init.defaultBranch", "a..b")] + + +@pytest.mark.parametrize("handed", FROM_A_PARENT) +def test_a_parent_gits_default_branch_is_asked_of_the_splits_own_clone( + tmp_path, handed): + """`init.defaultBranch = a..b`, handed down by a parent git, as the + global config's is asked above: git 2.43's `git clone` stops at it, + and `check` says so as one `adopt-git-unasked`. On bdcf9952 the clone + took no `GIT_CONFIG_PARAMETERS`, and `check` said `plan ok` where + `execute`'s clone, which takes the whole environment, stopped after + both legs existed. git 2.55's clone takes the name, and there the CRLF + file the plan keeps is found.""" + plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, + NOTES: CRLF}), + tmp_path, {NOTES: ROOT}) + work = tmp_path / "work" + env = handed(tmp_path, BAD_DEFAULT_BRANCH) + clone = this_gits_clone(tmp_path, env) + result = checked(plan, env, "--work-dir", str(work)) + assert "REFUSED" not in result.stderr, result.stderr + assert result.returncode == 1, result.stderr + if clone.returncode == 0: + assert codes(result) == [REENCODED_FILES], result.stderr + else: + assert finding_lines(result) == [unasked_clone( + work, clone.returncode, clone.stderr.decode().strip())] + assert list(work.iterdir()) == [] + + +@pytest.mark.parametrize("handed", FROM_A_PARENT) +def test_execute_refuses_a_parent_gits_default_branch_before_any_leg( + adopter, monkeypatch, tmp_path, handed): + """Measured on bdcf9952 with git 2.43 and `GIT_CONFIG_PARAMETERS`: + `execute` made both legs and stopped extracting the spec leg, `fatal: + invalid branch name`, with nothing rolled back.""" + plan = planned(a_source(tmp_path), tmp_path, {}) + env = handed(tmp_path, BAD_DEFAULT_BRANCH) + if this_gits_clone(tmp_path, env).returncode == 0: + pytest.skip("this git's `git clone` takes `init.defaultBranch = " + "a..b`, as 2.55's does: the split clones there") + for name, value in env.items(): + monkeypatch.setenv(name, value) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + remotes = tmp_path / "remotes" + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan), source=None, local_remote_dir=remotes, + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work")) + assert caught.value.code == UNASKED + assert "invalid branch name" in caught.value.detail + assert not remotes.exists() + + +#: The setting the stub `git` of `refusing_a_setting` reads, under a key no +#: config of a machine this runs on sets, and what it says when it is set. +REFUSING_KEY = "test.refusesplitclone" +REFUSED_BY_SETTING = "fatal: f1119: a setting this clone reads refuses it" + + +def refusing_a_setting(base: Path) -> dict: + """The environment of a git whose `git clone` stops at a setting, on + every version: a stub first on PATH asks the real git whether the + config that clone reads -- the files, `GIT_CONFIG_COUNT` and + `GIT_CONFIG_PARAMETERS` alike -- sets `REFUSING_KEY` to `yes`, and + refuses the clone where it does. Any other command is the real git's. + `init.defaultBranch = a..b` stops git 2.43's clone and not 2.55's (CI + on #1031), so the rows below do not lean on it.""" + stubs = base / "setting-stubs" + stubs.mkdir() + stub = stubs / "git" + stub.write_bytes("\n".join([ + "#!/bin/sh", + 'for arg in "$@"; do', + ' [ "$arg" = clone ] || continue', + f' if [ "$("$REAL_GIT" config --get {REFUSING_KEY})" = yes ]; then', + f' echo "{REFUSED_BY_SETTING}" >&2', + " exit 128", + " fi", + " break", + "done", + 'exec "$REAL_GIT" "$@"', ""]).encode("ascii")) + stub.chmod(0o755) + return {"PATH": str(stubs) + os.pathsep + os.environ["PATH"], + "REAL_GIT": shutil.which("git")} + + +#: Each form a setting can reach the split's clone in: a file, as the rows +#: of #1083 ask it, and the two a parent git hands down. +EVERY_FORM = [pytest.param(with_global_config, id="global-config"), + *FROM_A_PARENT] + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH +@pytest.mark.parametrize("handed", EVERY_FORM) +def test_a_setting_that_stops_the_clone_is_asked_in_every_form(tmp_path, + handed): + """Whatever form the setting comes in, `check`'s clone reads it as the + split's does, and one that stops it is one `adopt-git-unasked`, on any + version of git. On bdcf9952 `git-config-parameters` was `plan ok`.""" + plan = planned(a_source(tmp_path), tmp_path, {}) + work = tmp_path / "work" + env = {**refusing_a_setting(tmp_path), + **handed(tmp_path, [(REFUSING_KEY, "yes")])} + result = checked(plan, env, "--work-dir", str(work)) + assert "REFUSED" not in result.stderr, result.stderr + assert result.returncode == 1, result.stderr + assert finding_lines(result) == [ + unasked_clone(work, 128, REFUSED_BY_SETTING)] + assert list(work.iterdir()) == [] + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH +@pytest.mark.parametrize("handed", EVERY_FORM) +def test_execute_refuses_a_setting_that_stops_its_clone_before_any_leg( + adopter, monkeypatch, tmp_path, handed): + """`execute`'s first clone of the source reads the setting in the whole + environment; on bdcf9952, under `git-config-parameters`, it was the + first to read it, after both legs existed.""" + plan = planned(a_source(tmp_path), tmp_path, {}) + env = {**refusing_a_setting(tmp_path), + **handed(tmp_path, [(REFUSING_KEY, "yes")])} + for name, value in env.items(): + monkeypatch.setenv(name, value) + monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) + remotes = tmp_path / "remotes" + with pytest.raises(adopter.Refusal) as caught: + adopter.cmd_execute(Namespace( + plan=str(plan), source=None, local_remote_dir=remotes, + allow_empty_leg=[], yes=True, work_dir=tmp_path / "work")) + assert caught.value.code == UNASKED + assert REFUSED_BY_SETTING in caught.value.detail + assert "no leg repository was created" in caught.value.remediation + assert not remotes.exists() + + +def test_the_clone_takes_a_parent_gits_settings_and_no_other_repository( + adopter, monkeypatch, tmp_path): + """`_split_clone_env()`: `_inherited_env()`, which keeps + `GIT_CONFIG_COUNT` but an `attr.tree` in it, and the parent's + `GIT_CONFIG_PARAMETERS` as it stands; and none of the variables that + name another repository, its objects, an index or a work tree, nor + `GIT_ATTR_SOURCE`. With no parent setting there is none.""" + named = {"GIT_DIR": "git-dir", "GIT_WORK_TREE": "work-tree", + "GIT_INDEX_FILE": "index", "GIT_COMMON_DIR": "common", + "GIT_OBJECT_DIRECTORY": "objects", "GIT_ATTR_SOURCE": "HEAD"} + for name, value in named.items(): + monkeypatch.setenv(name, str(tmp_path / value)) + parameters = "'test.parentone'='one' 'core.hooksPath'='/nowhere'" + monkeypatch.setenv("GIT_CONFIG_PARAMETERS", parameters) + for name, value in appended_to_command_scope( + {}, [("attr.tree", "HEAD"), ("test.heldone", "one")]).items(): + monkeypatch.setenv(name, value) + env = adopter._split_clone_env() + assert env["GIT_CONFIG_PARAMETERS"] == parameters + assert [env.get(name) for name in named] == [None] * len(named) + assert (env["GIT_CONFIG_COUNT"], env["GIT_CONFIG_KEY_0"], + env["GIT_CONFIG_VALUE_0"]) == ("1", "test.heldone", "one") + monkeypatch.delenv("GIT_CONFIG_PARAMETERS") + assert "GIT_CONFIG_PARAMETERS" not in adopter._split_clone_env() + + +def objects_of(repository: Path) -> dict: + """Each file below `repository`'s object directory, and its bytes.""" + objects = repository / ".git" / "objects" + return {path.relative_to(objects).as_posix(): path.read_bytes() + for path in sorted(objects.rglob("*")) if path.is_file()} + + +def test_the_clone_writes_nothing_into_an_object_directory_around_it( + tmp_path): + """r1119's SHOULD-1: `GIT_OBJECT_DIRECTORY` names the source's own + objects, as it does for a command git runs in the source. `check` + promises never to write into the source, and its clone of it takes + no such variable (`_inherited_env()`): `plan ok`, and the source's + object directory as it was. In the whole environment the clone wrote + `objects/info/alternates` there, naming the throwaway's own objects, + which `check` then removes (r1119, M6).""" + source = a_source(tmp_path) + plan = planned(source, tmp_path, {}) + before = objects_of(source) + work = tmp_path / "work" + result = checked(plan, {"GIT_OBJECT_DIRECTORY": str( + source / ".git" / "objects")}, "--work-dir", str(work)) + assert result.returncode == 0, result.stderr + assert "plan ok" in result.stdout + assert objects_of(source) == before + assert list(work.iterdir()) == [] + + +def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): + """`_answers` asks each url in a throwaway repository of its own, so + that no repository's config around this process rewrites a url, as + the assembly's recursive clone would not: here `GIT_DIR` names the + source, as it does for a hook git runs there, and the source's own + config rewrites the on-disk url to a directory that holds nothing + (the control: git there expands it so). `check` says what it says with + no `GIT_DIR`; in the whole environment `_answers` read the rewrite, + and found the url answering nothing (r1119, M14). Nor is the source's + config written: `_resolved` sets `remote.origin.url` in its own + repository, which in the whole environment is the source.""" + dependency = tmp_path / "dependency" + source = source_with(tmp_path, SIBLING, second=dependency.as_posix(), + dependency=dependency) + nowhere = (tmp_path / "nowhere").as_posix() + git("config", f"url.{nowhere}.insteadOf", dependency.as_posix(), + cwd=source) + assert git("ls-remote", "--get-url", dependency.as_posix(), + cwd=source).stdout.strip() == nowhere + plan = planned_with_urls(source, tmp_path, + answers={**BOTH_IN_CODE, EXTRA: "code"}) + config = (source / ".git" / "config").read_bytes() + for env in ({}, {"GIT_DIR": str(source / ".git")}): + result = checked(plan, env) + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [RELATIVE_URL], (env, result.stderr) + assert (source / ".git" / "config").read_bytes() == config, env + + +@pytest.mark.parametrize("asked", [ + pytest.param(lambda adopter, source: adopter._resolved( + "..", ["../x"], None, source), id="resolved"), + pytest.param(lambda adopter, source: adopter._answers(["/x"], source), + id="answers"), + pytest.param(lambda adopter, source: adopter._split_clone_asked(source), + id="split-clone")]) +def test_an_environment_read_that_raises_leaves_no_throwaway( + adopter, monkeypatch, tmp_path, asked): + """r1119's NIT-3: each reads its environment before it makes its + throwaway, so that a raise there leaves none in the work directory. + On bdcf9952 `_resolved` and `_answers` made it first. (By reading, a + `GIT_CONFIG_COUNT` of a superscript two raised in `_inherited_env()`, + whose `isdigit()` is true and whose `int()` raises; measured, git + refuses that count itself, `bogus count in GIT_CONFIG_COUNT`, at + `check`'s first command, before any throwaway.)""" + plan = adopter.Plan.load(planned(a_source(tmp_path), tmp_path, {})) + work = tmp_path / "work" + work.mkdir() + opened = plan.open_source(None, work) + opened.object_format() + + def unreadable(): + raise RuntimeError("f1119: no environment") + + monkeypatch.setattr(adopter, "_inherited_env", unreadable) + with pytest.raises(RuntimeError, match="f1119: no environment"): + asked(adopter, opened) + assert list(work.iterdir()) == [] From cea255306c041f3f577f54b96b27b451cd86cb67 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 21:17:22 +0000 Subject: [PATCH 04/17] adopt-project.py: `_resolved`'s and `_answers`' throwaways take `core.fsmonitor=false`, and the rows r1119b named pass on main: the clone-env row appends to the environment's count, and the ref-probe row fails only the probe's `git init` r1119b's second review of #1119, after the merge of main 038dafba (#1091, #1111): - MUST-1: `test_the_clone_takes_a_parent_gits_settings_and_no_other_repository` built its `GIT_CONFIG_COUNT` from `{}`, which `test_no_test_sets_a_git_config_count_of_its_own` refuses. It now appends to `os.environ`, and asserts the last entry kept and no `attr.tree`. - MUST-2: main's `test_check_says_where_git_would_not_make_the_ref_probe` set `GIT_DEFAULT_HASH=bogus`, which now stops the split's own clone first (`adopt-git-unasked`, exit 1). A stub `git` first on PATH fails only `git init` of a `refs-probe-*` directory, with git's words; skipped on Windows. - SHOULD-A: `_throwaway_made` returns `-c core.fsmonitor=false` beside `core.hooksPath`, as the rehearsal and `_stored_as_written` give theirs. A counting fsmonitor program ran 8 times in `_resolved`'s two repositories in one `check`; now 0, in the global config and in `GIT_CONFIG_COUNT`. New rows: that one and its control. - NIT-A: a row with `--work-dir` inside the source, whose config rewrites the url (kills R8). - NIT-B: a row with that rewrite handed down by a parent git in `GIT_CONFIG_PARAMETERS`, which `_answers` leaves out (kills A18). - NIT-C: `_split_clone_asked`'s docstring says a shallow source makes the clone run the `uploadpack.packObjectsHook` of protected config, as `execute`'s clones do. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 15 ++- tests/test_adopt_check_throwaway_repos.py | 156 +++++++++++++++++++--- tests/test_adopt_root_symlinks.py | 42 +++++- 3 files changed, 186 insertions(+), 27 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 770c4b98..010c9ccc 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -11529,11 +11529,16 @@ def _throwaway_made(scratch: Path, repository: Path, source: Source | None, throwaway repositories (#910): from the empty `THROWAWAY_EMPTY`, with the source's object format, and on `THROWAWAY_BRANCH`. Returns the `-c` every git command run there takes: `core.hooksPath` that same empty - directory. `_Unasked` where it cannot be made.""" + directory, and `core.fsmonitor=false`, as `_SplitRehearsal.git` and + `_stored_as_written` give each of theirs: no fsmonitor program or + daemon of this machine's is started for a repository that is about to + go. Without it, a counting `core.fsmonitor` program in the global + config ran 8 times in `_resolved`'s repositories in one `check` + (r1119b's SHOULD-A). `_Unasked` where it cannot be made.""" formats = ([] if source is None else [f"--object-format={source.object_format()}"]) empty = scratch / THROWAWAY_EMPTY - hooks_off = ["-c", f"core.hooksPath={empty}"] + hooks_off = ["-c", f"core.hooksPath={empty}", "-c", "core.fsmonitor=false"] with _asked_there(scratch.parent): empty.mkdir() made = subprocess.run(["git", *hooks_off, "init", "-q", @@ -11645,7 +11650,11 @@ def _split_clone_asked(source: Source) -> None: nothing, so that clone is a full copy of the source's objects (r1119's NIT-1, measured: 117 MiB for a 115 MiB pack). That is a cost, not a fault: each of `execute`'s own clones of such a source is a full copy - too.""" + too. So such a clone also runs the `uploadpack.packObjectsHook` that + this machine's protected config names (its system or global config, or + a `-c`; git takes none from the source's own) to pack what it copies, + as each of `execute`'s clones of that source runs it (r1119b's + NIT-C).""" env = _split_clone_env() scratch = _throwaway("openreposhape-clone-", source) try: diff --git a/tests/test_adopt_check_throwaway_repos.py b/tests/test_adopt_check_throwaway_repos.py index d693c314..8b955664 100644 --- a/tests/test_adopt_check_throwaway_repos.py +++ b/tests/test_adopt_check_throwaway_repos.py @@ -847,17 +847,32 @@ def test_execute_refuses_a_default_branch_its_clone_will_not_take( # #1083 (b): the submodule urls' own throwaway repositories # --------------------------------------------------------------------------- -def url_plan(base: Path) -> Path: - """A plan that keeps in the code leg a submodule whose url is relative, - which `_resolved` resolves from the source and from the leg, and one - whose url is an absolute path on this disk, which `_answers` asks.""" +def a_url_and_nowhere(base: Path) -> tuple[Path, str, str]: + """`(source, url, nowhere)`: a source that keeps a submodule whose url + is relative and one whose url is `url`, an absolute path on this disk + where a repository answers; and `nowhere`, a directory beside it that + holds nothing, for a rewrite of that url to name.""" dependency = base / "dependency" source = source_with(base, SIBLING, second=dependency.as_posix(), dependency=dependency) + return source, dependency.as_posix(), (base / "nowhere").as_posix() + + +def kept_in_the_code_leg(source: Path, base: Path) -> Path: + """A plan of `a_url_and_nowhere`'s `source` that keeps both submodules + in the code leg.""" return planned_with_urls(source, base, answers={**BOTH_IN_CODE, EXTRA: "code"}) +def url_plan(base: Path) -> Path: + """A plan that keeps in the code leg a submodule whose url is relative, + which `_resolved` resolves from the source and from the leg, and one + whose url is an absolute path on this disk, which `_answers` asks.""" + source, _, _ = a_url_and_nowhere(base) + return kept_in_the_code_leg(source, base) + + def asked_with_urls(adopter, base: Path): """`(plan, opened, work)`: `url_plan`, opened in a work directory.""" plan = adopter.Plan.load(url_plan(base)) @@ -932,6 +947,60 @@ def test_no_hook_of_this_machines_runs_where_the_urls_are_asked( assert runs(log) == [] +def counting_fsmonitor(base: Path) -> tuple[Path, Path]: + """`(program, log)`: a `core.fsmonitor` program that adds a line to + `log` each time git runs it, and exits 1, which git takes as a monitor + that cannot say what changed, and goes on without it.""" + program = base / "fsmonitor" + log = base / "fsmonitor-runs.log" + program.write_bytes(f"#!/bin/sh\necho fsmonitor >> '{log.as_posix()}'\n" + "exit 1\n".encode()) + program.chmod(0o755) + return program, log + + +#: Where this machine's `core.fsmonitor` program is configured. +FSMONITOR_CONFIGURED = [ + pytest.param(lambda base, program: config_with( + base, "[core]", f'\tfsmonitor = "{program.as_posix()}"'), + id="global-config"), + pytest.param(lambda _base, program: appended_to_command_scope( + os.environ, [("core.fsmonitor", program.as_posix())]), + id="git-config-count")] + + +@pytest.mark.parametrize("configured", FSMONITOR_CONFIGURED) +def test_no_fsmonitor_of_this_machines_runs_where_the_urls_are_asked( + tmp_path, configured): + """`_resolved`'s and `_answers`' repositories are given + `core.fsmonitor=false`, as the rehearsal's and #619's are, so no + fsmonitor program or daemon of this machine's is started for a + repository that is about to go. On 9b998936 the counting program ran + 8 times in this `check`, in `_resolved`'s two repositories (r1119b's + SHOULD-A).""" + plan = url_plan(tmp_path) + program, log = counting_fsmonitor(tmp_path) + result = checked(plan, configured(tmp_path, program)) + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [RELATIVE_URL], result.stderr + assert runs(log) == [] + + +@pytest.mark.parametrize("configured", FSMONITOR_CONFIGURED) +def test_the_counting_fsmonitor_runs_where_this_machines_config_reaches( + tmp_path, configured): + """The control: the same config runs the program for a `git add` in a + repository of its own, so the row above counts something that would + run.""" + program, log = counting_fsmonitor(tmp_path) + repository = tmp_path / "ordinary" + git("init", "-q", str(repository), cwd=tmp_path) + (repository / "a").write_bytes(b"a\n") + subprocess.run(["git", "add", "a"], cwd=str(repository), check=True, + env={**os.environ, **configured(tmp_path, program)}) + assert runs(log) != [] + + def test_a_url_question_git_cannot_ask_is_one_finding_after_the_rest( adopter, monkeypatch, tmp_path): """Where `_answers`' repository cannot be made or asked, that is @@ -1232,7 +1301,10 @@ def test_the_clone_takes_a_parent_gits_settings_and_no_other_repository( `GIT_CONFIG_COUNT` but an `attr.tree` in it, and the parent's `GIT_CONFIG_PARAMETERS` as it stands; and none of the variables that name another repository, its objects, an index or a work tree, nor - `GIT_ATTR_SOURCE`. With no parent setting there is none.""" + `GIT_ATTR_SOURCE`. With no parent setting there is none. The two + entries are appended after those the environment holds, as every test + gives git a setting (`appended_to_command_scope`, #758), and the last + entry `_split_clone_env()` keeps is the second (r1119b's MUST-1).""" named = {"GIT_DIR": "git-dir", "GIT_WORK_TREE": "work-tree", "GIT_INDEX_FILE": "index", "GIT_COMMON_DIR": "common", "GIT_OBJECT_DIRECTORY": "objects", "GIT_ATTR_SOURCE": "HEAD"} @@ -1241,13 +1313,16 @@ def test_the_clone_takes_a_parent_gits_settings_and_no_other_repository( parameters = "'test.parentone'='one' 'core.hooksPath'='/nowhere'" monkeypatch.setenv("GIT_CONFIG_PARAMETERS", parameters) for name, value in appended_to_command_scope( - {}, [("attr.tree", "HEAD"), ("test.heldone", "one")]).items(): + os.environ, + [("attr.tree", "HEAD"), ("test.heldone", "one")]).items(): monkeypatch.setenv(name, value) env = adopter._split_clone_env() assert env["GIT_CONFIG_PARAMETERS"] == parameters assert [env.get(name) for name in named] == [None] * len(named) - assert (env["GIT_CONFIG_COUNT"], env["GIT_CONFIG_KEY_0"], - env["GIT_CONFIG_VALUE_0"]) == ("1", "test.heldone", "one") + scoped = [(env[f"GIT_CONFIG_KEY_{n}"], env[f"GIT_CONFIG_VALUE_{n}"]) + for n in range(int(env["GIT_CONFIG_COUNT"]))] + assert scoped[-1] == ("test.heldone", "one") + assert "attr.tree" not in [key.lower() for key, _ in scoped] monkeypatch.delenv("GIT_CONFIG_PARAMETERS") assert "GIT_CONFIG_PARAMETERS" not in adopter._split_clone_env() @@ -1280,6 +1355,14 @@ def test_the_clone_writes_nothing_into_an_object_directory_around_it( assert list(work.iterdir()) == [] +def rewritten_in_the_source(source: Path, url: str, nowhere: str) -> None: + """The source's own config rewrites `url` to `nowhere`; the control: + git there expands it so.""" + git("config", f"url.{nowhere}.insteadOf", url, cwd=source) + assert git("ls-remote", "--get-url", url, + cwd=source).stdout.strip() == nowhere + + def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): """`_answers` asks each url in a throwaway repository of its own, so that no repository's config around this process rewrites a url, as @@ -1291,16 +1374,9 @@ def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): and found the url answering nothing (r1119, M14). Nor is the source's config written: `_resolved` sets `remote.origin.url` in its own repository, which in the whole environment is the source.""" - dependency = tmp_path / "dependency" - source = source_with(tmp_path, SIBLING, second=dependency.as_posix(), - dependency=dependency) - nowhere = (tmp_path / "nowhere").as_posix() - git("config", f"url.{nowhere}.insteadOf", dependency.as_posix(), - cwd=source) - assert git("ls-remote", "--get-url", dependency.as_posix(), - cwd=source).stdout.strip() == nowhere - plan = planned_with_urls(source, tmp_path, - answers={**BOTH_IN_CODE, EXTRA: "code"}) + source, url, nowhere = a_url_and_nowhere(tmp_path) + rewritten_in_the_source(source, url, nowhere) + plan = kept_in_the_code_leg(source, tmp_path) config = (source / ".git" / "config").read_bytes() for env in ({}, {"GIT_DIR": str(source / ".git")}): result = checked(plan, env) @@ -1309,6 +1385,50 @@ def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): assert (source / ".git" / "config").read_bytes() == config, env +def test_a_work_directory_inside_the_source_rewrites_no_url_it_asks( + tmp_path): + """`--work-dir` inside the source's own work tree, whose config + rewrites the on-disk url to a directory that holds nothing: `_answers` + asks git in its throwaway repository, which git finds before the + source around it, so `check` says what it says with a work directory + outside the source, leaves nothing there and writes nothing into the + source's config. Asked in the throwaway's own directory, which is no + repository, git found the source around it and read its rewrite: a + false `plan-submodule-url-answers-nothing` (r1119b's NIT-A, R8).""" + source, url, nowhere = a_url_and_nowhere(tmp_path) + rewritten_in_the_source(source, url, nowhere) + plan = kept_in_the_code_leg(source, tmp_path) + config = (source / ".git" / "config").read_bytes() + for work in (tmp_path / "work", source / "work"): + result = checked(plan, None, "--work-dir", str(work)) + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [RELATIVE_URL], (work, result.stderr) + assert list(work.iterdir()) == [] + assert (source / ".git" / "config").read_bytes() == config + + +def test_a_parent_gits_url_rewrite_is_not_read_where_a_url_is_asked( + tmp_path): + """A parent git's `-c url..insteadOf=`: the split's own + clone takes the settings a parent hands down (`_split_clone_env`), and + the assembly's recursive clone, run later by no such parent, does not, + so `_answers` leaves `GIT_CONFIG_PARAMETERS` out, as its docstring + says, and `check` says what it says without it. Taking it, `_answers` + found the url answering nothing (r1119b's NIT-B, A18). The control: + git expands the url so under it.""" + source, url, nowhere = a_url_and_nowhere(tmp_path) + plan = kept_in_the_code_leg(source, tmp_path) + handed = appended_to_parameters([(f"url.{nowhere}.insteadOf", url)]) + expanded = subprocess.run(["git", "ls-remote", "--get-url", url], + cwd=str(tmp_path), capture_output=True, + env={**os.environ, **handed}, check=True) + assert expanded.stdout.decode().strip() == nowhere + for env in ({}, handed): + result = checked(plan, env) + assert "REFUSED" not in result.stderr, result.stderr + assert codes(result) == [RELATIVE_URL], (env, result.stderr) + + @pytest.mark.parametrize("asked", [ pytest.param(lambda adopter, source: adopter._resolved( "..", ["../x"], None, source), id="resolved"), diff --git a/tests/test_adopt_root_symlinks.py b/tests/test_adopt_root_symlinks.py index b1739eae..cd3b82fa 100644 --- a/tests/test_adopt_root_symlinks.py +++ b/tests/test_adopt_root_symlinks.py @@ -62,7 +62,7 @@ import pytest -from conftest import (ADOPT, REPO, appended_to_command_scope, +from conftest import (ADOPT, REPO, WINDOWS_SKIP, appended_to_command_scope, clear_ambient_pin_sources, cloned_just_pushed, git, make_source_repo, run_script, write_plan) @@ -2067,14 +2067,44 @@ def unwritable(_work_root): assert probe.notes() == [] +def refusing_the_ref_probe(base: Path) -> dict: + """The environment of a git that will not make the ref probe's + repository: a stub first on PATH fails each `git init` of a + `refs-probe-*` directory as `GIT_DEFAULT_HASH=bogus` fails it + (`UNKNOWN_HASH`, exit 128), and hands any other command to the real + git.""" + stubs = base / "ref-probe-stubs" + stubs.mkdir() + stub = stubs / "git" + stub.write_bytes("\n".join([ + "#!/bin/sh", + 'for arg in "$@"; do', + ' [ "$arg" = init ] || continue', + ' for word in "$@"; do', + ' case "$word" in */refs-probe-*) ;; *) continue ;; esac', + f' echo "{UNKNOWN_HASH}" >&2', + " exit 128", + " done", + "done", + 'exec "$REAL_GIT" "$@"', ""]).encode("ascii")) + stub.chmod(0o755) + return {"PATH": str(stubs) + os.pathsep + os.environ["PATH"], + "REAL_GIT": shutil.which("git")} + + +@WINDOWS_SKIP # a `#!` stub named `git` first on PATH def test_check_says_where_git_would_not_make_the_ref_probe( links_as_links, tmp_path, monkeypatch): - """`GIT_DEFAULT_HASH=bogus` fails the ref probe's `git init`, and not - `check`'s throwaway repositories, which name the source's format: - `check` passes a link to `.git/HEAD`, as git's defaults hold it, and - says why it took them.""" + """A git that will not make the ref probe's repository, and makes every + other: `check` passes a link to `.git/HEAD`, as git's defaults hold it, + and says why it took them, in git's words. The probe's `git init` is + failed by a stub, as `GIT_DEFAULT_HASH=bogus` failed it here before + #1083: that variable now stops the split's own `git clone` of the + source too, which `check` makes first and names `adopt-git-unasked`, + so it would no longer reach the probe (r1119b's MUST-2).""" _, plan = planned(tmp_path, {}, {CLAUDE: GIT_HEAD}, {CLAUDE: ROOT}) - monkeypatch.setenv("GIT_DEFAULT_HASH", "bogus") + for name, value in refusing_the_ref_probe(tmp_path).items(): + monkeypatch.setenv(name, value) checked = check(plan) assert checked.returncode == 0, checked.stderr + checked.stdout assert NO_REF_PROBE_NOTE in checked.stdout, checked.stdout From 0b326952cc8f7958ff10dc3ef05640d53c7f7ad7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 21:25:10 +0000 Subject: [PATCH 05/17] tests/test_adopt_check_throwaway_repos.py: the row with `--work-dir` inside the source also rewrites the url to a network, so a `--get-url` asked around the throwaway, which names it in a NOTE as not asked, fails it f1119c's mutant R8a, which moved only `_answers`' `git ls-remote --get-url` out of its throwaway repository into the throwaway's own directory, survived the row that kills r1119b's R8: the expansion there only decides between the disk and a network, and the url itself was still asked in the throwaway. A source config that rewrites the on-disk url to `https://f1119c.invalid/Thing` makes R8a print the NOTE of a url on a network it did not ask; on this head `check` prints none, inside the source or outside it. Now killed by the row's `a-network` case. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_check_throwaway_repos.py | 46 ++++++++++++++++------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tests/test_adopt_check_throwaway_repos.py b/tests/test_adopt_check_throwaway_repos.py index 8b955664..c71279fa 100644 --- a/tests/test_adopt_check_throwaway_repos.py +++ b/tests/test_adopt_check_throwaway_repos.py @@ -1355,12 +1355,12 @@ def test_the_clone_writes_nothing_into_an_object_directory_around_it( assert list(work.iterdir()) == [] -def rewritten_in_the_source(source: Path, url: str, nowhere: str) -> None: - """The source's own config rewrites `url` to `nowhere`; the control: - git there expands it so.""" - git("config", f"url.{nowhere}.insteadOf", url, cwd=source) +def rewritten_in_the_source(source: Path, url: str, target: str) -> None: + """The source's own config rewrites `url` to `target`; the control: + git there expands it so (`--get-url` contacts nothing).""" + git("config", f"url.{target}.insteadOf", url, cwd=source) assert git("ls-remote", "--get-url", url, - cwd=source).stdout.strip() == nowhere + cwd=source).stdout.strip() == target def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): @@ -1385,24 +1385,42 @@ def test_a_repository_around_check_rewrites_no_url_it_asks(tmp_path): assert (source / ".git" / "config").read_bytes() == config, env +#: How the NOTE begins that names each kept url `check` did not ask, as a +#: config here rewrites it off the disk. +URL_NOT_ASKED = ("NOTE `check` did not ask whether the url of each " + "submodule the plan keeps") +#: Where the source's own config sends the on-disk url: a directory that +#: holds nothing, which `git ls-remote` finds answering nothing, and a +#: network, which `check` names in `URL_NOT_ASKED` and does not ask. +REWRITTEN_TO = [ + pytest.param(lambda _source, nowhere: nowhere, + id="a-directory-holding-nothing"), + pytest.param(lambda source, _nowhere: "https://f1119c.invalid/" + + source.name, id="a-network")] + + +@pytest.mark.parametrize("rewritten_to", REWRITTEN_TO) def test_a_work_directory_inside_the_source_rewrites_no_url_it_asks( - tmp_path): + tmp_path, rewritten_to): """`--work-dir` inside the source's own work tree, whose config - rewrites the on-disk url to a directory that holds nothing: `_answers` - asks git in its throwaway repository, which git finds before the - source around it, so `check` says what it says with a work directory - outside the source, leaves nothing there and writes nothing into the - source's config. Asked in the throwaway's own directory, which is no - repository, git found the source around it and read its rewrite: a - false `plan-submodule-url-answers-nothing` (r1119b's NIT-A, R8).""" + rewrites the on-disk url: `_answers` asks git in its throwaway + repository, which git finds before the source around it, so `check` + says what it says with a work directory outside the source, leaves + nothing there and writes nothing into the source's config. Asked in + the throwaway's own directory, which is no repository, git found the + source around it and read its rewrite (r1119b's NIT-A): `git + ls-remote` there gave a false `plan-submodule-url-answers-nothing` + (R8), and `--get-url` there a NOTE of a url on a network not asked.""" source, url, nowhere = a_url_and_nowhere(tmp_path) - rewritten_in_the_source(source, url, nowhere) + rewritten_in_the_source(source, url, rewritten_to(source, nowhere)) plan = kept_in_the_code_leg(source, tmp_path) config = (source / ".git" / "config").read_bytes() for work in (tmp_path / "work", source / "work"): result = checked(plan, None, "--work-dir", str(work)) assert "REFUSED" not in result.stderr, result.stderr assert codes(result) == [RELATIVE_URL], (work, result.stderr) + assert URL_NOT_ASKED not in result.stdout + result.stderr, \ + (work, result.stdout) assert list(work.iterdir()) == [] assert (source / ".git" / "config").read_bytes() == config From bba168bdd1eb7906e74943fbf2114e8a9f998cad Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 21 Oct 2026 21:47:32 +0000 Subject: [PATCH 06/17] adopt-project.py: `_split_clone_asked`'s docstring names the global or system config as where the `uploadpack.packObjectsHook` a shallow source's clone runs comes from, since a parent's `-c` did not reach it cea25530's NIT-C sentence said "protected config ... or a `-c`". f1119c measured it on git 2.43, with a shallow source and a counting hook: - in the global config, 1 run in `check` and 1 in a clone made as `_extract_leg` makes it; - in the source's own config, 0 and 0; - in `GIT_CONFIG_PARAMETERS`, as a parent `git -c` hands it down, 0 and 0. So the `-c` is dropped. The commit message of cea25530 says "of protected config" for the same sentence; this is the measured form. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 010c9ccc..b17f7a41 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -11651,10 +11651,10 @@ def _split_clone_asked(source: Source) -> None: NIT-1, measured: 117 MiB for a 115 MiB pack). That is a cost, not a fault: each of `execute`'s own clones of such a source is a full copy too. So such a clone also runs the `uploadpack.packObjectsHook` that - this machine's protected config names (its system or global config, or - a `-c`; git takes none from the source's own) to pack what it copies, - as each of `execute`'s clones of that source runs it (r1119b's - NIT-C).""" + this machine's global or system config names, to pack what it copies, + as each of `execute`'s clones of that source runs it. git takes none + from the source's own config, and a parent's `-c` did not reach it + (measured with git 2.43: 1, 0 and 0 runs; r1119b's NIT-C).""" env = _split_clone_env() scratch = _throwaway("openreposhape-clone-", source) try: From 3403058ae3760a18581aa705b3033debb3dd9dfa Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 13:32:22 +0000 Subject: [PATCH 07/17] adopt-project.py: `check` and `execute` run with `GIT_NO_LAZY_FETCH=1` and refuse a partial-clone source that lacks an object its branches or tags reach as `source-partial-clone`, so neither fetches into the source any more (#1145) Measured with git 2.43 on a `--filter=blob:none --no-checkout` source: `check` ran the source's promisor `upload-pack` 26 times and wrote 26 promisor packs into it; on this commit it runs it 0 times and leaves the source's `objects/` unchanged. A full clone and a partial clone given every object again (`git fetch --refetch --no-filter origin`) still check `plan ok`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 218 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) diff --git a/adopt-project.py b/adopt-project.py index b17f7a41..8e1142bf 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -12906,6 +12906,218 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], return found +# --------------------------------------------------------------------------- +# A partial-clone source is never fetched into (#1145) +# --------------------------------------------------------------------------- +# +# `check` and `execute` read the source and write nothing into it. A partial +# clone (`git clone --filter`) broke that through git itself: git fetches an +# object such a clone lacks from a promisor remote on any read that needs +# one, with the `upload-pack` program the source's own config names +# (`remote..uploadpack`), and writes what it fetched into the source's +# `objects/` as a promisor pack. Measured with git 2.43 on a +# `--filter=blob:none --no-checkout` clone of a local `origin`, its plan +# written from a full clone at the same commit: `check` ran that +# `upload-pack` 26 times and took the source's `.promisor` files from 1 to +# 27, and so did `execute`, whose `git filter-repo` then died with `Blob +# not found` after both leg repositories were made. A promisor on a network +# is fetched from the same way, with the credentials of whoever runs the +# command. So there are two layers, as the doctor has them (#683, #754, +# #787): +# +# - `_lazy_fetch_off`, over the whole of both commands: with +# `GIT_NO_LAZY_FETCH=1`, a read of an object the source lacks fails where +# it fetched, whatever reads it and whichever commit it reads, a commit +# the plan names included; +# - `_refuse_a_partial_clone_short`, as soon as the source is opened: a +# partial clone that lacks an object its branches or tags reach is +# refused before anything reads it, on every git, the gits that ignore +# the variable included. +# +# NOT `-c protocol.allow=never`: measured, the source's own +# `protocol.file.allow=always` outranks it, and the lazy fetch ran. NOR AN +# EMPTY `GIT_ALLOW_PROTOCOL` over the whole command, which stops that fetch +# too: `check` and `execute` need their transports, for the clone of the +# source, `ls-remote` and the pushes. And `plan`, which reads its source +# the same way, is not changed here. + +#: The variable git reads to leave out the fetch a partial clone makes for an +#: object it lacks: `shape-doctor.py`'s `NO_LAZY_FETCH` (#787, #1145). +NO_LAZY_FETCH = "GIT_NO_LAZY_FETCH" +#: Refused: a source that is a partial clone and lacks an object its branches +#: or tags reach (#1145). +SOURCE_PARTIAL_CLONE = "source-partial-clone" +#: The config keys `_promisor_remotes` reads: git's, in `promisor-remote.c`, +#: as the doctor's `PROMISOR_KEYS` (#683). +PROMISOR_KEYS = (r"^(remote\..*\.(promisor|partialclonefilter)" + r"|extensions\.partialclone)$") +#: What `_objects_lacking` runs: every object the source's branches and tags +#: reach, one a line, and `?` for each the source lacks. It never +#: fetches, on any git that has `--missing` (measured with git 2.43 and +#: `GIT_NO_LAZY_FETCH` unset: no `upload-pack` ran). +LACKING_WALK = ("rev-list", "--objects", "--missing=print", "--branches", + "--tags") + + +@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: the doctor's `lazy_fetch_off` (#787), + over `check` and `execute` (#1145). + + EVERY GIT THE COMMAND STARTS inherits it, and every git those start: + each read of the source, the throwaway repositories `check` makes, the + clones and `git filter-repo` of `execute`. A read of an object a partial + clone lacks then fails, as one fails where the clone's remote is out of + reach (measured with git 2.43: exit 128, and no `upload-pack` ran), and + each reader here already answers a read that failed. Nothing printed for + a person's own shell carries it. + + WHICH GIT HONOURS IT: git 2.45.0 and later, and the security releases of + 2024-05-14 back to 2.39.4, whose fix for CVE-2024-32465 made + `promisor-remote.c` read it, as distributions' own builds of those do + (Ubuntu's 2.43.0-1ubuntu7.1 and later). An older git ignores it, and + there `_refuse_a_partial_clone_short` is what keeps the source whole.""" + 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 + + +def _config_listed(path: Path, *args: str) -> bytes: + """What `git config -z ` lists in `path`, as bytes; nothing where + it lists nothing (exit 1) or cannot run. Config only: no object is read, + so nothing is fetched.""" + try: + listed = subprocess.run(["git", GIT_CONFIG, "-z", *args], + cwd=str(path), capture_output=True, + check=False) + except OSError: + return b"" + return listed.stdout if listed.returncode == 0 else b"" + + +def _promisor_remotes(path: Path) -> list[str]: + """The remotes git fetches an object the repository at `path` lacks + from, by the rule git 2.43's `promisor-remote.c` reads its config with, + the doctor's `promisor_remotes` (#683): a remote whose `promisor` is + true, a remote with a `partialclonefilter`, and the remote + `extensions.partialclone` names, that one last, as git asks them; each + name once, in the config's order. `promisor = false` alone is not one. + + `_a_partial_clone`, which asks a pin's clone, keeps its older rule.""" + true = _config_listed(path, "--type=bool", "--get-regexp", + r"^remote\..*\.promisor$") + promised = {key for key, _newline, value in ( + record.partition(b"\n") for record in true.split(b"\0")) + if value == b"true"} + names: list[str] = [] + named = None + for record in _config_listed(path, "--get-regexp", + PROMISOR_KEYS).split(b"\0"): + key, _newline, value = record.partition(b"\n") + if key == b"extensions.partialclone": + named = value + elif key.endswith(b".partialclonefilter") or key in promised: + names.append(_git_path(key[len(b"remote."):key.rindex(b".")])) + last = [] if named is None else [_git_path(named)] + return [name for name in dict.fromkeys(names) if name not in last] + last + + +def _objects_lacking(path: Path) -> tuple[int | None, str]: + """`(how many objects the branches and tags of the repository at `path` + reach that it lacks, "")`, or `(None, what git said)` where `git + rev-list` cannot walk them: a branch whose commit it lacks, before git + 2.45, among them. The output is read as it comes, a line at a time, and + git's own words go to a file, so neither pipe can fill and stall the + other. A missing tree hides what is under it, so the count is a least.""" + lacking = 0 + try: + with tempfile.TemporaryFile() as said, subprocess.Popen( + ["git", *LACKING_WALK], cwd=str(path), + stdout=subprocess.PIPE, stderr=said) as walk: + for line in walk.stdout: + lacking += line.startswith(b"?") + if walk.wait(): + said.seek(0) + words = said.read().decode("utf-8", "replace").strip() + return None, (f"exited {walk.returncode}: " + f"{spelled_as_git_out(words)}") + except OSError as exc: + return None, f"could not run ({_os_reason(exc)})" + return lacking, "" + + +def _refetch(remote: str) -> str: + """The command that fetches every object again from `remote`, with no + filter, as a person pastes it: the name quoted, where it is printable and + no option; otherwise the step in words, as `_from_the_remote` says one.""" + if remote.isprintable() and not remote.startswith("-"): + return f"`git fetch --refetch --no-filter {shlex.quote(remote)}`" + return ("a `git fetch --refetch --no-filter` from the remote named " + f"{_spelled(remote)}") + + +def _refuse_a_partial_clone_short(source: Source) -> None: + """`source-partial-clone` for a source that is a partial clone + (`_promisor_remotes`) and lacks an object its branches or tags reach + (`_objects_lacking`), or whose branches and tags git cannot walk; asked + by `check` and `execute` as soon as the source is opened, before + anything else reads it (#1145). + + ONLY WHAT IS LACKING IS REFUSED. A partial clone given every object + again (`git fetch --refetch --no-filter`) keeps its promisor config, and + lacks nothing a read could fetch: it is read as a full clone is. A full + clone is asked its config alone, which reads no object. + + WHY BRANCHES AND TAGS: the source's branch is what `check` reads and + `execute` splits, and `execute`'s `git filter-repo` rewrites a clone of + the source, which copies every branch and tag. Measured with git 2.43, a + `--filter=blob:none` clone that had checked out its branch, and so + lacked only blobs of the history, passed `check` and failed `execute` + in `git filter-repo`, `Blob not found`, after both leg repositories + were made. A commit the plan names that no branch or tag reaches is + `_lazy_fetch_off`'s to keep unfetched. + + THE WAY OUT FETCHES, AND IS THE PERSON'S OWN: run in the source, from + each promisor remote, `git fetch --refetch --no-filter` (git 2.36 or + later), which leaves it lacking nothing; or give a full clone with + `--source`.""" + remotes = _promisor_remotes(source.path) + if not remotes: + return + lacking, said = _objects_lacking(source.path) + if lacking == 0: + return + where = shown_path(source.path) + walk = f"`git {' '.join(LACKING_WALK)}`" + short = (f"lacks at least {lacking} object(s) its branches and tags " + "reach" if lacking is not None else + f"lacks what its branches and tags reach, as far as git can " + f"tell: {walk}, which lists what a clone lacks without fetching " + f"it, {said}") + raise Refusal( + SOURCE_PARTIAL_CLONE, + f"the source {where} is a partial clone (`git clone --filter`) that " + f"{short}. Git fetches an object a partial clone lacks from its " + f"promisor remote ({', '.join(map(_spelled, remotes))}) on any read " + "that needs it, and `check` and `execute` never fetch into the " + "source: `execute`'s `git filter-repo` reads every branch and tag, " + "and a partial clone short of one of their blobs failed it after " + "both leg repositories were made", + f"Remediation: make the source whole, then run this command again: " + f"in {where}, run " + f"{', then '.join(map(_refetch, remotes))}, which fetches every " + "object again with no filter (git 2.36 or later). Or pass a full " + "clone (`git clone` with no `--filter`) with --source.") + + +@_lazy_fetch_off() def cmd_check(args) -> int: plan = Plan.load(Path(args.plan)) # FIRST, before anything is opened: the keys the rest is read from (#386). @@ -12931,6 +13143,9 @@ def cmd_check(args) -> int: if not unwritable: raise raise _work_dir_refusal(unwritable) from None + # Before anything reads it, and after the work directory's refusal, so + # that one is not masked (#1145). + _refuse_a_partial_clone_short(source) naming = NamingPolicy.load(NAMING_POLICY) findings: list[str] = [] if unwritable: @@ -15535,6 +15750,7 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, return split_commit, message, materialized.collisions +@_lazy_fetch_off() def cmd_execute(args) -> int: plan = Plan.load(Path(args.plan)) _refuse_a_mistyped_plan(plan) @@ -15542,6 +15758,8 @@ def cmd_execute(args) -> int: # Before the source is opened, so before any leg exists (#524). _refuse_an_unwritable_work_dir(args, work_root) source = plan.open_source(args.source, work_root, done=NO_LEG_YET) + # Before anything reads it, so before any leg exists (#1145). + _refuse_a_partial_clone_short(source) naming = NamingPolicy.load(NAMING_POLICY) _require_filter_repo() From 179b58efbdbe666c07fd9086bdc55eae08acf51c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 13:34:14 +0000 Subject: [PATCH 08/17] adopt-project.py and docs/cli.md: `check --source` and `execute --source` name `source-partial-clone`, the source it refuses, and the fetch that makes it whole (#1145) Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 7 ++++++- docs/cli.md | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/adopt-project.py b/adopt-project.py index 8e1142bf..ce02c50f 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -17941,7 +17941,12 @@ def _work_dir_help(does: str, refused: str, but: str = "") -> str: "`local_path` that is gone is refused as `plan-source-gone`; one that " "is a file, or a directory inside a repository, as " "`source-not-top-level`; and one this user cannot reach, a clone " - "`plan` made included, as `source-unreadable`, never cloned again.") + "`plan` made included, as `source-unreadable`, never cloned again. " + "Either source, where it is a partial clone (`git clone --filter`) " + "that lacks an object its branches or tags reach, is refused as " + "`source-partial-clone` before anything reads it: git would fetch that " + "object into it, and neither command fetches into the source. `git " + "fetch --refetch --no-filter `, run in it, makes it whole.") def main(argv: list[str] | None = None) -> int: diff --git a/docs/cli.md b/docs/cli.md index 837c7524..525dfa1c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -642,6 +642,13 @@ options: inside a repository, as `source-not-top-level`; and one this user cannot reach, a clone `plan` made included, as `source-unreadable`, never cloned again. + Either source, where it is a partial clone (`git clone + --filter`) that lacks an object its branches or tags + reach, is refused as `source-partial-clone` before + anything reads it: git would fetch that object into + it, and neither command fetches into the source. `git + fetch --refetch --no-filter `, run in it, + makes it whole. --local-remote-dir LOCAL_REMOTE_DIR the directory `execute --local-remote-dir` will be given (the TEST path): each relative submodule url the @@ -701,6 +708,13 @@ options: inside a repository, as `source-not-top-level`; and one this user cannot reach, a clone `plan` made included, as `source-unreadable`, never cloned again. + Either source, where it is a partial clone (`git clone + --filter`) that lacks an object its branches or tags + reach, is refused as `source-partial-clone` before + anything reads it: git would fetch that object into + it, and neither command fetches into the source. `git + fetch --refetch --no-filter `, run in it, + makes it whole. --local-remote-dir LOCAL_REMOTE_DIR create the legs as bare repositories here instead of calling `gh` (the TEST path). Both are asked about From 3b11751a14d5a3720ba6df9bca69dd3249c6bb6c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 20:13:01 +0000 Subject: [PATCH 09/17] tests/test_adopt_partial_clone_source.py: `check` and `execute` refuse a partial-clone source that lacks objects as `source-partial-clone` and run no `upload-pack`, and read no commit the plan names that the source lacks (#1145) Each row counts the promisor's `upload-pack` runs through a counting `remote.origin.uploadpack` and hashes the source's `objects/`. On #1119's head bba168bd, 18 of the 21 rows fail: `check` says `plan ok` for a `--no-checkout` partial clone, a checked-out one, and one whose own config sets `protocol.file.allow=always`; `execute` dies in `git filter-repo` after both legs; the guard alone is not there; and the plan's commit, read by the Triad advisory and by `_not_a_whole_clone`, is fetched once. The full clone, the cured clone and the cured `execute` pass on both. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_partial_clone_source.py | 416 +++++++++++++++++++++++ 1 file changed, 416 insertions(+) create mode 100644 tests/test_adopt_partial_clone_source.py diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py new file mode 100644 index 00000000..4c25646f --- /dev/null +++ b/tests/test_adopt_partial_clone_source.py @@ -0,0 +1,416 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`check` and `execute` never fetch into a partial-clone source (#1145). + +A partial clone (`git clone --filter=blob:none`) lacks objects, and git +fetches one it lacks from the promisor remote on any read that needs it: it +runs the `upload-pack` the source's own `remote.origin.uploadpack` names and +writes a promisor pack into the source's `objects/`. Measured on #1119's +head bba168bd with git 2.43, the plan written from a full clone and the +source then made again as the partial clone at the same commit: + +* a `--filter=blob:none --no-checkout` clone (28 objects lacking): `check` + said `plan ok` after running that `upload-pack` 26 times, and the source's + `.promisor` files went from 1 to 27; `execute` did the same, then made both + leg repositories, and its `git filter-repo` died with `Blob not found`; +* a `--filter=blob:none` clone that checked its branch out (2 history blobs + lacking): `check` said `plan ok`, and `execute` made both legs and died + the same way. + +Now both commands run with `GIT_NO_LAZY_FETCH=1` (`_lazy_fetch_off`), and +refuse a partial clone that lacks an object its branches or tags reach as +`source-partial-clone` before anything reads it +(`_refuse_a_partial_clone_short`). A partial clone given every object again +(`git fetch --refetch --no-filter`) lacks nothing and is read as a full clone +is. Each row counts the promisor's `upload-pack` runs through a counting +`remote.origin.uploadpack`, and hashes the source's `objects/` before and +after. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import os +import re +import shlex +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from conftest import (ADOPT, FILE_PROTOCOL, git, make_source_repo, resolve, + run_script, write_plan) +from test_adopt_temp_work_roots import gh_clones + +#: The refusal #1145 adds. +CODE = "source-partial-clone" +REFUSED = f"REFUSED {CODE}:" +#: The way out the refusal names, for the remote a clone calls `origin`. +CURE = "`git fetch --refetch --no-filter origin`" +#: The variable `_lazy_fetch_off` sets. +VARIABLE = "GIT_NO_LAZY_FETCH" +#: A name no git reads: `NO_LAZY_FETCH` set to it stands for a git that +#: ignores the variable. +IGNORED = "GIT_IGNORED_BY_GIT" +#: The answers the synthetic source's plan asks for. +ANSWERS = (("examples/", "spec"), (".claude/", "root"), + ("release.yaml", "root")) +#: How each source is cloned from `origin`: `cured` is fetched again with no +#: filter after its clone, and `fileallow` sets the source's own +#: `protocol.file.allow=always`, which outranked `-c protocol.allow=never`. +VARIANTS = {"nocheckout": ("--filter=blob:none", "--no-checkout"), + "checkout": ("--filter=blob:none",), + "fileallow": ("--filter=blob:none", "--no-checkout"), + "cured": ("--filter=blob:none", "--no-checkout"), + "full": ()} +#: A commit on a branch the source never clones. +SIDE = "side" + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git-filter-repo is not installed") + + +@pytest.fixture(scope="module") +def adopter(): + spec = importlib.util.spec_from_file_location( + "adopt_partial_clone_source", ADOPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(autouse=True) +def no_ambient_variable(monkeypatch): + """No `GIT_NO_LAZY_FETCH` from the shell the suite runs in: a row asks + what the tool sets.""" + monkeypatch.delenv(VARIABLE, raising=False) + + +@pytest.fixture(scope="module") +def planned_once(tmp_path_factory) -> dict: + """`origin`, which lets a clone filter, a plan written from a full clone + of its `main`, every entry answered, and a commit on a branch `side` no + clone below takes; made once, since every row reads it alone.""" + base = tmp_path_factory.mktemp("planned") + origin = make_source_repo(base / "origin") + git("config", "uploadpack.allowFilter", "true", cwd=origin) + full = base / "Thing" + git(*FILE_PROTOCOL, "clone", "-q", "--single-branch", "--no-local", + origin.as_uri(), str(full), cwd=base) + plan = base / "plan" / "adoption-plan.yaml" + plan.parent.mkdir() + written = write_plan(full, plan) + assert written.returncode == 0, written.stderr + written.stdout + for path, leg in ANSWERS: + resolve(plan, path, leg) + commit = git("rev-parse", "HEAD", cwd=origin).stdout.strip() + git("switch", "-q", "-c", SIDE, cwd=origin) + git("commit", "-q", "--allow-empty", "-m", "Elsewhere", cwd=origin) + side = git("rev-parse", "HEAD", cwd=origin).stdout.strip() + git("switch", "-q", "main", cwd=origin) + return {"origin": origin, "text": plan.read_text(encoding="utf-8"), + "at": full, "commit": commit, "side": side} + + +class Source: + """One source made as a `VARIANTS` clone of `origin` at `/Thing`, + its `upload-pack` counted, and the plan pointed at it.""" + + def __init__(self, planned: dict, base: Path, variant: str): + self.planned = planned + self.base = base + self.path = base / "Thing" + git(*FILE_PROTOCOL, "clone", "-q", "--single-branch", + *VARIANTS[variant], "--no-local", planned["origin"].as_uri(), + str(self.path), cwd=base) + if variant == "cured": + git(*FILE_PROTOCOL, "fetch", "-q", "--refetch", "--no-filter", + "origin", cwd=self.path) + if variant == "fileallow": + git("config", "protocol.file.allow", "always", cwd=self.path) + self.log = base / "upload-pack.log" + wrapper = base / "upload_pack.py" + wrapper.write_text( + "import subprocess, sys\n" + f"with open({str(self.log)!r}, 'a', encoding='utf-8') as log:\n" + " log.write('ran\\n')\n" + "sys.exit(subprocess.call(['git', 'upload-pack', " + "*sys.argv[1:]]))\n", encoding="utf-8") + git("config", "remote.origin.uploadpack", + f"{shlex.quote(sys.executable)} {shlex.quote(str(wrapper))}", + cwd=self.path) + self.plan = base / "plan" / "adoption-plan.yaml" + self.plan.parent.mkdir() + self.write_plan(planned["text"].replace( + f' local_path: "{planned["at"]}"\n', + f' local_path: "{self.path}"\n')) + self.before = self.store() + + def write_plan(self, text: str) -> None: + assert f' local_path: "{self.path}"\n' in text + self.plan.write_bytes(text.encode("utf-8")) + + def runs(self) -> int: + """How many times the promisor's `upload-pack` ran.""" + return (len(self.log.read_text(encoding="utf-8").splitlines()) + if self.log.exists() else 0) + + def store(self) -> str: + """A sha256 of every file under the source's `objects/`: each path + and its bytes.""" + objects = self.path / ".git" / "objects" + digest = hashlib.sha256() + for path in sorted(objects.rglob("*")): + if path.is_file(): + digest.update(path.relative_to(objects).as_posix().encode() + + b"\0" + path.read_bytes() + b"\0") + return digest.hexdigest() + + def promisor_packs(self) -> int: + return len(list((self.path / ".git" / "objects" / "pack") + .glob("*.promisor"))) + + def lacking(self) -> int: + listed = git("rev-list", "--objects", "--missing=print", "--all", + cwd=self.path).stdout + return sum(line.startswith("?") for line in listed.splitlines()) + + def untouched(self) -> None: + """No `upload-pack` ran, and `objects/` is as it was.""" + assert self.runs() == 0 + assert self.store() == self.before + + def check(self) -> subprocess.CompletedProcess: + return run_script(ADOPT, "check", "--plan", str(self.plan), + "--work-dir", str(self.base / "work")) + + def execute(self) -> subprocess.CompletedProcess: + return run_script(ADOPT, "execute", "--plan", str(self.plan), + "--yes", "--local-remote-dir", + str(self.base / "remotes"), + "--work-dir", str(self.base / "work")) + + def at_the_side_commit(self, cloned: bool = False) -> None: + """The plan names `side`'s commit, which `origin` holds and the + source lacks; `cloned`, as a clone `plan` made of a repository GitHub + does not have.""" + text = self.plan.read_text(encoding="utf-8") + planned = self.planned + # Quoted where YAML would read the hash as another type. + text, swapped = re.subn( + rf'^ commit: "?{planned["commit"]}"?$', + f' commit: "{planned["side"]}"', text, count=1, flags=re.M) + assert swapped == 1 + if cloned: + text = text.replace( + " repository: null\n", ' repository: "testorg/Elsewhere"\n', + 1).replace(" cloned: false\n", " cloned: true\n", 1) + assert " cloned: true\n" in text + self.write_plan(text) + + +@pytest.fixture +def made(planned_once, tmp_path): + return lambda variant: Source(planned_once, tmp_path, variant) + + +def skip_where_git_ignores_the_variable(source: Source) -> None: + """Skip where this git fetches a commit the source lacks with + `GIT_NO_LAZY_FETCH=1` set: before 2.39.4, and an unpatched 2.40 to + 2.44. Asked before the row, since an ignored variable fetches it.""" + asked = subprocess.run( + ["git", "cat-file", "-e", f"{source.planned['side']}^{{commit}}"], + cwd=str(source.path), capture_output=True, check=False, + env={**os.environ, VARIABLE: "1"}) + if source.runs(): + pytest.skip(f"this git ignores {VARIABLE}: `git cat-file -e` of a " + f"commit a partial clone lacks fetched it " + f"(exit {asked.returncode})") + + +@pytest.mark.parametrize("variant", ["nocheckout", "checkout", "fileallow"]) +def test_check_refuses_a_partial_clone_that_lacks_objects_and_fetches_none( + made, variant): + """Base: `plan ok`, after 26 `upload-pack` runs and `.promisor` files 1 + -> 27 for `nocheckout`, and none for `checkout`, whose 2 lacking history + blobs `execute` then failed on. Head: refused, nothing fetched.""" + source = made(variant) + assert source.lacking() > 0 + promisor = source.promisor_packs() + result = source.check() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + assert CURE in result.stderr + assert "plan ok" not in result.stdout + source.untouched() + assert source.promisor_packs() == promisor + + +@pytest.mark.parametrize("variant", ["full", "cured"]) +def test_check_reads_a_full_clone_and_a_cured_partial_clone_as_before( + made, variant): + """A full clone is asked its config alone, and a partial clone fetched + again with no filter lacks nothing: both `plan ok`, nothing fetched, on + base and head alike.""" + source = made(variant) + assert source.lacking() == 0 + result = source.check() + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert CODE not in result.stderr + result.stdout + source.untouched() + + +@pytest.mark.parametrize("variant", ["nocheckout", "checkout"]) +def test_execute_refuses_a_partial_clone_before_any_leg_and_fetches_none( + made, variant): + """Base: both leg repositories made, then `git filter-repo` died with + `Blob not found` (26 `upload-pack` runs for `nocheckout`). Head: refused + before the directory the legs go in is made, nothing fetched.""" + source = made(variant) + result = source.execute() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + assert not (source.base / "remotes").exists() + source.untouched() + + +@needs_filter_repo +def test_execute_splits_a_cured_partial_clone_and_fetches_none(made): + """A partial clone that lacks nothing is split as a full clone is.""" + source = made("cured") + result = source.execute() + assert result.returncode == 0, result.stderr + result.stdout + assert CODE not in result.stderr + result.stdout + assert sorted(path.name for path in (source.base / "remotes").iterdir()) + assert source.runs() == 0 + + +def test_the_guard_alone_refuses_where_git_ignores_the_variable( + adopter, made, monkeypatch, capsys): + """A git that ignores `GIT_NO_LAZY_FETCH` (stood for by setting another + name) still fetches nothing: the source is refused before it is read. + `raising=False`, so the base fails on what it does, not on the name.""" + monkeypatch.setattr(adopter, "NO_LAZY_FETCH", IGNORED, raising=False) + source = made("nocheckout") + code = adopter.main(["check", "--plan", str(source.plan), + "--work-dir", str(source.base / "work")]) + said = capsys.readouterr() + assert code == 2, said.err + said.out + assert REFUSED in said.err + source.untouched() + assert VARIABLE not in os.environ and IGNORED not in os.environ + + +def test_check_reads_no_commit_the_plan_names_and_the_source_lacks( + adopter, made, monkeypatch, capsys): + """A cured partial clone, whose branches lack nothing, and a plan whose + `commit:` the source lacks: `check` reads that commit (the Triad + advisory reads the plan's), and git would fetch it. The variable keeps + it unfetched; with the variable ignored, the same `check` fetches.""" + source = made("cured") + source.at_the_side_commit() + skip_where_git_ignores_the_variable(source) + args = ["check", "--plan", str(source.plan), + "--work-dir", str(source.base / "work")] + code = adopter.main(args) + said = capsys.readouterr() + source.untouched() + assert code == 1, said.err + said.out + assert "FINDING plan-stale" in said.err + assert CODE not in said.err + monkeypatch.setattr(adopter, "NO_LAZY_FETCH", IGNORED, raising=False) + adopter.main(args) + capsys.readouterr() + assert source.runs() >= 1 + + +def test_execute_reads_no_commit_a_cloned_plan_names_and_the_source_lacks( + adopter, made, monkeypatch, capsys, tmp_path): + """The same through a plan whose source `plan` cloned (`source.cloned: + true`): whether that clone still holds the plan's commit is asked before + the guard, and git would fetch it. The variable keeps it unfetched, and + the clone is taken as no longer whole (cloned again, from a GitHub that + does not have it); with the variable ignored, the same `execute` + fetches.""" + gh_clones(adopter, monkeypatch, tmp_path / "nowhere") + source = made("cured") + source.at_the_side_commit(cloned=True) + skip_where_git_ignores_the_variable(source) + args = ["execute", "--plan", str(source.plan), "--yes", + "--local-remote-dir", str(source.base / "remotes"), + "--work-dir", str(source.base / "work")] + code = adopter.main(args) + said = capsys.readouterr() + source.untouched() + assert code == 2, said.err + said.out + assert "no longer whole" in said.out + said.err + monkeypatch.setattr(adopter, "NO_LAZY_FETCH", IGNORED, raising=False) + adopter.main(args) + capsys.readouterr() + assert source.runs() >= 1 + assert not (source.base / "remotes").exists() + + +def test_a_branch_whose_commit_the_source_lacks_is_refused(made): + """A branch git cannot walk, written by hand at a commit the source + lacks: before git 2.45 `rev-list --missing=print` exits 128 on it, and + from 2.45 it lists it as lacking. Refused either way, by its code + alone, and nothing is fetched.""" + source = made("cured") + ghost = source.path / ".git" / "refs" / "heads" / "ghost" + ghost.write_text(source.planned["side"] + "\n", encoding="ascii") + source.before = source.store() + result = source.check() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + source.untouched() + + +@pytest.mark.parametrize(("config", "remotes"), [ + ((("remote.origin.promisor", "true"),), ["origin"]), + ((("remote.origin.promisor", "yes"),), ["origin"]), + ((("remote.origin.partialclonefilter", "blob:none"),), ["origin"]), + ((("extensions.partialclone", "origin"),), ["origin"]), + ((("remote.origin.promisor", "false"),), []), + ((("extensions.partialclone", "first"), + ("remote.second.promisor", "true"), + ("remote.first.partialclonefilter", "blob:none")), + ["second", "first"]), + ((), []), +], ids=["promisor", "promisor-yes", "filter", "extension", "promisor-false", + "extension-last", "none"]) +def test_promisor_remotes_reads_git_s_rule(adopter, tmp_path, config, + remotes): + """Each key git's `promisor-remote.c` reads, the remote + `extensions.partialclone` names last; `promisor = false` alone is + none.""" + git("init", "-q", str(tmp_path / "repo"), cwd=tmp_path) + for key, value in config: + git("config", key, value, cwd=tmp_path / "repo") + assert adopter._promisor_remotes(tmp_path / "repo") == remotes + + +@pytest.mark.parametrize("before", [None, "0"], ids=["unset", "set"]) +def test_lazy_fetch_off_puts_the_environment_back(adopter, monkeypatch, + before): + """`GIT_NO_LAZY_FETCH=1` inside, and as it was afterwards, an exception + included.""" + if before is not None: + monkeypatch.setenv(VARIABLE, before) + + with pytest.raises(LookupError): + with adopter._lazy_fetch_off(): + assert os.environ[VARIABLE] == "1" + raise LookupError + assert os.environ.get(VARIABLE) == before + + @adopter._lazy_fetch_off() + def inside() -> str: + return os.environ[VARIABLE] + + assert inside() == "1" + assert os.environ.get(VARIABLE) == before From 2c8082a2a265b38a8dbf82a92c2823b6199fcade Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 20:14:15 +0000 Subject: [PATCH 10/17] tests/test_adopt_partial_clone_source.py: a partial clone whose walk fails is refused on any git, and not read as lacking nothing (#1145) The hand-written branch at a commit the source lacks makes `git rev-list --missing=print` exit 128 only before git 2.45; from 2.45 it lists the commit as lacking, so a `None` read as `0` would pass that row there. A walk made to fail in-process (`LACKING_WALK` with an option no git has) holds the `None` path on every git. It fails on bba168bd (`plan ok`). Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- tests/test_adopt_partial_clone_source.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 4c25646f..adae25a4 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -370,6 +370,23 @@ def test_a_branch_whose_commit_the_source_lacks_is_refused(made): source.untouched() +def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( + adopter, made, monkeypatch, capsys): + """A walk that fails tells nothing of what is lacking, so it is no + `0`: a cured partial clone, its walk made to fail (an option no git + has), is refused, naming how git exited.""" + monkeypatch.setattr(adopter, "LACKING_WALK", + ("rev-list", "--no-such-option"), raising=False) + source = made("cured") + code = adopter.main(["check", "--plan", str(source.plan), + "--work-dir", str(source.base / "work")]) + said = capsys.readouterr() + assert code == 2, said.err + said.out + assert REFUSED in said.err + assert "`git rev-list --no-such-option`" in said.err + source.untouched() + + @pytest.mark.parametrize(("config", "remotes"), [ ((("remote.origin.promisor", "true"),), ["origin"]), ((("remote.origin.promisor", "yes"),), ["origin"]), From 70f324859c122ff8adf2f5724308d38b49aef664 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 21:46:52 +0000 Subject: [PATCH 11/17] tests/test_adopt_partial_clone_source.py: a tag alone that reaches a blob the source lacks is refused through `check` and `execute` as `source-partial-clone`, and the remote `extensions.partialclone` names comes last where it is a promisor too, so neither the `--tags` of the walk nor that order is left unpinned (#1145) A cured partial clone that fetches an annotated tag with the filter it keeps lacks the blob its commit changes, and only the tag reaches it: branches lack 0 objects, tags 1. With `--tags` out of `LACKING_WALK`, `check` said `plan ok` and `execute` made both legs, then died in `git filter-repo`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_partial_clone_source.py | 55 +++++++++++++++++++++--- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index adae25a4..4397bf6c 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -67,6 +67,8 @@ "full": ()} #: A commit on a branch the source never clones. SIDE = "side" +#: An annotated tag on a commit no branch reaches, which changes a spec file. +TAG = "tagged" needs_filter_repo = pytest.mark.skipif( shutil.which("git-filter-repo") is None, @@ -93,7 +95,8 @@ def no_ambient_variable(monkeypatch): def planned_once(tmp_path_factory) -> dict: """`origin`, which lets a clone filter, a plan written from a full clone of its `main`, every entry answered, and a commit on a branch `side` no - clone below takes; made once, since every row reads it alone.""" + clone below takes, and an annotated tag `TAG` on a commit no branch + reaches; made once, since every row reads it alone.""" base = tmp_path_factory.mktemp("planned") origin = make_source_repo(base / "origin") git("config", "uploadpack.allowFilter", "true", cwd=origin) @@ -111,6 +114,12 @@ def planned_once(tmp_path_factory) -> dict: git("commit", "-q", "--allow-empty", "-m", "Elsewhere", cwd=origin) side = git("rev-parse", "HEAD", cwd=origin).stdout.strip() git("switch", "-q", "main", cwd=origin) + git("switch", "-q", "--detach", cwd=origin) + (origin / "specs" / "001-feature" / "spec.md").write_text( + "# The feature, as the tag has it\n", encoding="utf-8") + git("commit", "-q", "-a", "-m", "Only a tag reaches this", cwd=origin) + git("tag", "-a", "-m", "A tag", TAG, cwd=origin) + git("switch", "-q", "main", cwd=origin) return {"origin": origin, "text": plan.read_text(encoding="utf-8"), "at": full, "commit": commit, "side": side} @@ -173,11 +182,21 @@ def promisor_packs(self) -> int: return len(list((self.path / ".git" / "objects" / "pack") .glob("*.promisor"))) - def lacking(self) -> int: - listed = git("rev-list", "--objects", "--missing=print", "--all", + def lacking(self, refs: str = "--all") -> int: + """How many objects `refs` (`--all`, `--branches` or `--tags`) + reach that the source lacks.""" + listed = git("rev-list", "--objects", "--missing=print", refs, cwd=self.path).stdout return sum(line.startswith("?") for line in listed.splitlines()) + def fetch_the_tag(self) -> None: + """Fetch `TAG` with the filter this clone keeps, so that its commit's + changed blob is lacking; what that fetch ran is not counted.""" + git(*FILE_PROTOCOL, "fetch", "-q", "origin", + f"refs/tags/{TAG}:refs/tags/{TAG}", cwd=self.path) + self.log.write_text("", encoding="utf-8") + self.before = self.store() + def untouched(self) -> None: """No `upload-pack` ran, and `objects/` is as it was.""" assert self.runs() == 0 @@ -355,6 +374,27 @@ def test_execute_reads_no_commit_a_cloned_plan_names_and_the_source_lacks( assert not (source.base / "remotes").exists() +@pytest.mark.parametrize("command", ["check", "execute"]) +def test_a_tag_whose_blob_the_source_lacks_is_refused(made, command): + """Only a tag reaches what is lacking: a cured partial clone lacks + nothing, then fetches `TAG` with the filter it keeps, and that tag's + commit changes a spec file, whose blob is not fetched. Its branches lack + 0 objects and its tags lack some. Refused through `check` and through + `execute`, no leg made, nothing fetched. With `--tags` out of the walk, + `check` said `plan ok`, and `execute` made both legs and then died in + `git filter-repo`, `Blob not found`.""" + source = made("cured") + source.fetch_the_tag() + assert source.lacking("--branches") == 0 + assert source.lacking("--tags") > 0 + result = getattr(source, command)() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + assert "plan ok" not in result.stdout + assert not (source.base / "remotes").exists() + source.untouched() + + def test_a_branch_whose_commit_the_source_lacks_is_refused(made): """A branch git cannot walk, written by hand at a commit the source lacks: before git 2.45 `rev-list --missing=print` exits 128 on it, and @@ -397,14 +437,17 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( ("remote.second.promisor", "true"), ("remote.first.partialclonefilter", "blob:none")), ["second", "first"]), + ((("extensions.partialclone", "o"), + ("remote.o.promisor", "true"), + ("remote.p.promisor", "true")), ["p", "o"]), ((), []), ], ids=["promisor", "promisor-yes", "filter", "extension", "promisor-false", - "extension-last", "none"]) + "extension-last", "extension-also-promisor", "none"]) def test_promisor_remotes_reads_git_s_rule(adopter, tmp_path, config, remotes): """Each key git's `promisor-remote.c` reads, the remote - `extensions.partialclone` names last; `promisor = false` alone is - none.""" + `extensions.partialclone` names last, one that is also a promisor + included; `promisor = false` alone is none.""" git("init", "-q", str(tmp_path / "repo"), cwd=tmp_path) for key, value in config: git("config", key, value, cwd=tmp_path / "repo") From 6cd96b8b12e88215874a1dad2df9b5f875f6370c Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Thu, 22 Oct 2026 21:49:13 +0000 Subject: [PATCH 12/17] adopt-project.py and tests/test_adopt_partial_clone_source.py: the `source-partial-clone` refusal says a later fetch stays filtered while `remote..partialclonefilter` is set, says git could not walk the refs, and names `git fsck`, where the walk fails, and the comments say `_not_a_whole_clone` reads a cloned plan's commit before the guard in any partial clone (#1145) The refetch the refusal names leaves `remote..partialclonefilter` as it was, so the next plain fetch can leave the source lacking again. Where `git rev-list` cannot walk the branches and tags, nothing is known of what is lacking: the refusal said "lacks what its branches and tags reach, as far as git can tell", and named a refetch that does not bring in a ref outside the remote's refspec. It now says git could not walk them and names `git fsck`, as `Source.tree` does, with the refetch as a second way. The code stays `source-partial-clone`, so the doc trio is unchanged. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 61 ++++++++++++++++++------ tests/test_adopt_partial_clone_source.py | 31 +++++++++++- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index ce02c50f..713a6f94 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -12931,8 +12931,11 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], # the plan names included; # - `_refuse_a_partial_clone_short`, as soon as the source is opened: a # partial clone that lacks an object its branches or tags reach is -# refused before anything reads it, on every git, the gits that ignore -# the variable included. +# refused before anything else reads it, on every git, the gits that +# ignore the variable included. What comes first is `Plan.open_source`'s +# `_not_a_whole_clone`, for a `source.cloned: true` plan: it reads the +# plan's commit in ANY partial clone, so on a git that ignores the +# variable it fetches a commit the clone lacks. # # NOT `-c protocol.allow=never`: measured, the source's own # `protocol.file.allow=always` outranks it, and the lazy fetch ran. NOR AN @@ -12977,7 +12980,9 @@ def _lazy_fetch_off(): 2024-05-14 back to 2.39.4, whose fix for CVE-2024-32465 made `promisor-remote.c` read it, as distributions' own builds of those do (Ubuntu's 2.43.0-1ubuntu7.1 and later). An older git ignores it, and - there `_refuse_a_partial_clone_short` is what keeps the source whole.""" + there `_refuse_a_partial_clone_short` is what keeps the source whole, + but for the read `_not_a_whole_clone` makes before it, of the commit a + `source.cloned: true` plan names.""" before = os.environ.get(NO_LAZY_FETCH) os.environ[NO_LAZY_FETCH] = "1" try: @@ -13063,6 +13068,12 @@ def _refetch(remote: str) -> str: f"{_spelled(remote)}") +def _filter_key(remote: str) -> str: + """The config key that makes a later fetch from `remote` filtered, as a + person reads it: `_spelled`, as every name here is.""" + return _spelled(f"remote.{remote}.partialclonefilter") + + def _refuse_a_partial_clone_short(source: Source) -> None: """`source-partial-clone` for a source that is a partial clone (`_promisor_remotes`) and lacks an object its branches or tags reach @@ -13087,7 +13098,14 @@ def _refuse_a_partial_clone_short(source: Source) -> None: THE WAY OUT FETCHES, AND IS THE PERSON'S OWN: run in the source, from each promisor remote, `git fetch --refetch --no-filter` (git 2.36 or later), which leaves it lacking nothing; or give a full clone with - `--source`.""" + `--source`. It leaves `remote..partialclonefilter` as it was, and + a later fetch stays filtered while that is set, so the refusal says so. + + WHERE GIT CANNOT WALK THE REFS, what is lacking is not known, and the + refusal says that, not that it lacks something: it names `git fsck`, as + `Source.tree` does, which lists each object a repository lacks. The + refetch is named too, but it fetches what the remote's refspec names + and no more.""" remotes = _promisor_remotes(source.path) if not remotes: return @@ -13096,25 +13114,38 @@ def _refuse_a_partial_clone_short(source: Source) -> None: return where = shown_path(source.path) walk = f"`git {' '.join(LACKING_WALK)}`" - short = (f"lacks at least {lacking} object(s) its branches and tags " - "reach" if lacking is not None else - f"lacks what its branches and tags reach, as far as git can " - f"tell: {walk}, which lists what a clone lacks without fetching " - f"it, {said}") + refetch = ", then ".join(map(_refetch, remotes)) + later = (f"Later fetches stay filtered while " + f"{' or '.join(map(_filter_key, remotes))} is set, so the " + "source can lack objects again.") + if lacking is None: + short = (f"whose branches and tags git could not walk, so what it " + f"lacks is not known: {walk}, which lists what a clone " + f"lacks without fetching it, {said}") + remediation = ( + f"Remediation: run `git fsck` in {where}: it names each object " + f"the repository lacks. Restore them, or fetch every object " + f"again with {refetch} (git 2.36 or later), or pass a full " + "clone (`git clone` with no `--filter`) with --source, and run " + f"this command again. {later}") + else: + short = (f"that lacks at least {lacking} object(s) its branches and " + "tags reach") + remediation = ( + f"Remediation: make the source whole, then run this command " + f"again: in {where}, run {refetch}, which fetches every object " + "again with no filter (git 2.36 or later). Or pass a full clone " + f"(`git clone` with no `--filter`) with --source. {later}") raise Refusal( SOURCE_PARTIAL_CLONE, - f"the source {where} is a partial clone (`git clone --filter`) that " + f"the source {where} is a partial clone (`git clone --filter`) " f"{short}. Git fetches an object a partial clone lacks from its " f"promisor remote ({', '.join(map(_spelled, remotes))}) on any read " "that needs it, and `check` and `execute` never fetch into the " "source: `execute`'s `git filter-repo` reads every branch and tag, " "and a partial clone short of one of their blobs failed it after " "both leg repositories were made", - f"Remediation: make the source whole, then run this command again: " - f"in {where}, run " - f"{', then '.join(map(_refetch, remotes))}, which fetches every " - "object again with no filter (git 2.36 or later). Or pass a full " - "clone (`git clone` with no `--filter`) with --source.") + remediation) @_lazy_fetch_off() diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 4397bf6c..26f0d424 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -49,6 +49,9 @@ REFUSED = f"REFUSED {CODE}:" #: The way out the refusal names, for the remote a clone calls `origin`. CURE = "`git fetch --refetch --no-filter origin`" +#: What the refusal says about a fetch run after the cure. +LATER = ("Later fetches stay filtered while " + "`remote.origin.partialclonefilter` is set") #: The variable `_lazy_fetch_off` sets. VARIABLE = "GIT_NO_LAZY_FETCH" #: A name no git reads: `NO_LAZY_FETCH` set to it stands for a git that @@ -255,7 +258,9 @@ def test_check_refuses_a_partial_clone_that_lacks_objects_and_fetches_none( made, variant): """Base: `plan ok`, after 26 `upload-pack` runs and `.promisor` files 1 -> 27 for `nocheckout`, and none for `checkout`, whose 2 lacking history - blobs `execute` then failed on. Head: refused, nothing fetched.""" + blobs `execute` then failed on. Head: refused, nothing fetched, and the + refusal says a later fetch stays filtered (the cure leaves + `partialclonefilter` set).""" source = made(variant) assert source.lacking() > 0 promisor = source.promisor_packs() @@ -263,6 +268,7 @@ def test_check_refuses_a_partial_clone_that_lacks_objects_and_fetches_none( assert result.returncode == 2, result.stderr + result.stdout assert REFUSED in result.stderr assert CURE in result.stderr + assert LATER in result.stderr assert "plan ok" not in result.stdout source.untouched() assert source.promisor_packs() == promisor @@ -374,6 +380,22 @@ def test_execute_reads_no_commit_a_cloned_plan_names_and_the_source_lacks( assert not (source.base / "remotes").exists() +def test_the_refusal_names_the_filter_key_of_each_promisor_remote(made): + """Two promisor remotes: each is fetched again, and each keeps the key + that filters a later fetch.""" + source = made("nocheckout") + git("config", "remote.second.promisor", "true", cwd=source.path) + result = source.check() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + assert ("`git fetch --refetch --no-filter origin`, then `git fetch " + "--refetch --no-filter second`") in result.stderr + assert ("Later fetches stay filtered while " + "`remote.origin.partialclonefilter` or " + "`remote.second.partialclonefilter` is set") in result.stderr + source.untouched() + + @pytest.mark.parametrize("command", ["check", "execute"]) def test_a_tag_whose_blob_the_source_lacks_is_refused(made, command): """Only a tag reaches what is lacking: a cured partial clone lacks @@ -414,7 +436,8 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( adopter, made, monkeypatch, capsys): """A walk that fails tells nothing of what is lacking, so it is no `0`: a cured partial clone, its walk made to fail (an option no git - has), is refused, naming how git exited.""" + has), is refused, naming how git exited, saying git could not walk the + refs (and no object count), and naming `git fsck`.""" monkeypatch.setattr(adopter, "LACKING_WALK", ("rev-list", "--no-such-option"), raising=False) source = made("cured") @@ -424,6 +447,10 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( assert code == 2, said.err + said.out assert REFUSED in said.err assert "`git rev-list --no-such-option`" in said.err + assert "branches and tags git could not walk" in said.err + assert "lacks at least" not in said.err + assert "Remediation: run `git fsck` in " in said.err + assert LATER in said.err source.untouched() From 41230f6406dadf0ce2ca083fc61e0a1953aee1f7 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 07:48:29 +0000 Subject: [PATCH 13/17] adopt-project.py, docs/cli.md and tests/test_adopt_partial_clone_source.py: the `source-partial-clone` walk also reaches the source's HEAD where `git rev-parse --verify -q HEAD` gives an id, so a HEAD detached at a commit no branch or tag reaches is refused through `check` and `execute` and an unborn HEAD is read as before, and the `git fsck` sentence says what fsck names in a partial clone, and the count and walk-failure words are pinned (#1145) SHOULD-1 of r1180b: `LACKING_WALK` ended at `--branches --tags`, but `execute`'s `git clone` checks out the source's HEAD. HEAD is walked by its id, asked with `rev-parse --verify -q HEAD` and not `HEAD^{commit}`, which fetched a commit the clone lacks (measured with git 2.43, 1 `upload-pack` run) and exits 1 under `GIT_NO_LAZY_FETCH`. A plain `HEAD` fails on an unborn HEAD, exit 128, and on a file named HEAD in the work tree. NIT-1: the `None` remedy names what `git fsck` shows, a branch, tag or HEAD whose object is lacking, and no object a promisor remote holds. NIT-2 and NIT-3: the count arm's words and the failing walk's command, `exited`, the source's path after `git fsck` in, and the cure are asserted. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 134 +++++++++++++------- docs/cli.md | 20 +-- tests/test_adopt_partial_clone_source.py | 153 +++++++++++++++++++++-- 3 files changed, 239 insertions(+), 68 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index 713a6f94..b1e44868 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -12930,7 +12930,7 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], # it fetched, whatever reads it and whichever commit it reads, a commit # the plan names included; # - `_refuse_a_partial_clone_short`, as soon as the source is opened: a -# partial clone that lacks an object its branches or tags reach is +# partial clone that lacks an object its branches, tags or HEAD reach is # refused before anything else reads it, on every git, the gits that # ignore the variable included. What comes first is `Plan.open_source`'s # `_not_a_whole_clone`, for a `source.cloned: true` plan: it reads the @@ -12947,19 +12947,29 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], #: The variable git reads to leave out the fetch a partial clone makes for an #: object it lacks: `shape-doctor.py`'s `NO_LAZY_FETCH` (#787, #1145). NO_LAZY_FETCH = "GIT_NO_LAZY_FETCH" -#: Refused: a source that is a partial clone and lacks an object its branches -#: or tags reach (#1145). +#: Refused: a source that is a partial clone and lacks an object its branches, +#: tags or HEAD reach (#1145). SOURCE_PARTIAL_CLONE = "source-partial-clone" #: The config keys `_promisor_remotes` reads: git's, in `promisor-remote.c`, #: as the doctor's `PROMISOR_KEYS` (#683). PROMISOR_KEYS = (r"^(remote\..*\.(promisor|partialclonefilter)" r"|extensions\.partialclone)$") #: What `_objects_lacking` runs: every object the source's branches and tags -#: reach, one a line, and `?` for each the source lacks. It never +#: reach, one a line, and `?` for each the source lacks; and, where +#: `HEAD_ID` gives one, the objects the source's HEAD reaches too. It never #: fetches, on any git that has `--missing` (measured with git 2.43 and -#: `GIT_NO_LAZY_FETCH` unset: no `upload-pack` ran). +#: `GIT_NO_LAZY_FETCH` unset: no `upload-pack` ran, with HEAD at a commit the +#: clone lacks included). LACKING_WALK = ("rev-list", "--objects", "--missing=print", "--branches", "--tags") +#: What `_head_id` asks: HEAD's id, printed, and exit 1 where HEAD is unborn +#: (`git switch --orphan`). A plain `HEAD` in the walk fails there, +#: `ambiguous argument 'HEAD'`, exit 128. NOT `HEAD^{commit}`: it reads the +#: object, and where the clone lacks it that fetched it (measured with git +#: 2.43, `GIT_NO_LAZY_FETCH` unset: 1 `upload-pack` run), and under the +#: variable it exits 1, as an unborn HEAD does. `HEAD` prints the id of a +#: commit the clone lacks and runs nothing. +HEAD_ID = ("rev-parse", "--verify", "-q", "HEAD") @contextlib.contextmanager @@ -13034,27 +13044,52 @@ def _promisor_remotes(path: Path) -> list[str]: return [name for name in dict.fromkeys(names) if name not in last] + last +def _head_id(path: Path) -> str | None: + """The id HEAD names in the repository at `path` (`HEAD_ID`), or None + where it names none: an unborn HEAD, or git cannot run. It reads no + object, so it fetches nothing.""" + try: + asked = subprocess.run(["git", *HEAD_ID], cwd=str(path), + capture_output=True, check=False) + except OSError: + return None + named = asked.stdout.decode("utf-8", "replace").strip() + return named if asked.returncode == 0 and named else None + + def _objects_lacking(path: Path) -> tuple[int | None, str]: - """`(how many objects the branches and tags of the repository at `path` - reach that it lacks, "")`, or `(None, what git said)` where `git - rev-list` cannot walk them: a branch whose commit it lacks, before git - 2.45, among them. The output is read as it comes, a line at a time, and - git's own words go to a file, so neither pipe can fill and stall the - other. A missing tree hides what is under it, so the count is a least.""" + """`(how many objects the branches and tags of the repository at `path`, + and its HEAD where that names a commit, reach that it lacks, "")`, or + `(None, the walk, and what git said)` where `git rev-list` cannot walk + them: a branch whose commit it lacks, before git 2.45, among them. The + output is read as it comes, a line at a time, and git's own words go to + a file, so neither pipe can fill and stall the other. A missing tree + hides what is under it, so the count is a least. + + HEAD IS WALKED BY ITS ID, AND ONLY WHERE `_head_id` has one: a plain + `HEAD` fails on an unborn HEAD (exit 128), and a file of the source's + work tree named `HEAD` would be read as a path (`ambiguous argument + 'HEAD': both revision and filename`). The walk is named with `HEAD`, as + a person asks it.""" + head = _head_id(path) + shown = (*LACKING_WALK, "HEAD") if head else LACKING_WALK + walked = (f"`git {' '.join(shown)}`, which lists what a clone lacks " + "without fetching it") lacking = 0 try: with tempfile.TemporaryFile() as said, subprocess.Popen( - ["git", *LACKING_WALK], cwd=str(path), + ["git", *LACKING_WALK, *([head] if head else [])], + cwd=str(path), stdout=subprocess.PIPE, stderr=said) as walk: for line in walk.stdout: lacking += line.startswith(b"?") if walk.wait(): said.seek(0) words = said.read().decode("utf-8", "replace").strip() - return None, (f"exited {walk.returncode}: " + return None, (f"{walked}, exited {walk.returncode}: " f"{spelled_as_git_out(words)}") except OSError as exc: - return None, f"could not run ({_os_reason(exc)})" + return None, f"{walked}, could not run ({_os_reason(exc)})" return lacking, "" @@ -13076,24 +13111,30 @@ def _filter_key(remote: str) -> str: def _refuse_a_partial_clone_short(source: Source) -> None: """`source-partial-clone` for a source that is a partial clone - (`_promisor_remotes`) and lacks an object its branches or tags reach - (`_objects_lacking`), or whose branches and tags git cannot walk; asked - by `check` and `execute` as soon as the source is opened, before - anything else reads it (#1145). + (`_promisor_remotes`) and lacks an object its branches, tags or HEAD + reach (`_objects_lacking`), or whose branches, tags and HEAD git cannot + walk; asked by `check` and `execute` as soon as the source is opened, + before anything else reads it (#1145). ONLY WHAT IS LACKING IS REFUSED. A partial clone given every object again (`git fetch --refetch --no-filter`) keeps its promisor config, and lacks nothing a read could fetch: it is read as a full clone is. A full clone is asked its config alone, which reads no object. - WHY BRANCHES AND TAGS: the source's branch is what `check` reads and - `execute` splits, and `execute`'s `git filter-repo` rewrites a clone of - the source, which copies every branch and tag. Measured with git 2.43, a - `--filter=blob:none` clone that had checked out its branch, and so - lacked only blobs of the history, passed `check` and failed `execute` - in `git filter-repo`, `Blob not found`, after both leg repositories - were made. A commit the plan names that no branch or tag reaches is - `_lazy_fetch_off`'s to keep unfetched. + WHY BRANCHES, TAGS AND HEAD: the source's branch is what `check` reads + and `execute` splits, `execute`'s `git filter-repo` rewrites a clone of + the source, which copies every branch and tag, and that clone (`git + clone`) checks out the source's HEAD before the branch is reset to the + plan's commit. Measured with git 2.43, a `--filter=blob:none` clone that + had checked out its branch, and so lacked only blobs of the history, + passed `check` and failed `execute` in `git filter-repo`, `Blob not + found`, after both leg repositories were made. And a cured partial + clone with its HEAD detached at a commit no branch or tag reaches, one + that changed a blob the clone lacks (the commit of a tag fetched with + the filter the clone keeps, the tag then deleted), passed `check`, and + `execute` made both leg repositories and died in that clone's checkout, + `unable to read sha1 file`. A commit the plan names that no branch, tag + or HEAD reaches is `_lazy_fetch_off`'s to keep unfetched. THE WAY OUT FETCHES, AND IS THE PERSON'S OWN: run in the source, from each promisor remote, `git fetch --refetch --no-filter` (git 2.36 or @@ -13102,10 +13143,13 @@ def _refuse_a_partial_clone_short(source: Source) -> None: a later fetch stays filtered while that is set, so the refusal says so. WHERE GIT CANNOT WALK THE REFS, what is lacking is not known, and the - refusal says that, not that it lacks something: it names `git fsck`, as - `Source.tree` does, which lists each object a repository lacks. The - refetch is named too, but it fetches what the remote's refspec names - and no more.""" + refusal says that, not that it lacks something: it names `git fsck`, + which names a branch, tag or HEAD whose object the repository lacks + (measured with git 2.43: `error: refs/heads/ghost: invalid sha1 + pointer`, `error: HEAD: invalid sha1 pointer`, and no `upload-pack` + ran). It lists no object a partial clone's promisor remote holds, so + the refusal does not say it does. The refetch is named too, but it + fetches what the remote's refspec names and no more.""" remotes = _promisor_remotes(source.path) if not remotes: return @@ -13113,24 +13157,23 @@ def _refuse_a_partial_clone_short(source: Source) -> None: if lacking == 0: return where = shown_path(source.path) - walk = f"`git {' '.join(LACKING_WALK)}`" refetch = ", then ".join(map(_refetch, remotes)) later = (f"Later fetches stay filtered while " f"{' or '.join(map(_filter_key, remotes))} is set, so the " "source can lack objects again.") if lacking is None: - short = (f"whose branches and tags git could not walk, so what it " - f"lacks is not known: {walk}, which lists what a clone " - f"lacks without fetching it, {said}") + short = (f"whose branches, tags and HEAD git could not walk, so what " + f"it lacks is not known: {said}") remediation = ( - f"Remediation: run `git fsck` in {where}: it names each object " - f"the repository lacks. Restore them, or fetch every object " - f"again with {refetch} (git 2.36 or later), or pass a full " - "clone (`git clone` with no `--filter`) with --source, and run " - f"this command again. {later}") + f"Remediation: run `git fsck` in {where}: it names a branch, " + "tag or HEAD whose object the repository lacks, and does not " + "list what a partial clone's promisor remote holds. Restore " + f"it, or fetch every object again with {refetch} (git 2.36 or " + "later), or pass a full clone (`git clone` with no `--filter`) " + f"with --source, and run this command again. {later}") else: - short = (f"that lacks at least {lacking} object(s) its branches and " - "tags reach") + short = (f"that lacks at least {lacking} object(s) its branches, " + "tags and HEAD reach") remediation = ( f"Remediation: make the source whole, then run this command " f"again: in {where}, run {refetch}, which fetches every object " @@ -13142,9 +13185,10 @@ def _refuse_a_partial_clone_short(source: Source) -> None: f"{short}. Git fetches an object a partial clone lacks from its " f"promisor remote ({', '.join(map(_spelled, remotes))}) on any read " "that needs it, and `check` and `execute` never fetch into the " - "source: `execute`'s `git filter-repo` reads every branch and tag, " - "and a partial clone short of one of their blobs failed it after " - "both leg repositories were made", + "source: `execute`'s clone of it checks out its HEAD, and its `git " + "filter-repo` reads every branch and tag, and a partial clone short " + "of one of their blobs failed it after both leg repositories were " + "made", remediation) @@ -17974,7 +18018,7 @@ def _work_dir_help(does: str, refused: str, but: str = "") -> str: "`source-not-top-level`; and one this user cannot reach, a clone " "`plan` made included, as `source-unreadable`, never cloned again. " "Either source, where it is a partial clone (`git clone --filter`) " - "that lacks an object its branches or tags reach, is refused as " + "that lacks an object its branches, tags or HEAD reach, is refused as " "`source-partial-clone` before anything reads it: git would fetch that " "object into it, and neither command fetches into the source. `git " "fetch --refetch --no-filter `, run in it, makes it whole.") diff --git a/docs/cli.md b/docs/cli.md index 525dfa1c..68cd8cd3 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -643,11 +643,11 @@ options: one this user cannot reach, a clone `plan` made included, as `source-unreadable`, never cloned again. Either source, where it is a partial clone (`git clone - --filter`) that lacks an object its branches or tags - reach, is refused as `source-partial-clone` before - anything reads it: git would fetch that object into - it, and neither command fetches into the source. `git - fetch --refetch --no-filter `, run in it, + --filter`) that lacks an object its branches, tags or + HEAD reach, is refused as `source-partial-clone` + before anything reads it: git would fetch that object + into it, and neither command fetches into the source. + `git fetch --refetch --no-filter `, run in it, makes it whole. --local-remote-dir LOCAL_REMOTE_DIR the directory `execute --local-remote-dir` will be @@ -709,11 +709,11 @@ options: one this user cannot reach, a clone `plan` made included, as `source-unreadable`, never cloned again. Either source, where it is a partial clone (`git clone - --filter`) that lacks an object its branches or tags - reach, is refused as `source-partial-clone` before - anything reads it: git would fetch that object into - it, and neither command fetches into the source. `git - fetch --refetch --no-filter `, run in it, + --filter`) that lacks an object its branches, tags or + HEAD reach, is refused as `source-partial-clone` + before anything reads it: git would fetch that object + into it, and neither command fetches into the source. + `git fetch --refetch --no-filter `, run in it, makes it whole. --local-remote-dir LOCAL_REMOTE_DIR create the legs as bare repositories here instead of diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 26f0d424..36a399ba 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -17,8 +17,8 @@ the same way. Now both commands run with `GIT_NO_LAZY_FETCH=1` (`_lazy_fetch_off`), and -refuse a partial clone that lacks an object its branches or tags reach as -`source-partial-clone` before anything reads it +refuse a partial clone that lacks an object its branches, tags or HEAD reach +as `source-partial-clone` before anything reads it (`_refuse_a_partial_clone_short`). A partial clone given every object again (`git fetch --refetch --no-filter`) lacks nothing and is read as a full clone is. Each row counts the promisor's `upload-pack` runs through a counting @@ -186,8 +186,8 @@ def promisor_packs(self) -> int: .glob("*.promisor"))) def lacking(self, refs: str = "--all") -> int: - """How many objects `refs` (`--all`, `--branches` or `--tags`) - reach that the source lacks.""" + """How many objects `refs` (`--all`, `--branches`, `--tags` or + `HEAD`) reach that the source lacks.""" listed = git("rev-list", "--objects", "--missing=print", refs, cwd=self.path).stdout return sum(line.startswith("?") for line in listed.splitlines()) @@ -200,6 +200,24 @@ def fetch_the_tag(self) -> None: self.log.write_text("", encoding="utf-8") self.before = self.store() + def detach_at_the_fetched_tag(self) -> None: + """Fetch `TAG` as `fetch_the_tag` does, move HEAD (not a branch) to + its commit, and delete the tag: only HEAD reaches the blob that is + lacking, as in a clone detached at a commit it fetched with its + filter. `update-ref --no-deref`, not `switch --detach`, which would + check the commit out and fetch that blob.""" + self.fetch_the_tag() + commit = git("rev-parse", f"{TAG}^{{commit}}", + cwd=self.path).stdout.strip() + git("update-ref", "--no-deref", "HEAD", commit, cwd=self.path) + git("tag", "-d", TAG, cwd=self.path) + self.before = self.store() + + def unborn_head(self) -> None: + """HEAD on a branch that does not exist, as `git switch --orphan` + leaves it: it names no commit.""" + git("symbolic-ref", "HEAD", "refs/heads/unborn", cwd=self.path) + def untouched(self) -> None: """No `upload-pack` ran, and `objects/` is as it was.""" assert self.runs() == 0 @@ -258,15 +276,17 @@ def test_check_refuses_a_partial_clone_that_lacks_objects_and_fetches_none( made, variant): """Base: `plan ok`, after 26 `upload-pack` runs and `.promisor` files 1 -> 27 for `nocheckout`, and none for `checkout`, whose 2 lacking history - blobs `execute` then failed on. Head: refused, nothing fetched, and the - refusal says a later fetch stays filtered (the cure leaves - `partialclonefilter` set).""" + blobs `execute` then failed on. Head: refused, nothing fetched, the + refusal gives the count it lacks at least and the cure, and says a later + fetch stays filtered (the cure leaves `partialclonefilter` set).""" source = made(variant) assert source.lacking() > 0 promisor = source.promisor_packs() result = source.check() assert result.returncode == 2, result.stderr + result.stdout assert REFUSED in result.stderr + assert "that lacks at least " in result.stderr + assert "Remediation: make the source whole" in result.stderr assert CURE in result.stderr assert LATER in result.stderr assert "plan ok" not in result.stdout @@ -432,28 +452,135 @@ def test_a_branch_whose_commit_the_source_lacks_is_refused(made): source.untouched() +@pytest.mark.parametrize("head", ["born", "unborn"]) def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( - adopter, made, monkeypatch, capsys): + adopter, made, monkeypatch, capsys, head): """A walk that fails tells nothing of what is lacking, so it is no `0`: a cured partial clone, its walk made to fail (an option no git - has), is refused, naming how git exited, saying git could not walk the - refs (and no object count), and naming `git fsck`.""" + has), is refused, naming the walk as run (`HEAD` last where HEAD names a + commit) and how git exited, saying git could not walk the refs (and no + object count), naming `git fsck` in the source, as what names a ref + whose object is lacking and no object a promisor holds, and the cure.""" monkeypatch.setattr(adopter, "LACKING_WALK", ("rev-list", "--no-such-option"), raising=False) source = made("cured") + if head == "unborn": + source.unborn_head() code = adopter.main(["check", "--plan", str(source.plan), "--work-dir", str(source.base / "work")]) said = capsys.readouterr() + walk = "`git rev-list --no-such-option" + ( + " HEAD`" if head == "born" else "`") assert code == 2, said.err + said.out assert REFUSED in said.err - assert "`git rev-list --no-such-option`" in said.err - assert "branches and tags git could not walk" in said.err + assert (f"{walk}, which lists what a clone lacks without fetching it, " + "exited ") in said.err + assert "branches, tags and HEAD git could not walk" in said.err assert "lacks at least" not in said.err - assert "Remediation: run `git fsck` in " in said.err + assert (f"Remediation: run `git fsck` in " + f"{adopter.shown_path(source.path)}: it names a branch, tag or " + "HEAD whose object the repository lacks, and does not list what " + "a partial clone's promisor remote holds.") in said.err + assert CURE in said.err assert LATER in said.err source.untouched() +@pytest.mark.parametrize("command", ["check", "execute"]) +def test_a_head_whose_blob_the_source_lacks_is_refused(made, command): + """Only HEAD reaches what is lacking: a cured partial clone lacks + nothing, then fetches `TAG` with the filter it keeps, so that the blob + that tag's commit changes is lacking, moves HEAD to that commit and + deletes the tag. Its branches and tags lack 0 objects and HEAD's commit + lacks some. Refused through `check` and through `execute`, no leg made, + nothing fetched. With HEAD out of the walk, `check` said `plan ok`, and + `execute` made both legs and then its clone died in the checkout of + HEAD, `unable to read sha1 file`.""" + source = made("cured") + source.detach_at_the_fetched_tag() + assert source.lacking("--branches") == 0 + assert source.lacking("--tags") == 0 + assert source.lacking("HEAD") > 0 + result = getattr(source, command)() + assert result.returncode == 2, result.stderr + result.stdout + assert REFUSED in result.stderr + assert "that lacks at least " in result.stderr + assert "plan ok" not in result.stdout + assert not (source.base / "remotes").exists() + source.untouched() + + +def test_a_head_at_a_commit_the_source_lacks_is_refused_and_not_fetched( + adopter, made, monkeypatch, capsys): + """HEAD names a commit the source lacks, written into `.git/HEAD` by + hand (`git update-ref` would fetch it): refused, and fetched by nothing, + with the variable ignored too. Asking HEAD for a commit + (`HEAD^{commit}`) read the object, and fetched it.""" + monkeypatch.setattr(adopter, "NO_LAZY_FETCH", IGNORED, raising=False) + source = made("cured") + (source.path / ".git" / "HEAD").write_text( + source.planned["side"] + "\n", encoding="ascii") + code = adopter.main(["check", "--plan", str(source.plan), + "--work-dir", str(source.base / "work")]) + said = capsys.readouterr() + assert code == 2, said.err + said.out + assert REFUSED in said.err + source.untouched() + + +@pytest.mark.parametrize("command", [ + "check", pytest.param("execute", marks=needs_filter_repo)]) +def test_an_unborn_head_is_read_as_before(made, command): + """HEAD on a branch that does not exist (`git switch --orphan`) names no + commit, and a cured partial clone lacks nothing: read as a full clone is, + nothing fetched. A plain `HEAD` in the walk exits 128 on it, `ambiguous + argument 'HEAD'`, and would refuse it.""" + source = made("cured") + source.unborn_head() + assert source.lacking("--branches") == 0 + result = getattr(source, command)() + assert result.returncode == 0, result.stderr + result.stdout + assert CODE not in result.stderr + result.stdout + if command == "check": + assert "plan ok" in result.stdout + source.untouched() + else: + assert sorted(path.name + for path in (source.base / "remotes").iterdir()) + assert source.runs() == 0 + + +@pytest.mark.parametrize("lacked", ["branch", "tag", "head", "promised"]) +def test_git_fsck_names_a_ref_whose_object_is_lacking_and_no_promised_one( + made, lacked): + """What the refusal of a walk git could not make says of `git fsck`: it + names a branch, tag or HEAD whose object the source lacks, and lists no + object its promisor remote holds. The blobs `nocheckout` lacks are + promised, and fsck is silent on them; and it fetches nothing.""" + source = made("nocheckout") + assert source.lacking("--branches") > 0 + ghost = source.planned["side"] + "\n" + git_dir = source.path / ".git" + if lacked == "branch": + (git_dir / "refs" / "heads" / "ghost").write_text(ghost, + encoding="ascii") + elif lacked == "tag": + (git_dir / "refs" / "tags" / "ghost").write_text(ghost, + encoding="ascii") + elif lacked == "head": + (git_dir / "HEAD").write_text(ghost, encoding="ascii") + fsck = git("fsck", cwd=source.path, check=False) + said = fsck.stdout + fsck.stderr + if lacked == "promised": + assert fsck.returncode == 0, said + assert said == "" + else: + assert fsck.returncode != 0, said + assert {"branch": "refs/heads/ghost", "tag": "refs/tags/ghost", + "head": "HEAD"}[lacked] in said + source.untouched() + + @pytest.mark.parametrize(("config", "remotes"), [ ((("remote.origin.promisor", "true"),), ["origin"]), ((("remote.origin.promisor", "yes"),), ["origin"]), From b316f415761c573fa37ed7962bf73b9288025a3d Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 07:50:01 +0000 Subject: [PATCH 14/17] tests/test_adopt_partial_clone_source.py: the refusal of a HEAD whose blob the source lacks names HEAD in what its branches and tags reach, and a file of the source named HEAD is no path of the walk (#1145) Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_partial_clone_source.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 36a399ba..66fee9f2 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -505,6 +505,8 @@ def test_a_head_whose_blob_the_source_lacks_is_refused(made, command): assert result.returncode == 2, result.stderr + result.stdout assert REFUSED in result.stderr assert "that lacks at least " in result.stderr + assert "its branches, tags and HEAD reach" in result.stderr + assert "checks out its HEAD" in result.stderr assert "plan ok" not in result.stdout assert not (source.base / "remotes").exists() source.untouched() @@ -550,6 +552,20 @@ def test_an_unborn_head_is_read_as_before(made, command): assert source.runs() == 0 +def test_a_file_named_head_in_the_source_is_no_path_of_the_walk(made): + """HEAD is walked by its id: with `HEAD` in the walk, a file of the + source's work tree named `HEAD` made git exit 128, `ambiguous argument + 'HEAD': both revision and filename`, and refused a source that lacks + nothing.""" + source = made("cured") + (source.path / "HEAD").write_text("not a revision\n", encoding="utf-8") + result = source.check() + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert CODE not in result.stderr + result.stdout + source.untouched() + + @pytest.mark.parametrize("lacked", ["branch", "tag", "head", "promised"]) def test_git_fsck_names_a_ref_whose_object_is_lacking_and_no_promised_one( made, lacked): From 195d9936e17c99dd299009df1f16a220e4634b0f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 15:27:57 +0000 Subject: [PATCH 15/17] tests/test_adopt_check_throwaway_repos.py: the rows that give check and execute a default branch git will not take share one helper each, so SonarCloud's new-code duplication on #1119 falls under its 3% gate (#1083) The parent-git rows and the global-config rows repeated one body each; each body is now a module-level helper that takes the function building the environment, and no row asserts or sets up anything else than before. All test ids keep their names. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- tests/test_adopt_check_throwaway_repos.py | 84 ++++++++++------------- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/tests/test_adopt_check_throwaway_repos.py b/tests/test_adopt_check_throwaway_repos.py index c71279fa..d6b9420e 100644 --- a/tests/test_adopt_check_throwaway_repos.py +++ b/tests/test_adopt_check_throwaway_repos.py @@ -596,19 +596,16 @@ def unasked_clone(work: Path, code: int, said: str) -> str: + said + UNASKED_ENDS) -def test_a_default_branch_is_asked_of_the_splits_own_clone(tmp_path): - """`init.defaultBranch = a..b`: `check` answers as the split's own `git - clone` of the source does on this git (#1083). git 2.43's stops at it, - `fatal: invalid branch name`, and `check` says so as one - `adopt-git-unasked`, where 2a119f28 said `plan ok` and `execute` then - stopped after both legs existed. git 2.55's takes it (CI on #1031), and - there no `git init` stops a question: each names its branch, and the - CRLF file the plan keeps is found.""" +def check_asks_the_clone_a_default_branch(tmp_path: Path, env_of) -> None: + """The rows that give `check` an `init.defaultBranch = a..b` in the + form `env_of(tmp_path)` builds: `check` answers as this git's `git + clone` does, one `adopt-git-unasked` where it stops at the name and, + where it takes it, the CRLF file the plan keeps.""" plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, NOTES: CRLF}), tmp_path, {NOTES: ROOT}) work = tmp_path / "work" - env = bad_default_branch(tmp_path) + env = env_of(tmp_path) clone = this_gits_clone(tmp_path, env) result = checked(plan, env, "--work-dir", str(work)) assert "REFUSED" not in result.stderr, result.stderr @@ -621,6 +618,17 @@ def test_a_default_branch_is_asked_of_the_splits_own_clone(tmp_path): assert list(work.iterdir()) == [] +def test_a_default_branch_is_asked_of_the_splits_own_clone(tmp_path): + """`init.defaultBranch = a..b`: `check` answers as the split's own `git + clone` of the source does on this git (#1083). git 2.43's stops at it, + `fatal: invalid branch name`, and `check` says so as one + `adopt-git-unasked`, where 2a119f28 said `plan ok` and `execute` then + stopped after both legs existed. git 2.55's takes it (CI on #1031), and + there no `git init` stops a question: each names its branch, and the + CRLF file the plan keeps is found.""" + check_asks_the_clone_a_default_branch(tmp_path, bad_default_branch) + + def test_a_default_branch_git_will_not_take_stops_no_throwaway_init( adopter, monkeypatch, tmp_path): """Each throwaway `git init` names its branch, as the split's own `git @@ -820,13 +828,14 @@ def test_no_url_is_asked_once_the_clone_cannot_be_made(tmp_path): assert list(work.iterdir()) == [] -def test_execute_refuses_a_default_branch_its_clone_will_not_take( - adopter, monkeypatch, tmp_path): - """`init.defaultBranch = a..b`, on a git whose `git clone` stops at it, - as 2.43's does: `execute` refuses it before any leg exists, where on - 2a119f28 it made both and stopped extracting the spec leg (#1083).""" +def execute_refuses_a_default_branch_its_clone_will_not_take( + adopter, monkeypatch, tmp_path, env_of) -> None: + """The rows that give `execute` an `init.defaultBranch = a..b` in the + form `env_of(tmp_path)` builds: on a git whose `git clone` stops at it, + `execute` refuses it before any leg exists; where the clone takes it, + the row is skipped.""" plan = planned(a_source(tmp_path), tmp_path, {}) - env = bad_default_branch(tmp_path) + env = env_of(tmp_path) if this_gits_clone(tmp_path, env).returncode == 0: pytest.skip("this git's `git clone` takes `init.defaultBranch = " "a..b`, as 2.55's does: the split clones there") @@ -843,6 +852,15 @@ def test_execute_refuses_a_default_branch_its_clone_will_not_take( assert not remotes.exists() +def test_execute_refuses_a_default_branch_its_clone_will_not_take( + adopter, monkeypatch, tmp_path): + """`init.defaultBranch = a..b`, on a git whose `git clone` stops at it, + as 2.43's does: `execute` refuses it before any leg exists, where on + 2a119f28 it made both and stopped extracting the spec leg (#1083).""" + execute_refuses_a_default_branch_its_clone_will_not_take( + adopter, monkeypatch, tmp_path, bad_default_branch) + + # --------------------------------------------------------------------------- # #1083 (b): the submodule urls' own throwaway repositories # --------------------------------------------------------------------------- @@ -1172,21 +1190,8 @@ def test_a_parent_gits_default_branch_is_asked_of_the_splits_own_clone( `execute`'s clone, which takes the whole environment, stopped after both legs existed. git 2.55's clone takes the name, and there the CRLF file the plan keeps is found.""" - plan = planned(a_source(tmp_path, files={ROOT_ATTRIBUTES: TEXT_EOL_LF, - NOTES: CRLF}), - tmp_path, {NOTES: ROOT}) - work = tmp_path / "work" - env = handed(tmp_path, BAD_DEFAULT_BRANCH) - clone = this_gits_clone(tmp_path, env) - result = checked(plan, env, "--work-dir", str(work)) - assert "REFUSED" not in result.stderr, result.stderr - assert result.returncode == 1, result.stderr - if clone.returncode == 0: - assert codes(result) == [REENCODED_FILES], result.stderr - else: - assert finding_lines(result) == [unasked_clone( - work, clone.returncode, clone.stderr.decode().strip())] - assert list(work.iterdir()) == [] + check_asks_the_clone_a_default_branch( + tmp_path, lambda base: handed(base, BAD_DEFAULT_BRANCH)) @pytest.mark.parametrize("handed", FROM_A_PARENT) @@ -1195,22 +1200,9 @@ def test_execute_refuses_a_parent_gits_default_branch_before_any_leg( """Measured on bdcf9952 with git 2.43 and `GIT_CONFIG_PARAMETERS`: `execute` made both legs and stopped extracting the spec leg, `fatal: invalid branch name`, with nothing rolled back.""" - plan = planned(a_source(tmp_path), tmp_path, {}) - env = handed(tmp_path, BAD_DEFAULT_BRANCH) - if this_gits_clone(tmp_path, env).returncode == 0: - pytest.skip("this git's `git clone` takes `init.defaultBranch = " - "a..b`, as 2.55's does: the split clones there") - for name, value in env.items(): - monkeypatch.setenv(name, value) - monkeypatch.setattr(adopter, "_require_filter_repo", lambda: None) - remotes = tmp_path / "remotes" - with pytest.raises(adopter.Refusal) as caught: - adopter.cmd_execute(Namespace( - plan=str(plan), source=None, local_remote_dir=remotes, - allow_empty_leg=[], yes=True, work_dir=tmp_path / "work")) - assert caught.value.code == UNASKED - assert "invalid branch name" in caught.value.detail - assert not remotes.exists() + execute_refuses_a_default_branch_its_clone_will_not_take( + adopter, monkeypatch, tmp_path, + lambda base: handed(base, BAD_DEFAULT_BRANCH)) #: The setting the stub `git` of `refusing_a_setting` reads, under a key no From a4ec51d2e3eeb5f5e57a84a58615cf9f02cac5f6 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 15:31:33 +0000 Subject: [PATCH 16/17] adopt-project.py and tests/test_adopt_partial_clone_source.py: the `source-partial-clone` walk ends with `--`, so a file of the source's work tree named as HEAD's id is no path of it (#1145) `_objects_lacking` walked `git rev-list --objects --missing=print --branches --tags `, and a file of the source's work tree named as HEAD's id made git exit 128 (`ambiguous argument '': both revision and filename`), so `check` and `execute` refused a source that lacks nothing as `source-partial-clone`. The walk now ends with `--`, after the id, so no file of the work tree is read as a path of it, and the walk the refusal names ends with `--` too. The new row puts a file named as HEAD's id in a cured partial clone's work tree and expects `plan ok` with nothing fetched; it fails at `b316f415` and passes here, and the failing-walk row, born and unborn, which asserts the walk as named, now expects the `--`. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Sonnet 5.5 --- adopt-project.py | 16 +++++++++---- tests/test_adopt_partial_clone_source.py | 29 ++++++++++++++++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index b1e44868..c98cc216 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -12956,7 +12956,9 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], r"|extensions\.partialclone)$") #: What `_objects_lacking` runs: every object the source's branches and tags #: reach, one a line, and `?` for each the source lacks; and, where -#: `HEAD_ID` gives one, the objects the source's HEAD reaches too. It never +#: `HEAD_ID` gives one, the objects the source's HEAD reaches too; then `--`, +#: which `_objects_lacking` appends, so no file of the work tree is a path of +#: the walk, whatever it is named (#1145). It never #: fetches, on any git that has `--missing` (measured with git 2.43 and #: `GIT_NO_LAZY_FETCH` unset: no `upload-pack` ran, with HEAD at a commit the #: clone lacks included). @@ -13069,16 +13071,20 @@ def _objects_lacking(path: Path) -> tuple[int | None, str]: HEAD IS WALKED BY ITS ID, AND ONLY WHERE `_head_id` has one: a plain `HEAD` fails on an unborn HEAD (exit 128), and a file of the source's work tree named `HEAD` would be read as a path (`ambiguous argument - 'HEAD': both revision and filename`). The walk is named with `HEAD`, as - a person asks it.""" + 'HEAD': both revision and filename`). A file named as the id is read + as a path too, `ambiguous argument '': both revision and + filename`, so the walk ENDS WITH `--`: everything before it is a + revision or an option, and no file of the work tree is read at all. + The walk is named with `HEAD`, as a person asks it, and with the `--` + it ends with, so what a person pastes is the walk that ran.""" head = _head_id(path) - shown = (*LACKING_WALK, "HEAD") if head else LACKING_WALK + shown = (*LACKING_WALK, *(("HEAD",) if head else ()), "--") walked = (f"`git {' '.join(shown)}`, which lists what a clone lacks " "without fetching it") lacking = 0 try: with tempfile.TemporaryFile() as said, subprocess.Popen( - ["git", *LACKING_WALK, *([head] if head else [])], + ["git", *LACKING_WALK, *([head] if head else []), "--"], cwd=str(path), stdout=subprocess.PIPE, stderr=said) as walk: for line in walk.stdout: diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 66fee9f2..0ebc1227 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -457,10 +457,11 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( adopter, made, monkeypatch, capsys, head): """A walk that fails tells nothing of what is lacking, so it is no `0`: a cured partial clone, its walk made to fail (an option no git - has), is refused, naming the walk as run (`HEAD` last where HEAD names a - commit) and how git exited, saying git could not walk the refs (and no - object count), naming `git fsck` in the source, as what names a ref - whose object is lacking and no object a promisor holds, and the cure.""" + has), is refused, naming the walk as run (`HEAD` where HEAD names a + commit, then `--` last) and how git exited, saying git could not walk + the refs (and no object count), naming `git fsck` in the source, as what + names a ref whose object is lacking and no object a promisor holds, and + the cure.""" monkeypatch.setattr(adopter, "LACKING_WALK", ("rev-list", "--no-such-option"), raising=False) source = made("cured") @@ -470,7 +471,7 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( "--work-dir", str(source.base / "work")]) said = capsys.readouterr() walk = "`git rev-list --no-such-option" + ( - " HEAD`" if head == "born" else "`") + " HEAD --`" if head == "born" else " --`") assert code == 2, said.err + said.out assert REFUSED in said.err assert (f"{walk}, which lists what a clone lacks without fetching it, " @@ -566,6 +567,24 @@ def test_a_file_named_head_in_the_source_is_no_path_of_the_walk(made): source.untouched() +def test_a_file_named_as_the_head_id_is_no_path_of_the_walk(made): + """The walk ends with `--`: HEAD is walked by its id, and a file of the + source's work tree named as that id made git exit 128, `ambiguous + argument '': both revision and filename`, and refused a source that + lacks nothing. A commit cannot hold a file named as its own id, so the + file is in the work tree only.""" + source = made("cured") + head = git("rev-parse", "HEAD", cwd=source.path).stdout.strip() + (source.path / head).write_text("not a revision\n", encoding="utf-8") + assert git("rev-list", head, cwd=source.path, + check=False).returncode == 128 + result = source.check() + assert result.returncode == 0, result.stderr + result.stdout + assert "plan ok" in result.stdout + assert CODE not in result.stderr + result.stdout + source.untouched() + + @pytest.mark.parametrize("lacked", ["branch", "tag", "head", "promised"]) def test_git_fsck_names_a_ref_whose_object_is_lacking_and_no_promised_one( made, lacked): From a0e8cdeda53001b2f61e2444b627972cbcddfdb3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 23 Oct 2026 20:31:31 +0000 Subject: [PATCH 17/17] adopt-project.py and tests/test_adopt_partial_clone_source.py: the text beside the `source-partial-clone` walk says a plain `HEAD` fails on an unborn HEAD as `bad revision 'HEAD'` now that the walk ends with `--`, and the rows pin git's own words after the failed walk's exit and the exact count the source lacks (#1145) r1180d's NIT-1 to NIT-3. - NIT-1: the `HEAD_ID` comment, the `_objects_lacking` docstring and two row docstrings described the walk as it was before the `--`. Measured with git 2.43: `rev-list ... HEAD --` on an unborn HEAD says `fatal: bad revision 'HEAD'` (exit 128); a file named `HEAD` is read as a path (`both revision and filename`) only without the `--`. - NIT-2: the failing-walk row asks this git for its own words, running the same walk, and asserts `exited : `. Git 2.43 prints its usage (exit 129) for `--no-such-option`, which does not name the option. Kills M15 (git's words dropped). - NIT-3: the count row counts the `?` lines of the tool's own walk (`--branches --tags HEAD --`), asserts more than 1, and asserts `that lacks at least object(s) `. Kills M17 (count held at 1). Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 --- adopt-project.py | 30 ++++++++-------- tests/test_adopt_partial_clone_source.py | 45 ++++++++++++++++-------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/adopt-project.py b/adopt-project.py index c98cc216..76b860ff 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -12965,12 +12965,12 @@ def _grouped_findings(stored: list[str], taken: list[str], copies: list[str], LACKING_WALK = ("rev-list", "--objects", "--missing=print", "--branches", "--tags") #: What `_head_id` asks: HEAD's id, printed, and exit 1 where HEAD is unborn -#: (`git switch --orphan`). A plain `HEAD` in the walk fails there, -#: `ambiguous argument 'HEAD'`, exit 128. NOT `HEAD^{commit}`: it reads the -#: object, and where the clone lacks it that fetched it (measured with git -#: 2.43, `GIT_NO_LAZY_FETCH` unset: 1 `upload-pack` run), and under the -#: variable it exits 1, as an unborn HEAD does. `HEAD` prints the id of a -#: commit the clone lacks and runs nothing. +#: (`git switch --orphan`). A plain `HEAD` in the walk, which ends with +#: `--`, fails there, `bad revision 'HEAD'`, exit 128. NOT `HEAD^{commit}`: +#: it reads the object, and where the clone lacks it that fetched it +#: (measured with git 2.43, `GIT_NO_LAZY_FETCH` unset: 1 `upload-pack` run), +#: and under the variable it exits 1, as an unborn HEAD does. `HEAD` prints +#: the id of a commit the clone lacks and runs nothing. HEAD_ID = ("rev-parse", "--verify", "-q", "HEAD") @@ -13069,14 +13069,16 @@ def _objects_lacking(path: Path) -> tuple[int | None, str]: hides what is under it, so the count is a least. HEAD IS WALKED BY ITS ID, AND ONLY WHERE `_head_id` has one: a plain - `HEAD` fails on an unborn HEAD (exit 128), and a file of the source's - work tree named `HEAD` would be read as a path (`ambiguous argument - 'HEAD': both revision and filename`). A file named as the id is read - as a path too, `ambiguous argument '': both revision and - filename`, so the walk ENDS WITH `--`: everything before it is a - revision or an option, and no file of the work tree is read at all. - The walk is named with `HEAD`, as a person asks it, and with the `--` - it ends with, so what a person pastes is the walk that ran.""" + `HEAD` fails on an unborn HEAD (`bad revision 'HEAD'`, exit 128). + Without the `--`, a file of the source's work tree named `HEAD`, or + named as the id, is read as a path (`ambiguous argument 'HEAD': both + revision and filename`, and the same for the id, exit 128), so the + walk ENDS WITH `--`: everything before it is a revision or an option, + and no file of the work tree is read at all. With the `--`, `HEAD` by + name would now do as well where HEAD names a commit (measured with git + 2.43); the walk keeps the id `_head_id` gives. The walk is named with + `HEAD`, as a person asks it, and with the `--` it ends with, so what a + person pastes is the walk that ran.""" head = _head_id(path) shown = (*LACKING_WALK, *(("HEAD",) if head else ()), "--") walked = (f"`git {' '.join(shown)}`, which lists what a clone lacks " diff --git a/tests/test_adopt_partial_clone_source.py b/tests/test_adopt_partial_clone_source.py index 0ebc1227..7c9a8951 100644 --- a/tests/test_adopt_partial_clone_source.py +++ b/tests/test_adopt_partial_clone_source.py @@ -277,15 +277,20 @@ def test_check_refuses_a_partial_clone_that_lacks_objects_and_fetches_none( """Base: `plan ok`, after 26 `upload-pack` runs and `.promisor` files 1 -> 27 for `nocheckout`, and none for `checkout`, whose 2 lacking history blobs `execute` then failed on. Head: refused, nothing fetched, the - refusal gives the count it lacks at least and the cure, and says a later - fetch stays filtered (the cure leaves `partialclonefilter` set).""" + refusal gives as the count it lacks at least the `?` lines of its own + walk (more than 1, so a count held at 1 is told), and the cure, and says + a later fetch stays filtered (the cure leaves `partialclonefilter` + set).""" source = made(variant) - assert source.lacking() > 0 + listed = git("rev-list", "--objects", "--missing=print", "--branches", + "--tags", "HEAD", "--", cwd=source.path).stdout + lacks = sum(line.startswith("?") for line in listed.splitlines()) + assert lacks > 1 promisor = source.promisor_packs() result = source.check() assert result.returncode == 2, result.stderr + result.stdout assert REFUSED in result.stderr - assert "that lacks at least " in result.stderr + assert f"that lacks at least {lacks} object(s) " in result.stderr assert "Remediation: make the source whole" in result.stderr assert CURE in result.stderr assert LATER in result.stderr @@ -458,10 +463,12 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( """A walk that fails tells nothing of what is lacking, so it is no `0`: a cured partial clone, its walk made to fail (an option no git has), is refused, naming the walk as run (`HEAD` where HEAD names a - commit, then `--` last) and how git exited, saying git could not walk - the refs (and no object count), naming `git fsck` in the source, as what - names a ref whose object is lacking and no object a promisor holds, and - the cure.""" + commit, then `--` last), how git exited and git's own words after it, + saying git could not walk the refs (and no object count), naming `git + fsck` in the source, as what names a ref whose object is lacking and no + object a promisor holds, and the cure. Git's words are asked of this + git, running the same walk: git 2.43 prints its usage, exit 129, which + does not name the option.""" monkeypatch.setattr(adopter, "LACKING_WALK", ("rev-list", "--no-such-option"), raising=False) source = made("cured") @@ -472,10 +479,18 @@ def test_a_partial_clone_git_cannot_walk_is_refused_on_any_git( said = capsys.readouterr() walk = "`git rev-list --no-such-option" + ( " HEAD --`" if head == "born" else " --`") + ran = ["git", "rev-list", "--no-such-option", *( + [git("rev-parse", "HEAD", cwd=source.path).stdout.strip()] + if head == "born" else []), "--"] + asked = subprocess.run(ran, cwd=str(source.path), capture_output=True, + check=False) + words = adopter.spelled_as_git_out( + asked.stderr.decode("utf-8", "replace").strip()) + assert asked.returncode != 0 and words assert code == 2, said.err + said.out assert REFUSED in said.err assert (f"{walk}, which lists what a clone lacks without fetching it, " - "exited ") in said.err + f"exited {asked.returncode}: {words}") in said.err assert "branches, tags and HEAD git could not walk" in said.err assert "lacks at least" not in said.err assert (f"Remediation: run `git fsck` in " @@ -536,8 +551,8 @@ def test_a_head_at_a_commit_the_source_lacks_is_refused_and_not_fetched( def test_an_unborn_head_is_read_as_before(made, command): """HEAD on a branch that does not exist (`git switch --orphan`) names no commit, and a cured partial clone lacks nothing: read as a full clone is, - nothing fetched. A plain `HEAD` in the walk exits 128 on it, `ambiguous - argument 'HEAD'`, and would refuse it.""" + nothing fetched. A plain `HEAD` in the walk, which ends with `--`, exits + 128 on it, `bad revision 'HEAD'`, and would refuse it.""" source = made("cured") source.unborn_head() assert source.lacking("--branches") == 0 @@ -554,10 +569,10 @@ def test_an_unborn_head_is_read_as_before(made, command): def test_a_file_named_head_in_the_source_is_no_path_of_the_walk(made): - """HEAD is walked by its id: with `HEAD` in the walk, a file of the - source's work tree named `HEAD` made git exit 128, `ambiguous argument - 'HEAD': both revision and filename`, and refused a source that lacks - nothing.""" + """HEAD is walked by its id, and the walk ends with `--`: with `HEAD` in + the walk and no `--`, a file of the source's work tree named `HEAD` made + git exit 128, `ambiguous argument 'HEAD': both revision and filename`, + and refused a source that lacks nothing.""" source = made("cured") (source.path / "HEAD").write_text("not a revision\n", encoding="utf-8") result = source.check()