From 66ace9bfd480fbdbffe12a2eb9b07e3b10f0788e Mon Sep 17 00:00:00 2001 From: openRepoShape CI Date: Sun, 11 Oct 2026 16:38:00 +0000 Subject: [PATCH 1/2] family.py: `bump` to the commit a member is already pinned at says `nothing to do` and exits 1, so a re-run of a bump that landed is no longer `REFUSED a git or gh command failed`, exit 2, around git's `nothing to commit` (#210) `cmd_bump` checked the member out at `--to`, rewrote the `members:` block with the row it already had and handed `git commit -- family.yaml` nothing. git's own `nothing to commit` (exit 1) reached `main`'s generic handler as `REFUSED a git or gh command failed`, exit 2 - where this file's docstring documents exit 1 for nothing and `bump-leg.py` answers the same re-run with `nothing to do`, exit 1. A lane or script re-running a landed bump read a refusal to act on. After that rewrite and before `_commit_one`, `cmd_bump` now asks git whether the commit has anything to record: `_nothing_to_record` runs `git diff --quiet --ignore-submodules=dirty HEAD -- family.yaml`, reads its exit 1 as "there is a difference" and lets any other failure propagate. - When there is nothing to record, `bump` prints its usual `old -> new tree` line, then `bump-leg.py`'s sentence - `nothing to do: / is already pinned at and every fact agrees.` - on stdout, says nothing on stderr and exits 1. It writes no commit and stages nothing. - `family.yaml` goes back byte for byte as it was found. The rewrite recorded nothing, and an uncommitted hand edit it happened to overwrite is not this run's to discard (on main the same run threw the edit away before it refused). - git is asked, and `was == --to` is not read. A row that names `--to` is not by itself a bump with nothing in it, and three cases keep committing: a gitlink that moved alone and a digest that does not recompute (`validate-family.py` names `bump --member --to ` as the repair for both), and a bump an earlier run wrote and staged but never committed. - `--ignore-submodules=dirty` is what `git commit` itself ignores: files the member holds untracked or modified are not recorded by this commit, its HEAD is, so they do not make a bump to the pinned commit look like one with something in it. - The module docstring's EXIT CODES says which bump is `nothing`. Measured with a throwaway family in local bare remotes, main a5b09c3 versus this head: - `bump --member IRRS --to ` (the issue's repro): exit 2 and `REFUSED ... git commit -q -F - -- members/IRRS family.yaml ... nothing to commit` on stderr, then exit 1 and the two stdout lines above with an empty stderr; HEAD, gitlink and `git status --porcelain` identical before and after in both. - The same bump run again after it landed, with `--trailer`: exit 2, then exit 1; `family.yaml` byte-identical before and after in both. - An uncommitted edit to the member's `id:` row, then a bump to the pinned commit: exit 2 and the edit gone from the working copy, then exit 1 and the edit still there (` M family.yaml` before and after). - An ordinary bump to a new commit, and a bump back to the commit a row already names after the gitlink moved alone: output and exit code diffed against main after normalizing the 12-hex shas and the scratch paths, byte-identical (exit 0, one commit, `validate-family.py` green after). Unchanged: every flag and `--help` text (`docs/cli.md` is not touched), the commit message and trailers of a bump that commits, `add` and `remove`, and the README and AGENTS.md, which do not describe `bump`'s exit codes. Left out: `family.py bump` has no `--dry-run`, though the docstring promises one prints and exits 0 and `bump-leg.py` has one; a nothing-to-do answer still costs the member fetch and checkout it always did. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 --- scripts/family.py | 44 +++++++++- tests/test_family.py | 187 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 1 deletion(-) diff --git a/scripts/family.py b/scripts/family.py index 560e2844..ebea9633 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -53,7 +53,10 @@ subcommand runs that file rather than a second copy of its logic. EXIT CODES: 0 done · 1 nothing (a dry run prints and exits 0) · 2 a refusal. -`siblings` exits with the template script's own code, unchanged. +`siblings` exits with the template script's own code, unchanged. A `bump` whose +commit would record nothing — the member is already pinned at `--to`, in the +gitlink and in `family.yaml` alike — is `nothing`: it says `nothing to do`, as +`bump-leg.py` does for a leg, and writes no commit. """ from __future__ import annotations @@ -870,6 +873,32 @@ def cmd_add(args) -> int: # --------------------------------------------------------------------------- +def _nothing_to_record(root: Path, paths: list[str]) -> bool: + """Whether `git commit -- ` has nothing to record: the gitlink the + member stands at and the manifest as rewritten are what `HEAD` already has. + + ASKED OF GIT, AFTER THE SAME REWRITE THE COMMIT WOULD CARRY (#210), and + not read off the pin alone. A row that names `--to` is not by itself a bump + with nothing in it: `validate-family.py` names `bump --to ` as the + repair for a gitlink that disagrees with the pin and for a digest that does + not recompute, and both leave the row's commit equal to `--to`; and so does + a bump an earlier run wrote and staged but never committed. In each of + them `git diff` finds something, and the commit goes ahead. + `--ignore-submodules=dirty` is what `git commit` itself ignores: files the + member holds untracked or modified are not a thing this commit records, + its HEAD is. Exit 1 is git's "there is a difference"; any other failure is + a real one and propagates. + """ + try: + run(["git", "diff", "--quiet", "--ignore-submodules=dirty", "HEAD", + "--", *paths], cwd=root) + except CommandFailed as exc: + if exc.code == 1: + return False + raise + return True + + def cmd_bump(args) -> int: family = Family.open(args.family_root) project = checked_value("--member", repo_basename(args.member)) @@ -916,7 +945,20 @@ def cmd_bump(args) -> int: rows = [(_member_row(project, project_id, str(row["repository"]), commit, digest) if str(r.get("project")) == project else r) for r in family.members] + original = family.path.read_bytes() family.write_members(rows) + if _nothing_to_record(family.root, [path, MANIFEST]): + # The same sentence `bump-leg.py` answers the same re-run with, and + # exit 1, which is what this file's docstring says `nothing` is. The + # manifest goes back byte for byte as it was found: the rewrite + # recorded nothing, and an uncommitted hand edit it happened to + # overwrite is not this run's to discard. + family.path.write_bytes(original) + print(f" {project:<16} {was[:12]} -> {commit[:12]} tree " + f"{digest[:12]}…") + print(f"\nnothing to do: {row['repository']} is already pinned at " + f"{commit[:12]} and every fact agrees.") + return 1 head = _commit_one( family.root, diff --git a/tests/test_family.py b/tests/test_family.py index 421f5105..b2dd0fd1 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -636,6 +636,8 @@ def test_bump_moves_the_gitlink_and_the_pin_together(family, tmp_path): result = run_script(FAMILY, "bump", "--family-root", str(holder), "--member", "IRRS", "--to", moved) assert result.returncode == 0, result.stderr + result.stdout + assert "nothing to do" not in result.stdout, ( + "a bump that moves the pin is not the one #210 made say so") row = {r["project"]: r for r in manifest(holder)["members"]}["IRRS"] assert row["pin"]["commit"] == moved != was["pin"]["commit"] @@ -665,6 +667,191 @@ def test_bump_refuses_a_tag(holder): assert "member-to-not-a-commit" in result.stderr +# --- bump to the commit already pinned (#210) ------------------------------- + +def nothing_to_do_line(project: str, commit: str) -> str: + """The sentence `bump-leg.py` answers the same re-run with, repository + first, so a lane or script reads one wording from either tool.""" + return (f"nothing to do: {ORG}/{project} is already pinned at " + f"{commit[:12]} and every fact agrees.") + + +def commit_in_member(member, message: str = "The member moved again") -> str: + """An EMPTY commit on top of the member's HEAD: a new commit whose tree is + the old one's, so `pin.tree_sha256` still recomputes and the gitlink is the + only fact that moves. Returns its sha.""" + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--allow-empty", "-m", message, cwd=member) + return git("rev-parse", "HEAD", cwd=member).stdout.strip() + + +@pytest.mark.parametrize("extra", [ + (), ("--trailer", TRAILERS[0], "--trailer", TRAILERS[1])], + ids=["plain", "with-trailers"]) +def test_bump_to_the_commit_already_pinned_is_nothing_to_do(holder, extra): + """A re-run of a bump that already landed. It used to check the member out + at the commit, rewrite the same row and hand `git commit` nothing, which + exited 1 and reached `main` as "REFUSED a git or gh command failed", exit + 2 — a refusal, where `family.py` documents exit 1 for nothing and + `bump-leg.py` answers the same re-run with `nothing to do` (#210). + + NOTHING IS WRITTEN: no commit, no staged path, not one byte of + `family.yaml`. A lane re-runs a bump with its `Lane:` and `Co-Authored-By:` + trailers, so the run with them is the one that has to read the same. + """ + pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + row = {r["project"]: r for r in manifest(holder)["members"]}["IRRS"] + assert row["pin"]["commit"] == pinned, "fixture drift: gitlink != pin" + head = git("rev-parse", "HEAD", cwd=holder).stdout.strip() + status = git("status", "--porcelain", cwd=holder).stdout + before = (holder / "family.yaml").read_bytes() + + result = run_script(FAMILY, "bump", "--family-root", str(holder), + "--member", "IRRS", "--to", pinned, *extra) + + assert result.returncode == 1, result.stderr + result.stdout + assert nothing_to_do_line("IRRS", pinned) in result.stdout + assert result.stderr == "", ( + "nothing to do is not a refusal, and says nothing on stderr") + assert "REFUSED" not in result.stdout + result.stderr + assert "committed" not in result.stdout + assert git("rev-parse", "HEAD", cwd=holder).stdout.strip() == head, ( + "no commit, not even an empty one") + assert git("status", "--porcelain", cwd=holder).stdout == status == "" + assert (holder / "family.yaml").read_bytes() == before + assert validate(holder).returncode == 0 + + +def test_bump_to_the_commit_already_pinned_ignores_what_the_member_holds( + holder): + """The member's own untracked and modified files are not something the + holder's commit can record: `git commit` finds nothing to commit with them + there, so a bump that asked `git diff` without `--ignore-submodules=dirty` + would have gone on to commit and been refused again (#210).""" + member = holder / "members" / "IRRS" + pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + (member / "UNTRACKED.md").write_text("left lying in the mount\n") + (member / "NOTES.md").write_text("edited in place\n") + head = git("rev-parse", "HEAD", cwd=holder).stdout.strip() + + 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 git("rev-parse", "HEAD", cwd=holder).stdout.strip() == head + assert (member / "UNTRACKED.md").is_file(), "the member's files stay" + assert (member / "NOTES.md").read_text() == "edited in place\n" + + +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.""" + 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( + " id: irrs\n", " id: something-else\n")) + edited = manifest_path.read_bytes() + status = git("status", "--porcelain", cwd=holder).stdout + assert status.strip() == "M family.yaml", status + + 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 manifest_path.read_bytes() == edited + assert git("status", "--porcelain", cwd=holder).stdout == status + + +def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): + """THE MANIFEST ALREADY NAMES `--to`, and there is still something to + 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).""" + member = holder / "members" / "IRRS" + before = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + after = commit_in_member(member) + manifest_path = holder / "family.yaml" + text = manifest_path.read_text() + assert text.count(before) == 1, "fixture drift: IRRS's pin is not unique" + 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) + + assert result.returncode == 0, result.stderr + result.stdout + assert "nothing to do" not in result.stdout + assert git("rev-parse", "HEAD", cwd=holder).stdout.strip() != head + assert git("rev-parse", "HEAD:members/IRRS", + cwd=holder).stdout.strip() == after + assert set(git("show", "--name-only", "--format=", "HEAD", + cwd=holder).stdout.split()) == {"members/IRRS", + "family.yaml"} + assert validate(holder).returncode == 0 + + +def test_bump_to_the_pinned_commit_repairs_a_gitlink_that_moved_alone(holder): + """`validate-family.py`'s own remedy for `member-gitlink-mismatch` is + `family.py bump --member --to `, and when the commit it + is pointed at is the one the row already names the manifest agrees with + `--to` and the gitlink does not. That is a bump with something to commit + (#210).""" + member = holder / "members" / "IRRS" + pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + commit_in_member(member) + git("add", "--", "members/IRRS", cwd=holder) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", + "Move the gitlink and not the pin", cwd=holder) + broken = validate(holder) + assert broken.returncode == 1 + assert "member-gitlink-mismatch" in broken.stderr + + result = run_script(FAMILY, "bump", "--family-root", str(holder), + "--member", "IRRS", "--to", pinned) + + assert result.returncode == 0, result.stderr + result.stdout + assert "nothing to do" not in result.stdout + assert git("rev-parse", "HEAD:members/IRRS", + cwd=holder).stdout.strip() == pinned + assert validate(holder).returncode == 0, "and the lockstep rule holds" + + +def test_bump_to_the_pinned_commit_repairs_a_digest_that_does_not_recompute( + holder): + """The other half of the same remedy: a row whose `tree_sha256` is wrong + names the commit `--to` names, and the bump rewrites the digest from the + member's own objects. Comparing the commit and not the digest would have + called it done.""" + member = holder / "members" / "IRRS" + pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + row = {r["project"]: r for r in manifest(holder)["members"]}["IRRS"] + manifest_path = holder / "family.yaml" + manifest_path.write_text(manifest_path.read_text().replace( + row["pin"]["tree_sha256"], "b" * 64)) + git("add", "--", "family.yaml", cwd=holder) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "-m", + "Hand-edit a digest", cwd=holder) + broken = validate(holder) + assert broken.returncode == 1 + assert "member-digest-mismatch" in broken.stderr + + result = run_script(FAMILY, "bump", "--family-root", str(holder), + "--member", "IRRS", "--to", pinned) + + assert result.returncode == 0, result.stderr + result.stdout + assert "nothing to do" not in result.stdout + repaired = {r["project"]: r for r in manifest(holder)["members"]}["IRRS"] + assert repaired["pin"]["tree_sha256"] == tree_digest(member, pinned) + assert validate(holder).returncode == 0 + + # --- --trailer: the lines a bump's commit ends with (#111) ------------------ def test_bump_with_two_trailers_ends_the_commit_message_in_the_order_given( From c7e8f5d40940cc1c648fcb119915234fa0b9c091 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sun, 11 Oct 2026 19:21:00 +0000 Subject: [PATCH 2/2] family.py: `bump` re-stages the member's gitlink before it asks git whether there is anything to record, so a stray staged gitlink and a bump to the pinned commit say `nothing to do` and no longer end in `REFUSED ... nothing to commit`, exit 2 (#210) Fix round 1 on #219, from the independent review. `validate-family.py` reads the INDEX first. A member somebody moved and `git add`ed without committing is reported as `member-gitlink-mismatch`, and the validator names `family.py bump --member --to ` as the repair. `bump` checks the member out at `--to`, which for the pinned commit is HEAD's gitlink, but the index still held the stray one, so `git diff --quiet HEAD` found a difference, `_commit_one`'s own `git add` re-staged the pinned gitlink and `git commit` found nothing: the refusal this branch closes, exit 2, reached through the validator's remedy. `cmd_bump` now runs `git add -- ` immediately before `_nothing_to_record`, as `_commit_one` does first anyway. The checkout already moved the member to `--to`, so this stages that gitlink and nothing else: on a submodule path `git add` stages the gitlink and never the member's files, and `family.yaml`'s own index state is left as found. The commit path is unchanged, because `_commit_one` stages the same path again. - A stray gitlink staged in the index and a bump to the pinned commit: exit 1, the `nothing to do` line, `git status --porcelain` empty, the index back at the pinned gitlink, `family.yaml` byte-identical and `validate-family.py` green. On the previous head the same run was exit 2 with `REFUSED ... git commit -q -F - -- members/IRRS family.yaml ... nothing to commit`, as on main. - A new test, `test_bump_to_the_pinned_commit_unstages_a_stray_gitlink_and_says_nothing`, stages a stray gitlink, checks the validator reports `member-gitlink-mismatch`, bumps to the pinned commit and asserts the line above. It fails on 66ace9b with the refusal and passes here. Unchanged: every other bump path (the ordinary bump, a gitlink that moved alone, a digest that does not recompute and a bump an earlier run staged and never committed all still commit), every flag and `--help` text. Lane: openreposhape-3 (openRepoShape-3) Co-Authored-By: Claude Sonnet 5.5 --- scripts/family.py | 15 +++++++++++++-- tests/test_family.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/scripts/family.py b/scripts/family.py index ebea9633..e9e2298a 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -886,8 +886,9 @@ def _nothing_to_record(root: Path, paths: list[str]) -> bool: them `git diff` finds something, and the commit goes ahead. `--ignore-submodules=dirty` is what `git commit` itself ignores: files the member holds untracked or modified are not a thing this commit records, - its HEAD is. Exit 1 is git's "there is a difference"; any other failure is - a real one and propagates. + its HEAD is. The caller has staged the gitlink first, as `_commit_one` + would, so a stray one in the index is not a difference. Exit 1 is git's + "there is a difference"; any other failure is a real one and propagates. """ try: run(["git", "diff", "--quiet", "--ignore-submodules=dirty", "HEAD", @@ -947,6 +948,16 @@ def cmd_bump(args) -> int: for r in family.members] original = family.path.read_bytes() family.write_members(rows) + # THE GITLINK IS RE-STAGED BEFORE GIT IS ASKED, as `_commit_one` does it + # first anyway. An index still holding a stray gitlink somebody staged and + # never committed makes `git diff HEAD` find a difference the commit then + # does not find, and a bump to the pinned commit ended in the refusal #210 + # is about. `validate-family.py` reads the index first, reports + # `member-gitlink-mismatch` and names this very command as the repair. The + # checkout above already moved the member to `--to`, so this stages that + # and nothing else: a submodule path stages its gitlink, never the + # member's files, and the manifest's own index state is left as found. + run(["git", "add", "--", path], cwd=family.root) if _nothing_to_record(family.root, [path, MANIFEST]): # The same sentence `bump-leg.py` answers the same re-run with, and # exit 1, which is what this file's docstring says `nothing` is. The diff --git a/tests/test_family.py b/tests/test_family.py index b2dd0fd1..05bea68d 100644 --- a/tests/test_family.py +++ b/tests/test_family.py @@ -767,6 +767,44 @@ def test_bump_to_the_commit_already_pinned_leaves_an_uncommitted_edit_alone( assert git("status", "--porcelain", cwd=holder).stdout == status +def test_bump_to_the_pinned_commit_unstages_a_stray_gitlink_and_says_nothing( + holder): + """THE VALIDATOR'S OWN REMEDY, followed with the pinned commit. Somebody + moved the member and ran `git add members/IRRS` without committing: + `validate-family.py` reads the INDEX first, reports + `member-gitlink-mismatch` and names `family.py bump --member --to + ` as the repair. `bump` checks the member out at the pinned commit, + which is HEAD's gitlink, but the index still differs from HEAD, so + `git diff HEAD` found something and `git commit` then did not: the refusal + #210 is about, exit 2, reached through the remedy. The gitlink is + re-staged before git is asked, the index agrees with HEAD again, and there + is nothing to record (#210).""" + member = holder / "members" / "IRRS" + pinned = git("rev-parse", "HEAD:members/IRRS", cwd=holder).stdout.strip() + stray = commit_in_member(member, "A stray move, staged and never committed") + git("add", "--", "members/IRRS", cwd=holder) + assert git("ls-files", "-s", "--", "members/IRRS", + cwd=holder).stdout.split()[1] == stray, "fixture drift" + broken = validate(holder) + assert broken.returncode == 1 + assert "member-gitlink-mismatch" in broken.stderr + head = git("rev-parse", "HEAD", cwd=holder).stdout.strip() + before = (holder / "family.yaml").read_bytes() + + 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 "REFUSED" not in result.stdout + result.stderr + assert git("rev-parse", "HEAD", cwd=holder).stdout.strip() == head + assert git("ls-files", "-s", "--", "members/IRRS", + cwd=holder).stdout.split()[1] == pinned + assert git("status", "--porcelain", cwd=holder).stdout == "" + assert (holder / "family.yaml").read_bytes() == before + assert validate(holder).returncode == 0 + + def test_bump_finishes_a_bump_an_earlier_run_staged_and_did_not_commit(holder): """THE MANIFEST ALREADY NAMES `--to`, and there is still something to commit: a run that wrote the row and staged both paths and then died