From eeadc65c4b8c55bf6da2d219be8bec9a780637d4 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 22:54:45 +0000 Subject: [PATCH 1/2] family.py: `bump --to` asks `add`'s duplicate-id question and peels an annotated tag's own id as `add --at` does, and refuses a commit the member does not hold after its fetch as `member-to-unknown`, saying to push it first, so a bump no longer commits a holder its own validator refuses as `member-duplicate-id`, or fails in git's words `reference is not a tree` (#541) What changed, in `scripts/family.py`: - The duplicate id. `_checked_bumped_id` asks the question `add` asks since #330, of the row `bump` writes: does another row already record the id the member's `project.yaml` records at `--to`? The member's own row is left out (`_row_recording`'s `besides`), and every other row is asked as the holder's `validate-family.py` asks it. The dry run and the real run both ask it before the checkout, of the id `_member_id_at` reads from the member's objects; the real run asks it again of the id its checkout reads, so a linked `project.yaml` is refused there and put back by the `PinRollback` (#365). `add`'s `_checked_member_id` now finds the row through the same `_row_recording`, with its words unchanged. - The unpushed commit. `_checked_to_names_a_commit` used to return when `git cat-file -t` failed, leaving the id to the dry run's resolve and the real run's checkout, which failed in git's words. It now refuses `member-to-unknown`, naming the commit, saying no branch of the member's remote reaches it, and saying to push it first. When the member's fetch failed, the refusal says that instead and does not claim the remote lacks it. - The tag. As ruled by Brett Heap on 2026-10-14, `bump --to` peels an annotated tag's own id, as `add --at` does. `_checked_to_names_a_commit` returns the commit the tag names (through a tag of a tag too), `cmd_bump` goes on with that commit in both runs and prints one line saying so, so the gitlink, the row's `commit:`, the digest and the commit message all carry the commit. A tree, a blob or a tag of either is still refused `member-to-not-a-commit`, now in `add --at`'s words through one shared `_names_no_commit` ("names a tree", not "is an annotated tag of a tree"). The docstrings that said `bump` refuses a tag "as `bump-leg.py` refuses the same `--to`", and that an id the member does not hold is "left to the checkout", are rewritten; they compare with `family.py add --at` only. Tests: `tests/test_family_bump_to_checks.py` is new (the duplicate id, read from the objects and through a linked manifest; the unpushed commit, then pushed; the failed fetch). `tests/test_family_bump_to_a_tag.py` now holds the peel, in both runs and after the fetch, and the shared tree wording. `tests/test_family_dry_run.py`'s test of a commit the member lacks expects `member-to-unknown` from both runs. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- scripts/family.py | 294 +++++++++++++++++++--------- tests/test_family_bump_to_a_tag.py | 155 ++++++++------- tests/test_family_bump_to_checks.py | 227 +++++++++++++++++++++ tests/test_family_dry_run.py | 13 +- 4 files changed, 525 insertions(+), 164 deletions(-) create mode 100644 tests/test_family_bump_to_checks.py diff --git a/scripts/family.py b/scripts/family.py index 3d05f10d..f7edd8b0 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -2182,6 +2182,30 @@ def _checked_add_target(family: Family, repository: str, project: str, return commit.lower() +def _row_recording(family: Family, project_id: str, + besides: str | None = None) -> dict | None: + """The first row that records `project_id` as its `id:`, or None, asked + the way `_checked_member_id` says the validator asks it: a row with no + `project:` or no `id:`, and a row whose `project:` an earlier row + already spells exactly, are skipped. + + `besides` IS THE `project:` OF THE ROW `bump` MOVES, spelled exactly + (#541). `bump` replaces that row, and every row spelled the same way, + with the one it writes, so the id it records today is no other + project's; every other row is asked as `add` asks it. + """ + projects: set[str] = set() + for row in family.members: + name = row.get("project") + if not isinstance(name, str) or not name or name in projects: + continue + projects.add(name) + if name != besides and row.get("id") and \ + str(row["id"]) == project_id: + return row + return None + + def _checked_member_id(family: Family, repository: str, project: str, project_id: str) -> None: """Refused when a row already records the `id` the member's own @@ -2209,14 +2233,9 @@ def _checked_member_id(family: Family, repository: str, project: str, rule, as it keeps `member-path`'s: it ships INTO every holder, and a shared copy would move every holder's pin. """ - projects: set[str] = set() - for row in family.members: - name = row.get("project") - if not isinstance(name, str) or not name or name in projects: - continue - projects.add(name) - if not row.get("id") or str(row["id"]) != project_id: - continue + row = _row_recording(family, project_id) + if row is not None: + name = row["project"] raise Refusal( "member-duplicate-id", f"{project} is already a member of {family.data.get('name')} " @@ -2272,6 +2291,26 @@ def _probe_member(url: str, protocol: list[str], repository: str, "refers to it", file=sys.stderr) +def _names_no_commit(code: str, flag: str, value: str, kind: str, + repository: str, where: str) -> Refusal: + """The refusal for a `--at` or `--to` whose 40 hex names a tree or a + blob in the member, or an annotated tag of one: `kind` is what it names + once any tag is peeled (`^{}`), and `where` is where `git log` lists + the commits to pass instead. + + ONE WORDING FOR `add --at` AND `bump --to` (#541). The two worded a tag + of a tree differently, `names a tree` and `is an annotated tag of a + tree`. `bump` now peels a tag of a commit as `add` does, so it names a + tag of a tree as `add` does too, by the tree it tags (`_probe_commit`), + and each verb keeps only its own code. + """ + return Refusal( + code, f"{flag} {value} names a {kind} in {repository}, not a commit", + "Remediation: pass the commit itself — `git log --format=%H` in " + f"{where} lists them. A tree's or a blob's sha is part of a commit " + "and never one, and a pin is a commit or it is nothing.") + + def _probe_commit(clone: Path, repository: str, at: str | None) -> str: """`--at`, or the member's tip, as a commit of the member — or the refusal saying which of the two it was not. @@ -2307,13 +2346,8 @@ def _probe_commit(clone: Path, repository: str, at: str | None) -> str: except Refusal: kind = None if kind in ("tree", "blob"): - raise Refusal( - "member-at-unknown", - f"--at {at} names a {kind} in {repository}, not a commit", - "Remediation: pass the commit itself — `git log --format=%H` in " - "the member lists them. A tree's or a blob's sha is part of a " - "commit and never one, and a pin is a commit or it is " - "nothing.") from missing + raise _names_no_commit("member-at-unknown", "--at", at, kind, + repository, "the member") from missing raise Refusal( "member-at-unknown", f"--at {at} is not a commit any branch or tag of {repository} " @@ -3912,6 +3946,49 @@ def _member_id_at(member: Path, commit: str, project: str, shown=f"{repository} @ {commit[:12]}:project.yaml") +def _checked_bumped_id(family: Family, row: dict, commit: str, + project_id: str) -> None: + """`member-duplicate-id` for a `bump` whose `--to` records, in the + member's own `project.yaml`, the `id` another row already records + (#541): the question `add` asks since #330 (`_checked_member_id`), asked + of the row `bump` writes. + + `bump` READ THE ID AND NEVER ASKED IT. A member whose `project.yaml` at + `--to` records another member's id was committed, exit 0, and the + holder's own `validate-family.py` then refused it as + `member-duplicate-id`. So the dry run and the real run both ask it, + before the checkout, of the id `_member_id_at` reads from the member's + objects, and the real run asks it again of the id its checkout reads, + which is what the row records: a linked `project.yaml`, which only a + checkout reads through, is refused there and put back like any other + refusal after the first write (#365). + + THE MEMBER'S OWN ROW IS NOT ASKED (`_row_recording`'s `besides`): the + bump replaces it, so the id it records today is no other project's. + Every other row is asked as `add` asks it, so `bump` refuses what the + gate would refuse and nothing it would pass. + """ + project = str(row["project"]) + held = _row_recording(family, project_id, besides=project) + if held is None: + return + repository = row.get("repository") + raise Refusal( + "member-duplicate-id", + f"{repository} at {commit[:12]} is already a member of " + f"{family.data.get('name')} under another name: its project.yaml " + f"there records id {project_id!r}, and the row {held['project']} " + f"pins {held.get('repository')} with that id. Bumping {project} to " + "it would pin one project twice, which `validate-family.py` refuses", + "Remediation: a family pins each project once. Pass `--to` a commit " + f"of {repository} whose project.yaml records an id no other row " + f"records. If {repository} and {held.get('repository')} are one " + f"project, `family.py remove --member {held['project']}` takes the " + "other row out first. Two projects that record one id are a fault in " + "one of their `project.yaml` files, mended in that project, not in " + "the family.") + + def _head_oid(root: Path, path: str) -> str | None: """The object HEAD records at `path` — for a gitlink, the commit — or None where HEAD records nothing there. A tree path, not a pathspec, so @@ -3973,67 +4050,86 @@ def _print_trailers(trailers: list[str]) -> None: def _checked_to_names_a_commit(member: Path, path: str, row: dict, - commit: str) -> None: - """`member-to-not-a-commit` for a `--to` that is 40 hex and names, in the - member's objects, something that is not a commit: an annotated tag, a - tree or a blob (#405). `bump` and `bump --dry-run` both ask it, after - the fetch and before the checkout, so the two runs make one refusal and - the holder is left byte for byte as it was. - - AN ANNOTATED TAG'S OWN ID IS 40 HEX, and it is what `git rev-parse v2` - prints for a release tag. `COMMIT_RE` passed it, the checkout PEELED it - to the commit it names, which the gitlink then recorded, and the row - recorded the tag's id as typed: `bump` exited 0 with a commit the - holder's own `validate-family.py` refused as `member-gitlink-mismatch`, - and the dry run printed the two ids side by side on its `commit` and - `gitlink` lines. - - REFUSED, NOT PEELED, as `bump-leg.py` refuses the same `--to` for a leg: - its `validate-pins.py` goes red on the tag's id in the pin and the bump - is rolled back, exit 2. The pin is the id the person typed and the - `old -> new` line is what they read, so the refusal names the commit the - tag peels to (through a tag of a tag too, `^{}`) and the re-run is - theirs. A tree's or a blob's id, or a tag of one, is named for what it - is, as `add --at` names one (`member-at-unknown`); before, both runs - failed on it in git's words, exit 2, from the dry run's resolve or the - real run's checkout. - - AN ID THE MEMBER DOES NOT HOLD is not asked here. The dry run's resolve - and the real run's checkout fail on it in git's words, exit 2, as they - always have. + commit: str, fetched: bool) -> str: + """The COMMIT `--to` names in the member's objects: `--to` itself, or the + commit an annotated tag's own id names (#541), or the refusal saying why + there is none. `bump` and `bump --dry-run` both ask it, after the fetch + (`fetched` False when it failed) and before the dry run leaves or the + checkout, so the two runs go on with one commit or make one refusal, + and the holder is left byte for byte as it was. + + AN ANNOTATED TAG'S OWN ID IS PEELED, as `add --at` peels one (Brett + Heap, 2026-10-14, on #541). It is 40 hex, and it is what `git rev-parse + v2` prints for a release tag. Once the checkout peeled it while the row + recorded the tag's id as typed, so `bump` committed a holder its own + `validate-family.py` refused as `member-gitlink-mismatch`; #405 made + both runs refuse it instead, naming the commit for the person to type. + A tag object's id fixes its content, so it names one commit for ever + and, unlike a tag NAME, cannot be moved. The caller goes on with that + commit, in both runs, so the gitlink, the row's `commit:`, the digest + and the commit message carry it and never the tag's id. `^{}` peels + through a tag of a tag too. + + A TREE OR A BLOB IS NAMED AS ONE, and so is a tag of either, in `add + --at`'s words (`_names_no_commit`): its id is in the member, so "not + pushed" would send the person looking for a push that is not missing. + Before #405 both runs failed on it in git's words, exit 2. + + AN ID THE MEMBER DOES NOT HOLD AFTER THE FETCH is refused + `member-to-unknown` (#541), saying to push it first. The dry run's + resolve and the real run's checkout failed on it in git's words, + `fatal: reference is not a tree`, exit 2, which never said what was + wrong: the commonest such `--to` is a commit made in the member's + working clone beside the holder and not pushed yet, and the fetch of the + mount brings only what the member's remote has. When that fetch failed, + the refusal says so and does not claim the remote lacks it. """ + repository = row.get("repository") try: kind = run(["git", "cat-file", "-t", commit], cwd=member) except CommandFailed: - return - if kind == "commit": - return - try: - peeled = run(["git", "rev-parse", "-q", "--verify", f"{commit}^{{}}"], - cwd=member).lower() - names = run(["git", "cat-file", "-t", peeled], cwd=member) - except CommandFailed: - peeled, names = "", "" - repository = row.get("repository") - if names == "commit": + kind = None + if kind is None and fetched: raise Refusal( - "member-to-not-a-commit", - f"--to {commit} is an annotated tag in {repository}, not a " - f"commit; the commit it names is {peeled}", - f"Remediation: pass that commit, `--to {peeled}`. A pin is a " - "commit or it is nothing: checking the tag out would put " - f"{peeled[:12]} in the gitlink at {path} and the tag's own id in " - "the row, which `validate-family.py` refuses as " - "`member-gitlink-mismatch`.") - if kind != "tag": - what = f"names a {kind}" - elif names in ("tree", "blob"): - what = f"is an annotated tag of a {names}" - else: - what = "is an annotated tag" + "member-to-unknown", + f"--to {commit} is not a commit any branch of {repository} " + f"reaches: {path} fetched from its origin and holds no object " + "with that id", + f"Remediation: push it first, to a branch of {repository}, and " + "run the same command again. A pin the rest of the world cannot " + "fetch is a family the rest of the world cannot bootstrap. If it " + "is pushed, check the commit: it belongs to the member's " + "ASSEMBLY ROOT, not to one of its legs.") + if kind is None: + raise Refusal( + "member-to-unknown", + f"--to {commit} is not a commit {path} holds, and its fetch from " + f"its origin failed, so whether a branch of {repository} reaches " + "it is not known", + f"Remediation: push it first, to a branch of {repository}, if it " + f"is not pushed yet; make {repository} readable from here — if it " + "is PRIVATE, check `gh auth status` or your SSH key — and run the " + "same command again. A pin the rest of the world cannot fetch is " + "a family the rest of the world cannot bootstrap.") + if kind == "commit": + return commit + peeled, names = "", "" + if kind == "tag": + try: + peeled = run(["git", "rev-parse", "-q", "--verify", + f"{commit}^{{}}"], cwd=member).lower() + names = run(["git", "cat-file", "-t", peeled], cwd=member) + except CommandFailed: + peeled, names = "", "" + if names == "commit": + return peeled + named = names if kind == "tag" else kind + if named in ("tree", "blob"): + raise _names_no_commit("member-to-not-a-commit", "--to", commit, + named, repository, path) raise Refusal( "member-to-not-a-commit", - f"--to {commit} {what} in {repository}, not a commit", + f"--to {commit} is an annotated tag in {repository}, not a commit", "Remediation: pass the commit itself — `git log --format=%H` in " f"{path} lists them. A pin is a commit or it is nothing.") @@ -4045,10 +4141,13 @@ def _bump_dry_run(family: Family, row: dict, path: str, commit: str, (#326): the answers a real `bump` gives, and no write. THE REAL RUN'S QUESTIONS, IN ITS ORDER, ASKED OF THE MEMBER'S OBJECTS. - `--to` is resolved as the checkout would take it (`^{commit}`, which is - what `git checkout` accepts), and one the member does not have after the - fetch is exit 2, where the real run's checkout fails. The manifest is - read by `_member_id_at` and the digest by `tree_digest`, which reads + `commit` is the one `_checked_to_names_a_commit` returned, which both + runs asked after the fetch: an annotated tag's own id is already peeled, + and an id the member does not hold is already refused (#541). It is + resolved as the checkout would take it (`^{commit}`, which is what `git + checkout` accepts). The manifest is read by `_member_id_at`, its id is + asked against the other rows by `_checked_bumped_id` as the real run + asks it (#541), and the digest is computed by `tree_digest`, which reads objects and never a checkout, so the digest printed is the one the real run computes, never a guess. The rows are `_bumped_rows`', and `family.rewritten` asks `family-members-unrecognised` without writing. @@ -4078,6 +4177,8 @@ def _bump_dry_run(family: Family, row: dict, path: str, commit: str, linked = project_id is None if linked: project_id = str(row.get("id") or "") + else: + _checked_bumped_id(family, row, commit, project_id) digest = tree_digest(member, commit) new, rows = _bumped_rows(family, row, project_id, commit, digest) text = family.rewritten(rows) @@ -4131,6 +4232,10 @@ def _bump_and_pin(family: Family, row: dict, path: str, commit: str, was = str((row.get("pin") or {}).get("commit") or "") project_id = _read_member_project(member, project, str(row.get("repository"))) + # Asked again of THIS reading, which is the id the row records (#541), + # as `add` asks it of its mount's: a linked `project.yaml` is read + # through only here, and a refusal is put back like any other. + _checked_bumped_id(family, row, commit, project_id) digest = tree_digest(member, commit) _, rows = _bumped_rows(family, row, project_id, commit, digest) original = family.path.read_bytes() @@ -4179,19 +4284,21 @@ def _checked_bump_target(family: Family, row: dict, path: str, commit: str) -> None: """Every refusal the real `bump` can ask before its checkout, asked as `bump --dry-run` asks it and in its order (#365): the member's - `project.yaml` at `--to`, read from its objects (`_member_id_at`), and + `project.yaml` at `--to`, read from its objects (`_member_id_at`), the + id it records against the other rows (`_checked_bumped_id`, #541), and a `members:` block this tool will not rewrite. - THE REAL RUN ASKED BOTH AFTER ITS FIRST WRITE. A `--to` whose tree has no - `project.yaml` was refused `member-not-a-project` with the member left - checked out at `--to`, which the holder's own validator then reported, - while `bump --dry-run` refused the same commit and changed nothing. Asked - here, before the rollback is even recorded, the two runs refuse at the - same point in the same words, and the holder is byte for byte as it was. - A `--to` the member's objects do not hold is left to the checkout, which - fails on it in git's words, as it always has, before it writes anything; - a symbolic link is left to the read through the checkout, as the dry run - leaves it. + THE REAL RUN ASKED THEM AFTER ITS FIRST WRITE, OR NEVER. A `--to` whose + tree has no `project.yaml` was refused `member-not-a-project` with the + member left checked out at `--to`, which the holder's own validator then + reported, while `bump --dry-run` refused the same commit and changed + nothing; and an id another row records was committed. Asked here, before + the rollback is even recorded, the two runs refuse at the same point in + the same words, and the holder is byte for byte as it was. A `--to` the + member's objects do not hold never gets here: `_checked_to_names_a_commit` + refused it after the fetch (#541). A symbolic link is left to the read + through the checkout, as the dry run leaves it, and `_bump_and_pin` asks + the id of that reading. """ member = family.root / path try: @@ -4203,6 +4310,8 @@ def _checked_bump_target(family: Family, row: dict, path: str, str(row.get("repository"))) if project_id is None: project_id = str(row.get("id") or "") + else: + _checked_bumped_id(family, row, commit, project_id) # THE ROWS THE BUMP WRITES, not the rows the manifest has: a row with no # `path:` line is the member's own path by default (`_member_path`), and # only the row `_bumped_rows` makes for it spells one out to rewrite. @@ -4286,10 +4395,19 @@ def cmd_bump(args) -> int: fetched = False print(f" note: could not fetch {path} from its origin; the commit " "must already be in its object store", file=sys.stderr) - # WHAT `--to` NAMES IS ASKED OF THE FETCHED OBJECTS (#405), before the - # dry run leaves and before the checkout: a tag, a tree or a blob is - # refused by both runs alike, with nothing in the holder moved. - _checked_to_names_a_commit(member, path, row, commit) + # WHAT `--to` NAMES IS ASKED OF THE FETCHED OBJECTS (#405, #541), before + # the dry run leaves and before the checkout. From here on `commit` is + # the one it returns, in both runs: an annotated tag's own id is peeled + # to the commit it names, and said; an id the member does not hold, a + # tree or a blob is refused alike, with nothing in the holder moved. + # `suggested` keeps the id as typed: the real run's branch refusal named + # it before the fetch, and the dry run's `branch` line names the same. + typed = commit + commit = _checked_to_names_a_commit(member, path, row, typed, fetched) + if commit != typed: + print(f"--to {typed[:12]} is an annotated tag in " + f"{row.get('repository')}; the commit it names, " + f"{commit[:12]}, is what is pinned", flush=True) if args.dry_run: return _bump_dry_run(family, row, path, commit, args.trailer, fetched, suggested) diff --git a/tests/test_family_bump_to_a_tag.py b/tests/test_family_bump_to_a_tag.py index 0788db97..de855c5e 100644 --- a/tests/test_family_bump_to_a_tag.py +++ b/tests/test_family_bump_to_a_tag.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: Apache-2.0 -"""`family.py bump --to` an id that is 40 hex and names no commit (#405). +"""`family.py bump --to` an id that is 40 hex and names no commit itself +(#405, #541). An annotated tag's own id is 40 hex, and it is what `git rev-parse v2` prints for a release tag, so `COMMIT_RE` passed it. The checkout peeled it to the @@ -7,23 +8,24 @@ tag's id as typed: `bump` exited 0 and committed a holder whose own `validate-family.py` reported `member-gitlink-mismatch`, exit 1, and `bump --dry-run` printed the tag on its `commit` line and the commit on its -`gitlink` line. - -THE RULING THIS FILE HOLDS `bump` TO. A `--to` that names an annotated tag, a -tree or a blob in the member's objects is refused `member-to-not-a-commit`, -exit 2, by the real run and by its dry run alike, in the same two lines, with -nothing in the holder or the member's checkout moved. A tag of a commit (or -of a tag of one) is refused naming the commit it peels to, and the re-run -with that commit is the person's: it is what `bump-leg.py` does with the same -`--to` for a leg, whose own `validate-pins.py` goes red on the tag's id and -rolls the bump back. A tree, a blob, or a tag of either is named for what it -is. A `--to` the member does not hold at all is still git's to report, as -`test_family_dry_run.py` holds it. +`gitlink` line. #405 made both runs refuse it, naming the commit it tags. + +THE RULING THIS FILE HOLDS `bump` TO (Brett Heap, 2026-10-14, on #541): +`bump --to` peels an annotated tag's own id, as `family.py add --at` does. A +tag of a commit (or of a tag of one) is peeled to the commit it names by the +real run and by its dry run alike, and both say so in one line: the dry run +prints that commit and the row it would write and moves nothing, and the real +run pins that commit in the gitlink and the row alike and writes that row. A +tree, a blob, or a tag of either is refused `member-to-not-a-commit`, exit 2, +by both runs in the same two lines, named for what it is in `add --at`'s +words, with nothing in the holder or the member's checkout moved. A `--to` +the member does not hold at all is refused `member-to-unknown`, as +`test_family_bump_to_checks.py` holds it. ASKED AFTER THE FETCH. A release tag is pushed to the member's remote and reaches the member's mount only through `bump`'s own `git fetch`, which is the -issue's own flow; a check asked before the fetch finds nothing there and lets -the tag through to the checkout. One test holds that order. +issue's own flow; a question asked before the fetch finds nothing there. One +test holds that order. The holder, the offline environment and "changes nothing" are `test_family_dry_run.py`'s, imported rather than rebuilt. Every tag is made @@ -43,9 +45,12 @@ from test_family_add_refusals import holder, offline, refusal # noqa: F401 #: "Changes nothing", asked as bytes in the order a bump would write. from test_family_dry_run import ( - BUMP_ORDER, assert_as_it_was, bump, isolated, pinned_at, state, + BUMP_ORDER, assert_as_it_was, bump, isolated, pinned_at, printed_row, + state, ) from test_family_member_path import MEMBER, older, raw +#: The sentence a bump with nothing to record ends with, from its owner. +from test_family import nothing_to_do_line PATH = f"members/{MEMBER}" @@ -65,16 +70,27 @@ def oid(mount: Path, env: dict, rev: str) -> str: ).stdout.decode("ascii").strip() -def tag_refusal(to: str, target: str) -> tuple[str, str]: - """The two lines `bump --to ` is refused with when `to` is an - annotated tag that peels to the commit `target`.""" - return ( - f"REFUSED member-to-not-a-commit: --to {to} is an annotated tag in " - f"{ORG}/{MEMBER}, not a commit; the commit it names is {target}", - f"Remediation: pass that commit, `--to {target}`. A pin is a commit " - f"or it is nothing: checking the tag out would put {target[:12]} in " - f"the gitlink at {PATH} and the tag's own id in the row, which " - "`validate-family.py` refuses as `member-gitlink-mismatch`.") +def peeled_line(to: str, target: str) -> str: + """The line both runs of `bump --to ` begin with when `to` is an + annotated tag that names the commit `target`.""" + return (f"--to {to[:12]} is an annotated tag in {ORG}/{MEMBER}; the " + f"commit it names, {target[:12]}, is what is pinned") + + +def pins_the_tagged_commit(holder: Path, env: dict, to: str, + target: str) -> None: + """`bump --to `, an annotated tag of `target`, for real: it says + the tag is peeled, and pins `target` in the gitlink and the row alike, + and the holder passes its own gate.""" + real = bump(holder, env, to) + + assert real.returncode == 0, real.stdout + real.stderr + assert real.stderr == "" + assert real.stdout.splitlines()[0] == peeled_line(to, target) + row = {r["project"]: r for r in manifest(holder)["members"]}[MEMBER] + assert row["pin"]["commit"] == target + assert pinned_at(holder, env) == target + assert validate(holder).returncode == 0 def both_runs(holder: Path, env: dict, to: str) -> str: @@ -97,48 +113,55 @@ def both_runs(holder: Path, env: dict, to: str) -> str: @pytest.mark.parametrize("nested", [False, True], ids=["tag-of-a-commit", "tag-of-a-tag"]) -def test_bump_to_an_annotated_tag_is_refused_naming_the_commit_it_tags( +def test_bump_to_an_annotated_tag_pins_the_commit_it_tags( family, holder, offline, tmp_path, nested): - """THE ISSUE'S PROBE. `bump --to ` exited 0 and wrote the - tag's id into `pin.commit` over a gitlink at the commit it tags, and the - holder's own gate was red. Now both runs refuse it, name the commit the - tag peels to (through a tag of a tag as well) and move nothing; and the - re-run the refusal names pins that commit in the gitlink and the row - alike, and the gate is green.""" + """THE RULING'S PROBE (#541). `bump --to ` once wrote the + tag's id into `pin.commit` over a gitlink at the commit it tags, and + #405 refused it. Now it is peeled, through a tag of a tag as well, as + `add --at` peels it: the dry run says so, moves nothing and prints that + commit and its row; the real run says the same line and pins that commit + in the gitlink and the row alike, writing the row the dry run printed, + and the gate is green.""" env = isolated(offline, tmp_path) mount = holder / PATH - target = older(family) - assert target != pinned_at(holder, env), "premise: a bump with a move" + target, pinned = older(family), pinned_at(holder, env) + assert target != pinned, "premise: a bump with a move" to = annotated(mount, env, "v-release", target) if nested: to = annotated(mount, env, "v-release-again", to) + before = state(holder, env) - stderr = both_runs(holder, env, to) + dry = bump(holder, env, to, "--dry-run") - assert refusal(stderr) == tag_refusal(to, target) + assert_as_it_was(holder, env, before, BUMP_ORDER) + assert dry.returncode == 0, dry.stdout + dry.stderr + assert dry.stderr == "" + assert dry.stdout.splitlines()[:3] == [ + peeled_line(to, target), + f"member {MEMBER} {ORG}/{MEMBER} at {PATH}", + f"commit {pinned[:12]} -> {target[:12]}"] + row = printed_row(dry.stdout) + assert f' commit: "{target}"' in row offline.assert_left_nothing() - again = bump(holder, env, target) + pins_the_tagged_commit(holder, env, to, target) - assert again.returncode == 0, again.stdout + again.stderr - row = {r["project"]: r for r in manifest(holder)["members"]}[MEMBER] - assert row["pin"]["commit"] == target == pinned_at(holder, env) - assert validate(holder).returncode == 0 + assert "\n".join(row) + "\n" in (holder / "family.yaml").read_bytes( + ).decode("utf-8"), "the row printed is the row written" -def test_a_release_tag_only_the_fetch_brings_is_refused_after_the_fetch( +def test_a_release_tag_only_the_fetch_brings_is_peeled_after_the_fetch( family, holder, offline, tmp_path): """THE ISSUE'S OWN FLOW, AND THE ORDER IT NEEDS. The member releases: a new commit and an annotated tag on it, both pushed, after the holder was made. Neither is in the member's mount until `bump` fetches, so the question is asked of what the fetch brought, after it: asked before, it - found no such object, let `--to` through, and the checkout and the row - split the tag from its commit again. + finds no such object. THE REAL RUN GOES FIRST, so no dry run's fetch has handed the mount the - tag before it. Both runs then refuse in the same two lines and move - nothing, and the re-run with the commit the refusal names pins it in the - gitlink and the row alike.""" + tag before it: it peels the tag its own fetch brought and pins the + commit. The dry run of the same `--to` then peels it to the same commit, + in the same line, and finds `nothing to do`.""" env = isolated(offline, tmp_path) mount = holder / PATH # A PRIVATE COPY of the member's remote, which the mount is pointed at: @@ -160,32 +183,20 @@ def test_a_release_tag_only_the_fetch_brings_is_refused_after_the_fetch( assert raw(mount, env, "cat-file", "-e", name, check=False).returncode != 0, ( f"premise: {name} reaches the mount only through the fetch") - before = state(holder, env) - real = bump(holder, env, to) + pins_the_tagged_commit(holder, env, to, target) - assert_as_it_was(holder, env, before, BUMP_ORDER) - assert real.returncode == 2, real.stdout + real.stderr - assert real.stdout == "" - assert refusal(real.stderr) == tag_refusal(to, target) assert oid(mount, env, f"{to}^{{commit}}") == target, ( - "the fetch brought the tag, and the refusal was made after it") - + "the fetch brought the tag, and the peel was made after it") + before = state(holder, env) dry = bump(holder, env, to, "--dry-run") assert_as_it_was(holder, env, before, BUMP_ORDER) - assert dry.returncode == 2, dry.stdout + dry.stderr - assert dry.stdout == "" - assert dry.stderr == real.stderr + assert dry.returncode == 1, dry.stdout + dry.stderr + assert dry.stdout.splitlines()[0] == peeled_line(to, target) + assert dry.stdout.splitlines()[-1] == nothing_to_do_line(MEMBER, target) offline.assert_left_nothing() - again = bump(holder, env, target) - - assert again.returncode == 0, again.stdout + again.stderr - row = {r["project"]: r for r in manifest(holder)["members"]}[MEMBER] - assert row["pin"]["commit"] == target == pinned_at(holder, env) - assert validate(holder).returncode == 0 - #: Each id that is in the member's objects and is not a commit's, how a test #: makes it from a commit of the member, and what the refusal calls it. @@ -197,10 +208,10 @@ def test_a_release_tag_only_the_fetch_brings_is_refused_after_the_fetch( "names a blob"), "tag-of-a-tree": (lambda mount, env, target: annotated( mount, env, "v-tree", oid(mount, env, f"{target}^{{tree}}")), - "is an annotated tag of a tree"), + "names a tree"), "tag-of-a-blob": (lambda mount, env, target: annotated( mount, env, "v-blob", oid(mount, env, f"{target}:project.yaml")), - "is an annotated tag of a blob"), + "names a blob"), } @@ -210,8 +221,9 @@ def test_bump_to_a_tree_a_blob_or_a_tag_of_one_is_refused_by_name( """A tree's or a blob's id, or an annotated tag of one, is 40 hex and in the member's objects too. Both runs failed on it in git's own words, the dry run's resolve and the real run's checkout, so the two runs did not - even print the same refusal. Now both name it for what it is, as `add - --at` does, in the same two lines, and move nothing.""" + even print the same refusal. Now both name it for what it is, in the + words `add --at` names one with, a tag by what it tags (#541), in the + same two lines, and move nothing.""" env = isolated(offline, tmp_path) to = make(holder / PATH, env, older(family)) assert len(to) == 40, "premise: 40 hex, which `COMMIT_RE` passes" @@ -222,5 +234,6 @@ def test_bump_to_a_tree_a_blob_or_a_tag_of_one_is_refused_by_name( f"REFUSED member-to-not-a-commit: --to {to} {said} in " f"{ORG}/{MEMBER}, not a commit", "Remediation: pass the commit itself — `git log --format=%H` in " - f"{PATH} lists them. A pin is a commit or it is nothing.") + f"{PATH} lists them. A tree's or a blob's sha is part of a commit and " + "never one, and a pin is a commit or it is nothing.") offline.assert_left_nothing() diff --git a/tests/test_family_bump_to_checks.py b/tests/test_family_bump_to_checks.py new file mode 100644 index 00000000..029ddc20 --- /dev/null +++ b/tests/test_family_bump_to_checks.py @@ -0,0 +1,227 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`family.py bump --to` makes the checks `add` makes, and says to push an +unpushed commit first (#541). + +TWO LINES OF #541 ARE HELD HERE; its riding line, an annotated tag's own id, +is held by `test_family_bump_to_a_tag.py`. + +THE DUPLICATE ID. `add` refuses a member whose `project.yaml` records the +`id` another row records (#330, `member-duplicate-id`): that is one project +pinned twice. `bump` read the id at `--to` and never asked, so a member whose +`project.yaml` at `--to` records another member's id was committed, exit 0, +and the holder's own `validate-family.py` then refused it under that code. +Now the dry run and the real run both refuse it, before anything moves, in +the same two lines; and the real run asks again of the id its checkout +reads, so a linked `project.yaml`, which only a checkout reads through, is +refused there and put back. + +THE UNPUSHED COMMIT. `bump --to` a commit made in the member's working clone +and not pushed failed in git's own words, `fatal: reference is not a tree`, +exit 2, from the real run's checkout (and the dry run's resolve failed in +other words of git's), and neither said to push it first. Now both runs +refuse it `member-to-unknown`, naming the commit, saying no branch of the +member's remote reaches it, and saying to push it first; once it is pushed, +the same command pins it. When the member's fetch itself failed, the refusal +says so and does not claim the remote lacks the commit. + +THE SAME FAMILY, RULES AND OFFLINE ENVIRONMENT as `test_family_dry_run.py`, +imported from the modules that own them: members are scaffolded into local +bare repositories, nothing is created on GitHub, a `gh` on PATH logs and +exits 97, and the operator's git configuration is a file of the test's own. +The fixture's own remotes are only ever read: a commit to push goes to a +private bare repository the test makes and points the member's mount at. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +#: The family fixture and its names, from the module that owns them. +from test_family import ( # noqa: F401 + NAME, ORG, family, manifest, validate, +) +from test_family_add_refusals import holder, offline, refusal # noqa: F401 +#: "Changes nothing", asked as bytes in the order a bump would write. +from test_family_dry_run import bump, commit_in_mount, isolated, pinned_at +from test_family_bump_remove_rollback import ( + BUMP_PUT_BACK, assert_same, bump_undone, snapshot, +) +#: Both runs over one holder, refused alike with nothing moved. +from test_family_bump_to_a_tag import both_runs +from test_family_member_path import MEMBER, OTHER, raw + +PATH = f"members/{MEMBER}" + +#: The line `project.yaml` records the member's own id on, as the scaffold +#: writes it, and the id the neighbour's row records. +OWN_ID_LINE = f'\nid: "{MEMBER.lower()}"\n'.encode("ascii") +OTHER_ID = OTHER.lower() + + +def records_the_neighbours_id(manifest_path: Path) -> None: + """`manifest_path`, the member's `project.yaml`, rewritten as BYTES to + record the neighbour's id in place of the member's own.""" + data = manifest_path.read_bytes() + assert data.count(OWN_ID_LINE) == 1, "fixture drift: the id line" + manifest_path.write_bytes(data.replace( + OWN_ID_LINE, f'\nid: "{OTHER_ID}"\n'.encode("ascii"))) + + +def duplicate_id_refusal(to: str) -> tuple[str, str]: + """The two lines `bump --to ` is refused with when the member's + `project.yaml` at `to` records the neighbour's id.""" + return ( + f"REFUSED member-duplicate-id: {ORG}/{MEMBER} at {to[:12]} is " + f"already a member of {NAME} under another name: its project.yaml " + f"there records id '{OTHER_ID}', and the row {OTHER} pins " + f"{ORG}/{OTHER} with that id. Bumping {MEMBER} to it would pin one " + "project twice, which `validate-family.py` refuses", + "Remediation: a family pins each project once. Pass `--to` a commit " + f"of {ORG}/{MEMBER} whose project.yaml records an id no other row " + f"records. If {ORG}/{MEMBER} and {ORG}/{OTHER} are one project, " + f"`family.py remove --member {OTHER}` takes the other row out first. " + "Two projects that record one id are a fault in one of their " + "`project.yaml` files, mended in that project, not in the family.") + + +# --- the duplicate id --------------------------------------------------------- + +def test_bump_to_a_commit_recording_another_rows_id_is_refused_alike( + holder, offline, tmp_path): + """THE ISSUE'S FIRST LINE. The member's `project.yaml` at `--to` records + the neighbour's id. `bump` committed it, exit 0, and the holder's own + gate refused `member-duplicate-id`. Now both runs refuse it under that + code, in the same two lines, before the checkout, and the holder is as + it was and still passes its gate.""" + env = isolated(offline, tmp_path) + to = commit_in_mount(holder, env, lambda mount: records_the_neighbours_id( + mount / "project.yaml")) + + stderr = both_runs(holder, env, to) + + assert refusal(stderr) == duplicate_id_refusal(to) + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +@pytest.mark.skipif(sys.platform == "win32", + reason="a symbolic link in a checkout needs privileges " + "on Windows that a runner does not grant") +def test_a_duplicate_id_read_through_a_linked_manifest_is_put_back( + holder, offline, tmp_path): + """A `project.yaml` that is a SYMLINK at `--to` is only its link text in + the member's objects; a checkout reads through it, and the dry run + leaves its id to the real run, as it always has. The real run asks the + id its checkout read, which is the id the row would record: refused + `member-duplicate-id`, and the member put back where it stood, with the + sentence that says so: `commit_in_mount` left it detached at the pin.""" + env = isolated(offline, tmp_path) + + def link(mount: Path) -> None: + (mount / "project.yaml").rename(mount / "manifest.yaml") + records_the_neighbours_id(mount / "manifest.yaml") + (mount / "project.yaml").symlink_to("manifest.yaml") + + to = commit_in_mount(holder, env, link) + at = pinned_at(holder, env) + before = snapshot(holder, env) + + real = bump(holder, env, to) + + assert_same(before, snapshot(holder, env), BUMP_PUT_BACK) + assert real.returncode == 2, real.stdout + real.stderr + assert real.stdout == "" + said, remedy = duplicate_id_refusal(to) + assert refusal(real.stderr) == ( + f"{said}. {bump_undone(f'{PATH} checked out at {at[:12]} again')}", + remedy) + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +# --- the commit not yet pushed -------------------------------------------------- + +def a_private_remote(holder: Path, env: dict, tmp_path: Path) -> Path: + """A bare repository of the test's own holding the member's history, its + `main` at the member's tip, with the member's mount pointed at it: the + remote a person pushes to. Filled by a push from the mount, so the + fixture's own remote is not read through its files.""" + mount = holder / PATH + remote = tmp_path / f"{MEMBER}-private.git" + raw(tmp_path, env, "init", "-q", "--bare", str(remote)) + raw(mount, env, "push", "-q", str(remote), + "refs/remotes/origin/main:refs/heads/main") + raw(mount, env, "remote", "set-url", "origin", str(remote)) + return remote + + +def test_bump_to_a_commit_not_pushed_says_to_push_it_first( + holder, offline, tmp_path): + """THE ISSUE'S SECOND LINE, IN ITS OWN FLOW. A commit is made in a + working clone of the member and not pushed. Both runs refused it in + git's words, the real run's `reference is not a tree`. Now both refuse + it `member-to-unknown`, in the same two lines, naming the commit and + saying to push it first, with nothing moved; and once it is pushed, the + same command pins it and the holder passes its gate.""" + env = isolated(offline, tmp_path) + remote = a_private_remote(holder, env, tmp_path) + work = tmp_path / f"{MEMBER}-work" + raw(tmp_path, env, "clone", "-q", "--no-local", "-b", "main", "--", + str(remote), str(work)) + (work / "LOCAL.md").write_bytes(b"made here, not pushed yet\n") + raw(work, env, "add", "--", "LOCAL.md") + raw(work, env, "commit", "-q", "-m", "A commit only the clone has") + made = raw(work, env, "rev-parse", "HEAD").stdout.decode("ascii").strip() + + stderr = both_runs(holder, env, made) + + assert refusal(stderr) == ( + f"REFUSED member-to-unknown: --to {made} is not a commit any branch " + f"of {ORG}/{MEMBER} reaches: {PATH} fetched from its origin and " + "holds no object with that id", + f"Remediation: push it first, to a branch of {ORG}/{MEMBER}, and run " + "the same command again. A pin the rest of the world cannot fetch is " + "a family the rest of the world cannot bootstrap. If it is pushed, " + "check the commit: it belongs to the member's ASSEMBLY ROOT, not to " + "one of its legs.") + offline.assert_left_nothing() + + raw(work, env, "push", "-q", "origin", "HEAD:refs/heads/main") + again = bump(holder, env, made) + + assert again.returncode == 0, again.stdout + again.stderr + row = {r["project"]: r for r in manifest(holder)["members"]}[MEMBER] + assert row["pin"]["commit"] == made == pinned_at(holder, env) + assert validate(holder).returncode == 0 + + +def test_a_commit_missing_after_a_failed_fetch_is_not_called_unpushed( + holder, offline, tmp_path): + """The fetch fails, as for a private member with no credential, so the + member's object store is all either run can ask. A `--to` it does not + hold is refused `member-to-unknown` by both runs alike, after the fetch's + own note, saying the fetch failed and that whether the remote has it is + not known, and to push it first if it is not pushed yet.""" + env = isolated(offline, tmp_path) + raw(holder / PATH, env, "remote", "set-url", "origin", + str(tmp_path / "gone.git")) + missing = "deadbeef" * 5 + + stderr = both_runs(holder, env, missing) + + lines = stderr.splitlines() + assert lines[0] == (f" note: could not fetch {PATH} from its origin; " + "the commit must already be in its object store") + assert refusal("\n".join(lines[1:])) == ( + f"REFUSED member-to-unknown: --to {missing} is not a commit {PATH} " + "holds, and its fetch from its origin failed, so whether a branch of " + f"{ORG}/{MEMBER} reaches it is not known", + f"Remediation: push it first, to a branch of {ORG}/{MEMBER}, if it is " + f"not pushed yet; make {ORG}/{MEMBER} readable from here — if it is " + "PRIVATE, check `gh auth status` or your SSH key — and run the same " + "command again. A pin the rest of the world cannot fetch is a family " + "the rest of the world cannot bootstrap.") + offline.assert_left_nothing() diff --git a/tests/test_family_dry_run.py b/tests/test_family_dry_run.py index b8daf51a..78a9ecd0 100644 --- a/tests/test_family_dry_run.py +++ b/tests/test_family_dry_run.py @@ -431,8 +431,11 @@ def test_a_refusal_is_the_same_refusal_under_dry_run( def test_bump_dry_run_of_a_commit_the_member_does_not_have_exits_2( holder, offline, tmp_path): - """A `--to` the member's objects do not hold after the fetch is exit 2, - as the real run's checkout of it is, and neither prints a row.""" + """A `--to` the member's objects do not hold after the fetch is exit 2 + from both runs, and neither prints a row. The dry run's resolve and the + real run's checkout failed on it in git's words; both now refuse it + `member-to-unknown`, in the same words, saying to push it first, as + `test_family_bump_to_checks.py` holds it (#541).""" env = isolated(offline, tmp_path) missing = "deadbeef" * 5 before = state(holder, env) @@ -442,9 +445,9 @@ def test_bump_dry_run_of_a_commit_the_member_does_not_have_exits_2( assert dry.returncode == real.returncode == 2, dry.stdout + dry.stderr assert dry.stdout == real.stdout == "" - assert dry.stderr.startswith( - f"REFUSED resolving {missing[:12]} in members/{MEMBER}, where a real " - "bump checks it out.") + assert dry.stderr == real.stderr + assert refusal(dry.stderr)[0].startswith( + f"REFUSED member-to-unknown: --to {missing} ") assert_as_it_was(holder, env, before, BUMP_ORDER) From 8024f9c914fd7957e83ec376f9d851e5a86cccaa Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Wed, 14 Oct 2026 23:37:14 +0000 Subject: [PATCH 2/2] family.py: `bump`'s peel line says the commit a tag names is what WOULD be pinned, and `add --at` and `bump --to` say in `--help` that an annotated tag's own id is peeled, so the line is no longer false in a dry run or above a refusal, and docs/cli.md names the peel (#541) The review of eeadc65 (rv541: 0 blockers, 1 should, 5 nits) is answered: - S1. The line both runs print when `--to` is an annotated tag ended "is what is pinned". The dry run prints it, and so does a run refused by a question asked after the peel (`member-duplicate-id` through a tag), and neither pins anything. It now ends "is what would be pinned". A new test holds a duplicate id behind a tag: both runs print the line, refuse in the same two lines, exit 2, and leave the holder as it was. - N1. A new test holds the branch hint on the holder's tracking branch with a tag's id: the dry run's hint and the real run's refusal both name `family/bump--`, not a branch made from the commit the tag names. - N2. `_checked_to_names_a_commit`'s docstring names the one input where `add --at` and `bump --to` still differ: a tag whose commit is on no branch, which `bump`'s fetch does not bring and refuses `member-to-unknown`. The behaviour is left as it is. - N3. `add --at`'s and `bump --to`'s `--help` say that an annotated tag's own 40-hex id is peeled to the commit it names; `docs/cli.md` is regenerated by `scripts/render-cli-reference.py`. - N4 and N5 are the pull request body's, and are answered there. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 --- docs/cli.md | 6 ++- scripts/family.py | 21 ++++++-- tests/test_family_bump_to_a_tag.py | 14 +++--- tests/test_family_bump_to_checks.py | 77 ++++++++++++++++++++++++++--- 4 files changed, 100 insertions(+), 18 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index c793a3e0..c198acad 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -754,7 +754,8 @@ options: --member ORG/PROJECT the member's ASSEMBLY ROOT, `/` — never a leg --at COMMIT pin this 40-hex commit instead of the member's current - tip + tip (an annotated tag's own 40-hex id is peeled to the + commit it names) --local-remote-dir LOCAL_REMOTE_DIR resolve the member to a bare repository here instead of github.com (the TEST path) @@ -787,7 +788,8 @@ options: --family-root FAMILY_ROOT --member PROJECT --to COMMIT the 40-hex commit to move the gitlink and the pin to, - together + together (an annotated tag's own 40-hex id is peeled + to the commit it names, as `add --at` peels one) --trailer "KEY: VALUE" append this `: ` line to the commit this writes; repeatable, kept in the order given diff --git a/scripts/family.py b/scripts/family.py index f7edd8b0..e3b74b52 100755 --- a/scripts/family.py +++ b/scripts/family.py @@ -4083,6 +4083,16 @@ def _checked_to_names_a_commit(member: Path, path: str, row: dict, working clone beside the holder and not pushed yet, and the fetch of the mount brings only what the member's remote has. When that fetch failed, the refusal says so and does not claim the remote lacks it. + + A TAG WHOSE COMMIT IS ON NO BRANCH is refused `member-to-unknown` too, + and so is that commit, while `add --at` pins the same tag's id. The + fetch here brings every branch and, by git's own rule, only the tags + that point into the history those branches bring; `add`'s bare clone + brings every tag. A `git push origin v2` without the branch makes one, + and the refusal's push to a branch is what lets `bump` pin it. Left as + it is: `bump-leg.py` refuses its twin the same way, as + `bump-leg-commit-not-on-remote`, and fetching every tag would write refs + the person's member did not have, which is a new act. """ repository = row.get("repository") try: @@ -4402,12 +4412,14 @@ def cmd_bump(args) -> int: # tree or a blob is refused alike, with nothing in the holder moved. # `suggested` keeps the id as typed: the real run's branch refusal named # it before the fetch, and the dry run's `branch` line names the same. + # The line says WOULD BE PINNED: the dry run prints it, and so does a + # run refused by a question asked after it, and neither pins anything. typed = commit commit = _checked_to_names_a_commit(member, path, row, typed, fetched) if commit != typed: print(f"--to {typed[:12]} is an annotated tag in " f"{row.get('repository')}; the commit it names, " - f"{commit[:12]}, is what is pinned", flush=True) + f"{commit[:12]}, is what would be pinned", flush=True) if args.dry_run: return _bump_dry_run(family, row, path, commit, args.trailer, fetched, suggested) @@ -4911,7 +4923,8 @@ def build_parser() -> argparse.ArgumentParser: "never a leg") add.add_argument("--at", default=None, metavar="COMMIT", help="pin this 40-hex commit instead of the member's " - "current tip") + "current tip (an annotated tag's own 40-hex id is " + "peeled to the commit it names)") add.add_argument("--local-remote-dir", type=Path, default=None, help="resolve the member to a bare repository here " "instead of github.com (the TEST path)") @@ -4937,7 +4950,9 @@ def build_parser() -> argparse.ArgumentParser: bump.add_argument("--member", required=True, metavar="PROJECT") bump.add_argument("--to", required=True, metavar="COMMIT", help="the 40-hex commit to move the gitlink and the pin " - "to, together") + "to, together (an annotated tag's own 40-hex id is " + "peeled to the commit it names, as `add --at` " + "peels one)") bump.add_argument("--trailer", action="append", metavar='"KEY: VALUE"', default=[], type=trailer_line, help="append this `: ` line to the commit " diff --git a/tests/test_family_bump_to_a_tag.py b/tests/test_family_bump_to_a_tag.py index de855c5e..54f5d871 100644 --- a/tests/test_family_bump_to_a_tag.py +++ b/tests/test_family_bump_to_a_tag.py @@ -72,9 +72,10 @@ def oid(mount: Path, env: dict, rev: str) -> str: def peeled_line(to: str, target: str) -> str: """The line both runs of `bump --to ` begin with when `to` is an - annotated tag that names the commit `target`.""" + annotated tag that names the commit `target`: what WOULD be pinned, + since the dry run prints it and so does a run refused after it.""" return (f"--to {to[:12]} is an annotated tag in {ORG}/{MEMBER}; the " - f"commit it names, {target[:12]}, is what is pinned") + f"commit it names, {target[:12]}, is what would be pinned") def pins_the_tagged_commit(holder: Path, env: dict, to: str, @@ -93,10 +94,11 @@ def pins_the_tagged_commit(holder: Path, env: dict, to: str, assert validate(holder).returncode == 0 -def both_runs(holder: Path, env: dict, to: str) -> str: +def both_runs(holder: Path, env: dict, to: str, stdout: str = "") -> str: """`bump --to ` with `--dry-run` and then for real, over the same - holder: both refused, exit 2, in the same words, with nothing written to - stdout and nothing in the holder moved. Returns the shared stderr.""" + holder: both refused, exit 2, in the same words, with `stdout` and + nothing else written to stdout (nothing, unless a line came before the + refusal) and nothing in the holder moved. Returns the shared stderr.""" before = state(holder, env) dry = bump(holder, env, to, "--dry-run") @@ -106,7 +108,7 @@ def both_runs(holder: Path, env: dict, to: str) -> str: assert_as_it_was(holder, env, before, BUMP_ORDER) assert dry.returncode == real.returncode == 2, ( dry.stdout + dry.stderr + real.stdout + real.stderr) - assert dry.stdout == real.stdout == "" + assert dry.stdout == real.stdout == stdout assert dry.stderr == real.stderr return real.stderr diff --git a/tests/test_family_bump_to_checks.py b/tests/test_family_bump_to_checks.py index 029ddc20..ea27813e 100644 --- a/tests/test_family_bump_to_checks.py +++ b/tests/test_family_bump_to_checks.py @@ -3,7 +3,8 @@ unpushed commit first (#541). TWO LINES OF #541 ARE HELD HERE; its riding line, an annotated tag's own id, -is held by `test_family_bump_to_a_tag.py`. +is held by `test_family_bump_to_a_tag.py`, and here where it meets the other +two: a refusal after the peel, and the branch a person is told to make. THE DUPLICATE ID. `add` refuses a member whose `project.yaml` records the `id` another row records (#330, `member-duplicate-id`): that is one project @@ -45,13 +46,20 @@ ) from test_family_add_refusals import holder, offline, refusal # noqa: F401 #: "Changes nothing", asked as bytes in the order a bump would write. -from test_family_dry_run import bump, commit_in_mount, isolated, pinned_at +from test_family_dry_run import ( + BUMP_ORDER, assert_as_it_was, bump, commit_in_mount, isolated, pinned_at, + state, +) from test_family_bump_remove_rollback import ( BUMP_PUT_BACK, assert_same, bump_undone, snapshot, ) -#: Both runs over one holder, refused alike with nothing moved. -from test_family_bump_to_a_tag import both_runs -from test_family_member_path import MEMBER, OTHER, raw +#: Both runs over one holder, refused alike with nothing moved; an annotated +#: tag, and the line a peel prints. +from test_family_bump_to_a_tag import annotated, both_runs, peeled_line +from test_family_member_path import MEMBER, OTHER, older, raw +#: The shell's reading of a printed line, and of a refusal's `switch`. +from test_family_printed_quoting import host_words +from test_family_tracking_branch import remedy_switch PATH = f"members/{MEMBER}" @@ -87,7 +95,7 @@ def duplicate_id_refusal(to: str) -> tuple[str, str]: "`project.yaml` files, mended in that project, not in the family.") -# --- the duplicate id --------------------------------------------------------- +# --- the duplicate id -------------------------------------------------------- def test_bump_to_a_commit_recording_another_rows_id_is_refused_alike( holder, offline, tmp_path): @@ -142,7 +150,62 @@ def link(mount: Path) -> None: offline.assert_left_nothing() -# --- the commit not yet pushed -------------------------------------------------- +def test_a_duplicate_id_behind_a_tag_is_refused_after_the_peel_line( + holder, offline, tmp_path): + """THE PEEL LINE ABOVE A REFUSAL. `--to` is an annotated tag of a + commit whose `project.yaml` records the neighbour's id. Both runs peel + it and say so, in the line that says what WOULD be pinned, then refuse + `member-duplicate-id` for the commit the tag names, exit 2, and pin + nothing: the holder is as it was and still passes its gate.""" + env = isolated(offline, tmp_path) + made = commit_in_mount(holder, env, lambda mount: + records_the_neighbours_id(mount / "project.yaml")) + to = annotated(holder / PATH, env, "v-duplicate", made) + + stderr = both_runs(holder, env, to, peeled_line(to, made) + "\n") + + assert refusal(stderr) == duplicate_id_refusal(made) + assert validate(holder).returncode == 0 + offline.assert_left_nothing() + + +# --- the branch a person is told to make ------------------------------------- + +def test_both_runs_name_the_branch_for_the_tag_id_as_typed( + family, holder, offline, tmp_path): + """On the holder's tracking branch the real run refuses + `family-on-tracking-branch` before it fetches, so before any peel, + naming `family/bump--`. The dry run + fetches and peels, and its hint still names that branch, not one made + from the commit the tag names: the branch a person makes from the dry + run is the one the real run asks for.""" + env = isolated(offline, tmp_path) + raw(holder, env, "switch", "-q", "-C", "main") + target = older(family) + to = annotated(holder / PATH, env, "v-release", target) + assert to[:12] != target[:12], "premise: the two names differ" + switch = ["git", "-C", str(holder.resolve()), "switch", "-c", + f"family/bump-{MEMBER.lower()}-{to[:12]}"] + before = state(holder, env) + + dry = bump(holder, env, to, "--dry-run") + real = bump(holder, env, to) + + assert_as_it_was(holder, env, before, BUMP_ORDER) + assert dry.returncode == 0, dry.stdout + dry.stderr + lines = dry.stdout.splitlines() + assert lines[0] == peeled_line(to, target) + at = lines.index("branch main is the holder's tracking branch, " + "where a real bump refuses (family-on-tracking-branch); " + "make a branch first:") + assert host_words(lines[at + 1]) == switch + assert real.returncode == 2, real.stdout + real.stderr + assert real.stdout == "" + assert remedy_switch(real.stderr) == switch + offline.assert_left_nothing() + + +# --- the commit not yet pushed ----------------------------------------------- def a_private_remote(holder: Path, env: dict, tmp_path: Path) -> Path: """A bare repository of the test's own holding the member's history, its