diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 77fc23cc..76949537 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -2093,6 +2093,33 @@ def _replace_spans(line: str, replacements: list[tuple[int, int, str]]) -> str: return line +def blocks_the_pin(report: ProfileReport, accept_changed: bool = False) -> int: + """What would keep source_commit where it is after a recale. + + Two kinds. A ref the writer never regenerates, annotated or under a mode + key, whose parts have moved. And a prose run whose tokens cannot be + located well enough to rewrite, which stays describing the pinned + revision whatever else moves. Either one makes bump_commit refuse, so + either one has to stop the recale as well. + """ + movable = REBASE_STATUSES + (("CHANGED",) if accept_changed else ()) + blocked = pending_recale(report, accept_changed) + for entry in report.entries or []: + if entry.kind != "prose" or _prose_moves(entry, movable): + continue + if any( + part.status in movable + and part.start is not None + and ( + (part.start, part.end) != (part.part.start, part.part.end) + or part.new_path + ) + for part in entry.parts + ): + blocked += 1 + return blocked + + def rebase_refs( path: Path, report: ProfileReport, accept_changed: bool = False ) -> list[str]: @@ -2111,6 +2138,13 @@ def rebase_refs( blocking = [s for s in REVIEW_STATUSES if s not in statuses] if any((report.counts or {}).get(s) for s in blocking): return [] + if blocks_the_pin(report, accept_changed): + # Something here will not move: an annotated ref, one under a mode + # key, or a prose run this pass cannot rewrite without guessing. + # bump_commit will refuse while it stands, and recaling the rest + # would leave the profile describing two revisions at once. All or + # nothing means the pin too, not only the refs. + return [] text = path.read_text(encoding="utf-8") document = yaml.safe_load(text) citations = collect_citations(document) @@ -2603,22 +2637,38 @@ def _apply_writes(args, name: str, profile: dict, report: ProfileReport) -> None print(f"{name}: source_commit {report.pin[:7]}") if not (args.rebase_refs or args.bump_commit): return - with contextlib.ExitStack() as stack: - if args.dry_run: - # Plan on a copy through the production write path rather than a - # parallel branch: the bump then reads the text the rebase would - # have left, which is the only text under which it is reachable. - scratch = stack.enter_context(tempfile.TemporaryDirectory()) - target = Path(scratch) / path.name - target.write_bytes(path.read_bytes()) - recale, bumped = "would recale ", "would set source_commit ->" - else: - target, recale, bumped = path, "", "source_commit ->" + both = args.rebase_refs and args.bump_commit + with tempfile.TemporaryDirectory() as scratch: + # Always work on a copy. A dry run reports what it left there; a real + # run promotes it. Asking for both writes makes the pass atomic: the + # pin has to follow the refs or neither moves, because a profile whose + # refs describe one revision and whose pin names another is exactly + # what the all-or-nothing rule exists to prevent. + target = Path(scratch) / path.name + target.write_bytes(path.read_bytes()) + recale, bumped = ( + ("would recale ", "would set source_commit ->") if args.dry_run + else ("", "source_commit ->") + ) + applied = [] if args.rebase_refs and not report.skipped: - for line in rebase_refs(target, report, args.accept_changed): - print(f"{name}: {recale}{line}") - if args.bump_commit and bump_commit(target, report, args.accept_changed): + applied = rebase_refs(target, report, args.accept_changed) + moved = bool(args.bump_commit) and bump_commit( + target, report, args.accept_changed + ) + if both and applied and not moved: + print( + f"{name}: refs recaled but the pin will not follow, so nothing " + "was written; the prose or an annotated ref has to move first", + file=sys.stderr, + ) + return + for line in applied: + print(f"{name}: {recale}{line}") + if moved: print(f"{name}: {bumped} {report.head[:7]}") + if not args.dry_run and (applied or moved): + path.write_bytes(target.read_bytes()) def _cited_paths(report: ProfileReport) -> list[str]: diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 603407d5..364697c7 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -2145,6 +2145,77 @@ class TestWriteDryRun(unittest.TestCase): self.assertEqual(self.path.read_text(), before) +class TestRebaseWaitsForThePin(unittest.TestCase): + """Recaling refs the pin cannot follow manufactures the desync. + + An annotated ref is never rewritten, because regenerating it would drop + the author's prose, so its parts stay on the pinned line numbers and + bump_commit refuses. Moving the other refs anyway leaves the profile + describing two revisions at once, which is the state the all-or-nothing + rule exists to prevent. mariani was left in it. + """ + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.path = Path(self.tmp.name) / "p.yml" + self.path.write_text(SAMPLE, encoding="utf-8") + + def tearDown(self): + self.tmp.cleanup() + + def _report(self, entries, counts): + return ProfileReport( + name="p", repo="o/n", pin="pin", head="head", + entries=entries, counts=counts, + ) + + def test_nothing_moves_while_an_annotated_ref_holds_the_pin(self): + movable = PartResult(RefPart("a.c", 10, 12), "SHIFTED", None, 30, 32, []) + stuck = PartResult( + RefPart("b.c", 5, 5, "b.c:5"), "SHIFTED", None, 9, 9, [] + ) + report = self._report( + [ + EntryReport("a.bin", "a.c:10-12", "SHIFTED", [movable]), + EntryReport("b.bin", "b.c:5, (just a note)", "SHIFTED", [stuck]), + ], + {"SHIFTED": 2}, + ) + self.assertTrue(profile_sync.pending_recale(report)) + self.assertEqual(rebase_refs(self.path, report), []) + self.assertIn('source_ref: "a.c:10-12"', self.path.read_text()) + + def test_asking_for_both_writes_nothing_when_the_pin_cannot_follow(self): + """The pass is atomic: refs and pin move together or not at all.""" + movable = PartResult(RefPart("a.c", 10, 12), "SHIFTED", None, 30, 32, []) + report = self._report( + [EntryReport("a.bin", "a.c:10-12", "SHIFTED", [movable])], + {"SHIFTED": 1}, + ) + report.head = None # bump_commit refuses without a head + before = self.path.read_text() + args = argparse.Namespace( + emulators_dir=str(self.path.parent), backfill_commits=False, + rebase_refs=True, bump_commit=True, accept_changed=False, + dry_run=False, + ) + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + profile_sync._apply_writes(args, "p", yaml.safe_load(SAMPLE), report) + self.assertEqual(self.path.read_text(), before) + self.assertIn("nothing was written", err.getvalue()) + self.assertNotIn("a.c:30-32", out.getvalue()) + + def test_a_profile_the_pin_can_follow_still_moves(self): + movable = PartResult(RefPart("a.c", 10, 12), "SHIFTED", None, 30, 32, []) + report = self._report( + [EntryReport("a.bin", "a.c:10-12", "SHIFTED", [movable])], + {"SHIFTED": 1}, + ) + self.assertFalse(profile_sync.pending_recale(report)) + self.assertEqual(rebase_refs(self.path, report), ["a.c:10-12 -> a.c:30-32"]) + + class TestBumpCommit(unittest.TestCase): def setUp(self): self.tmp = tempfile.TemporaryDirectory() diff --git a/wiki/tools.md b/wiki/tools.md index 8f528cf4..416f5a49 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -345,6 +345,14 @@ python scripts/profile_sync.py --all --rebase-refs --bump-commit --dry-run python scripts/profile_sync.py --all --rebase-refs --bump-commit ``` +Asking for both makes the pass atomic. The work happens on a copy, and the +copy is promoted only when the pin follows the refs; a profile whose refs +describe one revision while its pin names another is the state the +all-or-nothing rule exists to prevent, and recaling alone produces it. What +holds a pin back is an annotated ref, one under a mode key, or a prose run the +pass cannot rewrite without guessing: those are repaired first, by hand or +with `--realign-prose`. + The first prints the plan and changes nothing: `would recale` per ref and `would set source_commit` per profile. It reaches that plan by running the real write path over a throwaway copy, so the planned bump reads the text