From 7015386262271dd2eebadc7f2a8da0ee14722a1f Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 14:16:39 +0000 Subject: [PATCH 1/5] WIP (account-swap checkpoint; partial, untested as a set): family.py: #684 holder states (#684) Saved for an account swap, on the coordinator's order. Not the final commit. In it: a bisect is refused as `family-operation-in-progress` with `git bisect reset` (E2, asked last); a rebase directory with no `head-name` is named with `git rebase --quit` (E5); non-cone sparse rules holding no pattern name `git sparse-checkout disable` alone (E3); a `family.yaml` that fails to parse while an act is unfinished gains a remediation naming the act (N4); a `members` file HEAD holds and the index no longer does is refused before the member is read (E1, the staged-move half). The new tests and test_family_refuse_up_front.py and test_family_member_path.py passed on these bytes (144 passed); the full targeted set has not run. Not in it: E1's unstaged-deletion half and E4 (reftable), which wait on rulings; the dirty `family.yaml` refusal (#459 line 66, on Brett Heap's word relayed by lane 5 on #473 at 13:56:50Z). Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 269 ++++++++++++++-- tests/test_family_holder_states.py | 474 +++++++++++++++++++++++++++++ 2 files changed, 710 insertions(+), 33 deletions(-) create mode 100644 tests/test_family_holder_states.py diff --git a/scripts/family.py b/scripts/family.py index e4f430cb..b2201d22 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -276,7 +276,17 @@ def open(cls, root_spec) -> "Family": "creates one. A PROJECT is scaffolded with " "`scaffold-project.py` and adopted with `adopt-project.py`; " "neither of those is a family.") - data = load_yaml(path) + try: + data = load_yaml(path) + except Refusal as exc: + # A MERGE CONFLICTED IN `family.yaml` ITSELF, the commonest one + # in a holder, is named as the act it is (#684, #612's review + # N4), asked ONLY HERE, once the parse has failed: asked before + # it, git would run ahead of the refusal of a hand-edited + # `path:`, which runs no git command at all. + if exc.code == "yaml-unparsable" and not exc.remediation: + exc.remediation = _unparsable_in_an_act(root) + raise if not isinstance(data, dict): raise Refusal("family-unreadable", f"{path}: not a mapping") if data.get("kind") != "family-manifest": @@ -824,6 +834,22 @@ def _branch_said(family: "Family", verb: str, suggested: str) -> list[str]: "HEAD the rebase has detached, in the middle of its sequence, where " "`git rebase --continue` replays the rest on top of it into the rebased " "branch and `git rebase --abort` drops it") +#: A BISECT, ASKED LAST (#684, #612's review E2). git keeps `BISECT_LOG` from +#: `git bisect start` to `git bisect reset`, and once the bisect checks out a +#: commit to test, HEAD is detached there: the real run met it as +#: `family-holder-detached`, whose `git switch -c` would branch from the +#: commit under test, and the dry run printed a plan. git does not refuse the +#: partial commit there, and `git bisect reset` leaves it behind (`you are +#: leaving 1 commit behind`, measured on git 2.43). Its exit is `git bisect +#: reset`, and nothing finishes it by hand. Asked after the others: a merge +#: or a cherry-pick begun at the commit under test leaves its own mark beside +#: `BISECT_LOG`, and `git bisect reset` cannot check a branch out over the +#: conflict it stopped on (`you need to resolve your current index first`, +#: exit 1, measured), so that act is named, and ended, first. +BISECT_EFFECT = ( + "git does not refuse that during a bisect: once the bisect has checked " + "out a commit to test, the commit would be made on it, on no branch, " + "and `git bisect reset` would leave it behind") OPERATION_MARKS = ( ("rebase-merge", "a rebase", "git rebase --continue", "git rebase --abort", REBASE_EFFECT), @@ -845,6 +871,7 @@ def _branch_said(family: "Family", verb: str, suggested: str) -> list[str]: "so `git revert --continue` and `--abort` then say `no cherry-pick or " "revert in progress` and the revert's changes are left half done in " "the index and the work tree"), + ("BISECT_LOG", "a bisect", None, "git bisect reset", BISECT_EFFECT), ) #: `git am` keeps its state in `rebase-apply` too, and git tells it from a @@ -880,6 +907,28 @@ def _branch_said(family: "Family", verb: str, suggested: str) -> list[str]: "`sequencer/` directory git can read no series from, which `--continue` " "fails on") +#: A REBASE GIT CAN READ NOTHING OF (#684, #612's review E5 and N5): a +#: `rebase-merge/` or `rebase-apply/` that holds no `head-name`, as a stale, +#: empty one does. `git status` still says `You are currently rebasing.`, +#: but `git rebase --continue` and `git rebase --abort` both fail there +#: (`could not read '.git/rebase-merge/head-name'`, exit 1), and `git rebase +#: --quit` takes the directory away and moves nothing (exit 0); measured on +#: git 2.43 in both. A real rebase writes `head-name` when it starts, and +#: `git am`, which writes none, is told apart by `applying` first. So it is +#: named as the sequencer's unreadable twin is: no `finish`, and `--quit`. +REBASE_HEAD_NAMES = {"rebase-merge": "rebase-merge/head-name", + "rebase-apply": "rebase-apply/head-name"} + + +def _rebase_unreadable(mark: str) -> tuple: + """`(what, finish, abort, effect)` for a `mark` directory that holds no + `head-name` (`REBASE_HEAD_NAMES`).""" + return ("a rebase", None, "git rebase --quit", + "git does not refuse that, and the commit would land beside a " + f"`{mark}/` directory that holds no `head-name`, so git can read " + "no rebase from it and `git rebase --continue` and `--abort` both " + "fail there") + def _git_paths(root: Path, names: list[str]) -> list[Path]: """Where git keeps each of `names` for the holder at `root`, ASKED OF GIT @@ -935,7 +984,7 @@ def _operation_in_progress( that is there, then a `sequencer/`. `finish` is None where git can continue nothing.""" names = [*(mark for mark, *_ in OPERATION_MARKS), AM_MARK, SEQUENCER, - SEQUENCER_TODO] + SEQUENCER_TODO, *REBASE_HEAD_NAMES.values()] marks = dict(zip(names, _git_paths(root, names))) for mark, what, finish, abort, effect in OPERATION_MARKS: where = marks[mark] @@ -943,6 +992,9 @@ def _operation_in_progress( continue if mark == "rebase-apply" and os.path.lexists(marks[AM_MARK]): what, finish, abort, effect = AM_OPERATION + elif mark in REBASE_HEAD_NAMES and not os.path.lexists( + marks[REBASE_HEAD_NAMES[mark]]): + what, finish, abort, effect = _rebase_unreadable(mark) return what, mark, where, finish, abort, effect if os.path.lexists(marks[SEQUENCER]): what, finish, abort, effect = _sequence_act(marks[SEQUENCER_TODO]) @@ -950,10 +1002,47 @@ def _operation_in_progress( return None +#: What the remedy says of an act git can continue nothing of (`finish` is +#: None), by the mark found, with `{mark}` and `{abort}` filled in: each +#: names only the exit that works there (#684). +REBASE_UNREADABLE_EXIT = ( + "`{mark}/` is what a rebase left behind, and it holds no `head-name`, so " + "git can read no rebase from it and there is nothing to continue or give " + "up: clear it without moving HEAD with `{abort}`.") +UNFINISHABLE = { + SEQUENCER: ("`sequencer/` is what a cherry-pick or revert series left " + "behind, and git can read no step from it, so there is " + "nothing to continue: give it up with `{abort}` (`git revert " + "--abort` is the same), or clear it without moving HEAD with " + "`git cherry-pick --quit`."), + "rebase-merge": REBASE_UNREADABLE_EXIT, + "rebase-apply": REBASE_UNREADABLE_EXIT, + "BISECT_LOG": ("end the bisect with `{abort}` once you have what it was " + "for: it takes HEAD back to where the bisect started."), +} + + +def _operation_exit(found: tuple) -> str: + """What a person does about the act `_operation_in_progress` found: its + own exit, and only what works there, then `Then run the same command + again.` The remediation of `family-operation-in-progress`, and of a + `family.yaml` an act left unparsable (`_unparsable_in_an_act`, #684), in + the same words.""" + _, mark, _, finish, abort, _ = found + if finish is None: + said = UNFINISHABLE[mark].format(mark=mark, abort=abort) + else: + said = ("end it, whichever way suits the work in it: finish it with " + f"`{finish}` once whatever stopped it is resolved and staged, " + f"or give it up with `{abort}`.") + return f"{said} Then run the same command again." + + def _checked_no_operation_in_progress(root: Path, verb: str) -> None: """`family-operation-in-progress`: `verb` refuses, before its first write and under `--dry-run` alike, a holder with a merge, a cherry-pick, - a revert, a rebase or a `git am` unfinished in it (#612, 571(e)). + a revert, a rebase, a `git am` or a bisect unfinished in it (#612, + 571(e); the bisect, #684). THE COMMIT GOES WRONG, AFTER THE WRITES. `add`, `bump` and `remove` commit with `git commit -- ` (`_commit_one`). git refuses a @@ -993,45 +1082,76 @@ def _checked_no_operation_in_progress(root: Path, verb: str) -> None: `--abort` and `--quit` both exit 0: the refusal names it generically and offers only what works. + A BISECT IS ASKED TOO, AND A REBASE DIRECTORY GIT CAN READ NOTHING OF + (#684, #612's review E2 and E5). A bisect holds HEAD detached on the + commit it tests, where the real run met it as `family-holder-detached`, + whose `git switch -c` would branch from that commit, and the dry run + printed a plan (`BISECT_EFFECT`); its exit is `git bisect reset`. A + stale, empty `rebase-merge/` was named as a rebase with `git rebase + --abort`, which fails there as `--continue` does; it is named with `git + rebase --quit`, the exit that works (`REBASE_HEAD_NAMES`). The remedy of + each act that has no `finish` is its own (`UNFINISHABLE`). + So the marks are asked, and the refusal names the one it found and that act's OWN exit: `git commit` for a merge, `--continue` for the others, - or `--abort`. Every mark is looked for WHERE GIT KEEPS IT (`_git_paths`), - which in a linked worktree is not `.git/`. Asked of the holder's - own git directory, never of a member's: a merge in a MEMBER's mount is - that checkout's own business. - - WHEN IT IS ASKED, AND WHAT IT DOES NOT NAME (#612's review, N4). Each - verb asks it right after `Family.open` and the checks of its own - arguments, so after `family.yaml` is parsed. The commonest merge conflict - in a holder is in `family.yaml` itself, whose `<<<<<<<` `load_yaml` reads - as a YAML merge key: `yaml-unparsable`, with no remediation, and no word - of a merge. That predates this check and is left for #459: asking before - `load_yaml` makes git run before the refusal of a hand-edited `path:`, - which this test holds to no git command at all: + or `--abort`; `--quit` or `git bisect reset` where nothing else works. + Every mark is looked for WHERE GIT KEEPS IT (`_git_paths`), which in a + linked worktree is not `.git/`. Asked of the holder's own git + directory, never of a member's: a merge in a MEMBER's mount is that + checkout's own business. + + WHEN IT IS ASKED (#612's review, N4; #684). Each verb asks it right + after `Family.open` and the checks of its own arguments, so after + `family.yaml` is parsed. The commonest merge conflict in a holder is in + `family.yaml` itself, whose `<<<<<<<` `load_yaml` reads as a YAML merge + key, so the parse fails first, and `Family.open` asks the same marks + then (`_unparsable_in_an_act`): its `yaml-unparsable` names the act and + this exit. Only then: asked before `load_yaml`, git would run before the + refusal of a hand-edited `path:`, which this test holds to no git + command at all: `test_family_member_path.py::test_the_refusal_is_made_before_any_git_call`. """ found = _operation_in_progress(root) if found is None: return - what, mark, where, finish, abort, effect = found - if finish is None: - end = ("Remediation: `sequencer/` is what a cherry-pick or revert " - "series left behind, and git can read no step from it, so " - "there is nothing to continue: give it up with " - f"`{abort}` (`git revert --abort` is the same), or clear it " - "without moving HEAD with `git cherry-pick --quit`. Then run " - "the same command again.") - else: - end = ("Remediation: end it, whichever way suits the work in it: " - f"finish it with `{finish}` once whatever stopped it is " - f"resolved and staged, or give it up with `{abort}`. Then " - "run the same command again.") + what, mark, where, _, _, effect = found raise Refusal( "family-operation-in-progress", f"{what} is unfinished in the family root: git keeps {mark} at " f"{where.as_posix()}. `{verb}` makes its commit with `git commit -- " f"`, and {effect.format(verb=verb)}. No git command that " - "writes was run and nothing was written.", end) + "writes was run and nothing was written.", + "Remediation: " + _operation_exit(found)) + + +def _unparsable_in_an_act(root: Path) -> str: + """The remediation of a `family.yaml` that does not parse while an act + git has unfinished is in the holder, or "" where none is (#684, #612's + review N4), for `Family.open`. + + A MERGE THAT CONFLICTED IN `family.yaml` WAS NEVER NAMED AS ONE. Every + verb parses the manifest first, and git's `<<<<<<<` line reads as a YAML + merge key, so the person was told `yaml-unparsable ... merge keys are not + supported`, with no remediation, before the operation check was ever + asked. The marks are asked here once the parse has failed, and the + refusal keeps its code and its line and gains the act and its exit + (`_operation_exit`). A holder git cannot answer for (not a repository, + no git on PATH) keeps the refusal as it was: this is a remediation, and + must never turn a refusal into a failure of its own. + """ + try: + found = _operation_in_progress(root) + except (CommandFailed, OSError): + return "" + if found is None: + return "" + what, mark, where, *_ = found + said = _operation_exit(found) + return (f"Remediation: {what} is unfinished in the family root: git " + f"keeps {mark} at {where.as_posix()}, and where it left " + f"{MANIFEST} conflicted, the reader cannot parse the lines git " + "marks the conflict with (`<<<<<<<` reads as a YAML merge key). " + + said[:1].upper() + said[1:]) def _sparse_rules_exclude(root: Path, paths: list[str]) -> list[str]: @@ -1095,6 +1215,24 @@ def _sparse_is_cone(root: Path) -> bool: return cone +def _sparse_rules_hold_no_pattern(root: Path) -> bool: + """Do the holder's NON-CONE sparse rules hold no pattern at all, as an + EMPTY `.git/info/sparse-checkout` does (#684, #612's review E3)? ASKED + OF GIT: `git sparse-checkout list` prints each pattern it reads from the + file, comments and blank lines left out, so it prints nothing, or a + blank line for a line of spaces, where there is none (measured on git + 2.43). Asked by the caller only of non-cone rules: in cone mode it lists + the directories in the cone, and prints nothing for a cone of none, whose + root files are inside and whose `git sparse-checkout add members` is + right. A `list` that fails is no answer, and leaves the remedy as it + was.""" + try: + listed = _git_bytes(["sparse-checkout", "list"], root) + except CommandFailed: + return False + return not listed.strip() + + def _checked_inside_sparse_rules(root: Path, verb: str, paths: list[str]) -> None: """`family-sparse-excluded`: `verb` refuses, before its first write and @@ -1147,6 +1285,15 @@ def _checked_inside_sparse_rules(root: Path, verb: str, back every path and leaves a copy already on disk as it is, as `gitmodules-sparse` says. + NON-CONE RULES THAT HOLD NO PATTERN get `disable` ALONE (#684, #612's + review E3). An empty rules file with sparse checkout on leaves out every + path, so every path a verb stages is refused, and `git sparse-checkout + add ` there makes those the only paths kept: measured on git + 2.43, it took every other file out of the work tree, and the holder's + top level went from 14 entries to `.git`, `.gitmodules`, `family.yaml` + and `members`. `disable` brings every path back and takes nothing away + (`_sparse_rules_hold_no_pattern`). + `git sparse-checkout check-rules` is a recent subcommand (the git here, 2.43, has it); a git without it is "cannot tell" and refuses nothing (`_sparse_rules_exclude`). @@ -1162,6 +1309,14 @@ def _checked_inside_sparse_rules(root: Path, verb: str, for path in out)) names = ", ".join(f"`{path}`" for path in out) one = len(out) == 1 + if not cone and _sparse_rules_hold_no_pattern(root): + bring = ("the rules hold no pattern at all, so they leave out every " + "path, and `git sparse-checkout add` would keep only the " + "paths it is given and take every other file out of the " + f"work tree: bring {'it' if one else 'them'} back with ") + else: + bring = (f"bring {'it' if one else 'them'} back into the rules: " + f"`git sparse-checkout add {' '.join(patterns)}`, or ") raise Refusal( "family-sparse-excluded", f"the holder's sparse checkout leaves out {names}, which `{verb}` " @@ -1170,8 +1325,7 @@ def _checked_inside_sparse_rules(root: Path, verb: str, f"{'it is' if one else 'they are'} outside, so `{verb}` would fail " "after its first writes. No git command that writes was run and " "nothing was written.", - f"Remediation: bring {'it' if one else 'them'} back into the rules: " - f"`git sparse-checkout add {' '.join(patterns)}`, or `git " + f"Remediation: {bring}`git " "sparse-checkout disable`, which brings back every path the sparse " "checkout leaves out and leaves a copy already on disk as it is. " "Then run the same command again.") @@ -3078,6 +3232,49 @@ def _submodule_paths( for name, keys in found.items() if "path" in keys} +def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: + """`member-path-unaddable` for a `members` FILE that HEAD holds and the + index no longer does, or None (#684, #612's review E1): the move + `_members_is_a_file`'s remedy names (`git mv -- members `), + staged and not committed, or the file's removal, staged. + + IT PASSED EVERY CHECK, AND THE ADD'S COMMIT TOOK HALF OF IT. Nothing is + at `members` on disk or in the index, so the mount went ahead, and its + commit, which names `.gitmodules`, the mount path and `family.yaml`, + took the deletion of the file into itself, git putting a directory + where the file was (`D members` beside `A members/`), and left + the new name staged alone, half a move in the holder's history + (measured by #612's review). The change is the person's and is committed + on its own first, as #363 has a change HEAD holds and the index no + longer does committed first (`_spellings_refusal`). HEAD's ROOT TREE is + asked, `git ls-tree` with no path (`_tree_entries`); a holder with no + commit yet holds nothing there.""" + if any(name == MEMBERS_DIR + for name, _ in _index_entries(root, MEMBERS_DIR)): + return None + try: + head = _tree_entries(root, "HEAD") + except Refusal: + return None + if not any(name == MEMBERS_DIR and kind == "a file" + for name, kind, _ in head): + return None + return Refusal( + "member-path-unaddable", + f"git would add the mount path {path} and take `{MEMBERS_DIR}` out of " + f"the holder with it: `{MEMBERS_DIR}` is a file HEAD holds and the " + "index no longer does, a move or a removal staged and not committed, " + "and the add's commit, putting a directory where the file was, would " + f"take that deletion into itself (`D {MEMBERS_DIR}`) and leave any " + "new name staged alone. No git command that writes was run and " + "nothing was written.", + f"Remediation: every member is mounted under {MEMBERS_DIR}/, which " + "must be a plain directory of the holder's own. Commit that change " + "on its own first (`git commit`), since the add's own commit names " + f"{GITMODULES}, {path} and {MANIFEST} alone, and run the same command " + "again.") + + def _members_is_a_file(root: Path, path: str) -> Refusal: """`member-path-unaddable` for a `members` that is a FILE (#612, 574/5): what it says, and the move that makes room. A file git tracks is moved @@ -3157,7 +3354,10 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: not take, whose remediation says already that `members/` must be a plain directory of the holder's own (a link and a submodule are the other two ways it is not). A file under another spelling of `members` - is `_members_spellings`'s, below. + is `_members_spellings`'s, below. One HEAD holds and the index no + longer does, its move or removal staged and not committed, is refused + under the same code (`_members_file_left_in_head`, #684): the add's + commit took the deletion half of that change into itself. - GIT'S OWN DRY RUN, `git add --dry-run --ignore-missing` of the path — the check `--force` skips — made here with `-f`, so an ignore rule passes it and nothing else does. It refuses a path beyond a symbolic @@ -3195,6 +3395,9 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: canon = _members_dir_on_disk(root, on_disk) if on_disk.get(MEMBERS_DIR, ("",))[0] == "a file": raise _members_is_a_file(root, path) + moved = _members_file_left_in_head(root, path) + if moved is not None: + raise moved held = _index_entries(root, path) # Each spelling of the mount path the index holds other than its own, # and whether git tracks a directory there. diff --git a/tests/test_family_holder_states.py b/tests/test_family_holder_states.py new file mode 100644 index 00000000..fe6b3003 --- /dev/null +++ b/tests/test_family_holder_states.py @@ -0,0 +1,474 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`add`, `bump` and `remove` meet five holder states #612's checks did not +model, and each refusal now fits the state it names (#684, from #612's +review lines on #459: E1, E2, E3, E5 and N4). + +- **A bisect** (E2). `BISECT_LOG` is asked with the other marks, LAST, and + the act is named with its own exit, `git bisect reset`, under both runs. + The real run said `family-holder-detached`, whose `git switch -c` would + branch from the commit under test; the dry run printed a plan. +- **A rebase directory git can read nothing of** (E5). An empty + `rebase-merge/` was named a rebase with `git rebase --abort`, which fails + there as `--continue` does; `git rebase --quit` is named, the exit that + works. `rebase-apply/` with no `applying` (no `git am`) is its twin. +- **Non-cone sparse rules that hold no pattern** (E3). Every staged path is + refused, as before, and the remedy names `git sparse-checkout disable` + alone: the `git sparse-checkout add ...` it printed took every other file + out of the work tree. +- **A merge conflicted in `family.yaml` itself** (N4). `yaml-unparsable` + keeps its code and its line, and gains a remediation naming the merge + and its exits; asked only once the parse has failed. +- **A `members` file HEAD holds and the index no longer does** (E1): the + move #612's own remedy names, staged and not committed, or a removal + staged. `add` went through and its commit took `D members` into itself; + now it is refused before the member is read. + +THE PROOF IS #612's, with the helpers of `test_family_refuse_up_front.py`: +exit 2 and one REFUSED paragraph, nothing on stdout (no member was read), +the holder as it was under the real run and `--dry-run` alike; then the +remedy typed as printed, and the same command makes its one commit. Each +state is made with git itself where git can make it; a stale, empty rebase +directory is what a crash leaves, and is made with `mkdir`, as the issue's +probe made it. +""" + +from __future__ import annotations + +import os +import re +from pathlib import Path + +import pytest + +from conftest import run_script +#: The family fixture and its names, imported from the modules that own them. +from test_family import FAMILY, ORG, SPARE, family, family_module # noqa: F401 +from test_family_add_excludes import empty_holder # noqa: F401 +from test_family_add_refusals import holder, offline, refusal # noqa: F401 +from test_family_dry_run import add, add_state, assert_add_as_it_was, isolated +from test_family_member_path import MEMBER, raw +#: #612's proof of a refusal before the first write, and its states. +from test_family_refuse_up_front import ( + NOTES, VERBS, assert_refused_alike, commit_file, head_of, mark_of, + needs_check_rules, operation_said, outside, run_verb, sparse_said, + sparse_set, type_the_remedy, work_branch, +) + +MANIFEST = "family.yaml" + + +def act_said(what: str, mark: str, effect: str, verb: str, + remedy: str) -> tuple: + """`(the REFUSED line as a pattern, the remediation)` of + `family-operation-in-progress` for an act `operation_said` does not + list: the path git keeps `mark` at is the one part that varies.""" + line = re.compile( + re.escape(f"REFUSED family-operation-in-progress: {what} is " + f"unfinished in the family root: git keeps {mark} at ") + + r".+/" + re.escape(mark) + + re.escape(f". `{verb}` makes its commit with `git commit -- " + f"`, and {effect}. No git command that writes " + "was run and nothing was written.")) + return line, remedy + + +def runs_after(verb: str, family, root: Path, env: dict) -> None: + """The same command once the remedy is typed: exit 0, and ONE commit, a + child of the HEAD it started from.""" + was = head_of(root, env) + done = run_verb(verb, family, root, env) + assert done.returncode == 0, done.stdout + done.stderr + assert raw(root, env, "rev-parse", "HEAD~1").stdout.decode().strip() \ + == was + + +# --- a bisect (E2) ---------------------------------------------------------- + +BISECT_EFFECT = ( + "git does not refuse that during a bisect: once the bisect has checked " + "out a commit to test, the commit would be made on it, on no branch, " + "and `git bisect reset` would leave it behind") +BISECT_REMEDY = ( + "Remediation: end the bisect with `git bisect reset` once you have what " + "it was for: it takes HEAD back to where the bisect started. Then run " + "the same command again.") + + +def bisect_said(verb: str) -> tuple: + return act_said("a bisect", "BISECT_LOG", BISECT_EFFECT, verb, + BISECT_REMEDY) + + +def bisecting(root: Path, env: dict, good: str | None = None) -> None: + """THE ISSUE'S PROBE: `git bisect start`, `git bisect bad`, `git bisect + good ` (or `good`), which checks out a commit to test and leaves + HEAD detached there, the state the real run met as + `family-holder-detached`. Each fact is a premise.""" + if good is None: + good = raw(root, env, "rev-list", "--max-parents=0", "HEAD" + ).stdout.decode().split()[0] + raw(root, env, "bisect", "start") + raw(root, env, "bisect", "bad") + raw(root, env, "bisect", "good", good) + assert raw(root, env, "symbolic-ref", "-q", "HEAD", + check=False).returncode != 0, "premise: HEAD is detached" + assert os.path.lexists(mark_of(root, env, "BISECT_LOG")), "premise" + + +@pytest.mark.parametrize("verb", VERBS) +def test_a_bisect_is_named_with_its_own_exit_in_both_runs( + family, holder, offline, tmp_path, verb): + """E2: the real run said `family-holder-detached` and offered `git + switch -c`, which branches from the commit under test, and the dry run + printed a plan. Now `family-operation-in-progress` names the bisect and + `git bisect reset`, in the same words under both runs, the holder + untouched and `BISECT_LOG` still there. Typed as printed, the reset + puts the holder back on its branch, and the verb runs.""" + env = isolated(offline, tmp_path) + bisecting(holder, env) + assert_refused_alike(verb, family, holder, env, bisect_said(verb), + mark="BISECT_LOG") + raw(holder, env, "bisect", "reset") + assert not os.path.lexists(mark_of(holder, env, "BISECT_LOG")) + runs_after(verb, family, holder, env) + + +def test_what_the_bisect_refusal_says_the_commit_would_do_is_what_git_does( + family, holder, offline, tmp_path): + """`BISECT_EFFECT`, TIED TO GIT: the partial commit the verbs make is + made at the commit under test (git does not refuse it), on no branch, + and `git bisect reset` leaves it behind: no branch contains it.""" + env = isolated(offline, tmp_path) + bisecting(holder, env) + manifest = holder / MANIFEST + manifest.write_bytes(manifest.read_bytes() + b"# a change\n") + raw(holder, env, "commit", "-q", "-m", "THE VERB'S COMMIT", "--", + f":(literal){MANIFEST}") + made = head_of(holder, env) + raw(holder, env, "bisect", "reset") + assert raw(holder, env, "branch", "--contains", made).stdout == b"" + + +def test_an_act_begun_inside_a_bisect_is_named_first_and_each_exit_works( + family, holder, offline, tmp_path): + """THE BISECT IS ASKED LAST. A cherry-pick stopped on a conflict at the + commit under test leaves `CHERRY_PICK_HEAD` beside `BISECT_LOG`, and + `git bisect reset` fails over it (`you need to resolve your current + index first`, the premise, measured here). So the cherry-pick is named + first, and its exit typed as printed works; the bisect is named next, + and its exit works; and `add` then runs.""" + env = isolated(offline, tmp_path) + work = work_branch(holder, env) + commit_file(holder, env, NOTES, b"base\n", "base") + good = head_of(holder, env) + raw(holder, env, "switch", "-q", "-c", "side") + commit_file(holder, env, NOTES, b"side\n", "side") + raw(holder, env, "switch", "-q", work) + for name, data in ((NOTES, b"main\n"), ("x1.txt", b"1\n"), + ("x2.txt", b"2\n")): + commit_file(holder, env, name, data, name) + bisecting(holder, env, good) + assert raw(holder, env, "cherry-pick", "side", + check=False).returncode != 0, "premise: it conflicts" + stuck = raw(holder, env, "bisect", "reset", check=False) + assert stuck.returncode != 0, "premise: the reset fails over it" + assert b"resolve your current index first" in stuck.stderr + assert_refused_alike("add", family, holder, env, + operation_said("cherry-pick", "add"), + mark="CHERRY_PICK_HEAD") + raw(holder, env, "cherry-pick", "--abort") + assert_refused_alike("add", family, holder, env, bisect_said("add"), + mark="BISECT_LOG") + raw(holder, env, "bisect", "reset") + runs_after("add", family, holder, env) + + +# --- a rebase directory git can read nothing of (E5) ------------------------ + +STALE_EFFECT = ( + "git does not refuse that, and the commit would land beside a `{mark}/` " + "directory that holds no `head-name`, so git can read no rebase from it " + "and `git rebase --continue` and `--abort` both fail there") +STALE_REMEDY = ( + "Remediation: `{mark}/` is what a rebase left behind, and it holds no " + "`head-name`, so git can read no rebase from it and there is nothing to " + "continue or give up: clear it without moving HEAD with `git rebase " + "--quit`. Then run the same command again.") + + +@pytest.mark.parametrize("mark, verb", [ + ("rebase-merge", "add"), ("rebase-merge", "bump"), + ("rebase-merge", "remove"), ("rebase-apply", "add")]) +def test_an_empty_rebase_directory_is_named_with_the_exit_that_works( + family, holder, offline, tmp_path, mark, verb): + """E5 (and N5), THE ISSUE'S PROBE: `mkdir .git/rebase-merge`. It was + refused as a rebase whose remedy named `git rebase --abort`, and that + fails there (`could not read '.git/rebase-merge/head-name'`), as `git + rebase --continue` does: both premises, measured here, the directory + left as it was. Now named with `git rebase --quit`, under both runs; + typed as printed, it takes the directory away, and the verb runs. An + empty `rebase-apply/`, which holds no `applying` and so is no `git am`, + is the same.""" + env = isolated(offline, tmp_path) + mark_of(holder, env, mark).mkdir() + for step in ("--abort", "--continue"): + tried = raw(holder, env, "rebase", step, check=False) + assert tried.returncode != 0, (step, tried.stderr) + assert f"{mark}/head-name".encode() in tried.stderr, tried.stderr + assert mark_of(holder, env, mark).is_dir() + ran = assert_refused_alike( + verb, family, holder, env, + act_said("a rebase", mark, STALE_EFFECT.format(mark=mark), verb, + STALE_REMEDY.format(mark=mark)), mark=mark) + assert "`git rebase --quit`" in ran.stderr + raw(holder, env, "rebase", "--quit") + assert not os.path.lexists(mark_of(holder, env, mark)) + runs_after(verb, family, holder, env) + + +# --- non-cone sparse rules that hold no pattern (E3) ------------------------ + +EMPTY_RULES_REMEDY = ( + "Remediation: the rules hold no pattern at all, so they leave out every " + "path, and `git sparse-checkout add` would keep only the paths it is " + "given and take every other file out of the work tree: bring them back " + "with `git sparse-checkout disable`, which brings back every path the " + "sparse checkout leaves out and leaves a copy already on disk as it is. " + "Then run the same command again.") + +#: What each verb stages, in the order its refusal names them. +STAGED = {"add": [".gitmodules", f"members/{SPARE}", MANIFEST], + "bump": [f"members/{MEMBER}", MANIFEST], + "remove": [".gitmodules", f"members/{MEMBER}", MANIFEST]} + + +def rules_emptied(root: Path, env: dict, init: bool = False) -> None: + """THE ISSUE'S STATE: sparse checkout on, NON-CONE rules, and the rules + file emptied by hand. `init` is the issue's own probe, `git + sparse-checkout init --no-cone`, whose rules (`/*`, `!/*/`) take every + directory but a member's checkout out of the work tree first; without + it the rules are `/*`, which keeps everything, so nothing is out of the + work tree before the file is emptied. Each fact is a premise: `list` + prints no pattern, and the cone setting is off.""" + if init: + raw(root, env, "sparse-checkout", "init", "--no-cone") + else: + sparse_set(root, env, "/*") + mark_of(root, env, "info/sparse-checkout").write_bytes(b"") + assert raw(root, env, "sparse-checkout", "list").stdout == b"" + assert raw(root, env, "config", "--bool", "core.sparseCheckoutCone", + check=False).stdout.strip() in (b"", b"false") + + +@needs_check_rules +@pytest.mark.parametrize("verb, init", [ + ("add", True), ("add", False), ("bump", False), ("remove", False)], + ids=["add-init", "add", "bump", "remove"]) +def test_rules_that_hold_no_pattern_name_disable_alone( + family, holder, offline, tmp_path, verb, init): + """E3: every path the verb stages is outside (the premise, asked of git + as the tool asks it), and it is refused as before, in both runs alike. + The remedy names `git sparse-checkout disable` ALONE, and says why: the + `git sparse-checkout add` of the paths it named took every other file + out of the work tree (the next test). Typed as printed, `disable` lets + the same command run.""" + env = isolated(offline, tmp_path) + rules_emptied(holder, env, init) + for path in STAGED[verb]: + assert outside(holder, env, path), f"premise: {path} is outside" + line, _ = sparse_said(verb, STAGED[verb]) + assert_refused_alike(verb, family, holder, env, + (line, EMPTY_RULES_REMEDY)) + raw(holder, env, "sparse-checkout", "disable") + runs_after(verb, family, holder, env) + + +@needs_check_rules +def test_the_add_the_refusal_named_there_takes_every_other_file_away( + family, holder, offline, tmp_path): + """WHY `disable` ALONE, tied to git (#612's review E3): with the rules + emptied, the `git sparse-checkout add` of the three paths `add`'s + refusal named before #684 leaves the holder's top level with nothing + but those, and `.git`; `disable`, the exit named now, brings every + entry back.""" + env = isolated(offline, tmp_path) + rules_emptied(holder, env) + before = sorted(os.listdir(holder)) + raw(holder, env, "sparse-checkout", "add", *STAGED["add"]) + assert sorted(os.listdir(holder)) == [ + ".git", ".gitmodules", MANIFEST, "members"], before + assert len(before) > 4, before + raw(holder, env, "sparse-checkout", "disable") + assert sorted(os.listdir(holder)) == before + + +@needs_check_rules +def test_a_cone_of_no_directories_keeps_its_add_remedy( + family, holder, offline, tmp_path): + """ONLY NON-CONE RULES ARE ASKED. In cone mode `git sparse-checkout + list` names the cone's directories, and prints nothing for a cone of + none (`set --cone` alone, the premise), whose root files are inside: + there `git sparse-checkout add members` brings the mount back, and it + is still named. Typed as printed, the verb runs.""" + env = isolated(offline, tmp_path) + sparse_set(holder, env, cone=True) + assert raw(holder, env, "sparse-checkout", "list").stdout == b"" + ran = assert_refused_alike( + "remove", family, holder, env, + sparse_said("remove", [f"members/{MEMBER}"], "members")) + type_the_remedy(holder, env, ran.stderr) + runs_after("remove", family, holder, env) + + +# --- a merge conflicted in family.yaml (N4) --------------------------------- + +def conflicted_manifest(root: Path, env: dict) -> int: + """THE ISSUE'S STATE (#612's review N4): two branches that change + `family.yaml` in the same place, merged, so `git merge` stops on + `CONFLICT (content): Merge conflict in family.yaml` and keeps + `MERGE_HEAD` (premises). Returns the number of the line holding git's + `<<<<<<<`, the one the reader stops at.""" + work = work_branch(root, env) + text = (root / MANIFEST).read_bytes() + raw(root, env, "switch", "-q", "-c", "side") + commit_file(root, env, MANIFEST, text + b"# side\n", "side") + raw(root, env, "switch", "-q", work) + commit_file(root, env, MANIFEST, text + b"# work\n", "work") + merged = raw(root, env, "merge", "side", check=False) + assert merged.returncode != 0, merged.stdout + assert b"Merge conflict in family.yaml" in merged.stdout, merged.stdout + lines = (root / MANIFEST).read_bytes().split(b"\n") + return next(number for number, line in enumerate(lines, 1) + if line.startswith(b"<<<<<<<")) + + +def unparsable_line(root: Path, at: int) -> str: + return (f"REFUSED yaml-unparsable: {root.resolve() / MANIFEST}: line " + f"{at}: merge keys are not supported") + + +@pytest.mark.parametrize("verb", VERBS) +def test_a_merge_conflicted_in_family_yaml_is_named_as_a_merge( + family, holder, offline, tmp_path, verb): + """N4: every verb parses `family.yaml` first, so the commonest holder + merge conflict said only `yaml-unparsable ... merge keys are not + supported`, with no remediation. The refusal keeps its code and that + line, in both runs alike, and gains a remediation that names the merge, + where git keeps it, and its two exits; the holder is untouched. Typed as + printed, `git merge --abort` lets the same command run.""" + env = isolated(offline, tmp_path) + at = conflicted_manifest(holder, env) + where = (holder.resolve() / ".git" / "MERGE_HEAD").as_posix() + remedy = ( + f"Remediation: a merge is unfinished in the family root: git keeps " + f"MERGE_HEAD at {where}, and where it left family.yaml conflicted, " + "the reader cannot parse the lines git marks the conflict with " + "(`<<<<<<<` reads as a YAML merge key). End it, whichever way suits " + "the work in it: finish it with `git commit` once whatever stopped " + "it is resolved and staged, or give it up with `git merge --abort`. " + "Then run the same command again.") + line = re.compile(re.escape(unparsable_line(holder, at))) + assert_refused_alike(verb, family, holder, env, (line, remedy), + mark="MERGE_HEAD") + raw(holder, env, "merge", "--abort") + runs_after(verb, family, holder, env) + + +def test_an_unparsable_family_yaml_with_no_act_in_progress_is_as_before( + family, holder, offline, tmp_path): + """THE CONTROL: the same `<<<<<<<` line put there by hand, with no act + in progress, is `yaml-unparsable` alone, one line and no remediation, + as before #684.""" + env = isolated(offline, tmp_path) + manifest = holder / MANIFEST + text = manifest.read_bytes() + manifest.write_bytes(text + b"<<<<<<< HEAD\n") + at = text.count(b"\n") + 1 + for extra in ((), ("--dry-run",)): + ran = run_verb("add", family, holder, env, *extra) + assert ran.returncode == 2, ran.stdout + ran.stderr + assert ran.stdout == "" + assert ran.stderr == unparsable_line(holder, at) + "\n", ran.stderr + + +def test_a_family_yaml_outside_any_repository_keeps_its_refusal( + offline, tmp_path): + """THE FALLBACK IS A REMEDIATION AND NOTHING ELSE: where git cannot + answer (no repository here, `GIT_CEILING_DIRECTORIES` stopping its + search, the premise), the refusal stays `yaml-unparsable`, one line, + exit 2, never a failed git command of its own.""" + env = {**isolated(offline, tmp_path), + "GIT_CEILING_DIRECTORIES": str(tmp_path)} + plain = tmp_path / "plain" + plain.mkdir() + (plain / MANIFEST).write_bytes(b"kind: family-manifest\n<<<<<<< HEAD\n") + assert raw(plain, env, "rev-parse", "--git-dir", + check=False).returncode != 0, "premise: no repository" + ran = run_script(FAMILY, "add", "--family-root", str(plain), "--member", + f"{ORG}/{SPARE}", env=env) + assert ran.returncode == 2, ran.stdout + ran.stderr + assert ran.stderr == unparsable_line(plain, 2) + "\n", ran.stderr + + +# --- a members file HEAD holds and the index no longer does (E1) ------------ + +MOVED_LINE = ( + "REFUSED member-path-unaddable: git would add the mount path " + f"members/{SPARE} and take `members` out of the holder with it: " + "`members` is a file HEAD holds and the index no longer does, a move or " + "a removal staged and not committed, and the add's commit, putting a " + "directory where the file was, would take that deletion into itself (`D " + "members`) and leave any new name staged alone. No git command that " + "writes was run and nothing was written.") +MOVED_REMEDY = ( + "Remediation: every member is mounted under members/, which must be a " + "plain directory of the holder's own. Commit that change on its own " + "first (`git commit`), since the add's own commit names .gitmodules, " + f"members/{SPARE} and family.yaml alone, and run the same command " + "again.") + +#: `{id: the git command that leaves the tracked file staged away}`: the +#: move #612's remedy names, and the file's removal. +STAGED_AWAY = {"git mv": ("mv", "--", "members", "members-file"), + "git rm": ("rm", "-q", "--", "members")} + + +@pytest.mark.parametrize("dry_run", [False, True], ids=["real", "dry-run"]) +@pytest.mark.parametrize("change", STAGED_AWAY.values(), ids=STAGED_AWAY) +def test_add_refuses_a_members_file_whose_move_is_staged_not_committed( + family, empty_holder, offline, tmp_path, change, dry_run): + """E1: a tracked `members` FILE moved aside with #612's own remedy, `git + mv -- members `, and not committed, or removed with `git rm`: + nothing is at `members` on disk or in the index (premises), `add` went + through, and its commit took `D members` into itself beside the mount, + leaving the new name staged alone. Now `member-path-unaddable`, nothing + on stdout, the holder as it was. The remedy typed as printed, `git + commit`, and `add` then makes a commit of its own three paths alone, + and leaves nothing staged.""" + holder = empty_holder + env = isolated(offline, tmp_path) + (holder / "members").write_bytes(b"a file where the directory goes\n") + raw(holder, env, "add", "-f", "--", "members") + raw(holder, env, "commit", "-q", "-m", "a file named members") + raw(holder, env, *change) + assert not os.path.lexists(holder / "members"), "premise: not on disk" + assert b"\tmembers\0" not in raw(holder, env, "ls-files", "-s", "-z" + ).stdout, "premise: not in the index" + before = add_state(holder, env) + extra = ("--dry-run",) if dry_run else () + ran = add(holder, env, family["remotes"], *extra) + assert ran.returncode == 2, ran.stdout + ran.stderr + assert ran.stdout == "" + line, remedy = refusal(ran.stderr) + assert line == MOVED_LINE + assert remedy == MOVED_REMEDY + assert_add_as_it_was(holder, env, before) + + raw(holder, env, "commit", "-q", "-m", "the change, on its own") + done = add(holder, env, family["remotes"]) + assert done.returncode == 0, done.stdout + done.stderr + shown = raw(holder, env, "show", "--name-status", "--format=", "HEAD" + ).stdout.decode().split("\n") + assert sorted(entry.split("\t")[1] for entry in shown if entry) == [ + ".gitmodules", MANIFEST, f"members/{SPARE}"], shown + assert raw(holder, env, "status", "--porcelain").stdout == b"" From 772105aa5752783872723357db5347789f5381bd Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 11:26:19 +0000 Subject: [PATCH 2/5] family.py: `add` refuses a `members` file whose deletion is not staged with `git rm --cached` and a commit, and `add`, `bump` and `remove` refuse a `family.yaml` holding a change HEAD does not, staged or not, both runs alike (#684) Finishes the WIP commit 70153862 (#684's E2, E5, E3, N4 and E1's staged half) with the rest of the issue and the scope Brett Heap added. E1's OTHER HALF. A tracked `members` FILE deleted from disk, the deletion not staged, passed every `add` check: the mount's `git submodule add`, putting a directory where the file was, took the entry out of the index, and the commit took `D members` into itself. `_members_file_left_in_head` now refuses it too, as `member-path-unaddable`, before the member is read and under `--dry-run` alike. The remedy is the house #318 one for an index entry with nothing on disk: take it out of the index, which touches no file (`git rm --cached -- members`), and commit the removal on its own first. The issue's `git mv` fails there (`fatal: bad source`, exit 128, measured), and `git checkout -- members` first would put the file back over a `members/` directory a hand made there and take the directory away (measured); the lane's coordinator ruled the remedy for that reason. A link on disk is left to git's own dry run. A DIRTY `family.yaml` (#459's line from #571's review, comment 6192725913, on Brett Heap's word "refuse a dirty `family.yaml`", relayed by lane 5 on #473 at 2026-10-16T13:56:50Z). `add`, `bump` and `remove` rewrite `family.yaml` as the work tree has it and commit it with `git commit -- `, so a comment line edited by hand went into their commit beside their own row. `_checked_manifest_committed`, asked right after the operation check in each verb, refuses `family-manifest-uncommitted` where the index holds the file unmerged, holds no copy, or holds one HEAD does not, or where `git ls-files -m` lists the work tree's copy; the remediation names git's two ways out, `git commit -- family.yaml` and `git checkout HEAD -- family.yaml`. It reads the index records, `HEAD:family.yaml` and `ls-files -m`, and no `hash-object`, which a dry run asks only for `bump`'s nothing-to-do. A holder whose HEAD holds no `family.yaml`, one made by hand with no commit yet, is not asked: `add` gives the file its first commit, as it always has. The two `member-path` remedies now say to commit a row put back by hand, or the run again would meet this refusal. Existing tests that left `family.yaml` changed and expected a later answer now commit that change first, or expect the new refusal: test_family.py's nothing-to-do over an uncommitted edit (now refused, the edit left alone) and #210's half-finished bump (now refused; `git checkout HEAD -- family.yaml` and the same bump finishes it in one commit of both paths); test_family_member_path.py's no-path-line control and its two remedy texts; and the members-line hand edits in test_family_bump_remove_rollback.py and test_family_dry_run.py. Not in it: #612's review E4 (reftable). This host's git is 2.43, and the issue asks for it to be measured on git 2.45 or later first. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 157 ++++++++++-- tests/test_family.py | 32 ++- tests/test_family_bump_remove_rollback.py | 7 +- tests/test_family_dry_run.py | 16 +- tests/test_family_holder_states.py | 291 ++++++++++++++++++++-- tests/test_family_member_path.py | 13 +- 6 files changed, 447 insertions(+), 69 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 798dbd2d..61812f98 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -490,6 +490,11 @@ def _member_path(project: str, row: dict | None = None) -> str: has no path of its own to be compared with, so it is refused here, under the same code, only when a row is given: what `add` writes (`_member_row`, no row) is as it was. + + A ROW PUT BACK BY HAND IS COMMITTED before the run again (#684): the + verbs refuse a `family.yaml` holding a change HEAD does not + (`_checked_manifest_committed`), so a row put back over an edit already + committed would be refused next, for want of that commit. """ expected = f"{MEMBERS_DIR}/{project}" if row is not None and (project in ("", ".", "..") or "/" in project): @@ -506,8 +511,8 @@ def _member_path(project: str, row: dict | None = None) -> str: f"edit. Restore {MANIFEST} from git (`git checkout -- " f"{MANIFEST}` if the edit is not committed), or put the row's " "`project:` and `path:` back as the tool wrote them " - f"(`project: `, `path: {MEMBERS_DIR}/`), then " - "run the command again naming that member.") + f"(`project: `, `path: {MEMBERS_DIR}/`) and " + "commit that, then run the command again naming that member.") declared = expected if row is None else str(row.get("path") or expected) if declared != expected: raise Refusal( @@ -517,8 +522,8 @@ def _member_path(project: str, row: dict | None = None) -> str: "git command was run and nothing was written.", "Remediation: this tool writes that field itself, so a different " f"value is a hand edit. Put `path: {expected}` back in " - f"{MANIFEST}, or `git checkout -- {MANIFEST}` if the edit is not " - "committed, then run the same command again.") + f"{MANIFEST} and commit that, or `git checkout -- {MANIFEST}` if " + "the edit is not committed, then run the same command again.") return expected @@ -1166,6 +1171,77 @@ def _unparsable_in_an_act(root: Path) -> str: + said[:1].upper() + said[1:]) +def _checked_manifest_committed(root: Path, verb: str) -> None: + """`family-manifest-uncommitted`: `verb` refuses, before its first write + and under `--dry-run` alike, a `family.yaml` that holds a change HEAD + does not, staged or not (#684; #459's line from #571's review, ruled by + Brett Heap and relayed by lane 5 on #473 at 2026-10-16T13:56:50Z: + "refuse a dirty `family.yaml`"). + + THE COMMAND'S COMMIT TOOK IT. `add`, `bump` and `remove` rewrite + `family.yaml` as the work tree has it (`Family.rewritten`) and commit it + with `git commit -- ` (`_commit_one`), which takes each path as + the work tree has it: a comment line edited by hand and never staged + went into the command's commit beside its own row, under its message. + This is #407's rule for `.gitmodules` (`_checked_gitmodules_as_staged`) + for the other file every one of those commits names, with one + difference: a change staged and not committed is refused too, since the + commit takes it just the same. So the remedies that have a person put a + row back by hand (`member-path`, `_member_path`) say to commit that. + + ASKED OF GIT, AND NOTHING WRITTEN: the index's records for the file + (`_index_records`, no pathspec), the blob HEAD holds (`_head_oid`), and + `git ls-files -m`, git's own answer to "modified", which compares the + work tree's copy with the index's by content where their stat data + differ. Not `git hash-object`, which a dry run asks only for `bump`'s + `nothing to do`; so a copy the index marks assume-unchanged, which git + does not compare, is not seen. + + A HOLDER WHOSE HEAD HOLDS NO `family.yaml`, as one made by hand with no + commit yet does, has no committed copy for a change to differ from: the + command's commit is the file's first, as `add` has always made it, and + nothing is asked. + + THE REMEDY IS GIT'S TWO WAYS OUT, both measured on git 2.43: `git commit + -- family.yaml` commits the work tree's copy and nothing else staged, + unmerged or not; `git checkout HEAD -- family.yaml` puts HEAD's back in + the index and the work tree. Where the change is a staged deletion over + a copy that is HEAD's byte for byte, the first has nothing to commit, + and the second is the way out. + """ + head = _head_oid(root, MANIFEST) + if head is None: + return + records = [record.partition(b"\t")[0].split() + for record in _index_records(root).get(MANIFEST.encode(), [])] + merged = [oid.decode("ascii") for _, oid, stage in records + if stage == b"0"] + if len(merged) != len(records): + # `ls-files -m` lists an unmerged path once for each stage the work + # tree's copy differs from, which says nothing more. + said = ["it is unmerged in the index"] + else: + said = ([] if merged == [head] else + ["a change to it is staged and not committed"] if merged else + ["its deletion is staged, so the index holds no copy"]) + if MANIFEST.encode() in _git_bytes(["ls-files", "-m", "-z"], + root).split(b"\0"): + said.append("git lists the work tree's copy as modified, a " + "change nobody has staged") + if not said: + return + raise Refusal( + "family-manifest-uncommitted", + f"{MANIFEST} holds a change HEAD does not: {', and '.join(said)}. " + f"`{verb}` rewrites {MANIFEST} as the work tree has it and commits " + "it, so that change would go into the command's commit beside its " + "own. No git command that writes was run and nothing was written.", + "Remediation: which change to keep is a person's to say. To keep it, " + f"commit it on its own first, `git commit -- {MANIFEST}`; to drop " + "it, put back HEAD's copy, in the index and the work tree, `git " + f"checkout HEAD -- {MANIFEST}`. Then run the same command again.") + + def _sparse_rules_exclude(root: Path, paths: list[str]) -> list[str]: """The `paths` the holder's sparse-checkout rules leave out, in the order given, ASKED OF GIT (`git sparse-checkout check-rules`), or none. @@ -3316,9 +3392,10 @@ def _submodule_paths( def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: """`member-path-unaddable` for a `members` FILE that HEAD holds and the - index no longer does, or None (#684, #612's review E1): the move + work tree no longer does, or None (#684, #612's review E1): the move `_members_is_a_file`'s remedy names (`git mv -- members `), - staged and not committed, or the file's removal, staged. + staged and not committed, or the file's removal, staged; or its + deletion from the work tree, not staged, which the index still holds. IT PASSED EVERY CHECK, AND THE ADD'S COMMIT TOOK HALF OF IT. Nothing is at `members` on disk or in the index, so the mount went ahead, and its @@ -3330,9 +3407,24 @@ def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: on its own first, as #363 has a change HEAD holds and the index no longer does committed first (`_spellings_refusal`). HEAD's ROOT TREE is asked, `git ls-tree` with no path (`_tree_entries`); a holder with no - commit yet holds nothing there.""" - if any(name == MEMBERS_DIR - for name, _ in _index_entries(root, MEMBERS_DIR)): + commit yet holds nothing there. + + THE DELETION NOT STAGED is the same commit: the index still holds the + file, nothing but a directory a hand made there, if any, is at + `members` on disk, and the mount's `git submodule add`, putting a + directory where the file was, takes the entry out of the index, so the + commit says `D members` all the same. It is taken out of the index, + which touches no file, and the removal committed, as #318 takes out of + the index what has nothing on disk (`UNSTAGE`). Not moved with the `git + mv` the issue proposed, which fails there (`fatal: bad source`, exit + 128, measured on git 2.43), nor put back first with `git checkout -- + members`, which would put the file back over such a directory and take + the directory away. A link on disk is left to git's own dry run, which + refuses a path beyond one.""" + held = [kind for name, kind in _index_entries(root, MEMBERS_DIR) + if name == MEMBERS_DIR] + if any(kind != "a file" for kind in held) or ( + held and os.path.islink(root / MEMBERS_DIR)): return None try: head = _tree_entries(root, "HEAD") @@ -3341,6 +3433,22 @@ def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: if not any(name == MEMBERS_DIR and kind == "a file" for name, kind, _ in head): return None + if held: + return Refusal( + "member-path-unaddable", + f"git would add the mount path {path} and take `{MEMBERS_DIR}` " + f"out of the holder with it: `{MEMBERS_DIR}` is a file HEAD and " + "the index hold and the work tree no longer does, its deletion " + "not staged, and the add's mount, putting a directory where the " + "file was, would take it out of the index, and its commit would " + f"take that deletion into itself (`D {MEMBERS_DIR}`). No git " + "command that writes was run and nothing was written.", + f"Remediation: every member is mounted under {MEMBERS_DIR}/, " + "which must be a plain directory of the holder's own. Take the " + "file out of the index, which touches no file (`git rm --cached " + f"-- {MEMBERS_DIR}`), and commit the removal on its own first " + f"(`git commit`), since the add's own commit names {GITMODULES}, " + f"{path} and {MANIFEST} alone, and run the same command again.") return Refusal( "member-path-unaddable", f"git would add the mount path {path} and take `{MEMBERS_DIR}` out of " @@ -3436,10 +3544,11 @@ def _checked_mount_path(root: Path, project: str, path: str) -> None: not take, whose remediation says already that `members/` must be a plain directory of the holder's own (a link and a submodule are the other two ways it is not). A file under another spelling of `members` - is `_members_spellings`'s, below. One HEAD holds and the index no - longer does, its move or removal staged and not committed, is refused - under the same code (`_members_file_left_in_head`, #684): the add's - commit took the deletion half of that change into itself. + is `_members_spellings`'s, below. One HEAD holds and the work tree no + longer does, its move or removal staged and not committed, or its + deletion not staged, is refused under the same code + (`_members_file_left_in_head`, #684): the add's commit took the + deletion half of that change into itself. - GIT'S OWN DRY RUN, `git add --dry-run --ignore-missing` of the path — the check `--force` skips — made here with `-f`, so an ignore rule passes it and nothing else does. It refuses a path beyond a symbolic @@ -4478,17 +4587,18 @@ def cmd_add(args) -> int: holder exactly as it was (#203); or with `--dry-run`, print what it would write and change nothing in the holder (#326, `_add_dry_run`). - IN THIS ORDER, AND ONLY THIS ORDER: an act git has unfinished in the - holder (`_checked_no_operation_in_progress`, #612), the holder's - refusals (`_checked_add_target`, which includes a `members` that is a - file, `_checked_mount_path`, #612), the paths the sparse rules leave out + IN THIS ORDER, AND ONLY THIS ORDER: an act git has unfinished in the holder + (`_checked_no_operation_in_progress`, #612), a `family.yaml` holding a + change HEAD does not (`_checked_manifest_committed`, #684), the holder's + refusals (`_checked_add_target`, which includes a `members` that is a file, + `_checked_mount_path`, #612), the paths the sparse rules leave out (`_checked_inside_sparse_rules`, #612), then the branch the commit lands on, which the real run refuses on the tracking branch or none (`_checked_branch`, #487) and a dry run only reports, then the member's (`_probe_member`, of a throwaway clone), then the member's id against the - rows (`_checked_member_id`, #330), and only then the mount, the pin and - the commit (`_mount_and_pin`), inside a `MountRollback` that puts the - holder back if anything still fails and says what it put back. + rows (`_checked_member_id`, #330), and only then the mount, the pin and the + commit (`_mount_and_pin`), inside a `MountRollback` that puts the holder + back if anything still fails and says what it put back. WHEN TWO APPLY, THE EARLIER ONE IS SAID. The holder's refusals need nothing cloned, so they come first: a section under another name that @@ -4507,6 +4617,9 @@ def cmd_add(args) -> int: # the rest is, refused after the writes or made inside the act (#612, # 571(e)). _checked_no_operation_in_progress(family.root, "add") + # A `family.yaml` holding a change HEAD does not, staged or not, next: + # the commit would take it beside the row (#684). + _checked_manifest_committed(family.root, "add") at = _checked_add_target(family, repository, project, path, args.at) # What the sparse rules leave out, after the refusals that read the same # paths by their marks (`gitmodules-sparse`), before the member is read. @@ -6022,6 +6135,7 @@ def cmd_bump(args) -> int: # BEFORE `member-uninitialized`, whose remedy a merge in progress or a # sparse checkout that leaves the mount out would make wrong (#612). _checked_no_operation_in_progress(family.root, "bump") + _checked_manifest_committed(family.root, "bump") _checked_inside_sparse_rules(family.root, "bump", [path, MANIFEST]) if not (member / ".git").exists(): raise Refusal( @@ -6718,6 +6832,9 @@ def cmd_remove(args) -> int: # holder's own files: the commit goes wrong however they are, refused # after the writes or made inside the act (#612). _checked_no_operation_in_progress(family.root, "remove") + # A `family.yaml` holding a change HEAD does not, staged or not, next: + # the commit would take it beside the row's removal (#684). + _checked_manifest_committed(family.root, "remove") # A `.gitmodules` the work tree lacks, or has not as the index has it, # is refused first (#407): the commit would take it out of HEAD, or # carry a change nobody staged. Where the index holds a copy, every diff --git a/tests/test_family.py b/tests/test_family.py index 960daf90..169ae191 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1046,10 +1046,11 @@ def test_bump_to_the_commit_already_pinned_ignores_what_the_member_holds( def test_bump_to_the_commit_already_pinned_leaves_an_uncommitted_edit_alone( holder): - """The rewrite equals `HEAD`, so there is nothing to record, and what the - working copy held before is put back: an uncommitted edit to the `members:` - block is not this run's to discard, however wrong it is. `git status` says - the same before and after.""" + """An uncommitted edit to the `members:` block is not this run's to + discard, however wrong it is. The rewrite equals `HEAD`, so there would be + nothing to record; since #684 `bump` refuses the edit before that, as a + `family.yaml` holding a change HEAD does not, and leaves it where it is. + `git status` says the same before and after.""" pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() manifest_path = holder / "family.yaml" manifest_path.write_text(manifest_path.read_text().replace( @@ -1061,8 +1062,9 @@ def test_bump_to_the_commit_already_pinned_leaves_an_uncommitted_edit_alone( result = run_script(FAMILY, "bump", "--family-root", str(holder), "--member", "IRRS", "--to", pinned) - assert result.returncode == 1, result.stderr + result.stdout - assert nothing_to_do_line("IRRS", pinned) in result.stdout + assert result.returncode == 2, result.stderr + result.stdout + assert result.stdout == "" + assert result.stderr.startswith("REFUSED family-manifest-uncommitted: ") assert manifest_path.read_bytes() == edited assert git("status", "--porcelain", cwd=holder).stdout == status @@ -1110,7 +1112,12 @@ def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): commit: a run that wrote the row and staged both paths and then died before `git commit` — a hook that refused, a lock — leaves exactly this. Reading `was == --to` as "nothing to do" would call the half-finished bump - done and leave the commit for somebody to make by hand (#210).""" + done and leave the commit for somebody to make by hand (#210). Since #684 + the staged `family.yaml` is refused first, as a change HEAD does not hold; + the way out that drops it, `git checkout HEAD -- family.yaml`, leaves the + gitlink staged, and the same bump then finishes in ONE commit of both + paths. (The other way out, `git commit -- family.yaml`, would commit the + row without its gitlink.)""" member = holder / "members" / "IRRS" before = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() after = commit_in_member(member) @@ -1120,9 +1127,14 @@ def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): manifest_path.write_text(text.replace(before, after, 1)) git("add", "--", "members/IRRS", "family.yaml", cwd=holder) head = git("rev-parse", "HEAD", cwd=holder).stdout.strip() - - result = run_script(FAMILY, "bump", "--family-root", str(holder), - "--member", "IRRS", "--to", after) + args = ["bump", "--family-root", str(holder), "--member", "IRRS", "--to", + after] + + refused = run_script(FAMILY, *args) + assert refused.returncode == 2, refused.stderr + refused.stdout + assert refused.stderr.startswith("REFUSED family-manifest-uncommitted: ") + git("checkout", "HEAD", "--", "family.yaml", cwd=holder) + result = run_script(FAMILY, *args) assert result.returncode == 0, result.stderr + result.stdout assert "nothing to do" not in result.stdout diff --git a/tests/test_family_bump_remove_rollback.py b/tests/test_family_bump_remove_rollback.py index 3dcfe47c..b6122367 100644 --- a/tests/test_family_bump_remove_rollback.py +++ b/tests/test_family_bump_remove_rollback.py @@ -429,13 +429,16 @@ def test_a_remove_dry_run_under_a_held_index_lock_is_unchanged( # --- refusals a dry run asks first are asked first (#365) ----------------- def comment_on_members(root: Path, env: dict) -> None: - """A comment on the `members:` line, uncommitted: YAML reads past it and - the line rewrite does not (`family-members-unrecognised`).""" + """A comment on the `members:` line, committed: YAML reads past it and + the line rewrite does not (`family-members-unrecognised`). Committed, + since the verbs refuse a `family.yaml` holding a change HEAD does not + first (#684).""" manifest_path = root / "family.yaml" text = manifest_path.read_bytes() assert text.count(b"\nmembers:\n") == 1, "fixture drift" manifest_path.write_bytes(text.replace( b"\nmembers:\n", b"\nmembers: # edited by hand\n")) + raw(root, env, "commit", "-q", "-m", "A hand edit", "--", "family.yaml") @pytest.mark.parametrize("verb", ["remove", "bump"]) diff --git a/tests/test_family_dry_run.py b/tests/test_family_dry_run.py index b024492e..ac832dc8 100644 --- a/tests/test_family_dry_run.py +++ b/tests/test_family_dry_run.py @@ -777,15 +777,12 @@ def test_a_dry_run_asks_the_manifest_refusal_the_real_run_asks_first_too( ask it after, with the holder changed). The dry run asks it too, exit 2, before anything is written. The hand edit is the one `test_family_add_refusals.py` makes: a comment on the `members:` line, - which YAML reads past and the line rewrite does not, left uncommitted. - The real runs are held to the same holder in + which YAML reads past and the line rewrite does not, committed, since + the verbs refuse a `family.yaml` holding a change HEAD does not first + (#684). The real runs are held to the same holder in `test_family_bump_remove_rollback.py`.""" env = isolated(offline, tmp_path) - manifest_path = holder / "family.yaml" - text = manifest_path.read_bytes().decode("utf-8") - assert text.count("\nmembers:\n") == 1, "fixture drift" - manifest_path.write_bytes(text.replace( - "\nmembers:\n", "\nmembers: # edited by hand\n").encode("utf-8")) + a_members_line_edited_by_hand(holder) before = state(holder, env) extra = ["--to", older(family)] if verb == "bump" else [] @@ -1042,12 +1039,15 @@ def a_cached_store(root: Path) -> None: def a_members_line_edited_by_hand(root: Path) -> None: """A comment on the `members:` line, which YAML reads past and the line - rewrite does not, left uncommitted.""" + rewrite does not, committed: the verbs refuse a `family.yaml` holding a + change HEAD does not before this (#684).""" manifest_path = root / "family.yaml" text = manifest_path.read_bytes().decode("utf-8") assert text.count("\nmembers:\n") == 1, "fixture drift" manifest_path.write_bytes(text.replace( "\nmembers:\n", "\nmembers: # edited by hand\n").encode("utf-8")) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "A hand edit", "--", "family.yaml", cwd=root) def a_renamed_copy_of_a_member(root: Path) -> Path: diff --git a/tests/test_family_holder_states.py b/tests/test_family_holder_states.py index fe6b3003..e38e3d17 100644 --- a/tests/test_family_holder_states.py +++ b/tests/test_family_holder_states.py @@ -18,10 +18,24 @@ - **A merge conflicted in `family.yaml` itself** (N4). `yaml-unparsable` keeps its code and its line, and gains a remediation naming the merge and its exits; asked only once the parse has failed. -- **A `members` file HEAD holds and the index no longer does** (E1): the - move #612's own remedy names, staged and not committed, or a removal - staged. `add` went through and its commit took `D members` into itself; - now it is refused before the member is read. +- **A `members` file HEAD holds and the work tree no longer does** (E1): + the move #612's own remedy names, staged and not committed, or a removal + staged; or the file deleted and the deletion not staged. `add` went + through and its commit took `D members` into itself; now it is refused + before the member is read. The unstaged deletion is taken out of the + index with `git rm --cached` and the removal committed: the issue's `git + mv` fails there (the premise, measured here). +- **A `family.yaml` holding a change HEAD does not** (#459's line from + #571's review, on Brett Heap's word relayed by lane 5 on #473 at + 2026-10-16T13:56:50Z, "refuse a dirty `family.yaml`"). `add`, `bump` and + `remove` commit the file as the work tree has it, so a comment line + edited by hand went into their commit; each now refuses it, staged or + not, and git's two ways out, typed as printed, let the command run. + `member-path`'s remedy says to commit a row put back by hand, so it no + longer leads into this refusal. + +#612's review E4 (reftable) is NOT here: this host's git is 2.43, and +the issue asks for it to be measured on git 2.45 or later first. THE PROOF IS #612's, with the helpers of `test_family_refuse_up_front.py`: exit 2 and one REFUSED paragraph, nothing on stdout (no member was read), @@ -46,7 +60,7 @@ from test_family_add_excludes import empty_holder # noqa: F401 from test_family_add_refusals import holder, offline, refusal # noqa: F401 from test_family_dry_run import add, add_state, assert_add_as_it_was, isolated -from test_family_member_path import MEMBER, raw +from test_family_member_path import GLOB, MEMBER, hand_edit, raw #: #612's proof of a refusal before the first write, and its states. from test_family_refuse_up_front import ( NOTES, VERBS, assert_refused_alike, commit_file, head_of, mark_of, @@ -410,7 +424,7 @@ def test_a_family_yaml_outside_any_repository_keeps_its_refusal( assert ran.stderr == unparsable_line(plain, 2) + "\n", ran.stderr -# --- a members file HEAD holds and the index no longer does (E1) ------------ +# --- a members file HEAD holds and the work tree no longer does (E1) -------- MOVED_LINE = ( "REFUSED member-path-unaddable: git would add the mount path " @@ -426,12 +440,64 @@ def test_a_family_yaml_outside_any_repository_keeps_its_refusal( "first (`git commit`), since the add's own commit names .gitmodules, " f"members/{SPARE} and family.yaml alone, and run the same command " "again.") +DELETED_LINE = ( + "REFUSED member-path-unaddable: git would add the mount path " + f"members/{SPARE} and take `members` out of the holder with it: " + "`members` is a file HEAD and the index hold and the work tree no " + "longer does, its deletion not staged, and the add's mount, putting a " + "directory where the file was, would take it out of the index, and its " + "commit would take that deletion into itself (`D members`). No git " + "command that writes was run and nothing was written.") +DELETED_REMEDY = ( + "Remediation: every member is mounted under members/, which must be a " + "plain directory of the holder's own. Take the file out of the index, " + "which touches no file (`git rm --cached -- members`), and commit the " + "removal on its own first (`git commit`), since the add's own commit " + f"names .gitmodules, members/{SPARE} and family.yaml alone, and run the " + "same command again.") #: `{id: the git command that leaves the tracked file staged away}`: the #: move #612's remedy names, and the file's removal. STAGED_AWAY = {"git mv": ("mv", "--", "members", "members-file"), "git rm": ("rm", "-q", "--", "members")} +#: What is at `members` once the file is deleted and the deletion is not +#: staged: nothing, or a directory a hand made there with a file of the +#: person's in it, which `git checkout -- members` would take away. +LEFT_THERE = {"nothing": None, "a-directory": "NOTES.txt"} + + +def a_members_file(root: Path, env: dict) -> None: + """A FILE named `members`, committed: what `_members_is_a_file` refuses + while it is on disk.""" + (root / "members").write_bytes(b"a file where the directory goes\n") + raw(root, env, "add", "-f", "--", "members") + raw(root, env, "commit", "-q", "-m", "a file named members") + + +def add_refused(family, root: Path, env: dict, dry_run: bool, line: str, + remedy: str) -> None: + """`add`, or its dry run, refused with `line` and `remedy` before the + member is read: nothing on stdout, and the holder as it was.""" + before = add_state(root, env) + ran = add(root, env, family["remotes"], + *(("--dry-run",) if dry_run else ())) + assert ran.returncode == 2, ran.stdout + ran.stderr + assert ran.stdout == "" + assert refusal(ran.stderr) == (line, remedy) + assert_add_as_it_was(root, env, before) + + +def adds_its_own_three_paths(family, root: Path, env: dict) -> None: + """`add` once the remedy is typed: exit 0 and a commit of `.gitmodules`, + `family.yaml` and the mount, and of nothing else (no `D members`).""" + done = add(root, env, family["remotes"]) + assert done.returncode == 0, done.stdout + done.stderr + shown = raw(root, env, "show", "--name-status", "--format=", "HEAD" + ).stdout.decode().split("\n") + assert sorted(entry.split("\t")[1] for entry in shown if entry) == [ + ".gitmodules", MANIFEST, f"members/{SPARE}"], shown + @pytest.mark.parametrize("dry_run", [False, True], ids=["real", "dry-run"]) @pytest.mark.parametrize("change", STAGED_AWAY.values(), ids=STAGED_AWAY) @@ -447,28 +513,205 @@ def test_add_refuses_a_members_file_whose_move_is_staged_not_committed( and leaves nothing staged.""" holder = empty_holder env = isolated(offline, tmp_path) - (holder / "members").write_bytes(b"a file where the directory goes\n") - raw(holder, env, "add", "-f", "--", "members") - raw(holder, env, "commit", "-q", "-m", "a file named members") + a_members_file(holder, env) raw(holder, env, *change) assert not os.path.lexists(holder / "members"), "premise: not on disk" assert b"\tmembers\0" not in raw(holder, env, "ls-files", "-s", "-z" ).stdout, "premise: not in the index" - before = add_state(holder, env) - extra = ("--dry-run",) if dry_run else () - ran = add(holder, env, family["remotes"], *extra) - assert ran.returncode == 2, ran.stdout + ran.stderr - assert ran.stdout == "" - line, remedy = refusal(ran.stderr) - assert line == MOVED_LINE - assert remedy == MOVED_REMEDY - assert_add_as_it_was(holder, env, before) + add_refused(family, holder, env, dry_run, MOVED_LINE, MOVED_REMEDY) raw(holder, env, "commit", "-q", "-m", "the change, on its own") - done = add(holder, env, family["remotes"]) - assert done.returncode == 0, done.stdout + done.stderr - shown = raw(holder, env, "show", "--name-status", "--format=", "HEAD" - ).stdout.decode().split("\n") - assert sorted(entry.split("\t")[1] for entry in shown if entry) == [ - ".gitmodules", MANIFEST, f"members/{SPARE}"], shown + adds_its_own_three_paths(family, holder, env) assert raw(holder, env, "status", "--porcelain").stdout == b"" + + +@pytest.mark.parametrize("dry_run", [False, True], ids=["real", "dry-run"]) +@pytest.mark.parametrize("left", LEFT_THERE.values(), ids=LEFT_THERE) +def test_add_refuses_a_members_file_deleted_and_the_deletion_not_staged( + family, empty_holder, offline, tmp_path, left, dry_run): + """E1's OTHER HALF: the tracked `members` file deleted from disk, the + deletion not staged, and nothing at `members` but, it may be, a + directory a hand made there. The index still holds the file, and the + `git mv` the issue proposed fails there, exit 128, changing nothing + (premises, measured here). `add` went through and its commit took `D + members`; now `member-path-unaddable`, nothing on stdout, the holder as + it was. Typed as printed, `git rm --cached -- members` and the commit + of the removal, `add` makes its commit of its own three paths alone, + and the person's directory and the file in it are still there.""" + holder = empty_holder + env = isolated(offline, tmp_path) + a_members_file(holder, env) + (holder / "members").unlink() + if left is not None: + (holder / "members").mkdir() + (holder / "members" / left).write_bytes(b"the person's own\n") + assert b"\tmembers\0" in raw(holder, env, "ls-files", "-s", "-z" + ).stdout, "premise: the index holds it" + status = raw(holder, env, "status", "--porcelain").stdout + moved = raw(holder, env, "mv", "--", "members", "members-file", + check=False) + assert moved.returncode == 128, moved.stderr + assert raw(holder, env, "status", "--porcelain").stdout == status + add_refused(family, holder, env, dry_run, DELETED_LINE, DELETED_REMEDY) + + raw(holder, env, "rm", "-q", "--cached", "--", "members") + raw(holder, env, "commit", "-q", "-m", "the removal, on its own") + adds_its_own_three_paths(family, holder, env) + if left is None: + assert raw(holder, env, "status", "--porcelain").stdout == b"" + else: + assert (holder / "members" / left).read_bytes() == \ + b"the person's own\n" + assert raw(holder, env, "status", "--porcelain").stdout == ( + f"?? members/{left}\n".encode()) + + +# --- a family.yaml holding a change HEAD does not (#459's line 66) ---------- + +#: The line's own example, a hand edit outside `members:`: a comment line, +#: put first in the file, where no verb's rewrite of `members:` reaches. +COMMENT = b"# a comment line, edited by hand\n" + +#: What the refusal says of each way the file holds such a change. +SAID_UNSTAGED = ("git lists the work tree's copy as modified, a change " + "nobody has staged") +SAID_STAGED = "a change to it is staged and not committed" +SAID_UNMERGED = "it is unmerged in the index" +SAID_DELETION = "its deletion is staged, so the index holds no copy" + +UNCOMMITTED_REMEDY = ( + "Remediation: which change to keep is a person's to say. To keep it, " + "commit it on its own first, `git commit -- family.yaml`; to drop it, " + "put back HEAD's copy, in the index and the work tree, `git checkout " + "HEAD -- family.yaml`. Then run the same command again.") + +#: The remedy's two ways out, as printed. +KEEP = ("commit", "-q", "-m", "the hand edit, on its own", "--", MANIFEST) +DROP = ("checkout", "HEAD", "--", MANIFEST) + + +def uncommitted_said(verb: str, *said: str) -> tuple: + """`(the REFUSED line as a pattern, the remediation)` of + `family-manifest-uncommitted` for `verb`, saying each of `said`.""" + line = ( + "REFUSED family-manifest-uncommitted: family.yaml holds a change " + f"HEAD does not: {', and '.join(said)}. `{verb}` rewrites " + "family.yaml as the work tree has it and commits it, so that change " + "would go into the command's commit beside its own. No git command " + "that writes was run and nothing was written.") + return re.compile(re.escape(line)), UNCOMMITTED_REMEDY + + +def commented(root: Path, comment: bytes = COMMENT) -> None: + """`comment` written first in `family.yaml` by hand, not staged.""" + manifest = root / MANIFEST + manifest.write_bytes(comment + manifest.read_bytes()) + + +@pytest.mark.parametrize("verb", VERBS) +def test_an_unstaged_hand_edit_to_family_yaml_is_refused_in_both_runs( + family, holder, offline, tmp_path, verb): + """THE LINE'S CASE: a comment line edited into `family.yaml` by hand + and never staged went into the verb's commit beside its own row. Each + verb now refuses it, in both runs alike, nothing on stdout, the holder + as it was and the edit still in the work tree. Typed as printed, `git + commit -- family.yaml` commits the edit on its own, and the verb then + makes its one commit on top: the comment is the person's commit's, and + the verb's commit leaves it as it was.""" + env = isolated(offline, tmp_path) + commented(holder) + assert_refused_alike(verb, family, holder, env, + uncommitted_said(verb, SAID_UNSTAGED)) + assert (holder / MANIFEST).read_bytes().startswith(COMMENT) + raw(holder, env, *KEEP) + runs_after(verb, family, holder, env) + for commit in ("HEAD~1", "HEAD"): + assert raw(holder, env, "show", f"{commit}:{MANIFEST}" + ).stdout.startswith(COMMENT), commit + + +def staged(root: Path, env: dict) -> None: + """The comment, staged and not committed.""" + commented(root) + raw(root, env, "add", "--", MANIFEST) + + +def staged_then_edited(root: Path, env: dict) -> None: + """The comment staged, and a second one written after it, not staged.""" + staged(root, env) + commented(root, b"# and another\n") + + +def deletion_staged(root: Path, env: dict) -> None: + """`git rm --cached -- family.yaml`: the copy on disk is HEAD's, and the + index holds none.""" + raw(root, env, "rm", "-q", "--cached", "--", MANIFEST) + + +def unmerged(root: Path, env: dict) -> None: + """`family.yaml` unmerged with NO ACT in progress, as a `git stash pop` + that conflicted leaves it (git keeps no `MERGE_HEAD` for it, the + premise), resolved in the work tree by hand, both lines kept, and not + staged.""" + commented(root, b"# stashed\n") + raw(root, env, "stash", "-q") + commented(root, b"# committed\n") + raw(root, env, "commit", "-q", "-m", "a comment", "--", MANIFEST) + popped = raw(root, env, "stash", "pop", check=False) + assert popped.returncode != 0, popped.stdout + assert not os.path.lexists(mark_of(root, env, "MERGE_HEAD")), "premise" + (root / MANIFEST).write_bytes( + b"# stashed\n" + raw(root, env, "show", f"HEAD:{MANIFEST}").stdout) + + +#: `{id: (how the holder gets the change, what the refusal says of it, the +#: way out typed)}`. +STATES = { + "staged": (staged, (SAID_STAGED,), DROP), + "staged-then-edited": (staged_then_edited, + (SAID_STAGED, SAID_UNSTAGED), KEEP), + "deletion-staged": (deletion_staged, (SAID_DELETION,), DROP), + "unmerged": (unmerged, (SAID_UNMERGED,), KEEP), +} + + +@pytest.mark.parametrize("set_up, said, typed", STATES.values(), ids=STATES) +def test_a_change_to_family_yaml_in_the_index_is_refused_too( + family, holder, offline, tmp_path, set_up, said, typed): + """A CHANGE STAGED AND NOT COMMITTED IS REFUSED AS WELL, since the + commit takes the work tree's copy all the same: staged, staged and then + edited again, a deletion staged over HEAD's own copy, and a copy left + unmerged with no act in progress. `remove` refuses each in both runs + alike, saying which it is, with the holder as it was; the way out typed + as printed lets it run.""" + env = isolated(offline, tmp_path) + set_up(holder, env) + assert_refused_alike("remove", family, holder, env, + uncommitted_said("remove", *said)) + raw(holder, env, *typed) + runs_after("remove", family, holder, env) + + +def test_a_row_put_back_as_member_path_says_is_committed_and_the_verb_runs( + family, holder, offline, tmp_path): + """THE REMEDY THAT EDITS `family.yaml` NO LONGER LEADS INTO THIS + REFUSAL. A `path:` edited by hand AND COMMITTED is `member-path`'s, + whose remedy now says to commit the row put back. Typed so, the verb + runs; put back and not committed, it is refused as uncommitted.""" + env = isolated(offline, tmp_path) + good = (holder / MANIFEST).read_bytes() + hand_edit(holder, MEMBER, GLOB) + raw(holder, env, "commit", "-q", "-m", "a hand edit", "--", MANIFEST) + ran = run_verb("remove", family, holder, env) + assert ran.returncode == 2, ran.stdout + ran.stderr + assert refusal(ran.stderr)[1] == ( + "Remediation: this tool writes that field itself, so a different " + f"value is a hand edit. Put `path: members/{MEMBER}` back in " + "family.yaml and commit that, or `git checkout -- family.yaml` if " + "the edit is not committed, then run the same command again.") + (holder / MANIFEST).write_bytes(good) + assert_refused_alike("remove", family, holder, env, + uncommitted_said("remove", SAID_UNSTAGED)) + raw(holder, env, "commit", "-q", "-m", "the row put back", "--", + MANIFEST) + runs_after("remove", family, holder, env) diff --git a/tests/test_family_member_path.py b/tests/test_family_member_path.py index a3734a5d..26f19d8a 100644 --- a/tests/test_family_member_path.py +++ b/tests/test_family_member_path.py @@ -217,8 +217,8 @@ def test_a_row_whose_path_is_not_the_members_is_refused_and_nothing_moves( assert remedy == ( "Remediation: this tool writes that field itself, so a different " f"value is a hand edit. Put `path: members/{MEMBER}` back in " - "family.yaml, or `git checkout -- family.yaml` if the edit is not " - "committed, then run the same command again.") + "family.yaml and commit that, or `git checkout -- family.yaml` if " + "the edit is not committed, then run the same command again.") assert state(holder, env) == before neighbour_intact(holder, env) assert not offline.gh_log.exists(), offline.gh_log.read_text() @@ -313,8 +313,8 @@ def test_a_row_whose_project_is_not_a_directory_name_is_refused( "edit. Restore family.yaml from git (`git checkout -- family.yaml` " "if the edit is not committed), or put the row's `project:` and " "`path:` back as the tool wrote them (`project: `, `path: " - "members/`), then run the command again naming that " - "member.") + "members/`) and commit that, then run the command again " + "naming that member.") assert state(holder, env) == before for mounted in MEMBERS: entry = raw(holder, env, "ls-tree", "HEAD", "--", f"members/{mounted}" @@ -408,10 +408,13 @@ def test_a_row_with_the_members_own_path_or_none_still_exits_zero_in_one_commit( """THE CONTROLS: the row the tool writes, and a row with no `path:` at all, which keeps defaulting to `members/`. Each is still ONE commit of exactly the paths the verb is for, and the holder still - validates.""" + validates. The hand edit is committed first: the verbs refuse a + `family.yaml` holding a change HEAD does not (#684).""" env = isolated(offline, tmp_path) if edit == "no-path-line": hand_edit(holder, MEMBER, None) + raw(holder, env, "commit", "-q", "-m", "a row with no path line", + "--", "family.yaml") was = commits(holder, env) result = run_script(FAMILY, *verb_args(verb, holder, family), env=env) From bc8024fce347c0a54d864890b8611f3a3357af68 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 12:11:46 +0000 Subject: [PATCH 3/5] family.py: the `members` remedies say a bare `git commit` takes every staged change, `family-members-unrecognised` says to commit the fix, and a dirty `family.yaml` is compared by mode as well as object and told the drop alone beside a staged gitlink (#684) Fix round 1, lane 1's review of #816 at 772105aa (issuecomment-6233536729), on lane 4's rulings. R1. Both remedies in `_members_file_left_in_head` said to commit "on its own" with a bare `git commit`, which takes every staged change: lane 1 measured an unrelated staged file committed beside `D members`, and beside the staged move. Each now says "(`git commit`, with nothing else staged: a bare `git commit` takes every staged change, and `git status` lists them)", lane 1's words. Not `git commit -- members`: it exits 128 where a directory is on disk (lane 1 measured). R2. `family-members-unrecognised` said "fix it by hand this once.", and the run again then met `family-manifest-uncommitted`. That refusal is reached only once the shape it names is committed, so it now says "fix it by hand this once and commit that (`git commit -- family.yaml`), then run the same command again." F4. "Dirty" compared the index's object with HEAD's and not its mode: a `chmod 755` staged went into `remove`'s commit as `mode change 100644 => 100755 family.yaml` (lane 1 measured). HEAD's entry is now read, mode and object, from `git ls-tree -r -z HEAD` with no pathspec (`_head_entries`), and compared with the stage-0 record. A mode staged with `git update-index --chmod=+x`, under each verb, is a new row; there `git commit -- family.yaml` has nothing to commit (measured), and the drop is the way out typed. F5. Where the index also stages a gitlink under `members/` that HEAD does not hold, as #210's half-finished bump leaves one, the "keep" way out committed the row without its gitlink (lane 1 measured). The refusal there names the drop alone (`_gitlink_staged`), and the same command run again makes the one commit of both paths. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 103 +++++++++++++++++++++++------ tests/test_family.py | 13 ++-- tests/test_family_add_refusals.py | 4 +- tests/test_family_holder_states.py | 95 +++++++++++++++++++++++--- 4 files changed, 177 insertions(+), 38 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 61812f98..cab6d9a4 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -378,6 +378,11 @@ def rewritten(self, rows: list[dict]) -> str: still exactly as the human left it, rather than after `git submodule add` has staged a gitlink and a `.gitmodules` section that the refusal would then leave behind. + + THE FIX BY HAND IS COMMITTED before the run again (#684): the verbs + refuse a `family.yaml` holding a change HEAD does not + (`_checked_manifest_committed`), and this refusal is only reached + once the shape it names is committed. """ text = self.path.read_text(encoding="utf-8") lines = text.splitlines() @@ -399,7 +404,8 @@ def rewritten(self, rows: list[dict]) -> str: "Remediation: the manifest was hand-edited into a shape this " "tool does not recognise. `members:` is the LAST block in the " "file and everything below it belongs to this tool; fix it by " - "hand this once.") + "hand this once and commit that (`git commit -- " + f"{MANIFEST}`), then run the same command again.") return "\n".join(out) + "\n" @@ -1190,12 +1196,15 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: row back by hand (`member-path`, `_member_path`) say to commit that. ASKED OF GIT, AND NOTHING WRITTEN: the index's records for the file - (`_index_records`, no pathspec), the blob HEAD holds (`_head_oid`), and - `git ls-files -m`, git's own answer to "modified", which compares the - work tree's copy with the index's by content where their stat data - differ. Not `git hash-object`, which a dry run asks only for `bump`'s - `nothing to do`; so a copy the index marks assume-unchanged, which git - does not compare, is not seen. + (`_index_records`, no pathspec), HEAD's entry for it, MODE AND OBJECT, + from `git ls-tree -r -z HEAD` with no pathspec (a mode change staged + and not committed went into the commit too, measured with `remove`: + `mode change 100644 => 100755 family.yaml`), and `git ls-files -m`, + git's own answer to "modified", which compares the work tree's copy + with the index's by content where their stat data differ. Not `git + hash-object`, which a dry run asks only for `bump`'s `nothing to do`; + so a copy the index marks assume-unchanged, which git does not compare, + is not seen. A HOLDER WHOSE HEAD HOLDS NO `family.yaml`, as one made by hand with no commit yet does, has no committed copy for a change to differ from: the @@ -1206,16 +1215,24 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: -- family.yaml` commits the work tree's copy and nothing else staged, unmerged or not; `git checkout HEAD -- family.yaml` puts HEAD's back in the index and the work tree. Where the change is a staged deletion over - a copy that is HEAD's byte for byte, the first has nothing to commit, - and the second is the way out. + a copy that is HEAD's byte for byte, or a mode staged with `git + update-index --chmod` that the work tree's copy does not have, the + first has nothing to commit, and the second is the way out. + + A GITLINK UNDER `members/` STAGED BESIDE IT (#210's half-finished bump, + lane 1's review of #816, F5) is told the second ALONE: there the first + commits the row without its gitlink, so the row and the gitlink HEAD + holds disagree (measured), while the second, and the same command run + again, makes the one commit of both. """ - head = _head_oid(root, MANIFEST) + held = _head_entries(root) + head = held.get(MANIFEST.encode()) if head is None: return + index = _index_records(root) records = [record.partition(b"\t")[0].split() - for record in _index_records(root).get(MANIFEST.encode(), [])] - merged = [oid.decode("ascii") for _, oid, stage in records - if stage == b"0"] + for record in index.get(MANIFEST.encode(), [])] + merged = [(mode, oid) for mode, oid, stage in records if stage == b"0"] if len(merged) != len(records): # `ls-files -m` lists an unmerged path once for each stage the work # tree's copy differs from, which says nothing more. @@ -1236,10 +1253,49 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: f"`{verb}` rewrites {MANIFEST} as the work tree has it and commits " "it, so that change would go into the command's commit beside its " "own. No git command that writes was run and nothing was written.", - "Remediation: which change to keep is a person's to say. To keep it, " - f"commit it on its own first, `git commit -- {MANIFEST}`; to drop " - "it, put back HEAD's copy, in the index and the work tree, `git " - f"checkout HEAD -- {MANIFEST}`. Then run the same command again.") + ("Remediation: the index also stages a gitlink under " + f"{MEMBERS_DIR}/ that HEAD does not hold, as a run stopped before " + f"its commit leaves it, and `git commit -- {MANIFEST}` would commit " + "the row without it: put back HEAD's copy, in the index and the " + f"work tree, `git checkout HEAD -- {MANIFEST}`." + if _gitlink_staged(index, held) else + "Remediation: which change to keep is a person's to say. To keep " + f"it, commit it on its own first, `git commit -- {MANIFEST}`; to " + "drop it, put back HEAD's copy, in the index and the work tree, " + f"`git checkout HEAD -- {MANIFEST}`.") + + " Then run the same command again.") + + +def _head_entries(root: Path) -> dict[bytes, tuple[bytes, bytes]]: + """`{path: (mode, object)}` of every file and gitlink HEAD holds, by + `git ls-tree -r -z HEAD` with NO PATHSPEC, as `_tree_entries` reads a + tree, the mode kept as git prints it; none where HEAD names no commit + yet, or none git can read (#684).""" + try: + listed = _git_bytes(["ls-tree", "-r", "-z", "HEAD"], root) + except CommandFailed: + return {} + held = {} + for record in listed.split(b"\0"): + meta, _, name = record.partition(b"\t") + if meta: + mode, _, oid = meta.split() + held[name] = (mode, oid) + return held + + +def _gitlink_staged(index: dict[bytes, list[bytes]], + held: dict[bytes, tuple[bytes, bytes]]) -> bool: + """Does `index` (`_index_records`) stage a gitlink under `members/` that + HEAD (`_head_entries`) does not hold, mode and commit, as a run stopped + between its stage and its commit leaves one (#684; lane 1's review of + #816, F5)?""" + prefix = f"{MEMBERS_DIR}/".encode() + return any( + mode == b"160000" and held.get(name) != (mode, oid) + for name, found in index.items() if name.startswith(prefix) + for mode, oid, _ in (record.partition(b"\t")[0].split() + for record in found)) def _sparse_rules_exclude(root: Path, paths: list[str]) -> list[str]: @@ -3447,8 +3503,10 @@ def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: "which must be a plain directory of the holder's own. Take the " "file out of the index, which touches no file (`git rm --cached " f"-- {MEMBERS_DIR}`), and commit the removal on its own first " - f"(`git commit`), since the add's own commit names {GITMODULES}, " - f"{path} and {MANIFEST} alone, and run the same command again.") + "(`git commit`, with nothing else staged: a bare `git commit` " + "takes every staged change, and `git status` lists them), since " + f"the add's own commit names {GITMODULES}, {path} and {MANIFEST} " + "alone, and run the same command again.") return Refusal( "member-path-unaddable", f"git would add the mount path {path} and take `{MEMBERS_DIR}` out of " @@ -3460,9 +3518,10 @@ def _members_file_left_in_head(root: Path, path: str) -> Refusal | None: "nothing was written.", f"Remediation: every member is mounted under {MEMBERS_DIR}/, which " "must be a plain directory of the holder's own. Commit that change " - "on its own first (`git commit`), since the add's own commit names " - f"{GITMODULES}, {path} and {MANIFEST} alone, and run the same command " - "again.") + "on its own first (`git commit`, with nothing else staged: a bare " + "`git commit` takes every staged change, and `git status` lists " + f"them), since the add's own commit names {GITMODULES}, {path} and " + f"{MANIFEST} alone, and run the same command again.") def _members_is_a_file(root: Path, path: str) -> Refusal: diff --git a/tests/test_family.py b/tests/test_family.py index 169ae191..34cd29e8 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -1113,11 +1113,12 @@ def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): before `git commit` — a hook that refused, a lock — leaves exactly this. Reading `was == --to` as "nothing to do" would call the half-finished bump done and leave the commit for somebody to make by hand (#210). Since #684 - the staged `family.yaml` is refused first, as a change HEAD does not hold; - the way out that drops it, `git checkout HEAD -- family.yaml`, leaves the - gitlink staged, and the same bump then finishes in ONE commit of both - paths. (The other way out, `git commit -- family.yaml`, would commit the - row without its gitlink.)""" + the staged `family.yaml` is refused first, as a change HEAD does not hold, + and with the gitlink staged beside it the refusal names one way out + alone: `git checkout HEAD -- family.yaml`, which leaves the gitlink + staged, and the same bump then finishes in ONE commit of both paths. + (`git commit -- family.yaml` would commit the row without its + gitlink.)""" member = holder / "members" / "IRRS" before = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() after = commit_in_member(member) @@ -1133,6 +1134,8 @@ def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): refused = run_script(FAMILY, *args) assert refused.returncode == 2, refused.stderr + refused.stdout assert refused.stderr.startswith("REFUSED family-manifest-uncommitted: ") + assert ("Remediation: the index also stages a gitlink under members/ " + in refused.stderr), refused.stderr git("checkout", "HEAD", "--", "family.yaml", cwd=holder) result = run_script(FAMILY, *args) diff --git a/tests/test_family_add_refusals.py b/tests/test_family_add_refusals.py index 7a037a25..4a0caf24 100644 --- a/tests/test_family_add_refusals.py +++ b/tests/test_family_add_refusals.py @@ -450,7 +450,9 @@ def test_a_manifest_this_tool_will_not_rewrite_is_refused_before_the_mount( "exactly one was expected, so this tool will not rewrite it", "Remediation: the manifest was hand-edited into a shape this tool " "does not recognise. `members:` is the LAST block in the file and " - "everything below it belongs to this tool; fix it by hand this once.") + "everything below it belongs to this tool; fix it by hand this once " + "and commit that (`git commit -- family.yaml`), then run the same " + "command again.") assert_unchanged(holder, before, SPARE) offline.assert_left_nothing() diff --git a/tests/test_family_holder_states.py b/tests/test_family_holder_states.py index e38e3d17..13d0cdda 100644 --- a/tests/test_family_holder_states.py +++ b/tests/test_family_holder_states.py @@ -30,9 +30,11 @@ 2026-10-16T13:56:50Z, "refuse a dirty `family.yaml`"). `add`, `bump` and `remove` commit the file as the work tree has it, so a comment line edited by hand went into their commit; each now refuses it, staged or - not, and git's two ways out, typed as printed, let the command run. - `member-path`'s remedy says to commit a row put back by hand, so it no - longer leads into this refusal. + not, its mode compared as well as its object, and git's two ways out, + typed as printed, let the command run; where a gitlink under `members/` + is staged beside it, as #210's half-finished bump leaves one, only the + way out that drops it is named. `member-path`'s remedy says to commit a + row put back by hand, so it no longer leads into this refusal. #612's review E4 (reftable) is NOT here: this host's git is 2.43, and the issue asks for it to be measured on git 2.45 or later first. @@ -60,7 +62,7 @@ from test_family_add_excludes import empty_holder # noqa: F401 from test_family_add_refusals import holder, offline, refusal # noqa: F401 from test_family_dry_run import add, add_state, assert_add_as_it_was, isolated -from test_family_member_path import GLOB, MEMBER, hand_edit, raw +from test_family_member_path import GLOB, MEMBER, hand_edit, older, raw #: #612's proof of a refusal before the first write, and its states. from test_family_refuse_up_front import ( NOTES, VERBS, assert_refused_alike, commit_file, head_of, mark_of, @@ -437,9 +439,10 @@ def test_a_family_yaml_outside_any_repository_keeps_its_refusal( MOVED_REMEDY = ( "Remediation: every member is mounted under members/, which must be a " "plain directory of the holder's own. Commit that change on its own " - "first (`git commit`), since the add's own commit names .gitmodules, " - f"members/{SPARE} and family.yaml alone, and run the same command " - "again.") + "first (`git commit`, with nothing else staged: a bare `git commit` " + "takes every staged change, and `git status` lists them), since the " + f"add's own commit names .gitmodules, members/{SPARE} and family.yaml " + "alone, and run the same command again.") DELETED_LINE = ( "REFUSED member-path-unaddable: git would add the mount path " f"members/{SPARE} and take `members` out of the holder with it: " @@ -452,9 +455,11 @@ def test_a_family_yaml_outside_any_repository_keeps_its_refusal( "Remediation: every member is mounted under members/, which must be a " "plain directory of the holder's own. Take the file out of the index, " "which touches no file (`git rm --cached -- members`), and commit the " - "removal on its own first (`git commit`), since the add's own commit " - f"names .gitmodules, members/{SPARE} and family.yaml alone, and run the " - "same command again.") + "removal on its own first (`git commit`, with nothing else staged: a " + "bare `git commit` takes every staged change, and `git status` lists " + "them), since the add's own commit names .gitmodules, " + f"members/{SPARE} and family.yaml alone, and run the same command " + "again.") #: `{id: the git command that leaves the tracked file staged away}`: the #: move #612's remedy names, and the file's removal. @@ -692,6 +697,76 @@ def test_a_change_to_family_yaml_in_the_index_is_refused_too( runs_after("remove", family, holder, env) +@pytest.mark.parametrize("verb", VERBS) +def test_a_mode_change_staged_to_family_yaml_is_refused_too( + family, holder, offline, tmp_path, verb): + """THE MODE IS COMPARED AS WELL AS THE OBJECT (lane 1's review of #816, + F4): a `remove` over a `chmod 755` and `git add` committed `mode change + 100644 => 100755 family.yaml` beside its own change. Staged with `git + update-index --chmod=+x`, which stages it on every disk, each verb + refuses it in both runs alike. Where the disk keeps modes, git lists the + work tree's copy, still 644, as modified as well, and the refusal says + what git lists (the premise, asked of git). `git checkout HEAD -- + family.yaml`, typed as printed, lets the verb run; `git commit -- + family.yaml` would have nothing to commit there, the work tree's copy + being HEAD's.""" + env = isolated(offline, tmp_path) + raw(holder, env, "update-index", "--chmod=+x", "--", MANIFEST) + assert raw(holder, env, "ls-files", "-s", "--", MANIFEST + ).stdout.startswith(b"100755 "), "premise: the mode is staged" + assert raw(holder, env, "ls-tree", "HEAD", "--", MANIFEST + ).stdout.startswith(b"100644 "), "premise: HEAD's is not" + listed = MANIFEST.encode() in raw(holder, env, "ls-files", "-m", "-z" + ).stdout.split(b"\0") + said = (SAID_STAGED, *((SAID_UNSTAGED,) if listed else ())) + assert_refused_alike(verb, family, holder, env, + uncommitted_said(verb, *said)) + raw(holder, env, *DROP) + runs_after(verb, family, holder, env) + + +#: The remediation where a gitlink under `members/` is staged beside the +#: change (F5): the way out that drops it, alone. +DROP_ALONE_REMEDY = ( + "Remediation: the index also stages a gitlink under members/ that HEAD " + "does not hold, as a run stopped before its commit leaves it, and `git " + "commit -- family.yaml` would commit the row without it: put back " + "HEAD's copy, in the index and the work tree, `git checkout HEAD -- " + "family.yaml`. Then run the same command again.") + + +def test_a_bump_stopped_before_its_commit_is_told_the_drop_alone( + family, holder, offline, tmp_path): + """#210'S HALF-FINISHED BUMP (lane 1's review of #816, F5): the row + rewritten to name an older commit of the member, and its gitlink moved + there, both staged, as a bump stopped between its stage and its commit + leaves them. "Keep" there commits the row without its gitlink, so HEAD's + row and gitlink disagree (lane 1 measured it); the refusal names the + drop ALONE, in both runs alike. Typed as printed, the same bump then + makes its one commit of both paths.""" + env = isolated(offline, tmp_path) + mount = holder / "members" / MEMBER + pinned = raw(holder, env, "rev-parse", f"HEAD:members/{MEMBER}" + ).stdout.decode().strip() + to = older(family) + raw(mount, env, "checkout", "-q", to) + manifest = holder / MANIFEST + text = manifest.read_bytes() + assert text.count(pinned.encode()) == 1, "fixture drift: the pin" + manifest.write_bytes(text.replace(pinned.encode(), to.encode())) + raw(holder, env, "add", "--", f"members/{MEMBER}", MANIFEST) + line, _ = uncommitted_said("bump", SAID_STAGED) + assert_refused_alike("bump", family, holder, env, + (line, DROP_ALONE_REMEDY)) + raw(holder, env, *DROP) + runs_after("bump", family, holder, env) + shown = raw(holder, env, "show", "--name-only", "--format=", "HEAD" + ).stdout.decode().split() + assert sorted(shown) == [MANIFEST, f"members/{MEMBER}"], shown + assert raw(holder, env, "rev-parse", f"HEAD:members/{MEMBER}" + ).stdout.decode().strip() == to + + def test_a_row_put_back_as_member_path_says_is_committed_and_the_verb_runs( family, holder, offline, tmp_path): """THE REMEDY THAT EDITS `family.yaml` NO LONGER LEADS INTO THIS From 4030ee6cbbc4d3c61d5592d44d73f9d26268b700 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 12:23:44 +0000 Subject: [PATCH 4/5] family.py: `_rebase_unreadable` leaves "a rebase" to its row in OPERATION_MARKS, and `_checked_manifest_committed` says the index's state with a plain if/elif chain, clearing the two Sonar smells #816 added (#684) S1192: `_rebase_unreadable` returned the literal "a rebase" a third time; it now returns `(finish, abort, effect)` alone, and the caller keeps `what` from the `rebase-merge` or `rebase-apply` row it already holds. S3358: the nested conditional that chose between no change, a staged change and a staged deletion is a flat if/elif chain, with the unmerged case first and `ls-files -m` asked only where the path is merged, as before. Nothing either refusal says changes. Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index cab6d9a4..179b0697 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -944,9 +944,10 @@ def _branch_said(family: "Family", verb: str, suggested: str) -> list[str]: def _rebase_unreadable(mark: str) -> tuple: - """`(what, finish, abort, effect)` for a `mark` directory that holds no - `head-name` (`REBASE_HEAD_NAMES`).""" - return ("a rebase", None, "git rebase --quit", + """`(finish, abort, effect)` for a `mark` directory that holds no + `head-name` (`REBASE_HEAD_NAMES`); what it is, a rebase, is its row's in + `OPERATION_MARKS`.""" + return (None, "git rebase --quit", "git does not refuse that, and the commit would land beside a " f"`{mark}/` directory that holds no `head-name`, so git can read " "no rebase from it and `git rebase --continue` and `--abort` both " @@ -1017,7 +1018,7 @@ def _operation_in_progress( what, finish, abort, effect = AM_OPERATION elif mark in REBASE_HEAD_NAMES and not os.path.lexists( marks[REBASE_HEAD_NAMES[mark]]): - what, finish, abort, effect = _rebase_unreadable(mark) + finish, abort, effect = _rebase_unreadable(mark) return what, mark, where, finish, abort, effect if os.path.lexists(marks[SEQUENCER]): what, finish, abort, effect = _sequence_act(marks[SEQUENCER_TODO]) @@ -1233,18 +1234,21 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: records = [record.partition(b"\t")[0].split() for record in index.get(MANIFEST.encode(), [])] merged = [(mode, oid) for mode, oid, stage in records if stage == b"0"] - if len(merged) != len(records): - # `ls-files -m` lists an unmerged path once for each stage the work - # tree's copy differs from, which says nothing more. + unmerged = len(merged) != len(records) + if unmerged: said = ["it is unmerged in the index"] + elif not merged: + said = ["its deletion is staged, so the index holds no copy"] + elif merged != [head]: + said = ["a change to it is staged and not committed"] else: - said = ([] if merged == [head] else - ["a change to it is staged and not committed"] if merged else - ["its deletion is staged, so the index holds no copy"]) - if MANIFEST.encode() in _git_bytes(["ls-files", "-m", "-z"], - root).split(b"\0"): - said.append("git lists the work tree's copy as modified, a " - "change nobody has staged") + said = [] + # `ls-files -m` lists an unmerged path once for each stage the work + # tree's copy differs from, which says nothing more. + if not unmerged and MANIFEST.encode() in _git_bytes( + ["ls-files", "-m", "-z"], root).split(b"\0"): + said.append("git lists the work tree's copy as modified, a change " + "nobody has staged") if not said: return raise Refusal( From fdc90a6e92b397ff6484c7609e221f386312610e Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 12:43:09 +0000 Subject: [PATCH 5/5] family.py: the drop alone is named only where a change to `family.yaml` is staged beside a gitlink under `members/`, so a gitlink staged by hand beside an unstaged edit is told both ways out (#684) Fix round 2, lane 1's re-verification of #816 at 4030ee6c (issuecomment-6233920232), N1. `_gitlink_staged` fired on any staged gitlink: a commit inside `members/IRRS` and `git add -- members/IRRS`, or `git mv -- members/IRRS members/MOVED`, beside a comment line not staged in `family.yaml`, was told to drop the edit, its only copy, under a sentence that says a run stopped there. Every stopped `add` or `bump` stages `family.yaml` beside its gitlink, so the gitlink is asked only where the index's own copy differs from HEAD too (lane 1's tested change, verbatim). Both states get the two-way remedy; a test holds each, and the old condition fails both. The docstrings say the drop and the same command make the one commit of both for `bump` only: a half-finished `add` then meets `member-path-taken` (lane 1 measured). Lane: openreposhape-4 (openRepoShape-4) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 22 ++++++++++----- tests/test_family_holder_states.py | 43 +++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index 179b0697..67b83191 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -1220,11 +1220,17 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: update-index --chmod` that the work tree's copy does not have, the first has nothing to commit, and the second is the way out. - A GITLINK UNDER `members/` STAGED BESIDE IT (#210's half-finished bump, - lane 1's review of #816, F5) is told the second ALONE: there the first - commits the row without its gitlink, so the row and the gitlink HEAD - holds disagree (measured), while the second, and the same command run - again, makes the one commit of both. + A CHANGE TO IT STAGED, AND A GITLINK UNDER `members/` STAGED BESIDE IT + (#210's half-finished bump, lane 1's review of #816, F5), is told the + second ALONE: there the first commits the row without its gitlink, so + the row and the gitlink HEAD holds disagree (measured), while the + second, and the same `bump` run again, makes the one commit of both. + That holds for `bump` only: after the second, a half-finished `add` + meets `member-path-taken` (lane 1 measured). Every stopped `add` or + `bump` stages `family.yaml` beside its gitlink, so a gitlink staged by + hand beside a change to the work tree's copy alone is told both ways + out: no run stopped there, and the second alone would drop the + person's edit (lane 1's re-verification, N1). """ held = _head_entries(root) head = held.get(MANIFEST.encode()) @@ -1262,7 +1268,8 @@ def _checked_manifest_committed(root: Path, verb: str) -> None: f"its commit leaves it, and `git commit -- {MANIFEST}` would commit " "the row without it: put back HEAD's copy, in the index and the " f"work tree, `git checkout HEAD -- {MANIFEST}`." - if _gitlink_staged(index, held) else + if merged and merged != [head] and _gitlink_staged(index, held) + else "Remediation: which change to keep is a person's to say. To keep " f"it, commit it on its own first, `git commit -- {MANIFEST}`; to " "drop it, put back HEAD's copy, in the index and the work tree, " @@ -1293,7 +1300,8 @@ def _gitlink_staged(index: dict[bytes, list[bytes]], """Does `index` (`_index_records`) stage a gitlink under `members/` that HEAD (`_head_entries`) does not hold, mode and commit, as a run stopped between its stage and its commit leaves one (#684; lane 1's review of - #816, F5)?""" + #816, F5)? Asked only where the index's copy of `family.yaml` differs + from HEAD's too (N1).""" prefix = f"{MEMBERS_DIR}/".encode() return any( mode == b"160000" and held.get(name) != (mode, oid) diff --git a/tests/test_family_holder_states.py b/tests/test_family_holder_states.py index 13d0cdda..69764902 100644 --- a/tests/test_family_holder_states.py +++ b/tests/test_family_holder_states.py @@ -743,7 +743,8 @@ def test_a_bump_stopped_before_its_commit_is_told_the_drop_alone( leaves them. "Keep" there commits the row without its gitlink, so HEAD's row and gitlink disagree (lane 1 measured it); the refusal names the drop ALONE, in both runs alike. Typed as printed, the same bump then - makes its one commit of both paths.""" + makes its one commit of both paths (a `bump` only: an `add` stopped so + then meets `member-path-taken`, lane 1 measured).""" env = isolated(offline, tmp_path) mount = holder / "members" / MEMBER pinned = raw(holder, env, "rev-parse", f"HEAD:members/{MEMBER}" @@ -767,6 +768,46 @@ def test_a_bump_stopped_before_its_commit_is_told_the_drop_alone( ).stdout.decode().strip() == to +def gitlink_committed_by_hand(root: Path, env: dict) -> None: + """A commit made inside `members/IRRS` by hand, and its gitlink staged, + `git add -- members/IRRS`.""" + raw(root / "members" / MEMBER, env, "commit", "-q", "--allow-empty", + "-m", "a commit made by hand") + raw(root, env, "add", "--", f"members/{MEMBER}") + + +def gitlink_moved_by_hand(root: Path, env: dict) -> None: + """The mount moved by hand, `git mv -- members/IRRS members/MOVED`.""" + raw(root, env, "mv", "--", f"members/{MEMBER}", "members/MOVED") + + +#: Lane 1's two states of a gitlink a person staged by hand (N1). +STAGED_BY_HAND = {"commit-and-add": gitlink_committed_by_hand, + "git-mv": gitlink_moved_by_hand} + + +@pytest.mark.parametrize("stage", STAGED_BY_HAND.values(), + ids=STAGED_BY_HAND) +def test_a_gitlink_staged_by_hand_beside_an_unstaged_edit_is_told_both( + family, holder, offline, tmp_path, stage): + """N1 (lane 1's re-verification of #816): a gitlink staged BY HAND, and + a comment line in `family.yaml` not staged. No run stopped there, and + the change is not the row, so the drop alone would discard the + person's only copy of their edit: the refusal names both ways out, as + for any unstaged edit, in both runs alike. Every stopped `add` or + `bump` stages `family.yaml` beside its gitlink; that is what the drop + alone is kept for. Typed, "keep" commits `family.yaml` alone.""" + env = isolated(offline, tmp_path) + stage(holder, env) + commented(holder) + assert_refused_alike("remove", family, holder, env, + uncommitted_said("remove", SAID_UNSTAGED)) + assert (holder / MANIFEST).read_bytes().startswith(COMMENT) + raw(holder, env, *KEEP) + assert raw(holder, env, "show", "--name-status", "--format=", "HEAD" + ).stdout == f"M\t{MANIFEST}\n".encode() + + def test_a_row_put_back_as_member_path_says_is_committed_and_the_verb_runs( family, holder, offline, tmp_path): """THE REMEDY THAT EDITS `family.yaml` NO LONGER LEADS INTO THIS