diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 67ae045c..378729c9 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -1305,12 +1305,22 @@ def _resolve_project_words( if part.path not in resolved: resolved[part.path] = resolve_project_word(part.path, tree_order()) if resolved[part.path]: - part = RefPart(resolved[part.path], part.start, part.end, part.raw) + part = replace(part, path=resolved[part.path]) rewritten.append(part) out.append((display, ref, tokens, hashes, rewritten, citation)) return out +def _path_roles(refs: list) -> dict[str, set[str]]: + """The roles each cited path is written under across the profile.""" + roles: dict[str, set[str]] = {} + for *_, parts, _citation in refs: + for part in parts: + if part.role: + roles.setdefault(part.path, set()).add(part.role) + return roles + + def build_report( name: str, profile: dict, cache_dir: str, offline: bool = False, text: str | None = None, @@ -1328,20 +1338,20 @@ def build_report( if (repo := upstream.parse_repo(url)) is not None ) refs = [] - # The repository a part says its lines are in (`upstream src/a.c:5`). - cited_roles: dict[str, str] = {} for citation in collect_citations(profile, text): if citation.kind == "ref": - for p in split_source_ref(citation.ref): - role, _bare = ref_role(p.path) - if role: - cited_roles.setdefault(strip_repo_word(p.path, known), role) entry = citation.entry display = str(entry.get("name") or "") or citation.field if citation.label: display += f" [{citation.label}]" + # The repository a part says its lines are in (`upstream + # src/a.c:5`) stays on that part: the same path can be cited + # under both roles in one profile. parts = [ - RefPart(strip_repo_word(p.path, known), p.start, p.end, p.raw) + RefPart( + strip_repo_word(p.path, known), p.start, p.end, p.raw, + ref_role(p.path)[0], + ) for p in split_source_ref(citation.ref) ] refs.append(( @@ -1353,6 +1363,19 @@ def build_report( citation.field, citation.ref, [], [], list(citation.parts), citation, )) + # A part that names no repository is looked up first where its path is + # cited elsewhere: a note citing `src/a.c:12` beside source_refs that + # all read `upstream src/a.c` means that tree. + # A path cited under both roles lends neither, and such a part is + # compared in every repository rather than in whichever comes first. + path_roles = _path_roles(refs) + + def role_of(part: RefPart) -> str: + if part.role: + return part.role + lent = path_roles.get(part.path, ()) + return next(iter(lent)) if len(lent) == 1 else "" + # Directories the profile already cites, deepest first, to settle a bare # filename that several trees of one repository carry. cited_dirs: set[str] = set() @@ -1448,7 +1471,7 @@ def build_report( pin_trees: dict[str, list[str] | None] = {} def _locate( - path: str, start: int | None = None, tokens: tuple = () + path: str, start: int | None = None, tokens: tuple = (), wanted: str = "" ) -> tuple[RepoView, str]: """Repository and real path carrying a cited path, HEAD before pin. @@ -1539,7 +1562,6 @@ def build_report( return 1 if line.strip() else 0 # A part that names its repository is looked up there first. - wanted = cited_roles.get(path, "") ordered = sorted(views, key=lambda v: _view_role(v) != wanted) if wanted else views best = None for candidate in candidates: @@ -1572,11 +1594,11 @@ def build_report( return (best[1], best[2]) if best else (primary, path) def resolve_path( - path: str, start: int | None = None, tokens: tuple = () + path: str, start: int | None = None, tokens: tuple = (), role: str = "" ) -> tuple[RepoView, str]: - key = (path, start, tokens) + key = (path, start, tokens, role) if key not in owners: - owners[key] = _locate(path, start, tokens) + owners[key] = _locate(path, start, tokens, role) return owners[key] def _pin_tree_for(view: RepoView) -> list[str] | None: @@ -1634,14 +1656,14 @@ def build_report( ) return context[key] - def rename_getter(path: str) -> tuple[str | None, list[str]]: + def rename_getter(path: str, role: 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) + owner, actual = resolve_path(path, role=role) ordered = [owner] + [v for v in views if v is not owner] candidates: list[str] = [] for view in ordered: @@ -1673,6 +1695,7 @@ def build_report( start: int | None = None, tokens: Iterable[str] = (), forced: RepoView | None = None, + role: str = "", ) -> list[str] | None: """Lines for one path at one revision, read once per run. @@ -1680,10 +1703,11 @@ def build_report( on identity. """ tokens = tuple(tokens) - key = (which, path, start, tokens, forced and forced.repo.slug) + key = (which, path, start, tokens, forced and forced.repo.slug, role) if key not in lines_cache: view, actual = ( - (forced, path) if forced else resolve_path(path, start, tokens) + (forced, path) if forced + else resolve_path(path, start, tokens, role) ) sha = view.pin if which == PIN else view.head found = fetch_from(view.repo, sha, actual) @@ -1701,9 +1725,11 @@ def build_report( start: int | None = None, tokens: Iterable[str] = (), forced: RepoView | None = None, + role: str = "", ) -> tuple[str | None, str, str]: view, actual = ( - (forced, path) if forced else resolve_path(path, start, tuple(tokens)) + (forced, path) if forced + else resolve_path(path, start, tuple(tokens), role) ) slug = view.repo.slug if view is not primary else None return slug, upstream.raw_url(view.repo, view.head, actual), actual @@ -1715,12 +1741,20 @@ def build_report( both mamedev/mame and its libretro fork hold, with the same delimited token on the cited line, so no single attribution can be right for all of them. Taking the best outcome across repositories settles it. + A part that names its repository is judged there alone when that + repository carries the path: hypseus-singe cites lair.cpp under + `source` for one entry and `upstream` for the next. """ - owner, _actual = resolve_path(part.path, part.start, tuple(tokens)) + owner, _actual = resolve_path( + part.path, part.start, tuple(tokens), role_of(part) + ) carrying = [ view for view in views if fetch_from(view.repo, view.pin, part.path) is not None ] + named = [view for view in carrying if _view_role(view) == part.role] + if part.role and named: + carrying = named live = { id(view) for view in carrying if fetch_from(view.repo, view.head, part.path) is not None @@ -1732,7 +1766,7 @@ def build_report( lambda which, path, start=None, toks=(), _v=view: fetch( which, path, start, toks, _v ), - rename_getter, + lambda path: rename_getter(path, role_of(part)), lambda path, start=None, toks=(), _v=view: describe( path, start, toks, _v ), @@ -1753,7 +1787,15 @@ def build_report( if best is None or rank < best[0] or (rank == best[0] and view is owner): best = (rank, result) return best[1] if best else anchor_part( - part, fetch, rename_getter, describe, tokens + part, + lambda which, path, start=None, toks=(): fetch( + which, path, start, toks, role=role_of(part) + ), + lambda path: rename_getter(path, role_of(part)), + lambda path, start=None, toks=(): describe( + path, start, toks, role=role_of(part) + ), + tokens, ) # Comparing a revision with itself anchors every ref whatever it cites, so @@ -1763,7 +1805,9 @@ def build_report( def self_checked(part: RefPart, tokens: Sequence[str]) -> bool: if report.pinned_tag: return True - view, _ = resolve_path(part.path, part.start, tuple(tokens)) + if not part.role and len(path_roles.get(part.path, ())) > 1: + return all(view.pin == view.head for view in views) + view, _ = resolve_path(part.path, part.start, tuple(tokens), role_of(part)) return view.pin == view.head def judge( @@ -1779,11 +1823,15 @@ def build_report( ) if flag: result = verify_at_pin( - part, fetch(PIN, part.path, part.start, tokens), tokens, hashes + part, + fetch(PIN, part.path, part.start, tokens, role=role_of(part)), + tokens, hashes, ) # Checked at its own revision, the part still lies in one tree: # the report names it, as a comparison does. - owner, _actual = resolve_path(part.path, part.start, tuple(tokens)) + owner, _actual = resolve_path( + part.path, part.start, tuple(tokens), role_of(part) + ) if result.repo is None and owner is not primary: result = replace(result, repo=owner.repo.slug) else: @@ -1791,7 +1839,9 @@ def build_report( # A forge that refused the request said nothing about the file: # git.eden-emu.dev answers 403 to scripts, and nine citations of # eden-nightly read "absent at HEAD" for a file that exists. - owner, _actual = resolve_path(part.path, part.start, tuple(tokens)) + owner, _actual = resolve_path( + part.path, part.start, tuple(tokens), role_of(part) + ) if ( result.status in ("GONE", "AMBIGUOUS") and (owner.repo.host, owner.repo.slug) in mute diff --git a/scripts/sitesources.py b/scripts/sitesources.py index 9833c2bb..7d9e0912 100644 --- a/scripts/sitesources.py +++ b/scripts/sitesources.py @@ -261,6 +261,10 @@ def link_prose_citations(profile: dict, text: str) -> str: continue if "/" not in path and path not in structured: continue + role = parts[0].role + if role and (role_sources(profile).get(role) or (None,))[0] != repo: + # `upstream a.c:5` names a repository this link would not reach. + continue if any(lo <= path_span[0] < hi for lo, hi in guarded): continue for index, ((lo, hi), part) in enumerate(zip(spans, parts)): diff --git a/scripts/sourceref.py b/scripts/sourceref.py index 598085f2..9ec48688 100644 --- a/scripts/sourceref.py +++ b/scripts/sourceref.py @@ -39,6 +39,10 @@ class RefPart: start: int | None end: int | None raw: str = "" + # The repository the part names (`upstream src/a.c:5`), empty when it + # names none. Carried per part: one profile cites the same path in both + # repositories, hypseus-singe's lair.cpp under `source` and `upstream`. + role: str = "" @property def last(self) -> int | None: @@ -247,6 +251,19 @@ def _prose_external( return not lead or lead[-1] in _PROSE_LIST_DELIMITERS +def _prose_role(line: str, path_start: int) -> str: + """The role word written right before a prose path, empty when none. + + Prose follows the source_ref convention: `(upstream retro_host.c:196)` + names the original emulator, `ref: source src/devices/floppy.cpp:1656` + the repository declared as source. Read as a linking word or a project + name, the word was dropped or made the citation external. + """ + before = line[:path_start].rstrip() + word = before[max(before.rfind(" "), before.rfind("\t"), before.rfind("(")) + 1:] + return word if word in REPO_ROLES else "" + + def resolve_project_word(path: str, trees: Iterable[list[str] | None]) -> str | None: """Path of `project file` inside a declared tree, None when out of reach. @@ -333,7 +350,8 @@ def _prose_runs( ): path_start -= 1 path = "." + path - if _prose_external(line, path_start, known): + role = _prose_role(line, path_start) + if not role and _prose_external(line, path_start, known): # Kept with its project word, the source_ref convention: the # report looks for the project inside the declared trees # (mupen64plus-ae vendors `rice Config.cpp`) and calls the @@ -358,7 +376,7 @@ def _prose_runs( for lo, hi in spans: rng = line[lo:hi] a, _, b = rng.partition("-") - parts.append(RefPart(path, int(a), int(b or a), rng)) + parts.append(RefPart(path, int(a), int(b or a), rng, role)) yield ( number, path_span, tuple(spans), tuple(parts), line[path_start:end], end > boundary, diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 60e09cfe..1dbc8b29 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -4340,5 +4340,107 @@ class EachCitationNamesTheCodeThatAsks(unittest.TestCase): self.assertEqual(self._attribute("source emu/memory.c:2"), "source emu/memory.c:2") +class ARoleIsReadPerPart(unittest.TestCase): + """The role a part writes (`source a.c:3`) was stored per path, first met + first kept: hypseus-singe's `upstream lair.cpp` decided where every later + `source lair.cpp` was read, and correct refs were reported MOVED.""" + + setUp = TestBuildReport.setUp + tearDown = TestBuildReport.tearDown + MD5A = "0123456789abcdef0123456789abcdef" + MD5B = "fedcba9876543210fedcba9876543210" + + def _report(self, files: str, notes: str = "") -> ProfileReport: + # The port inserted two lines above its copy of the table since its + # pin, while the original sits on HEAD with the same table. + trees = { + ("libretro/x", "pins"): {"a.c": ["pad", "pad", f"rom {self.MD5B}"]}, + ("libretro/x", "heads"): {"a.c": ["pad", "pad", "new", "new", + f"rom {self.MD5B}"]}, + ("alek/x", "pinu"): {"a.c": [f"rom {self.MD5A}", "x", + f"rom {self.MD5B}"]}, + } + profile_sync.upstream.fetch_file = ( + lambda repo, sha, path, cache_dir, offline=False: + trees.get((repo.slug, sha), {}).get(path) + ) + profile_sync.upstream.list_tree = ( + lambda repo, sha, cache_dir, offline=False: + (sorted(trees.get((repo.slug, sha), {})), False) + ) + profile_sync.upstream.resolve_head = ( + lambda repo, cache_dir, offline=False, branch=None: + "heads" if repo.owner == "libretro" else "pinu" + ) + profile_sync.upstream.compare = ( + lambda repo, base, head, cache_dir, offline=False: + profile_sync.CompareResult([], False) + ) + text = ( + 'emulator: T\n' + 'source: "https://github.com/libretro/x"\n' + 'upstream: "https://github.com/alek/x"\n' + 'source_commit: "pins"\n' + 'upstream_commit: "pinu"\n' + + (f"notes: |\n {notes}\n" if notes else "") + + "files:\n" + files + ) + profile = yaml.safe_load(text) + with contextlib.redirect_stderr(io.StringIO()): + return build_report("t", profile, self.dir, text=text) + + def test_a_part_naming_its_repository_is_judged_there(self): + report = self._report( + " - name: a.rom\n" + f" md5: {self.MD5A}\n" + ' source_ref: "upstream a.c:1"\n' + " - name: b.rom\n" + f" md5: {self.MD5B}\n" + ' source_ref: "source a.c:3"\n' + ) + parts = {e.name: e.parts[0] for e in report.entries} + self.assertEqual(parts["a.rom"].status, "ANCHORED") + self.assertEqual(parts["a.rom"].repo, "alek/x") + # Read in the original, where line 3 still holds the table, the + # port's own move went unseen. + self.assertEqual(parts["b.rom"].status, "SHIFTED") + self.assertEqual(parts["b.rom"].start, 5) + self.assertIsNone(parts["b.rom"].repo) + + def test_a_prose_source_word_is_a_role_not_a_project(self): + report = self._report( + " - name: b.rom\n" + f" md5: {self.MD5B}\n" + ' source_ref: "source a.c:3"\n', + notes="ref: source a.c:3, more", + ) + prose = [e for e in report.entries if e.kind == "prose"] + self.assertEqual(len(prose), 1) + self.assertNotEqual(prose[0].parts[0].status, "EXTERNAL") + self.assertEqual(prose[0].parts[0].part.role, "source") + + def test_a_prose_upstream_word_names_the_repository(self): + report = self._report( + " - name: b.rom\n" + f" md5: {self.MD5B}\n" + ' source_ref: "source a.c:3"\n', + notes="the original reads it (upstream a.c:1).", + ) + prose = [e for e in report.entries if e.kind == "prose"] + self.assertEqual(prose[0].parts[0].part.role, "upstream") + self.assertEqual(prose[0].parts[0].repo, "alek/x") + + def test_the_prose_grammar_keeps_the_role(self): + from sourceref import _prose_runs + + runs = list(_prose_runs( + "# ref: source src/a.cpp:1656-1685, 1709-1738, upstream b.c:5" + )) + roles = [(p.path, p.role) for run in runs for p in run[3]] + self.assertEqual(roles, [ + ("src/a.cpp", "source"), ("src/a.cpp", "source"), ("b.c", "upstream"), + ]) + + if __name__ == "__main__": unittest.main()