refactor: judge binary and linked citations

scripts/profile_sync.py

@@ -33,7 +33,7 @@ ANON_QUOTA = 60
 TRIAGE_PATH_SAMPLE = 5
 
 STATUS_ORDER = (
-    "ANCHORED", "EXTERNAL", "SHIFTED", "RENAMED", "MOVED", "AMBIGUOUS",
+    "ANCHORED", "EXTERNAL", "BINARY", "SHIFTED", "RENAMED", "MOVED", "AMBIGUOUS",
     "CHANGED", "GONE",
 )
 REVIEW_STATUSES = ("CHANGED", "GONE", "AMBIGUOUS")
@@ -233,6 +233,86 @@ def is_external_citation(path: str) -> bool:
     return "/" not in head and "." not in head
 
 
+_BINARY_SUFFIXES = (
+    ".dll", ".exe", ".so", ".dylib", ".apk", ".elf", ".o", ".a", ".jar", ".dex",
+)
+_HEX_ADDRESS_RE = re.compile(r"\b0x[0-9a-fA-F]{2,}")
+
+
+def is_binary_citation(path: str) -> bool:
+    """True when a ref locates code inside a shipped binary, not a source file.
+
+    `BAM.dll 1.5-408 .text:0x100c3960-0x100c3a97`, `libmain.so LoadNES
+    0x3bb49` and `.rdata dialog filter` were read by disassembly. No revision
+    of any repository holds those bytes, so the ref can neither anchor nor
+    break: 77 parts of `future-pinball-fploader` alone were reported GONE.
+    """
+    if _HEX_ADDRESS_RE.search(path):
+        return True
+    if " " not in path:
+        return False
+    head = path.split(" ", 1)[0]
+    if head.lower().endswith(_BINARY_SUFFIXES):
+        return True
+    return head.startswith(".") and "/" not in head
+
+
+def strip_repo_word(path: str, known=frozenset()) -> str:
+    """Drop a leading project word that names a declared repository.
+
+    `BAM_FPloader FPLoader.cpp:45` follows the `munt ROMInfo.cpp` convention,
+    but BAM_FPloader is the profile's own source: the path is FPLoader.cpp,
+    to be found in that tree, not an out-of-reach citation.
+    """
+    if " " not in path:
+        return path
+    head, _, tail = path.partition(" ")
+    if head.lower() in known and tail and "/" not in head and "." not in head:
+        return tail.strip()
+    return path
+
+
+def narrow_by_cited(matches: list[str], cited_dirs) -> list[str]:
+    """Keep the candidates under the deepest directory the profile cites.
+
+    A bare `version.h` matches BasiliskII/src/include/version.h and
+    SheepShaver/src/include/version.h in cebix/macemu; the profile's own
+    source_refs all live under SheepShaver/src, which is the tree the prose
+    meant. Candidates under no cited directory are kept only when none is.
+    """
+    best = -1
+    kept: list[str] = []
+    for match in matches:
+        depth = max(
+            (d.count("/") + 1 for d in cited_dirs if match.startswith(d + "/")),
+            default=-1,
+        )
+        if depth > best:
+            best, kept = depth, [match]
+        elif depth == best:
+            kept.append(match)
+    return kept if best >= 0 else matches
+
+
+def symlink_target(path: str, lines: list[str] | None) -> str | None:
+    """The file a one-line blob points at, when the blob is a symbolic link.
+
+    Git stores a symlink as a blob holding its target, and the raw endpoints
+    serve that text: SheepShaver/src/Unix/ether_unix.cpp is the single line
+    ../../../BasiliskII/src/Unix/ether_unix.cpp. Anchoring a line number in
+    it would report every citation as beyond the end of the file.
+    """
+    if not lines or len(lines) != 1:
+        return None
+    target = lines[0].strip()
+    if not target or " " in target or "/" not in target:
+        return None
+    if posixpath.basename(target) != posixpath.basename(path):
+        return None
+    resolved = posixpath.normpath(posixpath.join(posixpath.dirname(path), target))
+    return None if resolved.startswith("..") or resolved == path else resolved
+
+
 def _anchor_tokens(entry: dict) -> list[str]:
     """Values worth searching for when a cited line misses its subject.
 
@@ -657,6 +737,11 @@ def anchor_part(
     `describe(path)` returns (repo slug, raw URL at HEAD) for the repository
     that owns the path, or (None, None) when the caller does not track it.
     """
