diff --git a/tools/pr_overlap.py b/tools/pr_overlap.py index 9ef1da8377..65c24af853 100644 --- a/tools/pr_overlap.py +++ b/tools/pr_overlap.py @@ -343,8 +343,9 @@ def probe(pr): if status != "A": existing.add(path) if not any(path.startswith("core/libs/prx/") and path.endswith(".cpp") for path in files): - return files, set(), existing - return files, exports(git("diff", "-U0", "--no-color", f"refs/pr/base...{pr['ref']}", "--", "core/libs/prx/*.cpp")), existing + return files, (set(), {}), existing + diff = git("diff", "-U0", "--no-color", f"refs/pr/base...{pr['ref']}", "--", "core/libs/prx/*.cpp") + return files, (exports(diff), source_changed_exports(diff, "+")), existing def related(prs, executor): @@ -353,11 +354,15 @@ def related(prs, executor): others = {n: pr for n, pr in prs.items() if n} base_paths = set(git("ls-tree", "-r", "--name-only", "-z", "refs/pr/base").split("\0")) chosen = {} + mine_files = mine["files"] + mine_exports = mine["exports"] + mine_keys = set(mine.get("added_exports", {})) for index, ((n, pr), (files, found, existing)) in enumerate(zip(others.items(), executor.map(probe, others.values())), 1): - if files & mine["files"] or found & mine["exports"] or existing - base_paths: + found_exports, found_added = found + if files & mine_files or (found_exports & mine_exports) or (set(found_added) & mine_keys) or existing - base_paths: chosen[n] = pr else: - pr.update(tree=None, files=set(), added=set(), exports=set()) + pr.update(tree=None, files=set(), added=set(), exports=set(), added_exports={}, removed_exports={}) if index % PROGRESS_EVERY == 0 or index == len(others): progress(f"probed {index}/{len(others)} pull requests, {len(chosen)} share files or exports with the branch") return chosen @@ -418,7 +423,12 @@ def overlaps(prs, a, b): found = {"added": (pa["added"] & pb["added"]).difference(*(pr["added"] for pr in shared))} if pa["author"] != pb["author"]: found["conflicts"] = conflicts(pa, pb) - found["exports"] = (pa["exports"] & pb["exports"]).difference(*(pr["exports"] for pr in shared)) + same_exports = (pa.get("exports", set()) & pb.get("exports", set())).difference(*(pr.get("exports", set()) for pr in shared)) + shared_keys = set().union(*(pr.get("added_exports", {}) for pr in shared)) + for key in (set(pa.get("added_exports", {})) & set(pb.get("added_exports", {}))) - shared_keys: + same_exports.update(pa["added_exports"][key]) + same_exports.update(pb["added_exports"][key]) + found["exports"] = same_exports if not (pa["tree"] and pb["tree"]): uncompared = pa.get("files", set()) & pb.get("files", set()) if uncompared: diff --git a/tools/tests/test_pr_overlap_branch_scan.py b/tools/tests/test_pr_overlap_branch_scan.py index a0b747ecdd..6520140aff 100644 --- a/tools/tests/test_pr_overlap_branch_scan.py +++ b/tools/tests/test_pr_overlap_branch_scan.py @@ -146,6 +146,27 @@ class BranchScanTests(unittest.TestCase): pr_overlap.scan_all(prs, 2) self.assertIn("refs/pr/6", merged) + def test_a_pull_request_exporting_the_same_nid_under_another_name_is_merged(self): + base = run("rev-parse", "refs/pr/base") + nid = pr_overlap.nid_names.compute_nid("shared") + alias = commit_on_base(base, "same nid by alias", { + "core/libs/prx/lib/Alias.cpp": f'int APS5_VABI renamed(int value) {{ return value; }}\nAPS5_EXPORT("{nid}", renamed);\n' + }) + run("update-ref", "refs/pr/5", alias) + self.prs[5] = {"ref": "refs/pr/5", "sha": alias, "author": "author5", "depends": set()} + merged = [] + real = pr_overlap.merge + + def record(*args): + merged.append(args[1]) + return real(*args) + + with patch.object(pr_overlap, "merge", side_effect=record): + pr_overlap.scan_all(self.prs, 2) + + self.assertIn("refs/pr/5", merged) + self.assertEqual(pr_overlap.overlaps(self.prs, 0, 5)["exports"], {"shared", "renamed"}) + if __name__ == "__main__": unittest.main() diff --git a/tools/tests/test_pr_overlap_exports.py b/tools/tests/test_pr_overlap_exports.py index fbdb824013..8165982d0a 100644 --- a/tools/tests/test_pr_overlap_exports.py +++ b/tools/tests/test_pr_overlap_exports.py @@ -129,5 +129,60 @@ class ExportTests(unittest.TestCase): self.assertEqual(set(explicit) & set(named), {("core/libs/prx/libSceExample", "Pz262plj20w")}) + def test_overlaps_reports_same_exports_for_different_names_with_matching_nid(self): + nid = "vPP6vKHY8Tg" + prs = { + 1: { + "author": "author1", + "depends": set(), + "added": set(), + "exports": {"_sceSdmaNop"}, + "added_exports": {("core/libs/prx/libSceSdma", nid): {"_sceSdmaNop"}}, + "tree": None, + }, + 2: { + "author": "author2", + "depends": set(), + "added": set(), + "exports": set(), + "added_exports": {("core/libs/prx/libSceSdma", nid): {"sceSdmaPending_001_nid_no_patch"}}, + "tree": None, + }, + } + found = pr_overlap.overlaps(prs, 1, 2) + self.assertIsNotNone(found) + self.assertEqual(found["exports"], {"_sceSdmaNop", "sceSdmaPending_001_nid_no_patch"}) + + def test_overlaps_does_not_report_export_conflict_for_shared_dependency(self): + nid = "vPP6vKHY8Tg" + prs = { + 1: { + "author": "author1", + "depends": {3}, + "added": set(), + "exports": {"_sceSdmaNop"}, + "added_exports": {("core/libs/prx/libSceSdma", nid): {"_sceSdmaNop"}}, + "tree": None, + }, + 2: { + "author": "author2", + "depends": {3}, + "added": set(), + "exports": {"_sceSdmaNop"}, + "added_exports": {("core/libs/prx/libSceSdma", nid): {"_sceSdmaNop"}}, + "tree": None, + }, + 3: { + "author": "author3", + "depends": set(), + "added": set(), + "exports": {"_sceSdmaNop"}, + "added_exports": {("core/libs/prx/libSceSdma", nid): {"_sceSdmaNop"}}, + "tree": None, + }, + } + self.assertIsNone(pr_overlap.overlaps(prs, 1, 2)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/tests/test_pr_overlap_progress.py b/tools/tests/test_pr_overlap_progress.py index 80ed91007c..77197891ac 100644 --- a/tools/tests/test_pr_overlap_progress.py +++ b/tools/tests/test_pr_overlap_progress.py @@ -56,7 +56,7 @@ class ProgressTests(unittest.TestCase): def fake_probe(pr): number = int(pr["ref"].rsplit("/", 1)[1]) - return ({"shared.txt"} if number % 25 == 0 else {"other.txt"}), set(), set() + return ({"shared.txt"} if number % 25 == 0 else {"other.txt"}), (set(), {}), set() with patch.object(pr_overlap, "scan", side_effect=self.fake_scan), patch.object(pr_overlap, "probe", side_effect=fake_probe), \ patch.object(pr_overlap, "git", return_value="shared.txt\0other.txt\0"):