From 5342b182c786dfefce0d47bafad9cb1b8130191e Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 17:50:50 +0000 Subject: [PATCH 1/4] shape-doctor.py: the `legs` and `members` NEXT names every detached commit it would leave in a reflog alone, so the repository git kept for an emptied mount is read through `--git-dir` and kept with a line that names it, the holder's own bootstrap carries the same keep sentence, and a members line says `--recursive` checks out each member's own legs too (#413) `submodule_update_next` named a keep line only for a POPULATED mount detached off its pin (`off_pin`). The line moves three more HEADs that can hold the only copy of a commit, and after the paste each such commit was held by a reflog entry alone, with nothing said. (a) A mount emptied by `git submodule deinit -f` keeps its repository in `.git/modules/`, and `submodule update` links the mount to it again and checks the gitlink out over the commit. `emptied_reach` finds that repository by the submodule's NAME (`gitmodules_entry`, which reads `.gitmodules` where git reads it: the working tree, else the index, else HEAD) and `git rev-parse --git-path modules/`, and reads its HEAD through `--git-dir` (`kept_head`), never in the mount, where git walks up and answers about the root. Detached at a commit that is not the gitlink, the rule `off_pin` already used (`left_in_reflog`), the commit is named with `git --git-dir= branch NAME `: in the empty mount `git -C branch NAME` would branch the ROOT. With the mount directory itself gone, git refuses every command in that repository (`cannot chdir`, exit 128, measured), because its `core.worktree` names the mount, so the read and that keep line carry a `--work-tree`. (b) The holder's own `scripts/bootstrap.py`, the `members` NEXT where it can run, runs the same update over every member (`fetch_members`) and said only that it leaves each member detached at its pin. Its `why` now carries the same keep sentence, built by the same function (`checkout_notes`, factored out of `submodule_update_next`), for every member it would move off a detached commit, not only those the row sends to a checkout, because its update names no path. (c) `--recursive` checks out each member's own legs at the commits the MEMBER records. The row does not walk into them: a `members` line says so in general words, with `git -C LEG branch NAME` to run in that leg first, wherever a member it reaches has a repository for those legs to be in. `--recursive` stays, for parity with the holder's bootstrap. `detached` takes a `git_dir` for a kept repository, and answers None, not "detached", when `symbolic-ref` fails for any reason but a detached HEAD. Tests at the end of tests/test_doctor_legs_members_next.py, in the shape of `test_the_keep_line_in_the_explanation_keeps_a_detached_commit`: for each of (a), (b) and (c) the commit is named before the paste (by its sha in (a), by its mount's keep line in (b), in general words in (c), which does not walk), the keep line run first leaves it on a branch, and without that line no ref holds it afterwards. A member on a branch still gets no keep line of its own, and the existing test that asserted no `reflog` at all in its `why` asserts that instead. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 314 +++++++++++++++++++++--- tests/test_doctor_legs_members_next.py | 320 ++++++++++++++++++++++++- 2 files changed, 603 insertions(+), 31 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index 343a83ba..c137ce6a 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3432,9 +3432,225 @@ def lockstep_next(ctx: Context, gate: tuple, noun: str) -> tuple: "names the repair where it has one") +def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: + """`(name, url)` of the `.gitmodules` section whose `path` is `rel`, read + as `git submodule update` reads it, or `(None, None)` when no section + gives that path (#413). + + THE NAME SAYS WHERE GIT KEEPS THE MOUNT'S REPOSITORY (`kept_gitdir`), + and it is not always the path: git allows a submodule mounted by hand + under another one. + + READ WHERE GIT READS IT, WHICH IS NOT ALWAYS THE FILE. git reads the + working tree's `.gitmodules` when there is one, else the copy in the + index, else the one in HEAD. So a root whose `.gitmodules` was deleted + from the working tree alone still updates, and `git config -f + .gitmodules` by itself would find no section there. A file git cannot + parse gives nothing here. + + THE LAST VALUE GIVEN WINS, for a path two sections give and for a key a + section gives twice, because git reads `.gitmodules` with each later + value overwriting the one before. `-z`, because a section name may hold + a space, a dot or a quote. An empty `url =` is no url. Read only: + `--get-regexp` writes nothing. + """ + if os.path.lexists(root / ".gitmodules"): + sources = [["--file", ".gitmodules"]] + else: + sources = [["--blob", ":.gitmodules"], ["--blob", "HEAD:.gitmodules"]] + for source in sources: + try: + proc = subprocess.run( + ["git", "config", "-z", *source, "--get-regexp", + r"^submodule\..*\.(path|url)$"], + cwd=str(root), capture_output=True, check=False) + except OSError: + return None, None + if proc.returncode == 1: + # Read, and no section gives a path or a url. + return None, None + if proc.returncode != 0: + # No such copy, or one git cannot parse: the next place, as git. + continue + named = None + urls: dict[str, str] = {} + for record in proc.stdout.decode("utf-8", "replace").split("\0"): + key, _, value = record.partition("\n") + name, _, field = key[len("submodule."):].rpartition(".") + if field == "path" and value == rel: + named = name + elif field == "url": + urls[name] = value + if named is None: + return None, None + return named, urls.get(named) or None + return None, None + + +def kept_gitdir(root: Path, name: str) -> Path | None: + """The repository git keeps in `root` for the submodule NAMED `name`, or + None when it keeps none (#413). + + `git submodule deinit -f` empties a mount and KEEPS its repository under + the root's `.git/modules/`. `git submodule update` links the mount to + that repository again rather than fetching a new one (measured: a branch + made in it before the update is still there after), so what its HEAD + holds is what the line moves. + + BY NAME, which is not always the path: `--git-path modules/` says + where git keeps it, in the common directory of a linked worktree too. + """ + try: + where = git_out(["rev-parse", "--git-path", f"modules/{name}"], + cwd=root) + except (Refusal, OSError, UnicodeDecodeError): + return None + path = Path(where) + if not path.is_absolute(): + path = root / path + return path if path.is_dir() else None + + +def kept_head(root: Path, git_dir: Path) -> tuple[bool | None, str | None]: + """`(detached, commit)` of the HEAD in a repository git kept for a mount + (`kept_gitdir`), or `(None, None)` when git cannot read one (#413). + + ASKED THROUGH `--git-dir`, NEVER IN THE MOUNT. In an empty mount git + walks up and answers about the ROOT, which is why the row reads no HEAD + in one. `detached` is the same function the populated mounts are asked + through, so one question decides both. The root is the `--work-tree` + for the reason `emptied_reach` gives: the kept repository's own names + the mount, which may be gone. + """ + try: + proc = subprocess.run( + ["git", f"--git-dir={git_dir}", f"--work-tree={root}", + "rev-parse", "--verify", "-q", "HEAD"], + cwd=str(root), capture_output=True, check=False) + except OSError: + return None, None + if proc.returncode != 0: + return None, None + return (detached(root, git_dir=git_dir), + proc.stdout.decode("ascii", "replace").strip().lower()) + + +def left_in_reflog(is_detached: bool | None, head: str | None, + gitlink: str) -> bool: + """Does checking `gitlink` out leave the commit HEAD is on held by a + reflog entry alone? + + ONE RULE for a mount the row read a HEAD in and for a repository git + kept for an emptied one (#413): HEAD DETACHED, at a commit that is not + the gitlink. A commit on a branch stays on it, and a HEAD at the + gitlink does not move. + """ + return bool(is_detached) and head is not None and head != gitlink + + +def emptied_reach(root: Path, rel: str, + gitlink: str) -> tuple[tuple | None, bool]: + """`(keep, repository)` for the EMPTY mount at `rel`: what the line would + leave in a reflog alone there, and whether git kept a repository for it + at all (#413). + + `keep` is `(repository, commit, work tree)` when the kept repository's + HEAD is detached off the gitlink (`left_in_reflog`), and None otherwise. + `work tree` is the root when the mount directory is GONE, and None + otherwise: the kept repository's own `core.worktree` names the mount, + and git refuses every command in it while that directory is missing + (`cannot chdir`, exit 128, measured), the keep line included, unless a + `--work-tree` overrides it. Neither the read nor `branch` touches a work + tree, so which one is named changes nothing else. `repository` is + whether git kept one: a member's own legs live in it (#413 c). + """ + name, _ = gitmodules_entry(root, rel) + git_dir = kept_gitdir(root, name) if name is not None else None + if git_dir is None: + return None, False + is_detached, kept_at = kept_head(root, git_dir) + if not left_in_reflog(is_detached, kept_at, gitlink): + return None, True + return (git_dir, kept_at, None if (root / rel).is_dir() else root), True + + +def kept_branch_line(kept: tuple, platform: str) -> str: + """`git --git-dir= branch NAME `: the line that keeps + a commit a kept repository holds on a detached HEAD (#413 a). + + It names the REPOSITORY, because in a mount with no `.git` of its own + `git -C branch NAME` walks up and would branch the ROOT. And it + names the COMMIT, so the `why` says which commit is at stake, and the + line keeps that one wherever the repository's HEAD is by the time it + runs. Every value through `quote_arg`. + """ + git_dir, commit, work_tree = kept + words = ["git", quote_arg(f"--git-dir={git_dir}", platform)] + if work_tree is not None: + words.append(quote_arg(f"--work-tree={work_tree}", platform)) + return " ".join([*words, "branch", "NAME", commit]) + + +def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), + nested: bool = False, + platform: str | None = None) -> str: + """What a NEXT that moves HEAD owes a reader before it is pasted, as the + sentences that follow the one saying what it does, or `""` when it owes + nothing (#276, #413). + + ONE DEFINITION FOR THE GIT LINE AND THE HOLDER'S BOOTSTRAP. #276 wrote + the keep sentence into `submodule_update_next` alone, and the `members` + row's other NEXT, the holder's own `scripts/bootstrap.py`, runs the same + update (`fetch_members`) and moves the same HEADs: a commit made in the + pinned, detached `members/

` was left in its reflog with no word about + it (#413 b). So both build the sentence here. + + - `off_pin`: populated mounts detached off the commit the root records, + each kept first with `git -C branch NAME`. + - `kept`: `(repository, commit, work tree)` for emptied mounts whose kept + repository is detached off it (#413 a), each kept first with + `kept_branch_line`, which names the commit as well. + - `nested`, for the `members` row: the line is recursive, so it checks + out each member's own legs too, at the commits THE MEMBER records, and + a commit made on no branch in one of them is left in its reflog the + same way (#413 c). Said in general words, without walking into them, + and only where a member it reaches has a repository for them to be in. + `--recursive` stays: the holder's bootstrap recurses + (`fetch_members`), and a git line that did not would leave a member's + legs where a bootstrap would not. + + A commit made in a mount on a BRANCH stays on that branch, and nothing + is said where no mount is in any of these states, because a reader told + about a loss that cannot happen learns to skip the sentence on the day + it can. None of `;`, `'`, a backtick or a parenthesis is written here + (`submodule_update_next` says why). + """ + target = os.name if platform is None else platform + keeps = [f"git -C {quote_arg(mount, target)} branch NAME" + for mount in off_pin] + keeps += [kept_branch_line(entry, target) for entry in kept] + notes = "" + if keeps: + notes += (f". A commit made in a {noun} on no branch is left only in " + "its reflog by this line. To keep one, run first " + f"{' and '.join(keeps)} with a name of your choice") + if kept: + notes += (". A mount with no .git of its own is named by the " + "repository git kept for it, because git run inside such a " + "mount walks up and acts on this repository") + if nested: + notes += (". It is recursive, so it also checks out the legs of each " + "member at the commits that member records, and a commit " + "made on no branch in one of those legs is left only in its " + "reflog too. To keep one, run first git -C LEG branch NAME, " + "with LEG the directory of that leg") + return notes + + def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], noun: str, after: str = "", rerun: bool = False, - platform: str | None = None) -> tuple[str, str]: + platform: str | None = None, *, kept: list = (), + nested: bool = False) -> tuple[str, str]: """`(the line, what it does)`: the script-free command that checks each submodule at `paths` out at the commit `root` records for it, and the sentence a reader needs before pasting it (#276). @@ -3468,11 +3684,11 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], IT MOVES HEAD, AND THAT IS NOT FREE, so the sentence says so. A commit made in a leg on a BRANCH stays on that branch. A commit made on a detached HEAD is left only in the leg's reflog, which is what bootstrap's - own `checkout ` does to it too. `off_pin` is the mounts whose - HEAD is exactly that, and each is named with the line that keeps the - commit first, `git -C branch NAME`. Nothing is said when no mount - is in that state, because a reader told about a loss that cannot happen - learns to skip the sentence on the day it can. + own `checkout ` does to it too. `checkout_notes` names each + place that can happen with the line that keeps the commit first: `off_pin`, + the populated mounts whose HEAD is exactly that, and `kept`, the + repositories git kept for emptied mounts whose HEAD is (#413 a). + `nested` is its too. THE SENTENCE CARRIES NONE OF `;`, `'`, A BACKTICK OR A PARENTHESIS. It was written as a ` # ` tail, and a tail is pasted with the line: a @@ -3492,14 +3708,9 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], "update", "--init", "--recursive", "--checkout", "--", *(quote_arg(path, target) for path in paths)]) why = (f"checks each {noun} out at the commit this repository records " - "for it, and fetches a mount that is empty") - if off_pin: - keep = " and ".join(f"git -C {quote_arg(mount, target)} branch NAME" - for mount in off_pin) - why += (f". A commit made in a {noun} on no branch is left only in " - "its reflog by this line. To keep one, run first " - f"{keep} with a name of your choice") - why += after + "for it, and fetches a mount that is empty" + + checkout_notes(noun, off_pin, kept, nested, target) + + after) if rerun: why += (f". Run the doctor again afterwards, because a {noun} this " "line does not cure is named by the next run") @@ -3550,6 +3761,11 @@ def check_legs(ctx: Context) -> Row: walks up, and the HEAD it answers with is the ROOT's (see `member_state`), which the update does not cure either unless a leftover gitdir happens to still be there. + + WHAT THE LINE WOULD LEAVE IN A REFLOG IS NAMED, wherever it is (#413): + in a populated leg detached off the gitlink, and in the repository git + kept for an emptied one, read through `--git-dir` and kept with a line + that names that repository (`checkout_notes`). """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -3566,6 +3782,7 @@ def check_legs(ctx: Context) -> Row: problems: list[str] = [] checkout: list[str] = [] off_pin: list[Path] = [] + kept: list[tuple] = [] lockstep: list[str] = [] for leg in legs: role = str(leg.get("role") or "?") @@ -3586,6 +3803,10 @@ def check_legs(ctx: Context) -> Row: problems.append(f"{rel}: the gitlink is there and the directory " "is empty -- an unbootstrapped clone") checkout.append(rel) + # ITS KEPT REPOSITORY'S HEAD, read where it is (#413). + keep, _ = emptied_reach(ctx.root, rel, gitlink) + if keep is not None: + kept.append(keep) elif head is None: problems.append(f"{rel}: populated, but git cannot read a HEAD in " "it") @@ -3599,7 +3820,7 @@ def check_legs(ctx: Context) -> Row: f"{pinned[:12]}") if gitlink == pinned and (mount / ".git").exists(): checkout.append(rel) - if detached(mount): + if left_in_reflog(detached(mount), head, gitlink): off_pin.append(mount) else: lockstep.append(rel) @@ -3620,7 +3841,8 @@ def check_legs(ctx: Context) -> Row: # The line is the NEXT and what it does is the row's `why`, on a # line of its own (#277). next_command, why = submodule_update_next( - ctx.root, checkout, off_pin, "leg", after, bool(lockstep)) + ctx.root, checkout, off_pin, "leg", after, bool(lockstep), + kept=kept) else: # NO CHECKOUT CURES IT: the lockstep gate, one definition of it # for this row and the `pins` row (`lockstep_next`). @@ -5198,17 +5420,27 @@ def check_agent_files(ctx: Context) -> Row: "absent: " + ", ".join(absent), fix, detail, why=why) -def detached(path: Path) -> bool | None: +def detached(path: Path, git_dir: Path | None = None) -> bool | None: """Is this checkout on a detached HEAD? None when git cannot say. - `symbolic-ref -q HEAD` succeeds with a branch name and fails when HEAD - is detached, which is the one question with no ambiguity in it. + `symbolic-ref -q HEAD` succeeds with a branch name and exits 1 when HEAD + is detached, which is the one question with no ambiguity in it. Any + other exit is git unable to answer, not an answer: it was read as + "detached". + + `git_dir` asks it of a repository git KEPT for a mount with no `.git` of + its own, through `--git-dir`, with `path` as the work tree (`kept_head`): + asked in such a mount, git walks up and answers about the root (#413). """ - if not (path / ".git").exists(): + if git_dir is None and not (path / ".git").exists(): return None - proc = subprocess.run(["git", "symbolic-ref", "-q", "HEAD"], + where = ([] if git_dir is None + else [f"--git-dir={git_dir}", f"--work-tree={path}"]) + proc = subprocess.run(["git", *where, "symbolic-ref", "-q", "HEAD"], cwd=str(path), capture_output=True, check=False) - return proc.returncode != 0 + if proc.returncode not in (0, 1): + return None + return proc.returncode == 1 def is_this_member(sibling: Path, row: dict) -> bool: @@ -5573,22 +5805,39 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, # answers with the HOLDER's HEAD, `member_state`). checkout: list[str] = [] off_pin: list[Path] = [] + kept: list[tuple] = [] + moved: list[Path] = [] lockstep = False + nested = False for entry in rows: + rel = entry["path"] + mount = ctx.root / rel + gitlink = recorded_gitlink(ctx.root, rel) + # WHAT THE HOLDER'S BOOTSTRAP MOVES IS EVERY MEMBER (#413 b): its + # update names no path, so a member detached off its gitlink is + # moved whatever this row says of it, and is named for it. + if (gitlink is not None and entry["populated"] + and (mount / ".git").exists() + and left_in_reflog(entry["detached"], entry["head"], + gitlink)): + moved.append(mount) if member_problem(entry) is None: continue - mount = ctx.root / entry["path"] - gitlink = recorded_gitlink(ctx.root, entry["path"]) if gitlink is None: lockstep = True elif not entry["populated"]: - checkout.append(entry["path"]) + checkout.append(rel) + keep, repository = emptied_reach(ctx.root, rel, gitlink) + nested = nested or repository + if keep is not None: + kept.append(keep) elif (entry["pin"] is None or entry["head"] is None or not (mount / ".git").exists()): lockstep = True elif gitlink == entry["pin"]: - checkout.append(entry["path"]) - if entry["detached"]: + checkout.append(rel) + nested = True + if left_in_reflog(entry["detached"], entry["head"], gitlink): off_pin.append(mount) else: lockstep = True @@ -5603,14 +5852,21 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, f"{PYTHON} " f"{quote_arg(ctx.root / 'scripts' / 'bootstrap.py')} " f"--root {quote_arg(ctx.root)}") + # The same sentences the git line carries, built by the same + # function (#413 b), for every member it moves. Its update is + # recursive too, into every member that is there. why = ("what `make bootstrap` runs in a holder: it fetches every " - "member at its pin and leaves it detached there") + "member at its pin and leaves it detached there" + + checkout_notes( + "member", moved, kept, + nested or any(entry["populated"] for entry in rows))) elif checkout: after = (". This holder has no scripts/bootstrap.py that can " "run, so this is the plain git line, with no " "credential rewrite for a private member") next_command, why = submodule_update_next( - ctx.root, checkout, off_pin, "member", after, lockstep) + ctx.root, checkout, off_pin, "member", after, lockstep, + kept=kept, nested=nested) else: # THE FAMILY GATE, one definition of it for this row and the # `family` row (`lockstep_next`). diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 54670422..53123adf 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -745,7 +745,10 @@ def test_a_pasted_members_git_line_cures_the_row(standard, holder, tmp_path): def test_the_members_reflog_note_is_printed_for_a_detached_member_only( standard, holder, tmp_path): """The git line moves a member's HEAD too: a commit made on its detached - HEAD is left only in the reflog, and a commit made on a branch is not.""" + HEAD is left only in the reflog, and a commit made on a branch is not. + The branched member is still named in general words for its own legs, + which the recursive line checks out too (#413 c), but not by a keep + line of its own.""" detached = holder_copy(holder, tmp_path / "detached") holder_without(detached, "bootstrap.py") member = detached / "members" / FAMILY_MEMBER @@ -764,7 +767,9 @@ def test_the_members_reflog_note_is_printed_for_a_detached_member_only( commit_in(member) why = why_of(row_of(standard, branched, "members")) assert why, "the line must say what it does" - assert "reflog" not in why, why + assert "A commit made in a member on no branch" not in why, why + assert f"git -C {module.quote_arg(member.resolve())} branch NAME" \ + not in why, why @WINDOWS_SKIP @@ -986,3 +991,314 @@ def test_an_empty_member_with_no_pin_is_still_a_checkout_first( again = row_of(standard, root, "members") assert again["status"] == "FINDING", again assert "validate-family.py" in command_of(again), again["next"] + + +# --- #413: every detached commit the line moves is named -------------------- +# +# The keep sentence named a mount only where the row had read a HEAD in it, +# detached off its pin. The line moves three more HEADs that can hold the +# only copy of a commit: the repository git kept for an emptied mount (a), +# every member the holder's own bootstrap updates (b), and a member's own +# legs, which `--recursive` checks out (c). Each test has the shape of +# `test_the_keep_line_in_the_explanation_keeps_a_detached_commit`: the +# commit is named before the paste, the keep line the row names, run first, +# leaves it on a branch, and without that line no ref holds it afterwards. + +#: The keep line for a repository git kept (#413 a): it names the +#: REPOSITORY and the commit, and a work tree only where the mount directory +#: itself is gone. +KEPT_LINE = re.compile(r"git --git-dir=\S+(?: --work-tree=\S+)? branch NAME " + r"[0-9a-f]{40}") + + +def sh(line: str, cwd: Path) -> subprocess.CompletedProcess: + """A line the row's `why` names, run as a reader pastes it: in a POSIX + shell, from a directory that is not the root.""" + cwd.mkdir(exist_ok=True) + return subprocess.run(["sh", "-c", line], cwd=str(cwd), + capture_output=True, text=True, encoding="utf-8", + errors="replace", check=False, env=PASTE_ENV, + stdin=subprocess.DEVNULL) + + +def kept_repository(root: Path, name: str) -> Path: + """Where git keeps the repository of the submodule NAMED `name`.""" + where = Path(git("rev-parse", "--git-path", f"modules/{name}", + cwd=root).stdout.strip()) + return where if where.is_absolute() else root / where + + +def emptied_with_its_repository_kept(root: Path, mount: str) -> str: + """A commit made on a mount's detached HEAD, and then the mount emptied + with `git submodule deinit -f`, which KEEPS its repository: that + commit's sha. The row reads no HEAD in an empty mount, and the update + links the mount to the kept repository again and checks the gitlink out + over the commit.""" + commit = commit_in(root / mount, "work on a detached HEAD, then deinit") + git("submodule", "deinit", "-q", "-f", "--", mount, cwd=root) + assert not any((root / mount).iterdir()) + return commit + + +def detached_member(root: Path) -> Path: + """The fixture's member is on a branch (`git submodule add` checks one + out); a fresh recursive clone of a holder leaves it detached, which is + where a commit made in it is on no branch.""" + member = root / "members" / FAMILY_MEMBER + git("checkout", "-q", "--detach", "HEAD", cwd=member) + return member + + +def with_its_own_legs(root: Path) -> Path: + """`family.py add` does not recurse, so the fixture's member has empty + legs: fill them, as a recursive clone of the holder does. Each is then + detached at the commit the MEMBER records for it.""" + git("-c", "protocol.file.allow=always", "submodule", "update", "--init", + "--recursive", "--", f"members/{FAMILY_MEMBER}", cwd=root) + return root / "members" / FAMILY_MEMBER + + +def test_413a_an_emptied_legs_kept_repository_names_its_detached_commit( + standard, project): + """(a) The mount is empty, so the row read no HEAD in it, and the line + said nothing. git kept its repository, detached at a commit no branch + holds, and the line moves it. The commit is named, by a keep line that + names the REPOSITORY: run in the empty mount, git walks up and would + branch the root.""" + commit = emptied_with_its_repository_kept(project, "spec") + module = doctor_module(standard) + row = row_of(standard, project, "legs") + resolved = project.resolve() + assert command_of(row) == ( + f"git -C {module.quote_arg(resolved)} {UPDATE} -- spec"), row["next"] + why = why_of(row) + repository = kept_repository(resolved, "spec") + assert (f"git {module.quote_arg(f'--git-dir={repository}')} branch NAME " + f"{commit}") in why, why + assert "reflog" in why, why + assert f"git -C {module.quote_arg(resolved / 'spec')} branch" not in why + for character in (";", "'", "`", "(", ")"): + assert character not in why, (character, why) + + +def test_413a_a_kept_repository_on_a_branch_or_at_the_gitlink_is_not_named( + standard, project): + """The same rule as a populated mount's, `left_in_reflog`: a kept HEAD + on a branch keeps its commit there, and one at the gitlink does not + move, so neither is a loss to warn about.""" + commit_in(project / "spec") + git("checkout", "-q", "-B", "work", cwd=project / "spec") + git("submodule", "deinit", "-q", "-f", "--", "spec", cwd=project) + git("submodule", "deinit", "-q", "-f", "--", "code", cwd=project) + why = why_of(row_of(standard, project, "legs")) + assert "reflog" not in why, why + assert "--git-dir" not in why, why + + +@WINDOWS_SKIP +def test_413a_the_keep_line_keeps_the_emptied_legs_commit_through_the_paste( + standard, project, tmp_path): + commit = emptied_with_its_repository_kept(project, "spec") + row = row_of(standard, project, "legs") + keep = KEPT_LINE.search(why_of(row)) + assert keep, why_of(row) + kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + assert "kept" in git("branch", "--contains", commit, + cwd=project / "spec").stdout + + +@WINDOWS_SKIP +def test_413a_without_the_keep_line_the_emptied_legs_commit_is_in_no_ref( + standard, project, tmp_path): + commit = emptied_with_its_repository_kept(project, "spec") + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + spec = project / "spec" + assert git("for-each-ref", "--contains", commit, + cwd=spec).stdout.strip() == "" + assert git("rev-parse", "HEAD", cwd=spec).stdout.strip() != commit + assert commit in git("reflog", cwd=spec).stdout + + +@WINDOWS_SKIP +def test_413a_with_the_mount_directory_gone_the_keep_line_still_runs( + standard, project, tmp_path): + """The kept repository's own `core.worktree` names the mount, and with + that directory gone git refuses every command in the repository, the + keep line included (`cannot chdir`, exit 128). The line then carries a + `--work-tree`, and runs.""" + commit = emptied_with_its_repository_kept(project, "spec") + (project / "spec").rmdir() + row = row_of(standard, project, "legs") + keep = KEPT_LINE.search(why_of(row)) + assert keep and "--work-tree=" in keep.group(0), why_of(row) + kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + assert "kept" in git("branch", "--contains", commit, + cwd=project / "spec").stdout + + +def test_413a_the_kept_repositorys_keep_line_is_spelled_for_the_readers_shell( + standard): + """`quote_arg` over the whole `--git-dir=` word, in both dialects: a + root with a space is one argument to `sh` and to PowerShell.""" + module = doctor_module(standard) + commit = "a" * 40 + _, why = module.submodule_update_next( + PurePosixPath("/srv/my atlas"), ["spec"], [], "leg", + platform="posix", + kept=[(PurePosixPath("/srv/my atlas/.git/modules/spec"), commit, + None)]) + assert (f"git '--git-dir=/srv/my atlas/.git/modules/spec' branch NAME " + f"{commit}") in why, why + _, why = module.submodule_update_next( + PureWindowsPath("C:/Work/My Atlas"), ["spec"], [], "leg", + platform="nt", + kept=[(PureWindowsPath("C:/Work/My Atlas/.git/modules/spec"), commit, + PureWindowsPath("C:/Work/My Atlas"))]) + assert ("git '--git-dir=C:\\Work\\My Atlas\\.git\\modules\\spec' " + "'--work-tree=C:\\Work\\My Atlas' branch NAME " + commit) in why + + +def test_413a_an_emptied_members_kept_repository_names_its_detached_commit( + standard, holder, tmp_path): + """The members row takes the same rule: the holder's own bootstrap is + the NEXT here, and it names the commit the kept repository holds.""" + root = holder_copy(holder, tmp_path) + detached_member(root) + commit = emptied_with_its_repository_kept(root, f"members/{FAMILY_MEMBER}") + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert "bootstrap.py" in command_of(row), row["next"] + repository = kept_repository(root.resolve(), f"members/{FAMILY_MEMBER}") + assert (f"git {module.quote_arg(f'--git-dir={repository}')} branch NAME " + f"{commit}") in why_of(row), why_of(row) + + +def test_413b_the_holders_own_bootstrap_names_a_detached_members_keep_line( + standard, holder, tmp_path): + """(b) The holder's own bootstrap runs the same update the git line does + (`fetch_members`), so a commit made in the pinned, detached member is + left in its reflog by it too. Its `why` carries the same sentence, built + by the same function, `checkout_notes`, and the recursive one (c).""" + root = holder_copy(holder, tmp_path) + member = detached_member(root) + commit_in(member) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + resolved = root.resolve() + assert command_of(row) == ( + f"{module.PYTHON} " + f"{module.quote_arg(resolved / 'scripts' / 'bootstrap.py')} " + f"--root {module.quote_arg(resolved)}"), row["next"] + why = why_of(row) + assert module.checkout_notes("member", [member.resolve()]) in why, why + assert f"git -C {module.quote_arg(member.resolve())} branch NAME" in why + assert "git -C LEG branch NAME" in why, why + + +@WINDOWS_SKIP +def test_413b_the_keep_line_keeps_a_members_commit_through_the_bootstrap( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + member = detached_member(root) + commit = commit_in(member) + row = row_of(standard, root, "members") + keep = re.search(r"git -C \S+ branch NAME", why_of(row)) + assert keep and "LEG" not in keep.group(0), why_of(row) + kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, root, "members")["status"] == "ok" + assert "kept" in git("branch", "--contains", commit, cwd=member).stdout + + +@WINDOWS_SKIP +def test_413b_without_it_the_holders_bootstrap_leaves_the_commit_in_no_ref( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + member = detached_member(root) + commit = commit_in(member) + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, root, "members")["status"] == "ok" + assert git("for-each-ref", "--contains", commit, + cwd=member).stdout.strip() == "" + assert commit in git("reflog", cwd=member).stdout + + +def made_in_a_members_own_leg(root: Path) -> tuple[Path, str]: + """A commit in the member, so the row is red, and one in the member's + own detached `spec` leg, which the row never reads: the leg and the + second commit's sha.""" + member = with_its_own_legs(root) + git("checkout", "-q", "--detach", "HEAD", cwd=member) + commit_in(member, "work in the member") + leg = member / "spec" + return leg, commit_in(leg, "work in the member's own leg") + + +def test_413c_the_members_git_line_says_it_checks_out_each_members_legs( + standard, holder, tmp_path): + """(c) `--recursive` checks out the member's own legs at the commits the + MEMBER records, and the row walks into none of them. The `why` says so + in general words, with the keep line to run in that leg first.""" + root = holder_copy(holder, tmp_path) + holder_without(root, "bootstrap.py") + made_in_a_members_own_leg(root) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert command_of(row) == ( + f"git -C {module.quote_arg(root.resolve())} {UPDATE} -- " + f"members/{FAMILY_MEMBER}"), row["next"] + why = why_of(row) + assert "It is recursive" in why, why + assert "the legs of each member" in why, why + assert "git -C LEG branch NAME" in why, why + for character in (";", "'", "`", "(", ")"): + assert character not in why, (character, why) + + +@WINDOWS_SKIP +def test_413c_the_leg_keep_line_keeps_a_members_leg_commit_through_the_paste( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + holder_without(root, "bootstrap.py") + leg, commit = made_in_a_members_own_leg(root) + module = doctor_module(standard) + row = row_of(standard, root, "members") + keep = re.search(r"git -C LEG branch NAME", why_of(row)) + assert keep, why_of(row) + kept = sh(keep.group(0).replace("LEG", module.quote_arg(leg.resolve())) + .replace("NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, root, "members")["status"] == "ok" + assert "kept" in git("branch", "--contains", commit, cwd=leg).stdout + + +@WINDOWS_SKIP +def test_413c_without_it_the_members_leg_commit_is_in_no_ref( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + holder_without(root, "bootstrap.py") + leg, commit = made_in_a_members_own_leg(root) + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, root, "members")["status"] == "ok" + assert git("for-each-ref", "--contains", commit, + cwd=leg).stdout.strip() == "" + assert commit in git("reflog", cwd=leg).stdout + From 83b38657202f6ee04136eb157bfe3af42c960586 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 18:02:12 +0000 Subject: [PATCH 2/4] shape-doctor.py: the `legs` and `members` rows read no HEAD in a mount with no `.git` of its own and send it to the git line when git kept its repository, name an unborn HEAD and a mount `.gitmodules` gives no url for with no NEXT instead of a gate that says `pins ok` or a line that exits 128, and say to run the doctor again for a leg the line checks out whose own pin is wrong (#414) #414's four states, found by the #276 review, and the `members` row, which reaches each of them: (1) A populated mount with no `.git` of its own. `head_of` asked git in it, git walked up, and the row quoted the ROOT's HEAD as the leg's. No HEAD is read there now (`member_state` likewise), and the mount is routed on what `submodule update` routes on, whether git kept its repository (`checkout_reach`): kept, the update links the two again and the row is `ok`, so it is the git line's; gone, the update tries a clone into a directory that is not empty and exits 1, so it stays the lockstep gate's. The `why` says which. (2) An unborn HEAD (`unborn_branch`) is no lockstep fact: the gate said `pins ok` and the row stayed red, and the update exits 128. Nothing cures it without choosing a commit for the person, so the NEXT is None and the `why` names the state. (3) A mount `.gitmodules` gives no url for. The git line's `--init` exits 128 on `No url found`, for an empty mount and a populated one alike, so `gitmodules_entry` is asked before the line is chosen. The gate, measured, names no `.gitmodules` entry: `validate-pins.py` refuses `pin-submodule-uninitialized` and `validate-family.py` `member-uninitialized`, each handing back the same git line. So the NEXT is None and the `why` says a person restores the entry from the repository's history; no command is printed, so none discards an uncommitted `.gitmodules`. A `.gitmodules` deleted from the working tree alone is read from the index, as git reads it, and the line stands. In a holder, a member with no url takes the holder's own bootstrap off the line: its update names no path, git stops it at a section with a path and no url before it fetches any member (exit 128), and the bootstrap still exits 0. The git line over the other members is the NEXT. (4) `rerun` is asked per leg: a leg the line checks out whose own pin is not 40 hex, or not its gitlink, gets "run the doctor again" too, and so does the holder's bootstrap line. The precedence stays: a checkout first, then the lockstep gate, then no line. Both rows route through `checkout_reach`, one definition. And #533's two stale docstrings, moved here from P6: `submodule_update_next`, `check_legs` and tests/test_doctor_legs_members_next.py said bootstrap "never moves an existing branch whose tip is not the pin". Since #475 and #490 it fast-forwards one behind the pin and moves back one ahead of it with no commit of its own; any other branch holding a commit the pin does not contain stays where it is. They say so now. Tests at the end of tests/test_doctor_legs_members_next.py, for both rows: each NEXT, pasted, turns the row `ok`, or the NEXT is None and the `why` or the reason names the state. (1) never quotes the root's HEAD, (3) never prints a line that exits 128, and one test pins what the git line and the gate do there, and (4) says to run the doctor again. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 545 ++++++++++++++++++------- tests/test_doctor_legs_members_next.py | 349 +++++++++++++++- 2 files changed, 738 insertions(+), 156 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index c137ce6a..daf0de15 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3434,19 +3434,25 @@ def lockstep_next(ctx: Context, gate: tuple, noun: str) -> tuple: def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: """`(name, url)` of the `.gitmodules` section whose `path` is `rel`, read - as `git submodule update` reads it, or `(None, None)` when no section - gives that path (#413). - - THE NAME SAYS WHERE GIT KEEPS THE MOUNT'S REPOSITORY (`kept_gitdir`), - and it is not always the path: git allows a submodule mounted by hand - under another one. + as `git submodule update` reads it: `(None, None)` when no section gives + that path, and `(name, None)` when the one that does gives no url (#414). + + WHY THE GIT LINE ASKS IT FIRST. The line runs `submodule update --init`, + and `--init` needs a url for every path it names: with none, git prints + `No url found for submodule path ... in .gitmodules` and exits 128. + Measured for an empty mount whose section is gone and for one that lost + only its `url` key, and for a populated mount whose section is gone: a + mount git already holds is asked for its url all the same. A NEXT that + exits 128 is the defect #276 was about, so a mount this answers no url + for is never put on that line. READ WHERE GIT READS IT, WHICH IS NOT ALWAYS THE FILE. git reads the working tree's `.gitmodules` when there is one, else the copy in the index, else the one in HEAD. So a root whose `.gitmodules` was deleted - from the working tree alone still updates, and `git config -f - .gitmodules` by itself would find no section there. A file git cannot - parse gives nothing here. + from the working tree alone still updates: measured, the line exits 0 + there. `git config -f .gitmodules` by itself would call every mount of + that root url-less and withhold a line that works. A file git cannot + parse gives no url here. THE LAST VALUE GIVEN WINS, for a path two sections give and for a key a section gives twice, because git reads `.gitmodules` with each later @@ -3489,13 +3495,14 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: def kept_gitdir(root: Path, name: str) -> Path | None: """The repository git keeps in `root` for the submodule NAMED `name`, or - None when it keeps none (#413). + None when it keeps none (#413, #414). `git submodule deinit -f` empties a mount and KEEPS its repository under - the root's `.git/modules/`. `git submodule update` links the mount to - that repository again rather than fetching a new one (measured: a branch - made in it before the update is still there after), so what its HEAD - holds is what the line moves. + the root's `.git/modules/`, and a mount whose `.git` file was deleted + leaves it there too. `git submodule update` links such a mount to that + repository again rather than fetching a new one (measured: a branch made + in it before the update is still there after), so what its HEAD holds is + what the line moves. BY NAME, which is not always the path: `--git-path modules/` says where git keeps it, in the common directory of a linked worktree too. @@ -3515,11 +3522,12 @@ def kept_head(root: Path, git_dir: Path) -> tuple[bool | None, str | None]: """`(detached, commit)` of the HEAD in a repository git kept for a mount (`kept_gitdir`), or `(None, None)` when git cannot read one (#413). - ASKED THROUGH `--git-dir`, NEVER IN THE MOUNT. In an empty mount git - walks up and answers about the ROOT, which is why the row reads no HEAD - in one. `detached` is the same function the populated mounts are asked + ASKED THROUGH `--git-dir`, NEVER IN THE MOUNT. In an empty mount, or in + one with no `.git` of its own, git walks up and answers about the ROOT, + which is how the `legs` row came to quote the root's HEAD as a leg's + (#414). `detached` is the same function the populated mounts are asked through, so one question decides both. The root is the `--work-tree` - for the reason `emptied_reach` gives: the kept repository's own names + for the reason `checkout_reach` gives: the kept repository's own names the mount, which may be gone. """ try: @@ -3541,37 +3549,91 @@ def left_in_reflog(is_detached: bool | None, head: str | None, reflog entry alone? ONE RULE for a mount the row read a HEAD in and for a repository git - kept for an emptied one (#413): HEAD DETACHED, at a commit that is not - the gitlink. A commit on a branch stays on it, and a HEAD at the - gitlink does not move. + kept for a mount with no `.git` of its own (#413): HEAD DETACHED, at a + commit that is not the gitlink. A commit on a branch stays on it, and a + HEAD at the gitlink does not move. """ return bool(is_detached) and head is not None and head != gitlink -def emptied_reach(root: Path, rel: str, - gitlink: str) -> tuple[tuple | None, bool]: - """`(keep, repository)` for the EMPTY mount at `rel`: what the line would - leave in a reflog alone there, and whether git kept a repository for it - at all (#413). - - `keep` is `(repository, commit, work tree)` when the kept repository's - HEAD is detached off the gitlink (`left_in_reflog`), and None otherwise. - `work tree` is the root when the mount directory is GONE, and None - otherwise: the kept repository's own `core.worktree` names the mount, - and git refuses every command in it while that directory is missing - (`cannot chdir`, exit 128, measured), the keep line included, unless a - `--work-tree` overrides it. Neither the read nor `branch` touches a work - tree, so which one is named changes nothing else. `repository` is - whether git kept one: a member's own legs live in it (#413 c). - """ - name, _ = gitmodules_entry(root, rel) - git_dir = kept_gitdir(root, name) if name is not None else None +def unborn_branch(path: Path) -> str | None: + """The branch HEAD names in the checkout at `path` when that branch has + no commit, an UNBORN HEAD, or None (#414). + + NOT A LOCKSTEP FACT: the gitlink and the pin may agree, and the gate, + handed over for it, says `pins ok` and changes nothing. Nor does + `submodule update` cure it: git cannot find a current revision and exits + 128 (measured). No line cures it without choosing a commit for the + person, so the row says so and names none. + + Asked only of a mount with a `.git` of its own, where `head_of` found no + commit: `symbolic-ref` names a branch and `rev-parse` finds no commit + behind it. A HEAD git cannot read at all is neither, and stays the + lockstep gate's. + """ + try: + branch = subprocess.run(["git", "symbolic-ref", "-q", "HEAD"], + cwd=str(path), capture_output=True, + check=False) + commit = subprocess.run(["git", "rev-parse", "--verify", "-q", + "HEAD"], cwd=str(path), capture_output=True, + check=False) + except OSError: + return None + if branch.returncode != 0 or commit.returncode == 0: + return None + ref = branch.stdout.decode("utf-8", "replace").strip() + return ref[len("refs/heads/"):] if ref.startswith("refs/heads/") else ( + ref or None) + + +def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, + own_git: bool, + head: str | None) -> tuple[str, object, bool]: + """`(route, keep, repository)` for a mount a CHECKOUT would cure, as the + `legs` and `members` rows both read it, so the two rows route by one + definition (#414). `repository` is whether git has one for the mount, + its own or a kept one: a member's own legs live in it (#413 c). + + `route` is: + + - `checkout`: the git line cures it. + - `relink`: the same, for a mount with files but no `.git` of its own + whose repository git kept. The update links the two again and the row + is `ok` (#414 1, measured), so it is not the lockstep gate's. + - `gone`: files, no `.git` of its own and no kept repository. No update + links anything: it tries a clone into a directory that is not empty + and exits 1 (`destination path ... already exists`, measured), so it + is the lockstep gate's. + - `no-url`: `.gitmodules` gives no url for it, so the git line would exit + 128 (`gitmodules_entry`, #414 3). No line is printed for it. + + `keep` is what the line would leave in a reflog alone (`left_in_reflog`): + the mount itself, for `git -C branch NAME`, or `(repository, + commit, work tree)` for one with no `.git` of its own whose kept + repository is detached off the gitlink (#413 a). `work tree` is the root + when the mount directory is GONE, and None otherwise: the kept + repository's own `core.worktree` names the mount, and git refuses every + command in it while that directory is missing (`cannot chdir`, exit 128, + measured), the keep line included, unless a `--work-tree` overrides it. + Neither the read nor `branch` touches a work tree, so which one is named + changes nothing else. + """ + mount = root / rel + name, url = gitmodules_entry(root, rel) + if url is None: + return "no-url", None, False + if own_git: + return "checkout", (mount if left_in_reflog(detached(mount), head, + gitlink) else None), True + git_dir = kept_gitdir(root, name) if git_dir is None: - return None, False + return ("gone" if populated else "checkout"), None, False is_detached, kept_at = kept_head(root, git_dir) - if not left_in_reflog(is_detached, kept_at, gitlink): - return None, True - return (git_dir, kept_at, None if (root / rel).is_dir() else root), True + keep = None + if left_in_reflog(is_detached, kept_at, gitlink): + keep = (git_dir, kept_at, None if mount.is_dir() else root) + return ("relink" if populated else "checkout"), keep, True def kept_branch_line(kept: tuple, platform: str) -> str: @@ -3592,11 +3654,11 @@ def kept_branch_line(kept: tuple, platform: str) -> str: def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), - nested: bool = False, + relinked: list = (), nested: bool = False, platform: str | None = None) -> str: """What a NEXT that moves HEAD owes a reader before it is pasted, as the sentences that follow the one saying what it does, or `""` when it owes - nothing (#276, #413). + nothing (#276, #413, #414). ONE DEFINITION FOR THE GIT LINE AND THE HOLDER'S BOOTSTRAP. #276 wrote the keep sentence into `submodule_update_next` alone, and the `members` @@ -3605,11 +3667,14 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), pinned, detached `members/

` was left in its reflog with no word about it (#413 b). So both build the sentence here. + - `relinked`: mounts with files and no `.git` of their own whose + repository git kept. The line links each to it again, which is why + they are on it at all (#414 1). - `off_pin`: populated mounts detached off the commit the root records, each kept first with `git -C branch NAME`. - - `kept`: `(repository, commit, work tree)` for emptied mounts whose kept - repository is detached off it (#413 a), each kept first with - `kept_branch_line`, which names the commit as well. + - `kept`: `(repository, commit, work tree)` for mounts with no `.git` of + their own whose kept repository is detached off it (#413 a), each kept + first with `kept_branch_line`, which names the commit as well. - `nested`, for the `members` row: the line is recursive, so it checks out each member's own legs too, at the commits THE MEMBER records, and a commit made on no branch in one of them is left in its reflog the @@ -3626,10 +3691,12 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), (`submodule_update_next` says why). """ target = os.name if platform is None else platform + notes = "".join(f". {rel} has files but no .git of its own, and git kept " + "its repository, so this line links the two again" + for rel in relinked) keeps = [f"git -C {quote_arg(mount, target)} branch NAME" for mount in off_pin] keeps += [kept_branch_line(entry, target) for entry in kept] - notes = "" if keeps: notes += (f". A commit made in a {noun} on no branch is left only in " "its reflog by this line. To keep one, run first " @@ -3647,9 +3714,49 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), return notes +def rerun_sentence(noun: str) -> str: + """The sentence the `legs` and `members` rows add to a line that leaves + something wrong for the next run to name: in another mount, or in a + mount the line does check out, whose pin is not 40 hex or not the + gitlink (#414 4).""" + return (". Run the doctor again afterwards, because what this line " + f"leaves wrong, in a {noun} it checks out or in another, is named " + "by the next run") + + +def unborn_sentence(rel: str, noun: str) -> str: + """The `why` for an unborn HEAD, the NEXT being None (#414 2): it names + the state and says that nothing here picks the commit. Not the lockstep + gate, which says `pins ok` and changes nothing.""" + return (f"HEAD in {rel} names a branch with no commit, so there is no " + "commit to compare with the pin, and nothing here chooses one " + f"for you. Check out in that {noun} the commit you mean, then run " + "the doctor again") + + +def no_url_sentence(rel: str) -> str: + """The `why` for a mount `.gitmodules` gives no url for, the NEXT being + None (#414 3). No command, deliberately: the entry is restored from the + repository's history by a person, and a `git checkout -- .gitmodules` + would discard an edit nobody has committed yet.""" + return (f"git finds no url for {rel} in .gitmodules, so no submodule " + "update can fetch it or check it out, and the git line would exit " + "128. A person restores that entry from the history of this " + "repository, then runs the doctor again") + + +def gone_sentence(rel: str) -> str: + """What the lockstep gate's `why` adds for a mount with files, no `.git` + of its own and no kept repository (#414 1): which of the two such a + mount is, said where the line is the gate's.""" + return (f". {rel} has files but no .git of its own, and git kept no " + "repository for it, so no submodule update links it again") + + def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], noun: str, after: str = "", rerun: bool = False, platform: str | None = None, *, kept: list = (), + relinked: list = (), nested: bool = False) -> tuple[str, str]: """`(the line, what it does)`: the script-free command that checks each submodule at `paths` out at the commit `root` records for it, and the @@ -3661,8 +3768,12 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], `shape currency` row says `copy-missing` for the same file) pasted `can't open file`, exit 2. Where the file was there it did not cure the row's first problem either: bootstrap never initialises a mount, it - prints `NOT INITIALIZED` and stops, and it never moves an existing branch - whose tip is not the pin (`templates/assembly-root/scripts/bootstrap.py`, + prints `NOT INITIALIZED` and stops. Nor does it move a tracking branch + whose tip is not the pin, except where that loses nothing: it + fast-forwards one behind the pin, and moves back one ahead of it with no + commit of its own (#475, #490), and any other branch holding a commit the + pin does not contain stays where it is + (`templates/assembly-root/scripts/bootstrap.py`, `checkout_tracking_branch`). `git submodule update --init --recursive -- ` needs no script and no `PYTHONPATH`, and a probe of both cases shows it cures them: an empty mount is filled, and a leg committed past @@ -3687,8 +3798,8 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], own `checkout ` does to it too. `checkout_notes` names each place that can happen with the line that keeps the commit first: `off_pin`, the populated mounts whose HEAD is exactly that, and `kept`, the - repositories git kept for emptied mounts whose HEAD is (#413 a). - `nested` is its too. + repositories git kept for mounts with no `.git` of their own whose HEAD + is (#413 a). `relinked` and `nested` are its too. THE SENTENCE CARRIES NONE OF `;`, `'`, A BACKTICK OR A PARENTHESIS. It was written as a ` # ` tail, and a tail is pasted with the line: a @@ -3696,9 +3807,9 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], syntax. It was plain words so that moving it out of the NEXT (#277) would be a move and not a rewrite, and it was: it is the row's `why`, on a line of its own. `after` is the caller's own closing sentence, and - `rerun` adds the one the `legs` and `members` rows share: some other - leg or member is something this line does not cure, and the next run - names it. + `rerun` adds the one the `legs` and `members` rows share + (`rerun_sentence`): something this line leaves wrong, in another leg or + member or in one it checks out, is named by the next run. `platform` is `quote_arg`'s and `copy_command`'s, and for the same reason: the line is spelled for the reader's shell, not the host's. @@ -3709,11 +3820,10 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], *(quote_arg(path, target) for path in paths)]) why = (f"checks each {noun} out at the commit this repository records " "for it, and fetches a mount that is empty" - + checkout_notes(noun, off_pin, kept, nested, target) + + checkout_notes(noun, off_pin, kept, relinked, nested, target) + after) if rerun: - why += (f". Run the doctor again afterwards, because a {noun} this " - "line does not cure is named by the next run") + why += rerun_sentence(noun) return line, why @@ -3732,40 +3842,59 @@ def check_legs(ctx: Context) -> Row: THE NEXT IS CHOSEN BY WHAT IS WRONG, AND IT CURES THE FIRST KIND (#276). `make bootstrap` was named for all of it and does not answer it: it - never initialises an empty mount, it never moves an existing branch whose - tip is not the pin, and a root can have lost the script it runs. So there - are two lines. - - - A leg a CHECKOUT cures: an empty mount, or a HEAD that is not the pin - where the gitlink IS the pin. The line is - `git submodule update --init --recursive` over those legs - (`submodule_update_next`), which needs no script from the root. + never initialises an empty mount, it moves a tracking branch whose tip is + not the pin only where that loses nothing (behind the pin, or ahead of it + with no commit of its own: #475, #490), and a root can have lost the + script it runs. So there are two lines, and a third answer that is none. + + - A leg a CHECKOUT cures: an empty mount, a mount with files but no + `.git` of its own whose repository git kept (the update links the two + again, #414), or a HEAD that is not the pin where the gitlink IS the + pin. The line is `git submodule update --init --recursive` over those + legs (`submodule_update_next`), which needs no script from the root. - A leg NO checkout cures: no gitlink, no 40-hex pin, a gitlink that is not the pin (a checkout lands on the gitlink, so HEAD would still not - be the pin), or a mount git cannot read a HEAD in. These are lockstep - facts, `validate-pins.py` is the gate that compares them and names the - repair, and the line is that gate's, from `validator_for` and - `validator_command` (the root's own copy, else the standard's). - - WHEN BOTH ARE PRESENT THE CHECKOUT WINS AND A RE-RUN NAMES THE OTHER, - because it is the one command that changes the row and a reader pasting - the lockstep gate first would be told nothing new. + be the pin), a mount git cannot read a HEAD in, or one with files, no + `.git` of its own and no kept repository to link it to. These are + lockstep facts, `validate-pins.py` is the gate that compares them and + names the repair, and the line is that gate's, from `validator_for` + and `validator_command` (the root's own copy, else the standard's). + - A leg NO LINE cures without choosing for the person (#414), so the + NEXT is None and the `why` names the state. An UNBORN HEAD, one naming + a branch with no commit: the update exits 128 on it and the gate says + `pins ok` (`unborn_branch`). And a leg `.gitmodules` gives no url for: + the git line exits 128, and the gate, measured, names no `.gitmodules` + entry. It says `pin-submodule-uninitialized` and hands back the same + git line (`gitmodules_entry`). + + WHEN MORE THAN ONE KIND IS PRESENT THE CHECKOUT WINS AND A RE-RUN NAMES + THE REST, because it is the one command that changes the row and a reader + pasting the lockstep gate first would be told nothing new. The re-run is + asked PER LEG too (#414): a leg the line checks out can carry a lockstep + fact of its own, a pin that is not 40 hex or not its gitlink, which an + empty mount hides until the next run. + + NO HEAD IS READ IN A MOUNT WITH NO `.git` OF ITS OWN (#414). `head_of` + asks git in that directory, git walks up, and the HEAD it answered with + was the ROOT's, which this row then quoted as the leg's. Such a mount is + routed by whether git KEPT its repository (`checkout_reach`), because + that is what `submodule update` routes on: probed, a populated mount + whose repository is kept is linked to it again and the row is `ok`, and + one whose repository is gone makes the update try a clone into a + directory that is not empty. The `why` says which of the two it is. "POPULATED, BUT GIT CANNOT READ A HEAD" IS THE SECOND KIND, and it was - probed: a mount whose gitdir is gone or whose HEAD is corrupt, an unborn - HEAD, and a leg left on an orphan branch all make `git submodule update` - exit 128 ("could not get a repository handle", "Unable to find current - revision") and leave this row exactly as it was, so printing that line - would be a NEXT that does not cure. A populated mount with no `.git` of - its own is read the same way: `head_of` asks git in that directory, git - walks up, and the HEAD it answers with is the ROOT's (see `member_state`), - which the update does not cure either unless a leftover gitdir happens to - still be there. + probed: a mount whose gitdir is gone or whose HEAD is corrupt makes `git + submodule update` exit 128 ("could not get a repository handle") and + leaves this row exactly as it was, so printing that line would be a NEXT + that does not cure. An unborn HEAD, a leg left on an orphan branch among + them, exits 128 too ("Unable to find current revision"), and is the + third kind, above. WHAT THE LINE WOULD LEAVE IN A REFLOG IS NAMED, wherever it is (#413): in a populated leg detached off the gitlink, and in the repository git - kept for an emptied one, read through `--git-dir` and kept with a line - that names that repository (`checkout_notes`). + kept for a leg with no `.git` of its own, read through `--git-dir` and + kept with a line that names that repository (`checkout_notes`). """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -3783,7 +3912,11 @@ def check_legs(ctx: Context) -> Row: checkout: list[str] = [] off_pin: list[Path] = [] kept: list[tuple] = [] + relinked: list[str] = [] lockstep: list[str] = [] + gone: list[str] = [] + stuck: list[str] = [] + rerun = False for leg in legs: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) @@ -3791,41 +3924,73 @@ def check_legs(ctx: Context) -> Row: pinned = pin_commit(ctx.root, f"contracts/{role}-pin.yaml") gitlink = recorded_gitlink(ctx.root, rel) populated = mount.is_dir() and any(mount.iterdir()) - head = head_of(mount) if populated else None + own_git = (mount / ".git").exists() + head = head_of(mount) if populated and own_git else None + unborn = (unborn_branch(mount) if populated and own_git + and head is None else None) entry = {"role": role, "path": rel, "pin": pinned, - "gitlink": gitlink, "populated": populated, "head": head} + "gitlink": gitlink, "populated": populated, "head": head, + "own_git": own_git, "unborn": unborn} rows.append(entry) + problem = None + cures = False if gitlink is None: - problems.append(f"{rel}: no gitlink recorded, so this leg is not " - "mounted as a submodule at all") + problem = (f"{rel}: no gitlink recorded, so this leg is not " + "mounted as a submodule at all") lockstep.append(rel) elif not populated: - problems.append(f"{rel}: the gitlink is there and the directory " - "is empty -- an unbootstrapped clone") - checkout.append(rel) - # ITS KEPT REPOSITORY'S HEAD, read where it is (#413). - keep, _ = emptied_reach(ctx.root, rel, gitlink) - if keep is not None: - kept.append(keep) + problem = (f"{rel}: the gitlink is there and the directory is " + "empty -- an unbootstrapped clone") + cures = True + elif not own_git: + problem = (f"{rel}: populated, but with no .git of its own, so " + "git cannot read a HEAD in it") + cures = True + elif unborn is not None: + problem = (f"{rel}: HEAD names the branch {unborn}, which has no " + "commit") + stuck.append(unborn_sentence(rel, "leg")) elif head is None: - problems.append(f"{rel}: populated, but git cannot read a HEAD in " - "it") + problem = f"{rel}: populated, but git cannot read a HEAD in it" lockstep.append(rel) elif pinned is None: - problems.append(f"{rel}: contracts/{role}-pin.yaml carries no " - "40-hex commit to compare HEAD against") + problem = (f"{rel}: contracts/{role}-pin.yaml carries no 40-hex " + "commit to compare HEAD against") lockstep.append(rel) elif head != pinned: - problems.append(f"{rel}: checked out at {head[:12]}, pinned at " - f"{pinned[:12]}") - if gitlink == pinned and (mount / ".git").exists(): - checkout.append(rel) - if left_in_reflog(detached(mount), head, gitlink): - off_pin.append(mount) + problem = (f"{rel}: checked out at {head[:12]}, pinned at " + f"{pinned[:12]}") + if gitlink == pinned: + cures = True else: lockstep.append(rel) + if cures: + route, keep, _ = checkout_reach(ctx.root, rel, gitlink, + populated, own_git, head) + if route == "no-url": + problem += ", and git finds no url for it in .gitmodules" + stuck.append(no_url_sentence(rel)) + elif route == "gone": + lockstep.append(rel) + gone.append(rel) + else: + checkout.append(rel) + if route == "relink": + relinked.append(rel) + if isinstance(keep, tuple): + kept.append(keep) + elif keep is not None: + off_pin.append(keep) + # ITS OWN LOCKSTEP FACT (#414): the line checks the gitlink + # out, and a pin that is not 40 hex, or not that gitlink, is + # what the next run names. + if pinned is None or gitlink != pinned: + rerun = True + if problem: + problems.append(problem) detail = {"legs": rows} if problems: + rerun = rerun or bool(lockstep) or bool(stuck) if checkout: # `make bootstrap` IS NAMED ONLY WHERE IT CAN RUN, and said for # what it does: it puts a leg on its tracking branch, and @@ -3841,12 +4006,19 @@ def check_legs(ctx: Context) -> Row: # The line is the NEXT and what it does is the row's `why`, on a # line of its own (#277). next_command, why = submodule_update_next( - ctx.root, checkout, off_pin, "leg", after, bool(lockstep), - kept=kept) - else: + ctx.root, checkout, off_pin, "leg", after, rerun, kept=kept, + relinked=relinked) + elif lockstep: # NO CHECKOUT CURES IT: the lockstep gate, one definition of it - # for this row and the `pins` row (`lockstep_next`). + # for this row and the `pins` row (`lockstep_next`), and which + # mount with no `.git` of its own is the gate's because git kept + # no repository for it (#414). next_command, why = lockstep_next(ctx, PINS_GATE, "leg") + if why: + why += "".join(gone_sentence(rel) for rel in gone) + else: + # NO LINE CURES IT without choosing for the person (#414). + next_command, why = None, ". ".join(stuck) return Row("legs", "legs", FINDING, "; ".join(problems), next_command, detail, why=why) return Row("legs", "legs", OK, @@ -5699,6 +5871,12 @@ def member_state(ctx: Context, row: dict, members_dir: str) -> dict: reported as checked out at the holder's commit. `head` and `detached` are `None` there, and the caller says which of the two it is. + NOR OF ONE WITH NO `.git` OF ITS OWN, for the same reason (#414): a + populated directory that is not a repository answers with the holder's + HEAD too, and the row quoted it as the member's. `own_git` says which + mount that is, and `unborn` names the branch an UNBORN HEAD names + (`unborn_branch`), a state no line cures without choosing a commit. + THE PIN IS ONLY BELIEVED WHEN IT IS 40 HEX. A `pin.commit` carrying a branch name or a tag is not a commit to compare a HEAD against, and this reports no pin at all rather than a comparison that would fail for the @@ -5713,26 +5891,31 @@ def member_state(ctx: Context, row: dict, members_dir: str) -> dict: pinned = str(pin.get("commit") or "").lower() pinned = pinned if COMMIT_RE.match(pinned) else None populated = mount.is_dir() and any(mount.iterdir()) - head = head_of(mount) if populated else None + own_git = (mount / ".git").exists() + head = head_of(mount) if populated and own_git else None + unborn = (unborn_branch(mount) if populated and own_git and head is None + else None) loose = detached(mount) if populated else None sibling = ctx.root.parent / project remote = mounted_from(ctx.root, rel, str(row.get("repository") or "")) clone, elsewhere = working_clone(sibling, row, remote) return {"project": project, "path": rel, "pin": pinned, - "populated": populated, "head": head, - "detached": loose, "mounted_from": redacted(remote), + "populated": populated, "head": head, "own_git": own_git, + "unborn": unborn, "detached": loose, + "mounted_from": redacted(remote), "working_clone": clone, "other_origin": elsewhere} def member_problem(entry: dict) -> str | None: """What is wrong with one member's pinned copy, or None. - THE ORDER IS THE ONLY ORDER THE ANSWERS MAKE SENSE IN: an empty mount has - no HEAD to compare, a row with no 40-hex pin has nothing to compare it + THE ORDER IS THE ONLY ORDER THE ANSWERS MAKE SENSE IN: an empty mount, a + populated one with no `.git` of its own and an unborn HEAD have no HEAD + to compare (#414), a row with no 40-hex pin has nothing to compare it against, and only then is "checked out somewhere else" a thing that can - be said. Each of these three is a member the family's own gate would - fail, which is why they are the row's findings and the state below them - is not. + be said. Each of these is a member the family's own gate would fail, + which is why they are the row's findings and the state below them is + not. A MEMBER AT ITS PIN BUT ON A BRANCH IS NOT ONE OF THEM, deliberately; the branch in `check_members` that records it says at length why the standard @@ -5743,6 +5926,12 @@ def member_problem(entry: dict) -> str | None: if not entry["populated"]: return (f"{entry['path']}: not populated -- the pinned copy is an " "unbootstrapped submodule") + if not entry.get("own_git", True): + return (f"{entry['path']}: populated, but with no .git of its own, so " + "git cannot read a HEAD in it") + if entry.get("unborn"): + return (f"{entry['path']}: HEAD names the branch {entry['unborn']}, " + "which has no commit") if entry["pin"] is None: return (f"{entry['project']}: the row carries no 40-hex " "`pin.commit` to compare against") @@ -5792,22 +5981,31 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, if strangers: reason += ("; and, though it is not what fails this row: " + "; ".join(strangers)) - # THE NEXT IS CHOSEN BY WHAT IS WRONG, as `check_legs`' is, and the - # members that a CHECKOUT cures win over the ones none does (#276). - # An empty mount, or a HEAD that is not the pin where the holder's - # GITLINK is, is cured by fetching and checking out. No checkout - # cures a member the holder records no gitlink for (the `family` row - # says `member-gitlink-absent`, and the git line dies on `pathspec - # did not match`), a gitlink that is not the pin (a checkout lands on - # the gitlink, so HEAD would still not be the pin, `member-gitlink- - # mismatch`), a row with no 40-hex `pin.commit`, or a mount git - # cannot read a HEAD in (a populated one with no `.git` of its own - # answers with the HOLDER's HEAD, `member_state`). + # THE NEXT IS CHOSEN BY WHAT IS WRONG, as `check_legs`' is and BY ITS + # RULES (`checkout_reach`, #414), and the members that a CHECKOUT + # cures win over the ones none does (#276). An empty mount, one with + # files but no `.git` of its own whose repository the holder kept, + # or a HEAD that is not the pin where the holder's GITLINK is, is + # cured by fetching and checking out. No checkout cures a member the + # holder records no gitlink for (the `family` row says + # `member-gitlink-absent`, and the git line dies on `pathspec did not + # match`), a gitlink that is not the pin (a checkout lands on the + # gitlink, so HEAD would still not be the pin, `member-gitlink- + # mismatch`), a row with no 40-hex `pin.commit`, a mount git cannot + # read a HEAD in, or one with files, no `.git` of its own and no + # kept repository. And no line cures, without choosing for the + # person, an unborn HEAD or a member `.gitmodules` gives no url for. + # A member reaches each of #414's four states, as a leg does. checkout: list[str] = [] off_pin: list[Path] = [] kept: list[tuple] = [] + relinked: list[str] = [] + gone: list[str] = [] + stuck: list[str] = [] moved: list[Path] = [] lockstep = False + rerun = False + no_url = False nested = False for entry in rows: rel = entry["path"] @@ -5817,37 +6015,61 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, # update names no path, so a member detached off its gitlink is # moved whatever this row says of it, and is named for it. if (gitlink is not None and entry["populated"] - and (mount / ".git").exists() + and entry.get("own_git", True) and left_in_reflog(entry["detached"], entry["head"], gitlink)): moved.append(mount) if member_problem(entry) is None: continue + own_git = entry.get("own_git", True) if gitlink is None: lockstep = True - elif not entry["populated"]: - checkout.append(rel) - keep, repository = emptied_reach(ctx.root, rel, gitlink) - nested = nested or repository - if keep is not None: - kept.append(keep) - elif (entry["pin"] is None or entry["head"] is None - or not (mount / ".git").exists()): - lockstep = True - elif gitlink == entry["pin"]: - checkout.append(rel) - nested = True - if left_in_reflog(entry["detached"], entry["head"], gitlink): - off_pin.append(mount) - else: + continue + if entry["populated"] and own_git: + if entry.get("unborn"): + stuck.append(unborn_sentence(rel, "member")) + continue + if (entry["pin"] is None or entry["head"] is None + or gitlink != entry["pin"]): + lockstep = True + continue + route, keep, repository = checkout_reach( + ctx.root, rel, gitlink, entry["populated"], own_git, + entry["head"]) + if route == "no-url": + stuck.append(no_url_sentence(rel)) + no_url = True + continue + if route == "gone": lockstep = True + gone.append(rel) + continue + checkout.append(rel) + nested = nested or repository + if route == "relink": + relinked.append(rel) + if isinstance(keep, tuple): + kept.append(keep) + elif keep is not None: + off_pin.append(keep) + # Its own lockstep fact (#414), as `check_legs` asks it. + if entry["pin"] is None or gitlink != entry["pin"]: + rerun = True + rerun = rerun or lockstep or bool(stuck) # THE HOLDER'S OWN BOOTSTRAP STAYS THE NEXT WHEN IT CAN RUN: it does # what the plain git line does AND carries the credential rewrite a # PRIVATE member needs (`fetch_members`), so replacing it would # trade a missing script for a fetch that fails on a private member. # Each branch is the COMMAND and, apart from it, the sentence that # says why it is that command (#277). - if checkout and own_bootstrap(ctx.root): + # + # BUT NOT OVER A MEMBER WITH NO URL (#414). Its update names no path, + # so it inits every member, and git stops that whole run at a section + # that gives a path and no url (`No url found`, exit 128, measured) + # before it fetches any other; the bootstrap reports the fetch + # incomplete and exits 0 with the row as red as it was. The git line + # names only the members it cures, and git runs it. + if checkout and own_bootstrap(ctx.root) and not no_url: next_command = ( f"{PYTHON} " f"{quote_arg(ctx.root / 'scripts' / 'bootstrap.py')} " @@ -5858,19 +6080,34 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, why = ("what `make bootstrap` runs in a holder: it fetches every " "member at its pin and leaves it detached there" + checkout_notes( - "member", moved, kept, + "member", moved, kept, relinked, nested or any(entry["populated"] for entry in rows))) + if rerun: + why += rerun_sentence("member") elif checkout: - after = (". This holder has no scripts/bootstrap.py that can " - "run, so this is the plain git line, with no " - "credential rewrite for a private member") + if own_bootstrap(ctx.root): + after = (". This holder has its own scripts/bootstrap.py, " + "but it updates every member at once, and a member " + "with no url in .gitmodules can stop that run before " + "it fetches any other, so this is the plain git line " + "over the rest, with no credential rewrite for a " + "private member") + else: + after = (". This holder has no scripts/bootstrap.py that can " + "run, so this is the plain git line, with no " + "credential rewrite for a private member") next_command, why = submodule_update_next( - ctx.root, checkout, off_pin, "member", after, lockstep, - kept=kept, nested=nested) - else: + ctx.root, checkout, off_pin, "member", after, rerun, + kept=kept, relinked=relinked, nested=nested) + elif lockstep: # THE FAMILY GATE, one definition of it for this row and the # `family` row (`lockstep_next`). next_command, why = lockstep_next(ctx, FAMILY_GATE, "member") + if why: + why += "".join(gone_sentence(rel) for rel in gone) + else: + # NO LINE CURES IT without choosing for the person (#414). + next_command, why = None, ". ".join(stuck) return Row("members", "members", FINDING, reason, next_command, detail, why=why) note = f"{len(rows)} member(s) mounted at their pins" diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 53123adf..bad47349 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -7,8 +7,9 @@ the file (the `shape currency` row says `copy-missing` for it) pasted `can't open file`, exit 2. And where the file was there it did not cure the row: bootstrap never initialises an empty mount (it prints `NOT INITIALIZED` and -stops) and never moves an existing branch whose tip is not the pin, so the -re-run doctor said the same thing. +stops), and it moves a tracking branch whose tip is not the pin only where +that loses nothing, behind the pin or ahead of it with no commit of its own +(#475, #490), so the re-run doctor said the same thing. WHAT THE LINE IS NOW is `git -C submodule update --init --recursive --checkout -- `, which needs no script and no `PYTHONPATH`. A lockstep @@ -1302,3 +1303,347 @@ def test_413c_without_it_the_members_leg_commit_is_in_no_ref( cwd=leg).stdout.strip() == "" assert commit in git("reflog", cwd=leg).stdout + +# --- #414: four states the routing did not fit ----------------------------- +# +# Each NEXT, pasted, turns the row `ok`, or the NEXT is None and the `why` or +# the reason names the state. A member reaches all four, so each is asked of +# the `members` row too. + +def root_head(root: Path) -> str: + return git("rev-parse", "HEAD", cwd=root).stdout.strip() + + +def test_414_1_a_mount_with_no_git_whose_repository_is_kept_gets_the_line( + standard, project): + """(1) The repository git kept is what `submodule update` routes on: it + links the mount to it again. The row reads no HEAD in the mount, where + git walks up and answered with the ROOT's, and quoted it as the leg's.""" + head = root_head(project) + (project / "spec" / ".git").unlink() + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert row["status"] == "FINDING", row + assert head[:12] not in row["reason"], row["reason"] + assert "no .git of its own" in row["reason"], row["reason"] + [spec] = [leg for leg in row["detail"]["legs"] if leg["path"] == "spec"] + assert spec["head"] is None, spec + assert command_of(row) == ( + f"git -C {module.quote_arg(project.resolve())} {UPDATE} -- spec" + ), row["next"] + assert "git kept its repository" in why_of(row), why_of(row) + assert "links the two again" in why_of(row), why_of(row) + + +@WINDOWS_SKIP +def test_414_1_pasted_the_line_links_the_mount_again_and_the_row_is_ok( + standard, project, tmp_path): + (project / "spec" / ".git").unlink() + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + after = row_of(standard, project, "legs") + assert after["status"] == "ok", (after, result.stdout, result.stderr) + + +def test_414_1_with_the_repository_gone_too_the_gate_says_which(standard, + project): + """The other half: no kept repository, so no update links anything. The + lockstep gate stays the line, its `why` says why, and no HEAD is + quoted.""" + head = root_head(project) + make_mount_with_no_git_of_its_own(project) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert command_of(row) == pins_gate(module, standard, project, True), \ + row["next"] + assert head[:12] not in row["reason"], row["reason"] + assert "git kept no repository for it" in why_of(row), why_of(row) + + +def test_414_1_a_member_with_no_git_whose_repository_is_kept_is_a_checkout( + standard, holder, tmp_path): + """A member reaches (1): the holder's own bootstrap runs the update that + links it again, and the row quotes no HEAD for it, the holder's least + of all.""" + root = holder_copy(holder, tmp_path) + head = root_head(root) + (root / "members" / FAMILY_MEMBER / ".git").unlink() + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert head[:12] not in row["reason"], row["reason"] + assert row["detail"]["members"][0]["head"] is None, row["detail"] + assert "bootstrap.py" in command_of(row), row["next"] + assert "links the two again" in why_of(row), why_of(row) + + +@WINDOWS_SKIP +def test_414_1_pasted_the_holders_bootstrap_links_the_member_again( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + (root / "members" / FAMILY_MEMBER / ".git").unlink() + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + after = row_of(standard, root, "members") + assert after["status"] == "ok", (after, result.stdout, result.stderr) + + +def test_414_2_an_unborn_head_in_a_leg_gets_no_line_and_a_why_naming_it( + standard, project): + """(2) An unborn HEAD is no lockstep fact: the gate says `pins ok` and + the row stays red. No line cures it without choosing a commit for the + person, so there is none, and the `why` says what the state is.""" + git("symbolic-ref", "HEAD", "refs/heads/never-made", cwd=project / "spec") + row = row_of(standard, project, "legs") + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert "never-made" in row["reason"], row["reason"] + assert "no commit" in row["reason"], row["reason"] + assert "names a branch with no commit" in why_of(row), why_of(row) + assert "chooses" in why_of(row), why_of(row) + assert "validate-pins" not in why_of(row), why_of(row) + + +def test_414_2_an_unborn_head_in_a_member_gets_no_line_either( + standard, holder, tmp_path): + """And in a member, though the holder HAS its own bootstrap: the + update it runs cannot find a current revision either.""" + root = holder_copy(holder, tmp_path) + git("symbolic-ref", "HEAD", "refs/heads/never-made", + cwd=root / "members" / FAMILY_MEMBER) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert "never-made" in row["reason"], row["reason"] + assert "names a branch with no commit" in why_of(row), why_of(row) + + +def without_a_url(root: Path, mount: str, how: str) -> None: + """An emptied mount whose `.gitmodules` section is gone, or keeps its + path and has lost its `url`: `git submodule update --init -- ` + exits 128 on both (`No url found`).""" + git("submodule", "deinit", "-q", "-f", "--", mount, cwd=root) + if how == "section": + git("config", "-f", ".gitmodules", "--remove-section", + f"submodule.{mount}", cwd=root) + else: + git("config", "-f", ".gitmodules", "--unset", + f"submodule.{mount}.url", cwd=root) + + +@pytest.mark.parametrize("how", ["section", "url"]) +def test_414_3_a_leg_gitmodules_gives_no_url_for_gets_no_line( + standard, project, how): + """(3) The git line would exit 128, and the gate names no `.gitmodules` + entry (the next test pins both), so the NEXT is None and the `why` + names the missing entry and who restores it. No command: one that + restored `.gitmodules` from HEAD would discard an uncommitted edit.""" + without_a_url(project, "spec", how) + row = row_of(standard, project, "legs") + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert "no url for it in .gitmodules" in row["reason"], row["reason"] + why = why_of(row) + assert "no url for spec in .gitmodules" in why, why + assert "history" in why and "person" in why, why + + +def test_414_3_a_populated_leg_off_its_pin_with_no_url_gets_no_line_either( + standard, project): + """`--init` asks for the url of every path it is given, populated or + not, so a leg the line would check out is held to it too.""" + make_detached_past_the_pin(project) + git("config", "-f", ".gitmodules", "--remove-section", "submodule.spec", + cwd=project) + row = row_of(standard, project, "legs") + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert "no url for spec in .gitmodules" in why_of(row), why_of(row) + + +@WINDOWS_SKIP +def test_414_3_neither_the_git_line_nor_the_gate_cures_a_leg_with_no_url( + standard, project, tmp_path): + """WHY THE NEXT IS NONE AND NOT THE GATE, pinned by what both do, so + that the day either changes this says so: the git line exits 128 on `No + url found`, and the gate refuses `pin-submodule-uninitialized` and + names no `.gitmodules` entry at all.""" + without_a_url(project, "spec", "section") + module = doctor_module(standard) + line, _ = module.submodule_update_next(project.resolve(), ["spec"], [], + "leg") + result = sh(line, tmp_path / "elsewhere") + assert result.returncode == 128, result.stdout + result.stderr + assert "No url found" in result.stderr, result.stderr + gate = sh(pins_gate(module, standard, project, True), + tmp_path / "elsewhere") + output = gate.stdout + gate.stderr + assert gate.returncode != 0, output + assert "pin-submodule-uninitialized" in output, output + assert ".gitmodules" not in output, output + + +def test_414_3_a_gitmodules_deleted_from_the_working_tree_alone_is_read( + standard, project): + """git reads `.gitmodules` from the index when the working tree has + none, and so does `gitmodules_entry`: the line is still handed over.""" + unbootstrap(project, "spec") + (project / ".gitmodules").unlink() + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert command_of(row) == ( + f"git -C {module.quote_arg(project.resolve())} {UPDATE} -- spec" + ), row["next"] + + +@WINDOWS_SKIP +def test_414_3_pasted_there_the_line_runs_and_the_row_is_ok( + standard, project, tmp_path): + unbootstrap(project, "spec") + (project / ".gitmodules").unlink() + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + + +def test_414_3_a_member_gitmodules_gives_no_url_for_gets_no_line( + standard, holder, tmp_path): + """A member reaches (3), and the holder's own bootstrap is no line for + it: it exits 0 with the fetch incomplete and the row as red as it was, + and the family gate says `member-uninitialized`.""" + root = holder_copy(holder, tmp_path) + without_a_url(root, f"members/{FAMILY_MEMBER}", "section") + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert row["next"] is None, row + assert f"no url for members/{FAMILY_MEMBER} in .gitmodules" in \ + why_of(row), why_of(row) + + +def with_a_member_that_has_no_url(root: Path) -> None: + """A second member, `Ghost`, at the first one's commit, whose + `.gitmodules` section gives its path and NO url; and the first member + emptied, so a checkout cures it. The holder's own bootstrap updates + every member with no path named, and git stops that run at Ghost.""" + manifest = root / "family.yaml" + text = manifest.read_text(encoding="utf-8") + first = text[text.index(f" - project: {FAMILY_MEMBER}\n"):] + ghost = first + for was, now in ((f"project: {FAMILY_MEMBER}", "project: Ghost"), + (f"id: {FAMILY_MEMBER.lower()}", "id: ghost"), + (f"{FAMILY_NAME}/{FAMILY_MEMBER}", + f"{FAMILY_NAME}/Ghost"), + (f"members/{FAMILY_MEMBER}", "members/Ghost")): + assert ghost.count(was) == 1, (was, ghost) + ghost = ghost.replace(was, now) + with open(manifest, "w", encoding="utf-8", newline="\n") as handle: + handle.write(text.rstrip("\n") + "\n" + ghost) + record_gitlink(root, "members/Ghost", + git("rev-parse", f"HEAD:members/{FAMILY_MEMBER}", + cwd=root).stdout.strip()) + git("config", "-f", ".gitmodules", "submodule.members/Ghost.path", + "members/Ghost", cwd=root) + (root / "members" / "Ghost").mkdir() + member_unbootstrapped(root) + + +def test_414_3_a_member_with_no_url_takes_the_holders_bootstrap_off_the_line( + standard, holder, tmp_path): + """With a member the update cannot fetch beside one a checkout cures, + the holder's own bootstrap would be stopped at the first (the next test + pins it), so the line is the git line over the second alone, and says + why it is not the bootstrap.""" + root = holder_copy(holder, tmp_path) + with_a_member_that_has_no_url(root) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert command_of(row) == ( + f"git -C {module.quote_arg(root.resolve())} {UPDATE} -- " + f"members/{FAMILY_MEMBER}"), row["next"] + why = why_of(row) + assert "can stop that run" in why, why + assert "doctor again" in why, why + + +@WINDOWS_SKIP +def test_414_3_there_the_bootstrap_cures_nothing_and_the_git_line_cures_one( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + with_a_member_that_has_no_url(root) + module = doctor_module(standard) + tried = tmp_path / "tried" + shutil.copytree(root, tried, symlinks=True) + bootstrap = sh(f"{module.PYTHON} " + f"{module.quote_arg(tried / 'scripts' / 'bootstrap.py')} " + f"--root {module.quote_arg(tried)}", tmp_path / "boot") + output = bootstrap.stdout + bootstrap.stderr + assert bootstrap.returncode == 0, output + assert "No url found" in output, output + assert not any((tried / "members" / FAMILY_MEMBER).iterdir()), output + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + again = row_of(standard, root, "members") + assert again["next"] is None, again + assert "no url for members/Ghost" in why_of(again), why_of(again) + + +def make_gitlink_not_the_pin_in_an_emptied_mount(root: Path) -> None: + """The gitlink moved to a commit made in the leg, the pin left, and the + mount emptied with its repository kept, so the update can check the + gitlink out: HEAD is then the gitlink and not the pin.""" + record_gitlink(root, "spec", commit_in(root / "spec")) + git("submodule", "deinit", "-q", "-f", "--", "spec", cwd=root) + + +def make_pin_not_a_commit_in_an_emptied_mount(root: Path) -> None: + make_pin_not_a_commit(root) + unbootstrap(root, "spec") + + +@pytest.mark.parametrize("make", [make_pin_not_a_commit_in_an_emptied_mount, + make_gitlink_not_the_pin_in_an_emptied_mount + ], ids=["pin-not-40-hex", + "gitlink-not-the-pin"]) +def test_414_4_a_leg_the_line_checks_out_with_a_lockstep_fact_says_run_again( + standard, project, make): + """(4) The re-run sentence is asked per leg: the line checks the gitlink + out, and a pin that is not 40 hex, or not the gitlink, is what the next + run names. It was said only when ANOTHER leg was a lockstep one.""" + make(project) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert command_of(row) == ( + f"git -C {module.quote_arg(project.resolve())} {UPDATE} -- spec" + ), row["next"] + assert "doctor again" in why_of(row), why_of(row) + + +@WINDOWS_SKIP +def test_414_4_pasted_the_line_runs_and_the_rerun_names_the_pin( + standard, project, tmp_path): + make_pin_not_a_commit_in_an_emptied_mount(project) + module = doctor_module(standard) + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + again = row_of(standard, project, "legs") + assert again["status"] == "FINDING", again + assert "40-hex" in again["reason"], again["reason"] + assert command_of(again) == pins_gate(module, standard, project, True), \ + again["next"] + + +@pytest.mark.parametrize("bootstrap", ["with-its-own-bootstrap", + "without-it"]) +def test_414_4_a_member_the_line_checks_out_with_no_pin_says_run_again( + standard, holder, tmp_path, bootstrap): + """And a member: the holder's own bootstrap says it too.""" + root = holder_copy(holder, tmp_path) + make_member_pin_not_a_commit(root) + if bootstrap == "without-it": + holder_without(root, "bootstrap.py") + member_unbootstrapped(root) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert (" submodule update " in command_of(row)) == ( + bootstrap == "without-it"), row["next"] + assert "doctor again" in why_of(row), why_of(row) From aa63ff6889b7b1d330baaff4cafe75902602fd55 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 18:49:26 +0000 Subject: [PATCH 3/4] shape-doctor.py: a mount whose `.gitmodules` section lost only its `url` keeps the git line while the root's own config still holds one, a leg's own submodules are named in general words as a member's legs are, the lockstep gate's `why` carries the states no line cures, and tests pin `detached`'s None and every member the holder's bootstrap moves (#414, #413) Fix round 1 for PR #559, after the review R559 at 83b38657. The url check asked `.gitmodules` alone, and that was a regression from main. `git submodule init` copies a mount's url into the root's config, and `--init` reads it there first. So an INITIALIZED mount whose section kept its `path` and lost its `url` key still updates: measured, the git line exits 0 and cures the row, whether the leg is populated past its pin or emptied by `rm`, and the same holds for a member under the holder's own bootstrap. The row gave NEXT None and a `why` saying the line would exit 128. A mount is now `no-url` only when no section gives its path, or the section has no url and `git config --get submodule..url` in the root finds none (`configured_url`). `deinit` drops the configured url and a missing section leaves git no name to look one up by, so both still get NEXT None. The reason and the `why` say "in .gitmodules, nor in the config of this repository". The `legs` line's `--recursive` checks out the submodules a leg carries of its own, and a commit made on one's detached HEAD was left in its reflog unannounced (R559's p3). A leg the line names that carries a `.gitmodules`, or whose kept repository keeps `modules/`, now gets the general sentence a member's legs get (#413 c), with the placeholder keep line `git -C LEG/SUB branch NAME`. The row walks into none of them. `checkout_reach`'s third answer is now that "carries submodules" fact, for both rows. Where the lockstep gate is the NEXT, an unborn HEAD or a mount with no url beside it is said in the gate's `why` with the same sentence it gets when no line is printed, and that sentence ends by saying to run the doctor again. The gate itself cures nothing, so no separate re-run sentence is added. `checkout_notes` and `submodule_update_next` no longer say that the sentence carries none of `;`, `'`, a backtick or a parenthesis. That rule bound a ` # ` tail pasted with the line, and since #277 the sentence is the row's `why`, on a line of its own. New tests at the end of tests/test_doctor_legs_members_next.py: - the url the root's config keeps, for a leg past its pin and one emptied by `rm`, and for a member: the line is offered, and pasted it cures the row; - a leg carrying submodules gets the sentence and a leg without one does not, and with a real nested submodule the placeholder keep line, filled in and run first, keeps the commit, while without it no ref holds it; - a second member the row hands to the gate but the holder's bootstrap moves is named by its keep line, and pasting proves the move; - `detached` answers None where `symbolic-ref` fails, and a member whose gitdir is gone is not called detached in `--json`; - an unborn leg beside a lockstep one is said in the gate's `why`. `add_member_row` is factored out of the url-less member's helper. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 184 +++++++++++----- tests/test_doctor_legs_members_next.py | 284 +++++++++++++++++++++++-- 2 files changed, 397 insertions(+), 71 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index daf0de15..bd487bdc 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3443,8 +3443,10 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: Measured for an empty mount whose section is gone and for one that lost only its `url` key, and for a populated mount whose section is gone: a mount git already holds is asked for its url all the same. A NEXT that - exits 128 is the defect #276 was about, so a mount this answers no url - for is never put on that line. + exits 128 is the defect #276 was about. A section that gives the path + and no url is not the end of the question, though: `--init` reads the + url the root's own config already holds first (`configured_url`), and + `checkout_reach` asks that before it calls a mount url-less. READ WHERE GIT READS IT, WHICH IS NOT ALWAYS THE FILE. git reads the working tree's `.gitmodules` when there is one, else the copy in the @@ -3493,6 +3495,34 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: return None, None +def configured_url(root: Path, name: str) -> str | None: + """The url the root's own config gives the submodule NAMED `name`, or + None (#414). + + `git submodule init` copies a mount's url out of `.gitmodules` into the + root's config, and from then on `--init` reads it there first and asks + `.gitmodules` only when the config has none. So an initialized mount + whose `.gitmodules` section kept its `path` and lost its `url` key still + updates: measured, populated past its pin or emptied by `rm`, the git + line exits 0 and the row is `ok`. Calling that mount url-less withheld a + line that cures it. `deinit` removes the configured url, and a section + that is gone leaves git no NAME to look one up by (`No url found`, exit + 128, measured), so neither is helped by this. + + `git config --get` in the root, which reads what git reads: the + repository's config and the ones above it. Read only. + """ + try: + proc = subprocess.run(["git", "config", "--get", + f"submodule.{name}.url"], + cwd=str(root), capture_output=True, check=False) + except OSError: + return None + if proc.returncode != 0: + return None + return proc.stdout.decode("utf-8", "replace").strip() or None + + def kept_gitdir(root: Path, name: str) -> Path | None: """The repository git keeps in `root` for the submodule NAMED `name`, or None when it keeps none (#413, #414). @@ -3590,10 +3620,12 @@ def unborn_branch(path: Path) -> str | None: def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, own_git: bool, head: str | None) -> tuple[str, object, bool]: - """`(route, keep, repository)` for a mount a CHECKOUT would cure, as the + """`(route, keep, nested)` for a mount a CHECKOUT would cure, as the `legs` and `members` rows both read it, so the two rows route by one - definition (#414). `repository` is whether git has one for the mount, - its own or a kept one: a member's own legs live in it (#413 c). + definition (#414). `nested` is whether the mount carries submodules of + its own that the line's `--recursive` checks out too: a `.gitmodules` in + it, or a kept repository that keeps theirs (`modules/`). A member's legs + are those (#413 c), and so is anything a leg mounts. `route` is: @@ -3605,8 +3637,10 @@ def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, links anything: it tries a clone into a directory that is not empty and exits 1 (`destination path ... already exists`, measured), so it is the lockstep gate's. - - `no-url`: `.gitmodules` gives no url for it, so the git line would exit - 128 (`gitmodules_entry`, #414 3). No line is printed for it. + - `no-url`: no `.gitmodules` section gives its path, or the one that does + gives no url and the root's own config has none either + (`gitmodules_entry`, `configured_url`, #414 3), so the git line would + exit 128. No line is printed for it. `keep` is what the line would leave in a reflog alone (`left_in_reflog`): the mount itself, for `git -C branch NAME`, or `(repository, @@ -3621,11 +3655,14 @@ def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, """ mount = root / rel name, url = gitmodules_entry(root, rel) + if name is not None and url is None: + url = configured_url(root, name) if url is None: return "no-url", None, False + carries = populated and (mount / ".gitmodules").is_file() if own_git: - return "checkout", (mount if left_in_reflog(detached(mount), head, - gitlink) else None), True + stranded = left_in_reflog(detached(mount), head, gitlink) + return "checkout", (mount if stranded else None), carries git_dir = kept_gitdir(root, name) if git_dir is None: return ("gone" if populated else "checkout"), None, False @@ -3633,7 +3670,8 @@ def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, keep = None if left_in_reflog(is_detached, kept_at, gitlink): keep = (git_dir, kept_at, None if mount.is_dir() else root) - return ("relink" if populated else "checkout"), keep, True + return (("relink" if populated else "checkout"), keep, + carries or (git_dir / "modules").is_dir()) def kept_branch_line(kept: tuple, platform: str) -> str: @@ -3675,20 +3713,21 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), - `kept`: `(repository, commit, work tree)` for mounts with no `.git` of their own whose kept repository is detached off it (#413 a), each kept first with `kept_branch_line`, which names the commit as well. - - `nested`, for the `members` row: the line is recursive, so it checks - out each member's own legs too, at the commits THE MEMBER records, and - a commit made on no branch in one of them is left in its reflog the - same way (#413 c). Said in general words, without walking into them, - and only where a member it reaches has a repository for them to be in. - `--recursive` stays: the holder's bootstrap recurses - (`fetch_members`), and a git line that did not would leave a member's - legs where a bootstrap would not. + - `nested`: the line is recursive, so it checks out the submodules a + mount it names carries of its own too, at the commits THAT MOUNT + records, and a commit made on no branch in one of them is left in its + reflog the same way. For the `members` row those are each member's + legs (#413 c); for the `legs` row, whatever a leg mounts. Said in + general words, with a placeholder keep line, without walking into + them, and only where a mount the line names carries some + (`checkout_reach`). `--recursive` stays: the holder's bootstrap + recurses (`fetch_members`), and a git line that did not would leave a + member's legs where a bootstrap would not. A commit made in a mount on a BRANCH stays on that branch, and nothing is said where no mount is in any of these states, because a reader told about a loss that cannot happen learns to skip the sentence on the day - it can. None of `;`, `'`, a backtick or a parenthesis is written here - (`submodule_update_next` says why). + it can. """ target = os.name if platform is None else platform notes = "".join(f". {rel} has files but no .git of its own, and git kept " @@ -3705,12 +3744,19 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), notes += (". A mount with no .git of its own is named by the " "repository git kept for it, because git run inside such a " "mount walks up and acts on this repository") - if nested: + if nested and noun == "member": notes += (". It is recursive, so it also checks out the legs of each " "member at the commits that member records, and a commit " "made on no branch in one of those legs is left only in its " "reflog too. To keep one, run first git -C LEG branch NAME, " "with LEG the directory of that leg") + elif nested: + notes += (f". It is recursive, so it also checks out the submodules " + f"a {noun} carries of its own, at the commits that {noun} " + "records, and a commit made on no branch in one of them is " + "left only in its reflog too. To keep one, run first git -C " + "LEG/SUB branch NAME, with LEG/SUB the directory of that " + "submodule") return notes @@ -3725,9 +3771,10 @@ def rerun_sentence(noun: str) -> str: def unborn_sentence(rel: str, noun: str) -> str: - """The `why` for an unborn HEAD, the NEXT being None (#414 2): it names - the state and says that nothing here picks the commit. Not the lockstep - gate, which says `pins ok` and changes nothing.""" + """The `why` for an unborn HEAD, the NEXT being None (#414 2) or the + lockstep gate: it names the state and says that nothing here picks the + commit. Never the gate's own line for it, which says `pins ok` and + changes nothing.""" return (f"HEAD in {rel} names a branch with no commit, so there is no " "commit to compare with the pin, and nothing here chooses one " f"for you. Check out in that {noun} the commit you mean, then run " @@ -3735,14 +3782,16 @@ def unborn_sentence(rel: str, noun: str) -> str: def no_url_sentence(rel: str) -> str: - """The `why` for a mount `.gitmodules` gives no url for, the NEXT being - None (#414 3). No command, deliberately: the entry is restored from the + """The `why` for a mount git finds no url for, in `.gitmodules` or in the + root's own config, the NEXT being None (#414 3) or the lockstep gate. + No command, deliberately: the entry is restored from the repository's history by a person, and a `git checkout -- .gitmodules` would discard an edit nobody has committed yet.""" - return (f"git finds no url for {rel} in .gitmodules, so no submodule " - "update can fetch it or check it out, and the git line would exit " - "128. A person restores that entry from the history of this " - "repository, then runs the doctor again") + return (f"git finds no url for {rel} in .gitmodules, nor in the config " + "of this repository, so no submodule update can fetch it or check " + "it out, and the git line would exit 128. A person restores that " + "entry from the history of this repository, then runs the doctor " + "again") def gone_sentence(rel: str) -> str: @@ -3801,15 +3850,15 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], repositories git kept for mounts with no `.git` of their own whose HEAD is (#413 a). `relinked` and `nested` are its too. - THE SENTENCE CARRIES NONE OF `;`, `'`, A BACKTICK OR A PARENTHESIS. It - was written as a ` # ` tail, and a tail is pasted with the line: a - shell that does not treat `#` as a comment reads each of those as - syntax. It was plain words so that moving it out of the NEXT (#277) - would be a move and not a rewrite, and it was: it is the row's `why`, - on a line of its own. `after` is the caller's own closing sentence, and - `rerun` adds the one the `legs` and `members` rows share - (`rerun_sentence`): something this line leaves wrong, in another leg or - member or in one it checks out, is named by the next run. + THE SENTENCE IS THE ROW'S `why`, ON A LINE OF ITS OWN (#277). It was a + ` # ` tail, pasted with the line, and so was written with none of `;`, + `'`, a backtick or a parenthesis, each of which a shell that does not + read `#` as a comment takes as syntax. Off the line nothing in it is + pasted, so that rule no longer binds it, and the paths in its keep lines + are spelled by `quote_arg`, which may quote one. `after` is the caller's + own closing sentence, and `rerun` adds the one the `legs` and `members` + rows share (`rerun_sentence`): something this line leaves wrong, in + another leg or member or in one it checks out, is named by the next run. `platform` is `quote_arg`'s and `copy_command`'s, and for the same reason: the line is spelled for the reader's shell, not the host's. @@ -3894,7 +3943,13 @@ def check_legs(ctx: Context) -> Row: WHAT THE LINE WOULD LEAVE IN A REFLOG IS NAMED, wherever it is (#413): in a populated leg detached off the gitlink, and in the repository git kept for a leg with no `.git` of its own, read through `--git-dir` and - kept with a line that names that repository (`checkout_notes`). + kept with a line that names that repository (`checkout_notes`). A leg + that carries submodules of its own is said in general words: the line's + `--recursive` checks those out too, and the row does not walk into them. + + WHERE THE GATE IS THE LINE, the legs no line cures are said beside it: + an unborn HEAD or a leg with no url gets the same sentence it gets with + no line at all, which ends by saying to run the doctor again. """ waiting = waiting_row(ctx, "legs", "legs", "the legs it declares are not known and none is " @@ -3917,6 +3972,7 @@ def check_legs(ctx: Context) -> Row: gone: list[str] = [] stuck: list[str] = [] rerun = False + nested = False for leg in legs: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) @@ -3965,16 +4021,18 @@ def check_legs(ctx: Context) -> Row: else: lockstep.append(rel) if cures: - route, keep, _ = checkout_reach(ctx.root, rel, gitlink, - populated, own_git, head) + route, keep, carries = checkout_reach(ctx.root, rel, gitlink, + populated, own_git, head) if route == "no-url": - problem += ", and git finds no url for it in .gitmodules" + problem += (", and git finds no url for it in .gitmodules or " + "in the config of this repository") stuck.append(no_url_sentence(rel)) elif route == "gone": lockstep.append(rel) gone.append(rel) else: checkout.append(rel) + nested = nested or carries if route == "relink": relinked.append(rel) if isinstance(keep, tuple): @@ -4007,15 +4065,18 @@ def check_legs(ctx: Context) -> Row: # line of its own (#277). next_command, why = submodule_update_next( ctx.root, checkout, off_pin, "leg", after, rerun, kept=kept, - relinked=relinked) + relinked=relinked, nested=nested) elif lockstep: # NO CHECKOUT CURES IT: the lockstep gate, one definition of it - # for this row and the `pins` row (`lockstep_next`), and which - # mount with no `.git` of its own is the gate's because git kept - # no repository for it (#414). + # for this row and the `pins` row (`lockstep_next`), which mount + # with no `.git` of its own is the gate's because git kept no + # repository for it, and what no line cures (#414). next_command, why = lockstep_next(ctx, PINS_GATE, "leg") if why: why += "".join(gone_sentence(rel) for rel in gone) + why += "".join(f". {sentence}" for sentence in stuck) + elif stuck: + why = ". ".join(stuck) else: # NO LINE CURES IT without choosing for the person (#414). next_command, why = None, ". ".join(stuck) @@ -6033,7 +6094,7 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, or gitlink != entry["pin"]): lockstep = True continue - route, keep, repository = checkout_reach( + route, keep, carries = checkout_reach( ctx.root, rel, gitlink, entry["populated"], own_git, entry["head"]) if route == "no-url": @@ -6045,7 +6106,7 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, gone.append(rel) continue checkout.append(rel) - nested = nested or repository + nested = nested or carries if route == "relink": relinked.append(rel) if isinstance(keep, tuple): @@ -6065,10 +6126,11 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, # # BUT NOT OVER A MEMBER WITH NO URL (#414). Its update names no path, # so it inits every member, and git stops that whole run at a section - # that gives a path and no url (`No url found`, exit 128, measured) - # before it fetches any other; the bootstrap reports the fetch - # incomplete and exits 0 with the row as red as it was. The git line - # names only the members it cures, and git runs it. + # that gives a path and no url, where the holder's own config has + # none either (`No url found`, exit 128, measured), before it fetches + # any other; the bootstrap reports the fetch incomplete and exits 0 + # with the row as red as it was. The git line names only the members + # it cures, and git runs it. if checkout and own_bootstrap(ctx.root) and not no_url: next_command = ( f"{PYTHON} " @@ -6076,20 +6138,22 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, f"--root {quote_arg(ctx.root)}") # The same sentences the git line carries, built by the same # function (#413 b), for every member it moves. Its update is - # recursive too, into every member that is there. + # recursive too, into every member that carries legs. why = ("what `make bootstrap` runs in a holder: it fetches every " "member at its pin and leaves it detached there" + checkout_notes( "member", moved, kept, relinked, - nested or any(entry["populated"] for entry in rows))) + nested or any((ctx.root / entry["path"] + / ".gitmodules").is_file() + for entry in rows))) if rerun: why += rerun_sentence("member") elif checkout: if own_bootstrap(ctx.root): after = (". This holder has its own scripts/bootstrap.py, " "but it updates every member at once, and a member " - "with no url in .gitmodules can stop that run before " - "it fetches any other, so this is the plain git line " + "git finds no url for can stop that run before it " + "fetches any other, so this is the plain git line " "over the rest, with no credential rewrite for a " "private member") else: @@ -6101,10 +6165,14 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, kept=kept, relinked=relinked, nested=nested) elif lockstep: # THE FAMILY GATE, one definition of it for this row and the - # `family` row (`lockstep_next`). + # `family` row (`lockstep_next`), and, beside it, what no line + # cures, as `check_legs` says it (#414). next_command, why = lockstep_next(ctx, FAMILY_GATE, "member") if why: why += "".join(gone_sentence(rel) for rel in gone) + why += "".join(f". {sentence}" for sentence in stuck) + elif stuck: + why = ". ".join(stuck) else: # NO LINE CURES IT without choosing for the person (#414). next_command, why = None, ". ".join(stuck) diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index bad47349..0e206733 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -1519,24 +1519,31 @@ def test_414_3_a_member_gitmodules_gives_no_url_for_gets_no_line( why_of(row), why_of(row) -def with_a_member_that_has_no_url(root: Path) -> None: - """A second member, `Ghost`, at the first one's commit, whose - `.gitmodules` section gives its path and NO url; and the first member - emptied, so a checkout cures it. The holder's own bootstrap updates - every member with no path named, and git stops that run at Ghost.""" +def add_member_row(root: Path, project: str) -> None: + """A second row in `family.yaml`, `project` at `members/`, a + copy of the first member's row, pin included.""" manifest = root / "family.yaml" text = manifest.read_text(encoding="utf-8") first = text[text.index(f" - project: {FAMILY_MEMBER}\n"):] - ghost = first - for was, now in ((f"project: {FAMILY_MEMBER}", "project: Ghost"), - (f"id: {FAMILY_MEMBER.lower()}", "id: ghost"), + row = first + for was, now in ((f"project: {FAMILY_MEMBER}", f"project: {project}"), + (f"id: {FAMILY_MEMBER.lower()}", + f"id: {project.lower()}"), (f"{FAMILY_NAME}/{FAMILY_MEMBER}", - f"{FAMILY_NAME}/Ghost"), - (f"members/{FAMILY_MEMBER}", "members/Ghost")): - assert ghost.count(was) == 1, (was, ghost) - ghost = ghost.replace(was, now) + f"{FAMILY_NAME}/{project}"), + (f"members/{FAMILY_MEMBER}", f"members/{project}")): + assert row.count(was) == 1, (was, row) + row = row.replace(was, now) with open(manifest, "w", encoding="utf-8", newline="\n") as handle: - handle.write(text.rstrip("\n") + "\n" + ghost) + handle.write(text.rstrip("\n") + "\n" + row) + + +def with_a_member_that_has_no_url(root: Path) -> None: + """A second member, `Ghost`, at the first one's commit, whose + `.gitmodules` section gives its path and NO url; and the first member + emptied, so a checkout cures it. The holder's own bootstrap updates + every member with no path named, and git stops that run at Ghost.""" + add_member_row(root, "Ghost") record_gitlink(root, "members/Ghost", git("rev-parse", f"HEAD:members/{FAMILY_MEMBER}", cwd=root).stdout.strip()) @@ -1647,3 +1654,254 @@ def test_414_4_a_member_the_line_checks_out_with_no_pin_says_run_again( assert (" submodule update " in command_of(row)) == ( bootstrap == "without-it"), row["next"] assert "doctor again" in why_of(row), why_of(row) + + +# --- after review: a url the root's config keeps, a leg's own submodules, --- +# --- every member the holder's bootstrap moves, and `detached` ------------ + +def make_url_in_config_only(root: Path, how: str) -> None: + """An INITIALIZED leg whose `.gitmodules` section kept its `path` and + lost its `url` key. The root's own config still has the url `submodule + init` copied into it, and `--init` reads that first, so the git line + exits 0 here. Past its pin, or emptied by `rm`, which unlike `deinit` + leaves the configured url where it is.""" + if how == "past-the-pin": + make_detached_past_the_pin(root) + else: + rmtree(root / "spec") + (root / "spec").mkdir() + git("config", "-f", ".gitmodules", "--unset", "submodule.spec.url", + cwd=root) + assert git("config", "--get", "submodule.spec.url", cwd=root).stdout + + +@pytest.mark.parametrize("how", ["past-the-pin", "emptied-by-rm"]) +def test_414_3_a_url_the_roots_config_keeps_still_gets_the_line( + standard, project, how): + """`.gitmodules` alone does not decide that git has no url: only a + section that is gone, or one with no url where the root's config has + none either, makes the line exit 128. Here the line is handed over, as + main handed it over.""" + make_url_in_config_only(project, how) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert row["status"] == "FINDING", row + assert command_of(row) == ( + f"git -C {module.quote_arg(project.resolve())} {UPDATE} -- spec" + ), row["next"] + assert "no url" not in row["reason"] + why_of(row), row + + +@WINDOWS_SKIP +@pytest.mark.parametrize("how", ["past-the-pin", "emptied-by-rm"]) +def test_414_3_pasted_there_the_line_cures_the_row(standard, project, + tmp_path, how): + make_url_in_config_only(project, how) + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + after = row_of(standard, project, "legs") + assert after["status"] == "ok", (after, result.stdout, result.stderr) + + +def member_url_in_config_only(root: Path) -> None: + """The member's case: detached past its pin, its `.gitmodules` section + without a `url` key, and the holder's config still holding one.""" + member = detached_member(root) + commit_in(member) + git("config", "-f", ".gitmodules", "--unset", + f"submodule.members/{FAMILY_MEMBER}.url", cwd=root) + assert git("config", "--get", f"submodule.members/{FAMILY_MEMBER}.url", + cwd=root).stdout + + +def test_414_3_a_member_url_the_holders_config_keeps_still_gets_the_line( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + member_url_in_config_only(root) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert "bootstrap.py" in command_of(row), row["next"] + assert "no url" not in why_of(row), why_of(row) + + +@WINDOWS_SKIP +def test_414_3_pasted_the_holders_bootstrap_cures_that_member( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + member_url_in_config_only(root) + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + after = row_of(standard, root, "members") + assert after["status"] == "ok", (after, result.stdout, result.stderr) + + +def test_a_leg_that_carries_submodules_says_the_line_moves_them_too( + standard, project): + """The `legs` line's `--recursive` checks out the submodules a leg + carries of its own, at the commits the leg records, so a commit made on + no branch in one of them is left in its reflog as a member's leg's is + (#413 c). The row walks into none of them: a `.gitmodules` in the leg is + what it asks, and the sentence is general, with a placeholder keep + line. A leg with none gets no such sentence.""" + make_detached_past_the_pin(project) + assert "LEG/SUB" not in why_of(row_of(standard, project, "legs")) + (project / "spec" / ".gitmodules").write_bytes( + b'[submodule "vendor/lib"]\n\tpath = vendor/lib\n' + b"\turl = ../lib.git\n") + why = why_of(row_of(standard, project, "legs")) + assert "submodules a leg carries of its own" in why, why + assert "git -C LEG/SUB branch NAME" in why, why + + +def with_a_submodule_in_the_leg(project: Path, tmp_path: Path) -> Path: + """The spec leg mounts `vendor/lib` at a commit of its own, the root's + gitlink and pin record that spec commit, a commit is made on the + nested submodule's detached HEAD, and the leg walks past its pin: the + line checks the leg out, and `--recursive` the nested one. Its sha.""" + lib = tmp_path / "lib" + git("init", "-q", "-b", "main", str(lib), cwd=tmp_path) + commit_in(lib, "the library") + spec = project / "spec" + git("-c", "protocol.file.allow=always", "submodule", "add", "-q", + lib.as_posix(), "vendor/lib", cwd=spec) + git("commit", "-q", "-m", "mount the library", cwd=spec) + mounted = git("rev-parse", "HEAD", cwd=spec).stdout.strip() + record_gitlink(project, "spec", mounted) + pin = project / "contracts" / "spec-pin.yaml" + pin.write_text(re.sub(r'(?m)^commit: "[0-9a-f]{40}"$', + f'commit: "{mounted}"', + pin.read_text(encoding="utf-8")), encoding="utf-8") + nested = spec / "vendor" / "lib" + git("checkout", "-q", "--detach", cwd=nested) + commit = commit_in(nested, "work in the leg's own submodule") + commit_in(spec) + return commit + + +@WINDOWS_SKIP +def test_a_legs_own_submodule_keep_line_keeps_its_commit_through_the_paste( + standard, project, tmp_path): + commit = with_a_submodule_in_the_leg(project, tmp_path) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + keep = re.search(r"git -C LEG/SUB branch NAME", why_of(row)) + assert keep, why_of(row) + nested = project / "spec" / "vendor" / "lib" + kept = sh(keep.group(0).replace( + "LEG/SUB", module.quote_arg(nested.resolve())).replace( + "NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + assert "kept" in git("branch", "--contains", commit, cwd=nested).stdout + + +@WINDOWS_SKIP +def test_without_it_the_legs_own_submodule_commit_is_in_no_ref( + standard, project, tmp_path): + commit = with_a_submodule_in_the_leg(project, tmp_path) + nested = project / "spec" / "vendor" / "lib" + result = paste(row_of(standard, project, "legs"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + assert git("for-each-ref", "--contains", commit, + cwd=nested).stdout.strip() == "" + assert commit in git("reflog", cwd=nested).stdout + + +def with_a_second_member_off_its_gitlink(root: Path) -> tuple[Path, str]: + """A second member, `Ghost`, a clone of the first, recorded at a commit + that is not its pin and detached at another past that one; and the + first member emptied, so the holder's own bootstrap is the NEXT. The row + sends Ghost to the family gate, yet the bootstrap's update names no path + and checks Ghost's gitlink out over its HEAD (#413 b): the commit that + HEAD is on is the sha returned.""" + first = root / "members" / FAMILY_MEMBER + ghost = root / "members" / "Ghost" + url = git("config", "-f", ".gitmodules", "--get", + f"submodule.members/{FAMILY_MEMBER}.url", cwd=root).stdout + git("-c", "protocol.file.allow=always", "clone", "-q", "--no-local", + str(first), str(ghost), cwd=root) + add_member_row(root, "Ghost") + git("config", "-f", ".gitmodules", "submodule.members/Ghost.path", + "members/Ghost", cwd=root) + git("config", "-f", ".gitmodules", "submodule.members/Ghost.url", + url.strip(), cwd=root) + git("checkout", "-q", "--detach", cwd=ghost) + record_gitlink(root, "members/Ghost", + commit_in(ghost, "the commit the holder records")) + stranded = commit_in(ghost, "work on no branch, past it") + member_unbootstrapped(root) + return ghost, stranded + + +def test_413b_the_bootstrap_names_every_member_it_moves_not_only_checkouts( + standard, holder, tmp_path): + """The holder's bootstrap moves a member the row hands to the gate as + surely as one it hands to a checkout, so its keep line is named.""" + root = holder_copy(holder, tmp_path) + ghost, _ = with_a_second_member_off_its_gitlink(root) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert "bootstrap.py" in command_of(row), row["next"] + assert f"git -C {module.quote_arg(ghost.resolve())} branch NAME" in \ + why_of(row), why_of(row) + + +@WINDOWS_SKIP +@pytest.mark.parametrize("keep_first", [True, False]) +def test_413b_pasted_the_bootstrap_moves_that_member_off_its_commit( + standard, holder, tmp_path, keep_first): + """Run first, its keep line leaves the commit on a branch; without it, + after the paste no ref holds the commit. The paste's exit is not the + question: the member's own bootstrap may refuse a clone that is not its + project, after the update has moved it.""" + root = holder_copy(holder, tmp_path) + ghost, stranded = with_a_second_member_off_its_gitlink(root) + row = row_of(standard, root, "members") + if keep_first: + keep = re.search(r"git -C (\S+) branch NAME", why_of(row)) + assert keep, why_of(row) + kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + paste(row, tmp_path / "elsewhere") + assert git("rev-parse", "HEAD", cwd=ghost).stdout.strip() != stranded + holding = git("for-each-ref", "--contains", stranded, + "--format=%(refname:short)", cwd=ghost).stdout.split() + assert holding == (["kept"] if keep_first else []), holding + + +def test_detached_answers_none_where_git_cannot_say(standard, project): + """`symbolic-ref -q HEAD` exits 1 for a detached HEAD and 0 on a branch; + any other exit is git unable to answer, which main read as detached.""" + module = doctor_module(standard) + spec = project / "spec" + assert module.detached(spec) is True + git("checkout", "-q", "-B", "work", cwd=spec) + assert module.detached(spec) is False + rmtree(project / ".git" / "modules" / "spec") + assert module.detached(spec) is None + + +def test_a_member_whose_gitdir_is_gone_is_not_called_detached( + standard, holder, tmp_path): + root = holder_copy(holder, tmp_path) + rmtree(root / ".git" / "modules" / "members" / FAMILY_MEMBER) + row = row_of(standard, root, "members") + assert row["detail"]["members"][0]["detached"] is None, row["detail"] + + +def test_where_the_gate_is_the_line_the_states_no_line_cures_are_said( + standard, project): + """An unborn HEAD in one leg and a gitlink off the pin in the other: the + gate is the NEXT, for the second, and the first is said beside it, with + the same sentence it gets when it is the only one.""" + git("symbolic-ref", "HEAD", "refs/heads/never-made", cwd=project / "spec") + record_gitlink(project, "code", commit_in(project / "code")) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + assert command_of(row) == pins_gate(module, standard, project, True), \ + row["next"] + assert "names a branch with no commit" in why_of(row), why_of(row) + assert "doctor again" in why_of(row), why_of(row) From 66a93539b8ed5b8d7247cd883f45c69f3d9506a3 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:28:51 +0000 Subject: [PATCH 4/4] shape-doctor.py: the `members` NEXT is the git line and not the holder's own bootstrap whenever any member with a gitlink is one git finds no url for, routed or not, a commit in a submodule whose directory is gone is kept by a line spelled through its repository, a leg whose working tree dropped its submodule is asked for its repository's `modules/`, and the `.gitmodules` listing is read by a pure helper (#413, #414) Fix round 2 on #559, from R559v's verification of aa63ff68. S2 (#414). The holder's own bootstrap updates with no path named (`fetch_members`), so git stops that whole run at the first gitlinked member it finds no url for, whatever the row says of that member. R559v measured it: beside an emptied IRRS, a second member at its pin, which the row passes over, or recorded off it, which the row hands to the family gate, with no `.gitmodules` section or a path-only one and no url in the holder's config, left the bootstrap as the NEXT. Pasted, it printed `FETCH INCOMPLETE` and `No url found` and the row stayed red, while the git line over IRRS alone exits 0 and fills it. The url question, the section (`gitmodules_entry`) and then the root's own config (`configured_url`), is now one helper, `submodule_url`. `checkout_reach` asks it, and `url_less_members` asks it of every member with a gitlink before the bootstrap is chosen. Any such member takes the plain git line, and each one is named in the `why` by its `no_url_sentence`, so the reader knows which entry to restore. N6 (#413). Where the nested trigger is the `modules/` of a repository git kept for an EMPTY mount, the submodules' directories are gone with it, and `git -C LEG/SUB branch NAME` exits 128 (`No such file or directory`). `checkout_reach` now says which keep line a mount's own submodules need (`nested_kind`: `tree` where the working tree carries a `.gitmodules`, `repo` where only a repository keeps theirs), and `nested_sentence` spells the second: `git --git-dir=REPO/modules/SUB --work-tree=ROOT branch NAME HEAD`. The work tree is the root because the submodule repository's own `core.worktree` names the missing directory (`cannot chdir`, exit 128, without it). Filled in and run first, it exits 0 and the commit is on that branch after the paste, for a leg emptied by `deinit` and for a member's own leg after the member is. N7 (#413). A leg with its own `.git`, past its pin at a commit that dropped its submodule, carries no `.gitmodules`, yet the gitlink's commit mounts the submodule and `--recursive` links its kept repository again and checks it out. `keeps_modules` asks the mount's own repository (`rev-parse --absolute-git-dir`) for its `modules/`, and the same spelled line keeps the commit. It is under ten lines of code, so it is done here rather than put on #460's checklist. S3776. `gitmodules_entry`'s record parsing is now a pure helper, `section_url(listing, rel)`, unit-tested on git's own `-z` listing of a `.gitmodules` whose section name holds a space and a dot: the last url a section gives wins, the last section giving a path wins, and an empty url is none. S9073. The three composite test asserts are split into single asserts that keep their message, so a failure says which half failed. N8. The members twin of the test that the gate's `why` carries what no line cures: an unborn HEAD in IRRS beside a member recorded off its pin, the family gate as the NEXT, and the unborn sentence, which says to run the doctor again, beside it. S1192 (`".gitmodules"` repeated) is accepted, as the coordinator ruled. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- shape-doctor.py | 274 ++++++++++++++++++------- tests/test_doctor_legs_members_next.py | 255 ++++++++++++++++++++++- 2 files changed, 454 insertions(+), 75 deletions(-) diff --git a/shape-doctor.py b/shape-doctor.py index bd487bdc..dc315f43 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -3446,7 +3446,7 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: exits 128 is the defect #276 was about. A section that gives the path and no url is not the end of the question, though: `--init` reads the url the root's own config already holds first (`configured_url`), and - `checkout_reach` asks that before it calls a mount url-less. + `submodule_url` asks that before it calls a mount url-less. READ WHERE GIT READS IT, WHICH IS NOT ALWAYS THE FILE. git reads the working tree's `.gitmodules` when there is one, else the copy in the @@ -3456,11 +3456,9 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: that root url-less and withhold a line that works. A file git cannot parse gives no url here. - THE LAST VALUE GIVEN WINS, for a path two sections give and for a key a - section gives twice, because git reads `.gitmodules` with each later - value overwriting the one before. `-z`, because a section name may hold - a space, a dot or a quote. An empty `url =` is no url. Read only: - `--get-regexp` writes nothing. + `-z`, because a section name may hold a space, a dot or a quote, and the + listing is read by `section_url`. Read only: `--get-regexp` writes + nothing. """ if os.path.lexists(root / ".gitmodules"): sources = [["--file", ".gitmodules"]] @@ -3480,21 +3478,40 @@ def gitmodules_entry(root: Path, rel: str) -> tuple[str | None, str | None]: if proc.returncode != 0: # No such copy, or one git cannot parse: the next place, as git. continue - named = None - urls: dict[str, str] = {} - for record in proc.stdout.decode("utf-8", "replace").split("\0"): - key, _, value = record.partition("\n") - name, _, field = key[len("submodule."):].rpartition(".") - if field == "path" and value == rel: - named = name - elif field == "url": - urls[name] = value - if named is None: - return None, None - return named, urls.get(named) or None + return section_url(proc.stdout.decode("utf-8", "replace"), rel) return None, None +def section_url(listing: str, rel: str) -> tuple[str | None, str | None]: + """`(name, url)` of the section whose `path` is `rel`, read out of one + `git config -z --get-regexp` listing of `submodule..path` and + `.url` keys, which is what `gitmodules_entry` asks git for: `(None, + None)` when no section gives that path, and `(name, None)` when the one + that does gives no url (#414). + + PURE, so the reading is tested apart from the question of where git + found the file. Each record ends in a NUL and is the key, a newline and + the value. The NAME is all of the key between `submodule.` and its last + dot, so a name that holds a space or a dot is read whole. + + THE LAST VALUE GIVEN WINS, for a path two sections give and for a key a + section gives twice, because git reads `.gitmodules` with each later + value overwriting the one before. An empty `url =` is no url. + """ + named = None + urls: dict[str, str] = {} + for record in listing.split("\0"): + key, _, value = record.partition("\n") + name, _, field = key[len("submodule."):].rpartition(".") + if field == "path" and value == rel: + named = name + elif field == "url": + urls[name] = value + if named is None: + return None, None + return named, urls.get(named) or None + + def configured_url(root: Path, name: str) -> str | None: """The url the root's own config gives the submodule NAMED `name`, or None (#414). @@ -3523,6 +3540,25 @@ def configured_url(root: Path, name: str) -> str | None: return proc.stdout.decode("utf-8", "replace").strip() or None +def submodule_url(root: Path, rel: str) -> tuple[str | None, str | None]: + """`(name, url)`: the NAME a `.gitmodules` section gives the mount at + `rel`, and the url `git submodule update --init` fetches it from, each + None where git finds none (#414). A url None is a mount git stops at, + `No url found`, exit 128. + + ONE QUESTION, ASKED IN ONE PLACE: by `checkout_reach`, of a mount a line + would name, and by `url_less_members`, of every member a holder's own + bootstrap updates. The section first (`gitmodules_entry`), then, for a + section that names the mount and gives no url, the root's own config + (`configured_url`). A section that is gone leaves git no name to look a + configured url up by, so it is not asked there. + """ + name, url = gitmodules_entry(root, rel) + if name is not None and url is None: + url = configured_url(root, name) + return name, url + + def kept_gitdir(root: Path, name: str) -> Path | None: """The repository git keeps in `root` for the submodule NAMED `name`, or None when it keeps none (#413, #414). @@ -3617,15 +3653,47 @@ def unborn_branch(path: Path) -> str | None: ref or None) +def keeps_modules(mount: Path) -> bool: + """Does the repository of a mount with a `.git` of its own keep + repositories for submodules of its own, under its `modules/`? + + A leg whose working tree carries no `.gitmodules` can still mount one at + the commit the line checks out: past its pin at a commit that dropped + its submodule, the gitlink's commit mounts it, `--recursive` links the + repository kept for it again and checks it out, and a commit made there + on no branch is left in its reflog alone (measured). Asked through + `rev-parse --absolute-git-dir` in the mount, which has a `.git` of its + own, so git does not walk up. Read only. + """ + try: + where = git_out(["rev-parse", "--absolute-git-dir"], cwd=mount) + except (Refusal, OSError, UnicodeDecodeError): + return False + return (Path(where) / "modules").is_dir() + + +def nested_kind(carries: bool, keeps: bool) -> set: + """`checkout_reach`'s `nested`: `{"tree"}` where the mount's working + tree carries a `.gitmodules`, so each submodule of its own has a + directory to run `git -C` in; `{"repo"}` where only its repository keeps + theirs, so the directories may be gone and a keep line names the + repository instead; else nothing.""" + if carries: + return {"tree"} + return {"repo"} if keeps else set() + + def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, own_git: bool, - head: str | None) -> tuple[str, object, bool]: + head: str | None) -> tuple[str, object, set]: """`(route, keep, nested)` for a mount a CHECKOUT would cure, as the `legs` and `members` rows both read it, so the two rows route by one - definition (#414). `nested` is whether the mount carries submodules of - its own that the line's `--recursive` checks out too: a `.gitmodules` in - it, or a kept repository that keeps theirs (`modules/`). A member's legs - are those (#413 c), and so is anything a leg mounts. + definition (#414). `nested` says whether the mount carries submodules of + its own that the line's `--recursive` checks out too, and how a commit + in one is kept (`nested_kind`): a `.gitmodules` in its working tree, or + a repository that keeps theirs under `modules/`, kept for an emptied + mount (`kept_gitdir`) or the mount's own (`keeps_modules`). A member's + legs are those (#413 c), and so is anything a leg mounts. `route` is: @@ -3639,8 +3707,8 @@ def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, is the lockstep gate's. - `no-url`: no `.gitmodules` section gives its path, or the one that does gives no url and the root's own config has none either - (`gitmodules_entry`, `configured_url`, #414 3), so the git line would - exit 128. No line is printed for it. + (`submodule_url`, #414 3), so the git line would exit 128. No line is + printed for it. `keep` is what the line would leave in a reflog alone (`left_in_reflog`): the mount itself, for `git -C branch NAME`, or `(repository, @@ -3654,24 +3722,23 @@ def checkout_reach(root: Path, rel: str, gitlink: str, populated: bool, changes nothing else. """ mount = root / rel - name, url = gitmodules_entry(root, rel) - if name is not None and url is None: - url = configured_url(root, name) + name, url = submodule_url(root, rel) if url is None: - return "no-url", None, False + return "no-url", None, set() carries = populated and (mount / ".gitmodules").is_file() if own_git: stranded = left_in_reflog(detached(mount), head, gitlink) - return "checkout", (mount if stranded else None), carries + return "checkout", (mount if stranded else None), nested_kind( + carries, keeps_modules(mount)) git_dir = kept_gitdir(root, name) if git_dir is None: - return ("gone" if populated else "checkout"), None, False + return ("gone" if populated else "checkout"), None, set() is_detached, kept_at = kept_head(root, git_dir) keep = None if left_in_reflog(is_detached, kept_at, gitlink): keep = (git_dir, kept_at, None if mount.is_dir() else root) return (("relink" if populated else "checkout"), keep, - carries or (git_dir / "modules").is_dir()) + nested_kind(carries, (git_dir / "modules").is_dir())) def kept_branch_line(kept: tuple, platform: str) -> str: @@ -3691,8 +3758,54 @@ def kept_branch_line(kept: tuple, platform: str) -> str: return " ".join([*words, "branch", "NAME", commit]) +def nested_sentence(noun: str, nested: set | frozenset) -> str: + """What `checkout_notes` says of the submodules a mount on the line + carries of its own, which its `--recursive` checks out too (#413 c), or + `""` where none does. `nested` is `checkout_reach`'s kinds, gathered + over every mount the line names. + + TWO KEEP LINES, BECAUSE A DIRECTORY MAY BE GONE. `git -C LEG/SUB branch + NAME` runs where the submodule has a directory (`tree`). Where the mount + is empty, or its working tree no longer mounts the submodule (`repo`), + there is none, and `git -C` exits 128 (`No such file or directory`, + measured). The line then names the submodule's repository under the + mount's own, `REPO/modules/SUB`, with the root as the work tree, + because that repository's `core.worktree` names the missing directory + and git refuses every command in it otherwise (`cannot chdir`, exit + 128, as `kept_branch_line` found one level up). Measured: it exits 0, + and the branch holds the commit after the paste. Placeholders, because + the row walks into none of these submodules. + """ + if not nested: + return "" + if noun == "member": + moved = ("the legs of each member at the commits that member " + "records, and a commit made on no branch in one of those " + "legs") + inner, here = "leg", "LEG" + else: + moved = (f"the submodules a {noun} carries of its own, at the " + f"commits that {noun} records, and a commit made on no " + "branch in one of them") + inner, here = "submodule", "LEG/SUB" + tree = (f"git -C {here} branch NAME, with {here} the directory of that " + f"{inner}") + spelled = ("git --git-dir=REPO/modules/SUB --work-tree=ROOT branch NAME " + f"HEAD, with REPO the git directory of that {noun}, SUB the " + f"name of that {inner} and ROOT the directory of this " + "repository") + if "repo" not in nested: + how = tree + elif "tree" in nested: + how = f"{tree}, or, where that directory is not there, {spelled}" + else: + how = f"{spelled}, because the directory of that {inner} is not there" + return (f". It is recursive, so it also checks out {moved} is left only " + f"in its reflog too. To keep one, run first {how}") + + def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), - relinked: list = (), nested: bool = False, + relinked: list = (), nested: set | frozenset = frozenset(), platform: str | None = None) -> str: """What a NEXT that moves HEAD owes a reader before it is pasted, as the sentences that follow the one saying what it does, or `""` when it owes @@ -3720,9 +3833,9 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), legs (#413 c); for the `legs` row, whatever a leg mounts. Said in general words, with a placeholder keep line, without walking into them, and only where a mount the line names carries some - (`checkout_reach`). `--recursive` stays: the holder's bootstrap - recurses (`fetch_members`), and a git line that did not would leave a - member's legs where a bootstrap would not. + (`checkout_reach`'s kinds, `nested_sentence`). `--recursive` stays: + the holder's bootstrap recurses (`fetch_members`), and a git line + that did not would leave a member's legs where a bootstrap would not. A commit made in a mount on a BRANCH stays on that branch, and nothing is said where no mount is in any of these states, because a reader told @@ -3744,20 +3857,7 @@ def checkout_notes(noun: str, off_pin: list[Path], kept: list = (), notes += (". A mount with no .git of its own is named by the " "repository git kept for it, because git run inside such a " "mount walks up and acts on this repository") - if nested and noun == "member": - notes += (". It is recursive, so it also checks out the legs of each " - "member at the commits that member records, and a commit " - "made on no branch in one of those legs is left only in its " - "reflog too. To keep one, run first git -C LEG branch NAME, " - "with LEG the directory of that leg") - elif nested: - notes += (f". It is recursive, so it also checks out the submodules " - f"a {noun} carries of its own, at the commits that {noun} " - "records, and a commit made on no branch in one of them is " - "left only in its reflog too. To keep one, run first git -C " - "LEG/SUB branch NAME, with LEG/SUB the directory of that " - "submodule") - return notes + return notes + nested_sentence(noun, nested) def rerun_sentence(noun: str) -> str: @@ -3806,7 +3906,8 @@ def submodule_update_next(root: Path, paths: list[str], off_pin: list[Path], noun: str, after: str = "", rerun: bool = False, platform: str | None = None, *, kept: list = (), relinked: list = (), - nested: bool = False) -> tuple[str, str]: + nested: set | frozenset = frozenset() + ) -> tuple[str, str]: """`(the line, what it does)`: the script-free command that checks each submodule at `paths` out at the commit `root` records for it, and the sentence a reader needs before pasting it (#276). @@ -3911,10 +4012,11 @@ def check_legs(ctx: Context) -> Row: - A leg NO LINE cures without choosing for the person (#414), so the NEXT is None and the `why` names the state. An UNBORN HEAD, one naming a branch with no commit: the update exits 128 on it and the gate says - `pins ok` (`unborn_branch`). And a leg `.gitmodules` gives no url for: - the git line exits 128, and the gate, measured, names no `.gitmodules` - entry. It says `pin-submodule-uninitialized` and hands back the same - git line (`gitmodules_entry`). + `pins ok` (`unborn_branch`). And a leg git finds no url for, in + `.gitmodules` or in the root's own config: the git line exits 128, and + the gate, measured, names no `.gitmodules` entry. It says + `pin-submodule-uninitialized` and hands back the same git line + (`submodule_url`). WHEN MORE THAN ONE KIND IS PRESENT THE CHECKOUT WINS AND A RE-RUN NAMES THE REST, because it is the one command that changes the row and a reader @@ -3946,6 +4048,9 @@ def check_legs(ctx: Context) -> Row: kept with a line that names that repository (`checkout_notes`). A leg that carries submodules of its own is said in general words: the line's `--recursive` checks those out too, and the row does not walk into them. + Where their directories may be gone, an emptied leg or one whose working + tree no longer mounts them, the keep line names their repositories + (`nested_sentence`). WHERE THE GATE IS THE LINE, the legs no line cures are said beside it: an unborn HEAD or a leg with no url gets the same sentence it gets with @@ -3972,7 +4077,7 @@ def check_legs(ctx: Context) -> Row: gone: list[str] = [] stuck: list[str] = [] rerun = False - nested = False + nested: set = set() for leg in legs: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) @@ -4032,7 +4137,7 @@ def check_legs(ctx: Context) -> Row: gone.append(rel) else: checkout.append(rel) - nested = nested or carries + nested |= carries if route == "relink": relinked.append(rel) if isinstance(keep, tuple): @@ -6003,6 +6108,25 @@ def member_problem(entry: dict) -> str | None: return None +def url_less_members(root: Path, rows: list) -> list[str]: + """The path of every member the holder records a gitlink for and git + finds no url for (`submodule_url`), whatever the row says of it (#414). + + THE HOLDER'S OWN BOOTSTRAP UPDATES WITH NO PATH NAMED (`fetch_members`), + so it inits every gitlinked member, and git stops that whole run at the + first one with no url, before it fetches any other. A member at its pin, + which the row passes over, or one it hands to the family gate, stops it + as surely as one a checkout would cure: measured, the bootstrap prints + `FETCH INCOMPLETE` and `No url found` for it and the row stays red. + Asking only the members the row sends to a checkout kept the bootstrap + as the NEXT over them. A member with no gitlink is no submodule to git, + and is not asked. + """ + return [entry["path"] for entry in rows + if recorded_gitlink(root, entry["path"]) is not None + and submodule_url(root, entry["path"])[1] is None] + + def members_verdict(ctx: Context, detail: dict, members_dir: str, problems: list) -> Row: """What the members add up to: the one finding, or the two OK wordings. @@ -6066,8 +6190,7 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, moved: list[Path] = [] lockstep = False rerun = False - no_url = False - nested = False + nested: set = set() for entry in rows: rel = entry["path"] mount = ctx.root / rel @@ -6099,14 +6222,13 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, entry["head"]) if route == "no-url": stuck.append(no_url_sentence(rel)) - no_url = True continue if route == "gone": lockstep = True gone.append(rel) continue checkout.append(rel) - nested = nested or carries + nested |= carries if route == "relink": relinked.append(rel) if isinstance(keep, tuple): @@ -6125,13 +6247,16 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, # says why it is that command (#277). # # BUT NOT OVER A MEMBER WITH NO URL (#414). Its update names no path, - # so it inits every member, and git stops that whole run at a section - # that gives a path and no url, where the holder's own config has - # none either (`No url found`, exit 128, measured), before it fetches - # any other; the bootstrap reports the fetch incomplete and exits 0 - # with the row as red as it was. The git line names only the members - # it cures, and git runs it. - if checkout and own_bootstrap(ctx.root) and not no_url: + # so it inits every member, and git stops that whole run at a member + # with no section, or with a section that gives a path and no url + # where the holder's own config has none either (`No url found`, + # exit 128, measured), before it fetches any other; the bootstrap + # reports the fetch incomplete and the row is as red as it was. So + # EVERY member with a gitlink is asked, not only the ones routed to a + # checkout above (`url_less_members`). The git line names only the + # members it cures, and git runs it. + url_less = url_less_members(ctx.root, rows) + if checkout and own_bootstrap(ctx.root) and not url_less: next_command = ( f"{PYTHON} " f"{quote_arg(ctx.root / 'scripts' / 'bootstrap.py')} " @@ -6139,23 +6264,28 @@ def members_verdict(ctx: Context, detail: dict, members_dir: str, # The same sentences the git line carries, built by the same # function (#413 b), for every member it moves. Its update is # recursive too, into every member that carries legs. + carried = ({"tree"} if any((ctx.root / entry["path"] + / ".gitmodules").is_file() + for entry in rows) else set()) why = ("what `make bootstrap` runs in a holder: it fetches every " "member at its pin and leaves it detached there" + checkout_notes( "member", moved, kept, relinked, - nested or any((ctx.root / entry["path"] - / ".gitmodules").is_file() - for entry in rows))) + nested | carried)) if rerun: why += rerun_sentence("member") elif checkout: if own_bootstrap(ctx.root): + # Each member that stops the bootstrap is named, routed or + # not, so the reader knows which entry to restore. after = (". This holder has its own scripts/bootstrap.py, " "but it updates every member at once, and a member " "git finds no url for can stop that run before it " "fetches any other, so this is the plain git line " "over the rest, with no credential rewrite for a " - "private member") + "private member" + + "".join(f". {no_url_sentence(rel)}" + for rel in url_less)) else: after = (". This holder has no scripts/bootstrap.py that can " "run, so this is the plain git line, with no " diff --git a/tests/test_doctor_legs_members_next.py b/tests/test_doctor_legs_members_next.py index 0e206733..a88d71c9 100644 --- a/tests/test_doctor_legs_members_next.py +++ b/tests/test_doctor_legs_members_next.py @@ -1137,7 +1137,8 @@ def test_413a_with_the_mount_directory_gone_the_keep_line_still_runs( (project / "spec").rmdir() row = row_of(standard, project, "legs") keep = KEPT_LINE.search(why_of(row)) - assert keep and "--work-tree=" in keep.group(0), why_of(row) + assert keep, why_of(row) + assert "--work-tree=" in keep.group(0), why_of(row) kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") assert kept.returncode == 0, kept.stdout + kept.stderr result = paste(row, tmp_path / "elsewhere") @@ -1216,7 +1217,8 @@ def test_413b_the_keep_line_keeps_a_members_commit_through_the_bootstrap( commit = commit_in(member) row = row_of(standard, root, "members") keep = re.search(r"git -C \S+ branch NAME", why_of(row)) - assert keep and "LEG" not in keep.group(0), why_of(row) + assert keep, why_of(row) + assert "LEG" not in keep.group(0), why_of(row) kept = sh(keep.group(0).replace("NAME", "kept"), tmp_path / "keep") assert kept.returncode == 0, kept.stdout + kept.stderr result = paste(row, tmp_path / "elsewhere") @@ -1444,7 +1446,8 @@ def test_414_3_a_leg_gitmodules_gives_no_url_for_gets_no_line( assert "no url for it in .gitmodules" in row["reason"], row["reason"] why = why_of(row) assert "no url for spec in .gitmodules" in why, why - assert "history" in why and "person" in why, why + assert "history" in why, why + assert "person" in why, why def test_414_3_a_populated_leg_off_its_pin_with_no_url_gets_no_line_either( @@ -1905,3 +1908,249 @@ def test_where_the_gate_is_the_line_the_states_no_line_cures_are_said( row["next"] assert "names a branch with no commit" in why_of(row), why_of(row) assert "doctor again" in why_of(row), why_of(row) + + +# --- after verification: every member the holder's bootstrap stops at, the --- +# --- url read apart from git, a submodule whose directory is gone, and the --- +# --- members row's gate -------------------------------------------------- + +def with_a_second_member(root: Path, at_pin: bool, section: str) -> Path: + """`Ghost`, a clone of the first member at that member's HEAD, with a + `family.yaml` row copied from the first, pin included. Recorded at that + pin, where the row has nothing to say of it, or at a commit made past + it, which the row hands to the family gate. `section` is what + `.gitmodules` says of it: `full`, `path-only` or `none`. It is never + `submodule init`ed, so the holder's own config gives it no url.""" + first = root / "members" / FAMILY_MEMBER + ghost = root / "members" / "Ghost" + url = git("config", "-f", ".gitmodules", "--get", + f"submodule.members/{FAMILY_MEMBER}.url", cwd=root).stdout + git("-c", "protocol.file.allow=always", "clone", "-q", "--no-local", + str(first), str(ghost), cwd=root) + add_member_row(root, "Ghost") + if section != "none": + git("config", "-f", ".gitmodules", "submodule.members/Ghost.path", + "members/Ghost", cwd=root) + if section == "full": + git("config", "-f", ".gitmodules", "submodule.members/Ghost.url", + url.strip(), cwd=root) + if at_pin: + record_gitlink(root, "members/Ghost", + git("rev-parse", "HEAD", cwd=ghost).stdout.strip()) + else: + git("checkout", "-q", "--detach", cwd=ghost) + record_gitlink(root, "members/Ghost", + commit_in(ghost, "the commit the holder records")) + return ghost + + +def beside_a_member_git_finds_no_url_for(root: Path, section: str, + at_pin: bool) -> None: + """IRRS emptied, so a checkout cures it, beside a populated `Ghost` + that git finds no url for (`with_a_second_member`). The row never sends + Ghost to a checkout: at its pin it passes it over, and off it hands it + to the family gate. The holder's own bootstrap inits every gitlinked + member all the same, and git stops it at Ghost.""" + with_a_second_member(root, at_pin=at_pin, section=section) + member_unbootstrapped(root) + + +UNROUTED_NO_URL = pytest.mark.parametrize( + "section, at_pin", + [("none", True), ("path-only", True), ("none", False), + ("path-only", False)], + ids=["no-section-at-pin", "path-only-at-pin", "no-section-gate", + "path-only-gate"]) + + +@UNROUTED_NO_URL +def test_a_member_the_row_never_routes_with_no_url_takes_the_bootstrap_off( + standard, holder, tmp_path, section, at_pin): + """The holder's own bootstrap names no path, so a member git finds no + url for stops it whether the row sends that member to a checkout or + not. Every member with a gitlink is asked, the NEXT is the git line + over the member a checkout cures, and the `why` names the one with no + url.""" + root = holder_copy(holder, tmp_path) + beside_a_member_git_finds_no_url_for(root, section, at_pin) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + assert command_of(row) == ( + f"git -C {module.quote_arg(root.resolve())} {UPDATE} -- " + f"members/{FAMILY_MEMBER}"), row["next"] + why = why_of(row) + assert "can stop that run" in why, why + assert "no url for members/Ghost" in why, why + + +@WINDOWS_SKIP +@UNROUTED_NO_URL +def test_pasted_there_the_git_line_fills_the_member_the_bootstrap_would_not( + standard, holder, tmp_path, section, at_pin): + """Pasted, the line exits 0 and fills IRRS, which the bootstrap left + empty. With Ghost at its pin the row is then `ok`; off it, the row + names Ghost alone, for the gate.""" + root = holder_copy(holder, tmp_path) + beside_a_member_git_finds_no_url_for(root, section, at_pin) + result = paste(row_of(standard, root, "members"), tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert any((root / "members" / FAMILY_MEMBER).iterdir()) + after = row_of(standard, root, "members") + if at_pin: + assert after["status"] == "ok", (after, result.stdout, result.stderr) + else: + assert after["status"] == "FINDING", after + assert FAMILY_MEMBER not in after["reason"], after["reason"] + assert "members/Ghost" in after["reason"], after["reason"] + + +def test_section_url_reads_gits_listing_and_the_last_value_wins(standard, + tmp_path): + """`section_url` is the pure half of `gitmodules_entry`: one `git config + -z --get-regexp` listing in, `(name, url)` out. Fed git's own listing of + a `.gitmodules` whose section NAME holds a space and a dot and gives its + url twice, and of a path two sections give: the last value given wins + each time, as git reads the file, and an empty url is no url.""" + module = doctor_module(standard) + (tmp_path / ".gitmodules").write_bytes( + b'[submodule "my lib.v2"]\n\tpath = vendor/lib\n' + b"\turl = ../first.git\n\turl = ../second.git\n" + b'[submodule "old"]\n\tpath = vendor/both\n\turl = ../old.git\n' + b'[submodule "new"]\n\tpath = vendor/both\n\turl =\n') + listing = git("config", "-z", "--file", ".gitmodules", "--get-regexp", + r"^submodule\..*\.(path|url)$", cwd=tmp_path).stdout + assert module.section_url(listing, "vendor/lib") == ( + "my lib.v2", "../second.git"), listing + assert module.section_url(listing, "vendor/both") == ("new", None), \ + listing + assert module.section_url(listing, "vendor/none") == (None, None), \ + listing + + +#: The keep line for a commit in a submodule whose directory is gone: it +#: names that submodule's repository under the mount's own, with the root +#: as the work tree. +SPELLED = "git --git-dir=REPO/modules/SUB --work-tree=ROOT branch NAME HEAD" + + +def spelled_filled(module, repository: Path, sub: str, root: Path) -> str: + """`SPELLED` with its placeholders filled in as a reader fills them: + REPO the mount's git directory, SUB the submodule's name, ROOT the + root, and the branch `kept`.""" + return (SPELLED + .replace("--git-dir=REPO/modules/SUB", module.quote_arg( + f"--git-dir={repository.as_posix()}/modules/{sub}")) + .replace("--work-tree=ROOT", module.quote_arg( + f"--work-tree={root.as_posix()}")) + .replace("NAME", "kept")) + + +@WINDOWS_SKIP +def test_an_emptied_legs_own_submodule_is_kept_through_its_repository( + standard, project, tmp_path): + """`deinit` empties the leg and takes its submodule's directory with + it, so `git -C LEG/SUB branch NAME` has nowhere to run and exits 128. + The repository git kept for the leg keeps its submodule's under + `modules/`, and the `why` spells the line through it: filled in and run + first, it exits 0, and after the paste the commit is on that branch.""" + commit = with_a_submodule_in_the_leg(project, tmp_path) + git("submodule", "deinit", "-q", "-f", "--", "spec", cwd=project) + module = doctor_module(standard) + row = row_of(standard, project, "legs") + why = why_of(row) + assert SPELLED in why, why + assert "git -C LEG/SUB" not in why, why + resolved = project.resolve() + kept = sh(spelled_filled(module, kept_repository(resolved, "spec"), + "vendor/lib", resolved), tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + nested = project / "spec" / "vendor" / "lib" + assert git("rev-parse", "HEAD", cwd=nested).stdout.strip() != commit + assert "kept" in git("branch", "--contains", commit, cwd=nested).stdout + + +@WINDOWS_SKIP +def test_an_emptied_members_own_leg_is_kept_through_its_repository( + standard, holder, tmp_path): + """The members twin: the member is emptied by `deinit`, its legs' + directories with it, and the line spelled through the repository git + kept for the member keeps a commit made in its own `spec` leg.""" + root = holder_copy(holder, tmp_path) + holder_without(root, "bootstrap.py") + leg, commit = made_in_a_members_own_leg(root) + git("submodule", "deinit", "-q", "-f", "--", f"members/{FAMILY_MEMBER}", + cwd=root) + module = doctor_module(standard) + row = row_of(standard, root, "members") + why = why_of(row) + assert SPELLED in why, why + assert "git -C LEG branch" not in why, why + resolved = root.resolve() + repository = kept_repository(resolved, f"members/{FAMILY_MEMBER}") + kept = sh(spelled_filled(module, repository, "spec", resolved), + tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, root, "members")["status"] == "ok" + assert git("rev-parse", "HEAD", cwd=leg).stdout.strip() != commit + assert "kept" in git("branch", "--contains", commit, cwd=leg).stdout + + +@WINDOWS_SKIP +def test_a_leg_past_a_commit_that_dropped_its_submodule_still_names_it( + standard, project, tmp_path): + """The leg walks past its pin at a commit that removes its submodule, + `.gitmodules` included, so its working tree carries none. The gitlink's + commit still mounts it, and `--recursive` links the repository the leg + keeps for it again and checks it out. The leg's own repository is asked + for its `modules/`, the `why` spells the line through it, and that + line, run first, keeps the commit.""" + commit = with_a_submodule_in_the_leg(project, tmp_path) + spec = project / "spec" + git("rm", "-q", "-f", "vendor/lib", cwd=spec) + if (spec / ".gitmodules").exists(): + git("rm", "-q", "-f", ".gitmodules", cwd=spec) + commit_in(spec, "drop the library") + assert not (spec / ".gitmodules").exists() + module = doctor_module(standard) + row = row_of(standard, project, "legs") + why = why_of(row) + assert SPELLED in why, why + repository = Path(git("rev-parse", "--absolute-git-dir", + cwd=spec).stdout.strip()) + resolved = project.resolve() + kept = sh(spelled_filled(module, repository, "vendor/lib", resolved), + tmp_path / "keep") + assert kept.returncode == 0, kept.stdout + kept.stderr + result = paste(row, tmp_path / "elsewhere") + assert result.returncode == 0, result.stdout + result.stderr + assert row_of(standard, project, "legs")["status"] == "ok" + nested = spec / "vendor" / "lib" + assert git("rev-parse", "HEAD", cwd=nested).stdout.strip() != commit + assert "kept" in git("branch", "--contains", commit, cwd=nested).stdout + + +def test_where_the_gate_is_the_members_line_the_states_no_line_cures_are_said( + standard, holder, tmp_path): + """The members twin of the test above: an unborn HEAD in IRRS, and + `Ghost` recorded off its pin. The family gate is the NEXT, for Ghost, + and IRRS is said beside it, with the sentence it gets when it is the + only one.""" + root = holder_copy(holder, tmp_path) + with_a_second_member(root, at_pin=False, section="full") + git("symbolic-ref", "HEAD", "refs/heads/never-made", + cwd=root / "members" / FAMILY_MEMBER) + module = doctor_module(standard) + row = row_of(standard, root, "members") + assert row["status"] == "FINDING", row + resolved = root.resolve() + assert command_of(row) == module.validator_command( + standard, resolved / "scripts" / "validate-family.py", True, + ["--root", str(resolved)]), row["next"] + assert "names a branch with no commit" in why_of(row), why_of(row) + assert "doctor again" in why_of(row), why_of(row)