From 34876ba61008fd929aece97a4936f6193165ee36 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:16:01 +0200 Subject: [PATCH] fix: keep refs and pin on the same revision --- scripts/profile_sync.py | 39 ++++++++++++++++++++++++++++++++------ tests/test_profile_sync.py | 19 +++++++++++++++++++ 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 0127ed83..d3cf064c 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -527,9 +527,7 @@ def build_report( owners[path] = _locate(path) return owners[path] - def rename_getter(path: str) -> tuple[str | None, list[str]]: - """Resolved only when a cited file has vanished, then memoised.""" - view, actual = resolve_path(path) + def _context_for(view: RepoView): key = view.repo.slug if key not in context: comparison = upstream.compare( @@ -545,8 +543,28 @@ def build_report( "rename search is partial", file=sys.stderr, ) - comparison, tree = context[key] - return resolve_rename(comparison, actual, tree) + return context[key] + + def rename_getter(path: str) -> tuple[str | None, list[str]]: + """Resolved only when a cited file has vanished, then memoised. + + Every declared repository is searched, not just the one that owns the + path: a profile citing both a port and its upstream may point at a file + that only the other one carries. + """ + owner, actual = resolve_path(path) + ordered = [owner] + [v for v in views if v is not owner] + candidates: list[str] = [] + for view in ordered: + comparison, tree = _context_for(view) + found, near = resolve_rename(comparison, actual, tree) + if found: + return found, [] + # Only the owning repository reports near misses: pooling them + # across repositories turns a resolvable path into an ambiguous one. + if view is owner: + candidates = near + return None, candidates def fetch(which: str, path: str): view, actual = resolve_path(path) @@ -938,7 +956,16 @@ def _is_rewritable(ref: str) -> bool: def rebase_refs(path: Path, report: ProfileReport) -> list[str]: - """Recale the line ranges of parts whose content is unchanged.""" + """Recale the line ranges of parts whose content is unchanged. + + All or nothing per profile. `source_commit` names the revision the refs + are written against, so moving some refs to HEAD while others still + describe the pinned revision would leave the profile self-contradictory, + and the next comparison would read the pinned revision at line numbers + that only make sense at HEAD. + """ + if report.needs_review(): + return [] text = path.read_text(encoding="utf-8") document = yaml.safe_load(text) carriers = [ diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 5ecbf263..98c0fad8 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -662,6 +662,16 @@ class TestBuildReport(unittest.TestCase): report = build_report("test", self._profile(["other/absent.c:2"]), self.dir) self.assertEqual(report.entries[0].parts[0].status, "GONE") + def test_rename_is_searched_in_every_declared_repository(self): + self.files[("pinsha", "deep/src/stv.c")] = ["x", "hit"] + self.files[("headsha", "deep/src/stv.c")] = ["x", "hit"] + report = build_report( + "test", self._two_repo_profile(["ctrl/src/stv.c:2"]), self.dir + ) + part = report.entries[0].parts[0] + self.assertEqual(part.status, "RENAMED") + self.assertEqual(part.new_path, "deep/src/stv.c") + def test_missing_date_and_commit_is_skipped(self): report = build_report( "test", @@ -1062,6 +1072,15 @@ class TestRebaseRefs(unittest.TestCase): [], ) + def test_nothing_is_rebased_while_a_ref_needs_review(self): + shifted = PartResult(RefPart("a.c", 10, 12), "SHIFTED", None, 20, 22, []) + report = self._report( + [EntryReport("a.bin", "a.c:10-12", "SHIFTED", [shifted])] + ) + report.counts = {"SHIFTED": 1, "CHANGED": 1} + self.assertEqual(rebase_refs(self.path, report), []) + self.assertIn('source_ref: "a.c:10-12"', self.path.read_text()) + def test_annotated_ref_is_never_rewritten(self): self.path.write_text( SAMPLE.replace('"a.c:10-12"', '"a.c:10-12 (loads the kernel)"'),