fix: tell a contributor where a BIOS goes

scripts/validate_pr.py

@@ -237,6 +237,14 @@ def validate_file(
     else:
         result.add_check(True, f"File size OK ({result.size:,} bytes)")
 
+    # Paths are compared as the database writes them, relative to the
+    # checkout: an absolute path to the file itself read as its duplicate.
+    relative = filepath
+    if os.path.isabs(filepath):
+        candidate = os.path.relpath(filepath, os.getcwd())
+        if not candidate.startswith(".."):
+            relative = candidate
+
     if db:
         if result.sha1 in db.get("files", {}):
             existing = db["files"][result.sha1]
@@ -244,7 +252,7 @@ def validate_file(
                 os.path.normpath(p)
                 for p in [existing.get("path"), *(existing.get("paths") or [])] if p
             }
-            if os.path.normpath(filepath) in held_at:
+            if os.path.normpath(relative) in held_at:
                 # The file the database was built from, not a copy of it: a
                 # PR that regenerated database.json was told each new file
                 # duplicated itself.
@@ -284,7 +292,11 @@ def validate_file(
             "File not referenced in any platform config - needs manual review"
         )
 
-    normalized = os.path.normpath(filepath)
+    normalized = os.path.normpath(relative)
+    expected_folders = sorted(
+        platform_hashes.get("folders", {}).get(result.sha1)
+        or platform_hashes.get("folders", {}).get(result.md5) or set()
+    )
     if normalized.startswith("bios" + os.sep):
         parts = normalized.split(os.sep)
         expected = (platform_hashes.get("folders", {}).get(result.sha1)
@@ -301,8 +313,16 @@ def validate_file(
             result.add_check(True, f"Correct placement: bios/{parts[1]}/{parts[2]}/")
         else:
             result.add_warning("File should be in bios/Manufacturer/Console/ structure")
+    elif expected_folders:
+        # The contributor asks before forking: say where it goes.
+        result.add_warning(
+            "File is not under bios/ yet; its system's files are in "
+            + " or ".join(f"`{f}`" for f in expected_folders)
+        )
     else:
-        result.add_warning("File is not under bios/ directory")
+        result.add_warning(
+            "File is not under bios/ yet; place it in bios/Manufacturer/Console/"
+        )
 
     if name_known and not sha1_known and not md5_known:
         result.add_info(

tests/test_validate_pr.py

@@ -321,5 +321,45 @@ class FlagsAreAppliedOrRefused(unittest.TestCase):
         self.assertIn("not allowed with", result.stderr)
 
 
+
+class AFileOutsideBiosIsToldWhereToGo(unittest.TestCase):
+    """validate_pr knew bios/Sony/PlayStation/ for scph5501.bin and only said
+    "not under bios/"; given an absolute path, a placed file was reported a
+    duplicate of itself."""
+
+    def test_both(self):
+        import os as _os
+        import tempfile as _tempfile
+
+        from validate_pr import validate_file
+
+        content = b"bios bytes"
+        import hashlib as _hashlib
+        sha1 = _hashlib.sha1(content).hexdigest()
+        md5 = _hashlib.md5(content).hexdigest()
+        hashes = {"sha1": {sha1}, "md5": {md5}, "names": set(),
+                  "folders": {sha1: {"bios/Sony/PlayStation/"}}}
+        with _tempfile.TemporaryDirectory() as directory:
+            loose = _os.path.join(directory, "scph5501.bin")
+            with open(loose, "wb") as handle:
+                handle.write(content)
+            result = validate_file(loose, None, hashes)
+        self.assertTrue(any("bios/Sony/PlayStation/" in m for _s, m in result.checks))
+
+        previous = _os.getcwd()
+        with _tempfile.TemporaryDirectory() as root:
+            placed = _os.path.join(root, "bios", "Sony", "PlayStation", "scph5501.bin")
+            _os.makedirs(_os.path.dirname(placed))
+            with open(placed, "wb") as handle:
+                handle.write(content)
+            db = {"files": {sha1: {"path": "bios/Sony/PlayStation/scph5501.bin"}}}
+            _os.chdir(root)
+            try:
+                result = validate_file(placed, db, hashes)
+            finally:
+                _os.chdir(previous)
+        self.assertFalse(any("Duplicate" in m for _s, m in result.checks))
+
+
 if __name__ == "__main__":
     unittest.main()