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)

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
@@ -875,6 +878,33 @@ def cmd_add(args) -> int:
 # ---------------------------------------------------------------------------
 
 
+def _nothing_to_record(root: Path, paths: list[str]) -> bool:
+    """Whether `git commit -- <paths>` 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 <commit>` 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. 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",
+             "--", *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))
@@ -921,7 +951,30 @@ 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)
+    # 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
+        # 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,

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,229 @@ 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_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 <Project> --to
+    <commit>` 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
+    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 <Project> --to <commit>`, 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(