Merge pull request #1741 from SleglJan/fix/tools-pr-overlap-declarations

fix(tools): count only definitions as exports in the overlap report

CMakeLists.txt

@@ -49,6 +49,8 @@ if(BUILD_TESTING)
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_check_conventions_comments.py)
         add_test(NAME hardware_oracle COMMAND ${Python3_EXECUTABLE}
             ${CMAKE_CURRENT_SOURCE_DIR}/tools/hw-oracle/test_hw_oracle.py)
+        add_test(NAME pr_overlap_exports COMMAND ${Python3_EXECUTABLE}
+            ${CMAKE_CURRENT_SOURCE_DIR}/tools/tests/test_pr_overlap_exports.py)
     endif()
 endif()
 

tools/pr_overlap.py

@@ -60,6 +60,15 @@ def commit(tree):
                "-p", "refs/pr/base", "-m", "pr_overlap").strip()
 
 
+def exports(diff):
+    found = set()
+    for line in diff.splitlines():
+        m = EXPORT.match(line)
+        if m and not m.group(1).endswith("_nid_no_patch") and not line.rstrip().endswith(";"):
+            found.add(m.group(1))
+    return found
+
+
 def scan(pr):
     tree, conflicted = merge("refs/pr/base", pr["ref"])
     pr["tree"] = None if conflicted else commit(tree)
@@ -71,8 +80,7 @@ def scan(pr):
         if status == "A":
             pr["added"].add(path)
     diff = git("diff", "-U0", "--no-color", old, new, "--", "core/libs/prx/*.cpp")
-    pr["exports"] = {m.group(1) for m in map(EXPORT.match, diff.splitlines())
-                     if m and not m.group(1).endswith("_nid_no_patch")}
+    pr["exports"] = exports(diff)
 
 
 def hunks(pr, path):

tools/tests/test_pr_overlap_exports.py

@@ -0,0 +1,44 @@
+import sys
+import unittest
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+import pr_overlap
+
+TRAILING = "+extern \"C\" std::int64_t APS5_VABI recv_nid_postfix(int, void*, std::uint64_t, int) noexcept;   \n"
+DIFF = """diff --git a/core/libs/prx/libkernel/File/src/Open.cpp b/core/libs/prx/libkernel/File/src/Open.cpp
+@@ -96,0 +97,2 @@
++extern "C" int* APS5_VABI __error_nid_postfix();
+""" + TRAILING + """@@ -135,0 +136,5 @@
++int APS5_VABI mlock_nid_postfix(const void* address, std::size_t length) {
++std::int64_t APS5_VABI sceKernelRead(int d, void* buf, std::size_t nbytes) noexcept {
++int APS5_VABI getpid_nid_postfix() { return 1; }
++int APS5_VABI sceNpUniversalDataSystemCreateEvent(
++void APS5_VABI Helper_nid_no_patch(int value) {
+@@ -200 +205,0 @@
+-int APS5_VABI removed_nid_postfix(int) {
+diff --git a/core/libs/prx/libkernel/File/src/Stdio.cpp b/core/libs/prx/libkernel/File/src/Stdio.cpp
+@@ -10,2 +10,2 @@
+ int APS5_VABI context_nid_postfix(int) {
+"""
+
+
+class ExportTests(unittest.TestCase):
+    def test_added_definitions_are_exports(self):
+        self.assertEqual(pr_overlap.exports(DIFF), {"mlock_nid_postfix", "sceKernelRead", "getpid_nid_postfix", "sceNpUniversalDataSystemCreateEvent"})
+
+    def test_declarations_are_not_exports(self):
+        self.assertTrue(TRAILING.endswith(";   \n"))
+        found = pr_overlap.exports(DIFF)
+        self.assertNotIn("__error_nid_postfix", found)
+        self.assertNotIn("recv_nid_postfix", found)
+
+    def test_removed_context_and_helper_lines_are_not_exports(self):
+        found = pr_overlap.exports(DIFF)
+        self.assertNotIn("removed_nid_postfix", found)
+        self.assertNotIn("context_nid_postfix", found)
+        self.assertNotIn("Helper_nid_no_patch", found)
+
+
+if __name__ == "__main__":
+    unittest.main()