fix: move the refs and the pin together or not at all

Recaling refs while the pin stays put produces exactly the state the
all-or-nothing rule exists to prevent: a profile whose refs describe one
revision and whose source_commit names another. The tool manufactured it
on mariani, where three prose runs it could not rewrite kept
bump_commit refusing while eleven refs had already moved.

Asking for both writes is now atomic. The work happens on a copy, which
is promoted only when the pin follows, and a rebase is refused outright
when something visible beforehand will hold the pin: an annotated ref,
one under a mode key, or a prose run whose tokens cannot be located well
enough to rewrite. Where the block only appears after the write, the
copy is discarded and the profile is named on stderr rather than left
half moved.

mariani is back on its pin and stays at four refs to read, which is
honest: three of them have to be rewritten by hand before anything can
advance.
This commit is contained in:
Abdessamad Derraz committed 2026-09-04 18:55:15 +02:00
1 parent 09406ba5d4
commit c738073f66
3 files changed
+143 -14

No files matched your search

+64 -14
View File
@@ -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]:
+71
View File
@@ -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()
+8
View File
@@ -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