+    if is_binary_citation(part.path):
+        return PartResult(
+            part, "BINARY", None, None, None, [],
+            "cites a shipped binary, no source path to anchor",
+        )
     if is_external_citation(part.path):
         return PartResult(
             part, "EXTERNAL", None, None, None, [],
@@ -939,6 +1024,11 @@ def verify_at_pin(part: RefPart, pin_lines, tokens, hash_tokens=()) -> PartResul
     contains, so comparing the two says nothing. What can still be checked is
     self-consistency: does the cited range carry the value the entry declares?
     """
+    if is_binary_citation(part.path):
+        return PartResult(
+            part, "BINARY", None, None, None, [],
+            "cites a shipped binary, no source path to anchor",
+        )
     if is_external_citation(part.path):
         return PartResult(
             part, "EXTERNAL", None, None, None, [],
@@ -1006,7 +1096,7 @@ def reconcile_self_check(parts: list[PartResult]) -> list[PartResult]:
     """
     if not any(part.status == "ANCHORED" for part in parts):
         return parts
-    kept = ("ANCHORED", "GONE", "EXTERNAL")
+    kept = ("ANCHORED", "GONE", "EXTERNAL", "BINARY")
     return [
         part if part.status in kept
         else PartResult(part.part, "ANCHORED", None, None, None, [])
@@ -1067,6 +1157,11 @@ def build_report(
     # no declared value, so it anchors on content alone: the nudge and
     # relocation heuristics stay off rather than guess from a profile-wide
     # token pool.
+    known = frozenset(
+        repo.name.lower()
+        for _, _, url in declared_repositories(profile)
+        if (repo := upstream.parse_repo(url)) is not None
+    )
     refs = []
     for citation in collect_citations(profile):
         if citation.kind == "ref":
@@ -1074,15 +1169,28 @@ def build_report(
             display = str(entry.get("name") or "") or citation.field
             if citation.label:
                 display += f" [{citation.label}]"
+            parts = [
+                RefPart(strip_repo_word(p.path, known), p.start, p.end, p.raw)
+                for p in split_source_ref(citation.ref)
+            ]
             refs.append((
                 display, citation.ref, _anchor_tokens(entry),
-                entry_hashes(entry), split_source_ref(citation.ref), citation,
+                entry_hashes(entry), parts, citation,
             ))
         else:
             refs.append((
                 citation.field, citation.ref, [], [],
                 list(citation.parts), citation,
             ))
+    # Directories the profile already cites, deepest first, to settle a bare
+    # filename that several trees of one repository carry.
+    cited_dirs: set[str] = set()
+    for _, _, _, _, parts, _ in refs:
+        for part in parts:
+            directory = posixpath.dirname(part.path)
+            while directory:
+                cited_dirs.add(directory)
+                directory = posixpath.dirname(directory)
 
     if select_repo(profile) is None:
         declared = str(profile.get("source") or profile.get("upstream") or "")
@@ -1140,8 +1248,12 @@ def build_report(
                     p for p in tree or []
                     if posixpath.basename(p) == path
                 ]
-                if len(matches) == 1 and matches[0] not in candidates:
-                    candidates.append(matches[0])
+                if len(matches) > 1:
+                    matches = narrow_by_cited(matches, cited_dirs)
+                # A few survivors are told apart by the cited line below: a
+                # 266-line Windows configure.ac cannot carry line 754.
+                if len(matches) <= 4:
+                    candidates.extend(m for m in matches if m not in candidates)
 
         def score(lines) -> int:
             """How well a repository's cited line matches what the ref means.
@@ -1177,6 +1289,14 @@ def build_report(
                     )
                     if found is None:
                         continue
+                    target = symlink_target(candidate, found)
+                    if target:
+                        found = upstream.fetch_file(
+                            view.repo, sha_of(view), target, cache_dir, offline
+                        )
+                        if found is None:
+                            continue
+                        candidate = target
                     rank = score(found)
                     if rank == 3:
                         return view, candidate

scripts/validation.py

@@ -128,14 +128,15 @@ def _build_validation_index(profiles: dict) -> dict[str, dict]:
                     if norm.startswith("0x"):
                         norm = norm[2:]
                     index[fname]["crc32"].add(norm)
+            # A hash field may carry several accepted values, as a list or as
+            # the comma-separated string the schema spells out
             for hash_type in ("md5", "sha1", "sha256"):
                 if hash_type in checks and f.get(hash_type):
                     val = f[hash_type]
-                    if isinstance(val, list):
-                        for h in val:
-                            index[fname][hash_type].add(str(h).lower())
-                    else:
-                        index[fname][hash_type].add(str(val).lower())
+                    values = val if isinstance(val, list) else str(val).split(",")
+                    for h in values:
+                        if str(h).strip():
+                            index[fname][hash_type].add(str(h).strip().lower())
             # Adler32 -stored as known_hash_adler32 field (not in validation: list
             # for Dolphin, but support it in both forms for future profiles)
             adler_val = f.get("known_hash_adler32") or f.get("adler32")

tests/test_profile_sync.py

@@ -457,6 +457,117 @@ class TestExternalCitation(unittest.TestCase):
         self.assertNotIn("EXTERNAL", profile_sync.REVIEW_STATUSES)
 
 
+class TestBinaryCitation(unittest.TestCase):
+    """A ref read by disassembly has no source path to anchor."""
+
+    def test_hex_address_is_binary(self):
+        self.assertTrue(profile_sync.is_binary_citation(
+            "BAM.dll 1.5-408 .text:0x100c3960-0x100c3a97"
+        ))
+        self.assertTrue(profile_sync.is_binary_citation("libmain.so LoadNES 0x3bb49"))
+
+    def test_binary_suffix_followed_by_words_is_binary(self):
+        self.assertTrue(profile_sync.is_binary_citation("BAM-Tracker.exe import directory"))
+
+    def test_section_name_is_binary(self):
+        self.assertTrue(profile_sync.is_binary_citation(".rdata dialog filter"))
+
+    def test_a_source_path_is_not_binary(self):
+        for path in ("src/foo.c", "libs/libCg.so", "Source/Core/hook.c", "resources/font.h"):
+            self.assertFalse(profile_sync.is_binary_citation(path), path)
+
+    def test_binary_part_is_reported_not_reviewed(self):
+        result = anchor_part(
+            RefPart("BAM.dll 1.5-408 .text:0x10031bbb", None, None),
+            make_fetch({}),
+            renamer(CompareResult([], False)),
+        )
+        self.assertEqual(result.status, "BINARY")
+        self.assertNotIn("BINARY", profile_sync.REVIEW_STATUSES)
+
+    def test_binary_part_at_the_pin(self):
+        part = RefPart("ModelImporter.exe import directory", None, None, "")
+        self.assertEqual(profile_sync.verify_at_pin(part, None, []).status, "BINARY")
+
+
+class TestRepoWord(unittest.TestCase):
+    """A leading word naming a declared repository is a location, not a project."""
+
+    def test_declared_repository_word_is_stripped(self):
+        self.assertEqual(
+            profile_sync.strip_repo_word("BAM_FPloader FPLoader.cpp", {"bam_fploader"}),
+            "FPLoader.cpp",
+        )
+
+    def test_undeclared_project_word_stays(self):
+        self.assertEqual(
+            profile_sync.strip_repo_word("munt ROMInfo.cpp", {"bam_fploader"}),
+            "munt ROMInfo.cpp",
+        )
+
+    def test_a_directory_prefix_is_not_a_project_word(self):
+        self.assertEqual(
+            profile_sync.strip_repo_word("src/dir file.cpp", {"src/dir"}),
+            "src/dir file.cpp",
+        )
+
+
+class TestNarrowByCited(unittest.TestCase):
+    """A bare filename carried by two trees follows the tree the profile cites."""
+
+    MATCHES = [
+        "BasiliskII/src/include/version.h",
+        "SheepShaver/src/include/version.h",
+    ]
+
+    def test_deepest_cited_directory_wins(self):
+        self.assertEqual(
+            profile_sync.narrow_by_cited(self.MATCHES, {"SheepShaver", "SheepShaver/src"}),
+            ["SheepShaver/src/include/version.h"],
+        )
+
+    def test_equal_depth_keeps_both(self):
+        matches = [
+            "SheepShaver/src/EthernetDriver/cpu_emulation.h",
+            "SheepShaver/src/include/cpu_emulation.h",
+        ]
+        self.assertEqual(
+            profile_sync.narrow_by_cited(matches, {"SheepShaver/src"}), matches
+        )
+
+    def test_nothing_cited_keeps_all(self):
+        self.assertEqual(profile_sync.narrow_by_cited(self.MATCHES, set()), self.MATCHES)
+
+    def test_depth_counts_components_not_characters(self):
+        matches = [
+            "SheepShaver/src/Unix/configure.ac",
+            "SheepShaver/src/Windows/configure.ac",
+        ]
+        cited = {"SheepShaver", "SheepShaver/src", "SheepShaver/src/Unix", "SheepShaver/src/Windows"}
+        self.assertEqual(profile_sync.narrow_by_cited(matches, cited), matches)
+
+
+class TestSymlinkTarget(unittest.TestCase):
+    """A one-line blob naming a same-named file is a link to follow."""
+
+    def test_relative_link_resolves_from_the_link_directory(self):
+        self.assertEqual(
+            profile_sync.symlink_target(
+                "SheepShaver/src/Unix/ether_unix.cpp",
+                ["../../../BasiliskII/src/Unix/ether_unix.cpp"],
+            ),
+            "BasiliskII/src/Unix/ether_unix.cpp",
+        )
+
+    def test_a_real_one_line_file_is_not_a_link(self):
+        self.assertIsNone(profile_sync.symlink_target("a/VERSION", ["1.2.3"]))
+        self.assertIsNone(profile_sync.symlink_target("a/x.c", ["int main() { return 0; }"]))
+        self.assertIsNone(profile_sync.symlink_target("a/x.c", ["../other.c"]))
+
+    def test_a_link_leaving_the_tree_is_ignored(self):
+        self.assertIsNone(profile_sync.symlink_target("x.c", ["../../x.c"]))
+
+
 class TestAnchorTokens(unittest.TestCase):
     def test_archive_members_supply_the_content_values(self):
         tokens = profile_sync._anchor_tokens(