From c4e16e583eba7e8366aaa1b9e1337ce03142049c Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:27:33 +0200 Subject: [PATCH] fix: realign stale prose before a pin moves --- scripts/profile_sync.py | 67 +++++++++++++++++++++++++++----------- tests/test_profile_sync.py | 39 ++++++++++++++++++++-- 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index b73a541f..cfe1c17e 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -2628,10 +2628,14 @@ def _realign_part( def _writing_pairs( document: dict, repos: list, revisions: list[tuple[str, dict]], intro_sha: str -) -> tuple[list, list[str]]: - """(repo, writing pin, current pin) per declared repository, and the - repositories whose writing pin the history cannot single out.""" - pairs = [] +) -> tuple[list[list], list[str]]: + """Every reading of where the text was written, and the ambiguities. + + Each reading lists (repo, writing pin, current pin) per declared + repository. A repository whose text lived under several pins gives one + reading per pin; the caller settles them when they all agree. + """ + options: list[list] = [] ambiguous: list[str] = [] for pin_field, repo in repos: current = document.get(f"{pin_field}_commit") @@ -2640,9 +2644,15 @@ def _writing_pairs( written = _writing_pins(revisions, intro_sha, f"{pin_field}_commit") if len(written) > 1: ambiguous.append(f"{pin_field}_commit {' -> '.join(written)}") - elif written and written[0] != current: - pairs.append((repo, written[0], current)) - return pairs, ambiguous + moved = [(repo, pin, current) for pin in written if pin != current] + # The current pin among the candidates reads as "no move" for that + # repository, which the other candidates must then agree with. + options.append(moved + ([None] if current in written else [])) + readings = [ + [pair for pair in combo if pair is not None] + for combo in itertools.product(*options) + ] if options else [[]] + return readings, ambiguous def realign_prose( @@ -2703,22 +2713,29 @@ def realign_prose( if intro_sha is None: # The scalar is not committed yet: written now, under this pin. continue - pairs, ambiguous = _writing_pairs(document, repos, revisions, intro_sha) - if ambiguous: - # Moving from the wrong one rewrites a correct citation onto - # someone else's code, with nothing to show it happened. - messages.extend( - f"read again: {citation.field}: {citation.ref} (pin moved under " - f"this text: {item}; the history does not say which it describes)" - for item in ambiguous - ) - continue - if not pairs: + readings, ambiguous = _writing_pairs(document, repos, revisions, intro_sha) + if not any(readings): continue moves: dict[int, tuple[int, int, str | None]] = {} blocked: list[str] = [] for index, part in enumerate(citation.parts): - outcome = _realign_part(part, pairs, cache_dir, offline) + # Several pins under one text: the history cannot say which the + # text describes, but when the cited range lands in the same + # place from every one of them the question does not matter. + results = [ + _realign_part(part, pairs, cache_dir, offline) if pairs else None + for pairs in readings + ] + outcomes = {repr(result): result for result in results} + if len(outcomes) > 1: + # Moving from the wrong one rewrites a correct citation onto + # someone else's code, with nothing to show it happened. + blocked.extend( + f"pin moved under this text: {item}; the readings disagree" + for item in ambiguous + ) + continue + outcome = next(iter(outcomes.values())) if outcome is None: continue state, payload = outcome @@ -2873,6 +2890,18 @@ 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 + # Prose written under an older pin describes that revision: anchored from + # the current pin, a recale or a bump follows someone else's code + # (boom3 FileSystem.cpp:2125, written at b810234e, an unrelated + # OpenFileRead at 132dfddb). It moves first, from the pin it was written at. + stale = realign_prose(path, args.cache_dir, args.offline, dry_run=True) + if stale: + print( + f"{name}: {len(stale)} prose citation(s) written under an older pin; " + "run --realign-prose or read them before the pin moves", + file=sys.stderr, + ) + return with tempfile.TemporaryDirectory() as scratch: # Always work on a copy. A dry run reports what it left there; a real # run promotes it. A recale rewrites the refs for HEAD, so the pin diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 4cbe5f96..dba462f1 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -12,6 +12,7 @@ import sys import tempfile import unittest from pathlib import Path +from unittest import mock import yaml @@ -2162,7 +2163,7 @@ class TestWriteDryRun(unittest.TestCase): base = dict( emulators_dir=str(self.dir), backfill_commits=False, rebase_refs=False, bump_commit=False, accept_changed=False, - dry_run=True, + dry_run=True, cache_dir=str(self.dir), offline=True, ) base.update(over) return argparse.Namespace(**base) @@ -2214,6 +2215,40 @@ class TestWriteDryRun(unittest.TestCase): self.assertFalse(bump_commit(self.path, report)) self.assertEqual(self._run(self._args(bump_commit=True), report), "") + def test_two_writing_pins_give_two_readings(self): + """vitaquake2: written at one pin, re-pinned under the same text. Each + pin is a reading; a run settles when every reading agrees.""" + revisions = [ + ("c2", {"source_commit": "pin2", "notes": "x"}), + ("c1", {"source_commit": "pin1", "notes": "x"}), + ] + repo = object() + readings, ambiguous = profile_sync._writing_pairs( + {"source_commit": "pin2"}, [("source", repo)], revisions, "c1" + ) + self.assertEqual(readings, [[(repo, "pin1", "pin2")], []]) + self.assertTrue(ambiguous) + + def test_stale_prose_holds_the_pin(self): + """boom3: a note written at an older pin is not anchored from this one.""" + before = self.path.read_text() + part = PartResult( + RefPart("a.c", 10, 12, "a.c:10-12"), "ANCHORED", None, 10, 12, [] + ) + report = ProfileReport( + name="p", repo="o/n", pin="pin", head="newhead", + entries=[EntryReport("a.bin", "a.c:10-12", "ANCHORED", [part])], + counts={"ANCHORED": 1}, + ) + stale = ["read again: notes: FileSystem.cpp:2125 (pin moved under this text)"] + errors = io.StringIO() + with mock.patch.object(profile_sync, "realign_prose", return_value=stale), \ + contextlib.redirect_stderr(errors): + output = self._run(self._args(bump_commit=True, dry_run=False), report) + self.assertNotIn("source_commit ->", output) + self.assertIn("older pin", errors.getvalue()) + self.assertEqual(self.path.read_text(), before) + def test_bump_states_the_pin_and_leaves_the_file(self): before = self.path.read_text() part = PartResult( @@ -2438,7 +2473,7 @@ class TestRebaseWaitsForThePin(unittest.TestCase): args = argparse.Namespace( emulators_dir=str(self.path.parent), backfill_commits=False, rebase_refs=True, bump_commit=True, accept_changed=False, - dry_run=False, + dry_run=False, cache_dir=str(self.path.parent), offline=True, ) out, err = io.StringIO(), io.StringIO() with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):