From 275952598b3124621cc23c335d59aac04cbb967b Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 15:49:57 -0400
Subject: [PATCH 01/20] Support compact repo mapping format

---
 python/runfiles/runfiles.py     |  75 +++++++++++++--------
 tests/runfiles/runfiles_test.py | 116 ++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 29 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 3943be5646..af81c52122 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -20,7 +20,7 @@
 import os
 import posixpath
 import sys
-from typing import Dict, Optional, Tuple, Union
+from typing import Dict, List, Optional, Tuple, Union
 
 
 class _ManifestBased:
@@ -130,7 +130,7 @@ class Runfiles:
     def __init__(self, strategy: Union[_ManifestBased, _DirectoryBased]) -> None:
         self._strategy = strategy
         self._python_runfiles_root = _FindPythonRunfilesRoot()
-        self._repo_mapping = _ParseRepoMapping(
+        self._exact_repo_mapping, self._prefixed_repo_mapping = _ParseRepoMapping(
             strategy.RlocationChecked("_repo_mapping")
         )
 
@@ -179,7 +179,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if os.path.isabs(path):
             return path
 
-        if source_repo is None and self._repo_mapping:
+        if source_repo is None and (self._exact_repo_mapping or self._prefixed_repo_mapping):
             # Look up runfiles using the repository mapping of the caller of the
             # current method. If the repo mapping is empty, determining this
             # name is not necessary.
@@ -188,25 +188,26 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         # Split off the first path component, which contains the repository
         # name (apparent or canonical).
         target_repo, _, remainder = path.partition("/")
-        if not remainder or (source_repo, target_repo) not in self._repo_mapping:
-            # One of the following is the case:
-            # - not using Bzlmod, so the repository mapping is empty and
-            #   apparent and canonical repository names are the same
-            # - target_repo is already a canonical repository name and does not
-            #   have to be mapped.
-            # - path did not contain a slash and referred to a root symlink,
-            #   which also should not be mapped.
+        if not remainder:
+            # path did not contain a slash and referred to a root symlink,
+            # which should not be mapped.
             return self._strategy.RlocationChecked(path)
 
-        assert (
-            source_repo is not None
-        ), "BUG: if the `source_repo` is None, we should never go past the `if` statement above"
+        # Try exact mapping first
+        if (source_repo, target_repo) in self._exact_repo_mapping:
+            target_canonical = self._exact_repo_mapping[(source_repo, target_repo)]
+            return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
 
-        # target_repo is an apparent repository name. Look up the corresponding
-        # canonical repository name with respect to the current repository,
-        # identified by its canonical name.
-        target_canonical = self._repo_mapping[(source_repo, target_repo)]
-        return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
+        # Try prefixed mapping if no exact match found
+        for (prefix_source, target_apparent), target_canonical in self._prefixed_repo_mapping.items():
+            if (target_apparent == target_repo and 
+                source_repo is not None and 
+                source_repo.startswith(prefix_source)):
+                return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
+
+        # No mapping found - assume target_repo is already canonical or
+        # we're not using Bzlmod
+        return self._strategy.RlocationChecked(path)
 
     def EnvVars(self) -> Dict[str, str]:
         """Returns environment variables for subprocesses.
@@ -359,28 +360,44 @@ def _FindPythonRunfilesRoot() -> str:
     return root
 
 
-def _ParseRepoMapping(repo_mapping_path: Optional[str]) -> Dict[Tuple[str, str], str]:
-    """Parses the repository mapping manifest."""
+def _ParseRepoMapping(
+    repo_mapping_path: Optional[str]
+) -> Tuple[Dict[Tuple[str, str], str], Dict[Tuple[str, str], str]]:
+    """Parses the repository mapping manifest.
+    
+    Returns:
+        A tuple of (exact_mappings, prefixed_mappings) where:
+        - exact_mappings: Dict mapping (source_canonical, target_apparent) -> target_canonical
+        - prefixed_mappings: Dict mapping (source_prefix, target_apparent) -> target_canonical
+          where source_prefix entries in the input ended with '*'
+    """
     # If the repository mapping file can't be found, that is not an error: We
     # might be running without Bzlmod enabled or there may not be any runfiles.
-    # In this case, just apply an empty repo mapping.
+    # In this case, just apply empty repo mappings.
     if not repo_mapping_path:
-        return {}
+        return {}, {}
     try:
         with open(repo_mapping_path, "r", encoding="utf-8", newline="\n") as f:
             content = f.read()
     except FileNotFoundError:
-        return {}
+        return {}, {}
 
-    repo_mapping = {}
+    exact_mappings = {}
+    prefixed_mappings = {}
     for line in content.split("\n"):
         if not line:
             # Empty line following the last line break
             break
-        current_canonical, target_local, target_canonical = line.split(",")
-        repo_mapping[(current_canonical, target_local)] = target_canonical
-
-    return repo_mapping
+        source_canonical, target_apparent, target_canonical = line.split(",")
+        if source_canonical.endswith("*"):
+            # This is a prefixed mapping - remove the '*' for prefix matching
+            prefix = source_canonical[:-1]
+            prefixed_mappings[(prefix, target_apparent)] = target_canonical
+        else:
+            # This is an exact mapping
+            exact_mappings[(source_canonical, target_apparent)] = target_canonical
+
+    return exact_mappings, prefixed_mappings
 
 
 def CreateManifestBased(manifest_path: str) -> Runfiles:
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index a3837ac842..83a99d7d79 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -525,6 +525,122 @@ def testDirectoryBasedRlocationWithRepoMappingFromOtherRepo(self) -> None:
                 r.Rlocation("config.json", "protobuf~3.19.2"), dir + "/config.json"
             )
 
+    def testDirectoryBasedRlocationWithCompactRepoMappingFromMain(self) -> None:
+        """Test repository mapping with prefix-based entries (compact format)."""
+        with _MockFile(
+            name="_repo_mapping",
+            contents=[
+                # Exact mappings (no asterisk)
+                "_,config.json,config.json~1.2.3",
+                ",my_module,_main",
+                ",my_workspace,_main",
+                # Prefixed mappings (with asterisk) - these apply to any repo starting with the prefix
+                "deps+*,external_dep,external_dep~1.0.0",
+                "test_deps+*,test_lib,test_lib~2.1.0",
+            ],
+        ) as rm:
+            dir = os.path.dirname(rm.Path())
+            r = runfiles.CreateDirectoryBased(dir)
+
+            # Test exact mappings still work
+            self.assertEqual(
+                r.Rlocation("my_module/bar/runfile", ""), dir + "/_main/bar/runfile"
+            )
+            self.assertEqual(
+                r.Rlocation("my_workspace/bar/runfile", ""), dir + "/_main/bar/runfile"
+            )
+
+            # Test prefixed mappings - should match any repo starting with "deps+"
+            self.assertEqual(
+                r.Rlocation("external_dep/foo/file", "deps+dep1"),
+                dir + "/external_dep~1.0.0/foo/file",
+            )
+            self.assertEqual(
+                r.Rlocation("external_dep/bar/file", "deps+dep2"),
+                dir + "/external_dep~1.0.0/bar/file",
+            )
+            self.assertEqual(
+                r.Rlocation("external_dep/nested/path/file", "deps+some_long_dep_name"),
+                dir + "/external_dep~1.0.0/nested/path/file",
+            )
+
+            # Test that prefixed mappings work for test_deps+ prefix too
+            self.assertEqual(
+                r.Rlocation("test_lib/test/file", "test_deps+junit"),
+                dir + "/test_lib~2.1.0/test/file",
+            )
+
+            # Test that non-matching prefixes don't match
+            self.assertEqual(
+                r.Rlocation("external_dep/foo/file", "other_prefix"),
+                dir + "/external_dep/foo/file",  # No mapping applied, use as-is
+            )
+
+    def testDirectoryBasedRlocationWithCompactRepoMappingPrecedence(self) -> None:
+        """Test that exact mappings take precedence over prefixed mappings."""
+        with _MockFile(
+            name="_repo_mapping",
+            contents=[
+                # Exact mapping for a specific source repo
+                "deps+specific_repo,external_dep,external_dep~exact",
+                # Prefixed mapping for repos starting with "deps+"
+                "deps+*,external_dep,external_dep~prefix",
+                # Another prefixed mapping with different prefix
+                "other+*,external_dep,external_dep~other",
+            ],
+        ) as rm:
+            dir = os.path.dirname(rm.Path())
+            r = runfiles.CreateDirectoryBased(dir)
+
+            # Exact mapping should take precedence over prefix
+            self.assertEqual(
+                r.Rlocation("external_dep/foo/file", "deps+specific_repo"),
+                dir + "/external_dep~exact/foo/file",
+            )
+
+            # Other repos with deps+ prefix should use the prefixed mapping
+            self.assertEqual(
+                r.Rlocation("external_dep/foo/file", "deps+other_repo"),
+                dir + "/external_dep~prefix/foo/file",
+            )
+
+            # Different prefix should use its own mapping
+            self.assertEqual(
+                r.Rlocation("external_dep/foo/file", "other+some_repo"),
+                dir + "/external_dep~other/foo/file",
+            )
+
+    def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
+        """Test that order matters for prefixed mappings (first match wins)."""
+        with _MockFile(
+            name="_repo_mapping",
+            contents=[
+                # More specific prefix comes first
+                "deps+specific+*,lib,lib~specific",
+                # More general prefix comes second
+                "deps+*,lib,lib~general",
+            ],
+        ) as rm:
+            dir = os.path.dirname(rm.Path())
+            r = runfiles.CreateDirectoryBased(dir)
+
+            # Should match the more specific prefix first
+            self.assertEqual(
+                r.Rlocation("lib/foo/file", "deps+specific+repo"),
+                dir + "/lib~specific/foo/file",
+            )
+
+            # Should match the general prefix for non-specific repos
+            self.assertEqual(
+                r.Rlocation("lib/foo/file", "deps+other_repo"),
+                dir + "/lib~general/foo/file",
+            )
+
+# TODO: Add manifest-based test for compact repo mapping
+    # def testManifestBasedRlocationWithCompactRepoMapping(self) -> None:
+    #     """Test that compact repo mapping also works with manifest-based runfiles."""
+    #     pass
+
     def testCurrentRepository(self) -> None:
         # Under bzlmod, the current repository name is the empty string instead
         # of the name in the workspace file.

From 590637f465697767ab9739ffeb01724bf7a0ae67 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 16:08:06 -0400
Subject: [PATCH 02/20] Refactor into _RepositoryMapping

---
 python/runfiles/runfiles.py     | 193 +++++++++++++++++++++++---------
 tests/runfiles/runfiles_test.py |  70 +++++++++++-
 2 files changed, 209 insertions(+), 54 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index af81c52122..587b9773d9 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -16,11 +16,144 @@
 
 See @rules_python//python/runfiles/README.md for usage instructions.
 """
+import collections.abc
 import inspect
 import os
 import posixpath
 import sys
-from typing import Dict, List, Optional, Tuple, Union
+from typing import Dict, Iterator, List, Optional, Tuple, Union
+
+
+class _RepositoryMapping(collections.abc.Mapping[Tuple[str, str], str]):
+    """Repository mapping for resolving apparent repository names to canonical ones.
+    
+    Handles both exact mappings and prefix-based mappings introduced by the 
+    --incompatible_compact_repo_mapping_manifest flag.
+    
+    Implements the Mapping protocol for exact lookups, while providing a separate
+    lookup() method for prefix-aware resolution.
+    """
+
+    def __init__(
+        self, 
+        exact_mappings: Dict[Tuple[str, str], str], 
+        prefixed_mappings: Dict[Tuple[str, str], str]
+    ) -> None:
+        """Initialize repository mapping with exact and prefixed mappings.
+        
+        Args:
+            exact_mappings: Dict mapping (source_canonical, target_apparent) -> target_canonical
+            prefixed_mappings: Dict mapping (source_prefix, target_apparent) -> target_canonical
+        """
+        self._exact_mappings = exact_mappings
+        self._prefixed_mappings = prefixed_mappings
+
+    @staticmethod
+    def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
+        """Create RepositoryMapping from a repository mapping manifest file.
+        
+        Args:
+            repo_mapping_path: Path to the repository mapping file, or None if not available
+            
+        Returns:
+            RepositoryMapping instance with parsed mappings
+        """
+        # If the repository mapping file can't be found, that is not an error: We
+        # might be running without Bzlmod enabled or there may not be any runfiles.
+        # In this case, just apply empty repo mappings.
+        if not repo_mapping_path:
+            return _RepositoryMapping({}, {})
+        
+        try:
+            with open(repo_mapping_path, "r", encoding="utf-8", newline="\n") as f:
+                content = f.read()
+        except FileNotFoundError:
+            return _RepositoryMapping({}, {})
+
+        exact_mappings = {}
+        prefixed_mappings = {}
+        for line in content.split("\n"):
+            if not line:
+                # Empty line following the last line break
+                break
+            source_canonical, target_apparent, target_canonical = line.split(",")
+            if source_canonical.endswith("*"):
+                # This is a prefixed mapping - remove the '*' for prefix matching
+                prefix = source_canonical[:-1]
+                prefixed_mappings[(prefix, target_apparent)] = target_canonical
+            else:
+                # This is an exact mapping
+                exact_mappings[(source_canonical, target_apparent)] = target_canonical
+
+        return _RepositoryMapping(exact_mappings, prefixed_mappings)
+
+    def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
+        """Look up the canonical repository name for a target apparent repo name.
+        
+        Args:
+            source_repo: The canonical name of the source repository
+            target_apparent: The apparent name of the target repository to resolve
+            
+        Returns:
+            The canonical name of the target repository, or None if no mapping found
+        """
+        # Try exact mapping first
+        if (source_repo, target_apparent) in self._exact_mappings:
+            return self._exact_mappings[(source_repo, target_apparent)]
+
+        # Try prefixed mapping if no exact match found
+        for (prefix_source, target_app), target_canonical in self._prefixed_mappings.items():
+            if target_app == target_apparent and source_repo.startswith(prefix_source):
+                return target_canonical
+
+        # No mapping found
+        return None
+
+    def is_empty(self) -> bool:
+        """Check if the repository mapping is empty."""
+        return not self._exact_mappings and not self._prefixed_mappings
+
+    # Mapping protocol implementation (for exact mappings only)
+    def __getitem__(self, key: Tuple[str, str]) -> str:
+        """Get exact mapping for (source_canonical, target_apparent) key.
+        
+        Note: This only handles exact mappings, not prefix-based mappings.
+        For prefix-aware lookup, use the lookup() method instead.
+        
+        Args:
+            key: Tuple of (source_canonical, target_apparent)
+            
+        Returns:
+            target_canonical repository name
+            
+        Raises:
+            KeyError: if no exact mapping exists for the key
+        """
+        return self._exact_mappings[key]
+
+    def __iter__(self) -> Iterator[Tuple[str, str]]:
+        """Iterate over exact mapping keys."""
+        return iter(self._exact_mappings)
+
+    def __len__(self) -> int:
+        """Return the number of exact mappings."""
+        return len(self._exact_mappings)
+
+    # Additional mapping methods for convenience
+    def get(self, key: Tuple[str, str], default: Optional[str] = None) -> Optional[str]:
+        """Get exact mapping with default value.
+        
+        Note: This only handles exact mappings, not prefix-based mappings.
+        For prefix-aware lookup, use the lookup() method instead.
+        
+        Args:
+            key: Tuple of (source_canonical, target_apparent)
+            default: Default value to return if key not found
+            
+        Returns:
+            target_canonical repository name or default
+        """
+        return self._exact_mappings.get(key, default)
 
 
 class _ManifestBased:
@@ -130,7 +263,7 @@ class Runfiles:
     def __init__(self, strategy: Union[_ManifestBased, _DirectoryBased]) -> None:
         self._strategy = strategy
         self._python_runfiles_root = _FindPythonRunfilesRoot()
-        self._exact_repo_mapping, self._prefixed_repo_mapping = _ParseRepoMapping(
+        self._repo_mapping = _RepositoryMapping.create_from_file(
             strategy.RlocationChecked("_repo_mapping")
         )
 
@@ -179,7 +312,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if os.path.isabs(path):
             return path
 
-        if source_repo is None and (self._exact_repo_mapping or self._prefixed_repo_mapping):
+        if source_repo is None and not self._repo_mapping.is_empty():
             # Look up runfiles using the repository mapping of the caller of the
             # current method. If the repo mapping is empty, determining this
             # name is not necessary.
@@ -193,16 +326,10 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
             # which should not be mapped.
             return self._strategy.RlocationChecked(path)
 
-        # Try exact mapping first
-        if (source_repo, target_repo) in self._exact_repo_mapping:
-            target_canonical = self._exact_repo_mapping[(source_repo, target_repo)]
-            return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
-
-        # Try prefixed mapping if no exact match found
-        for (prefix_source, target_apparent), target_canonical in self._prefixed_repo_mapping.items():
-            if (target_apparent == target_repo and 
-                source_repo is not None and 
-                source_repo.startswith(prefix_source)):
+        # Look up the target repository using the repository mapping
+        if source_repo is not None:
+            target_canonical = self._repo_mapping.lookup(source_repo, target_repo)
+            if target_canonical is not None:
                 return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
 
         # No mapping found - assume target_repo is already canonical or
@@ -360,46 +487,6 @@ def _FindPythonRunfilesRoot() -> str:
     return root
 
 
-def _ParseRepoMapping(
-    repo_mapping_path: Optional[str]
-) -> Tuple[Dict[Tuple[str, str], str], Dict[Tuple[str, str], str]]:
-    """Parses the repository mapping manifest.
-    
-    Returns:
-        A tuple of (exact_mappings, prefixed_mappings) where:
-        - exact_mappings: Dict mapping (source_canonical, target_apparent) -> target_canonical
-        - prefixed_mappings: Dict mapping (source_prefix, target_apparent) -> target_canonical
-          where source_prefix entries in the input ended with '*'
-    """
-    # If the repository mapping file can't be found, that is not an error: We
-    # might be running without Bzlmod enabled or there may not be any runfiles.
-    # In this case, just apply empty repo mappings.
-    if not repo_mapping_path:
-        return {}, {}
-    try:
-        with open(repo_mapping_path, "r", encoding="utf-8", newline="\n") as f:
-            content = f.read()
-    except FileNotFoundError:
-        return {}, {}
-
-    exact_mappings = {}
-    prefixed_mappings = {}
-    for line in content.split("\n"):
-        if not line:
-            # Empty line following the last line break
-            break
-        source_canonical, target_apparent, target_canonical = line.split(",")
-        if source_canonical.endswith("*"):
-            # This is a prefixed mapping - remove the '*' for prefix matching
-            prefix = source_canonical[:-1]
-            prefixed_mappings[(prefix, target_apparent)] = target_canonical
-        else:
-            # This is an exact mapping
-            exact_mappings[(source_canonical, target_apparent)] = target_canonical
-
-    return exact_mappings, prefixed_mappings
-
-
 def CreateManifestBased(manifest_path: str) -> Runfiles:
     return Runfiles.CreateManifestBased(manifest_path)
 
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 83a99d7d79..44f85b6c3a 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -636,7 +636,75 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
                 dir + "/lib~general/foo/file",
             )
 
-# TODO: Add manifest-based test for compact repo mapping
+    def testRepositoryMappingImplementsMappingProtocol(self) -> None:
+        """Test that RepositoryMapping implements the Mapping protocol correctly."""
+        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
+        
+        # Create a RepositoryMapping with both exact and prefixed mappings
+        exact_mappings = {
+            ("", "my_workspace"): "_main",
+            ("", "config_lib"): "config_lib~1.0.0",
+            ("deps+specific_repo", "external_dep"): "external_dep~exact",
+        }
+        prefixed_mappings = {
+            ("deps+", "external_dep"): "external_dep~prefix",
+            ("test_deps+", "test_lib"): "test_lib~2.1.0",
+        }
+        
+        repo_mapping = _RepositoryMapping(exact_mappings, prefixed_mappings)
+        
+        # Test Mapping protocol methods
+        
+        # Test __len__ - should only count exact mappings
+        self.assertEqual(len(repo_mapping), 3)
+        
+        # Test __getitem__ - should work for exact mappings only
+        self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
+        self.assertEqual(repo_mapping[("", "config_lib")], "config_lib~1.0.0")
+        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
+        
+        # Test KeyError for non-existent exact mapping
+        with self.assertRaises(KeyError):
+            _ = repo_mapping[("nonexistent", "repo")]
+        
+        # Test iteration - should iterate over exact mapping keys only
+        keys = list(repo_mapping)
+        self.assertEqual(len(keys), 3)
+        self.assertIn(("", "my_workspace"), keys)
+        self.assertIn(("", "config_lib"), keys)
+        self.assertIn(("deps+specific_repo", "external_dep"), keys)
+        
+        # Test 'in' operator (via __contains__)
+        self.assertTrue(("", "my_workspace") in repo_mapping)
+        self.assertFalse(("nonexistent", "repo") in repo_mapping)
+        
+        # Test get() method
+        self.assertEqual(repo_mapping.get(("", "my_workspace")), "_main")
+        self.assertIsNone(repo_mapping.get(("nonexistent", "repo")))
+        self.assertEqual(repo_mapping.get(("nonexistent", "repo"), "default"), "default")
+        
+        # Test that lookup() still works for both exact and prefix-based
+        # Exact lookup
+        self.assertEqual(repo_mapping.lookup("", "my_workspace"), "_main")
+        
+        # Prefix-based lookup (not available via Mapping protocol)
+        self.assertEqual(repo_mapping.lookup("deps+some_repo", "external_dep"), "external_dep~prefix")
+        
+        # Exact takes precedence over prefix
+        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
+        
+        # Test items(), keys(), values() (inherited from Mapping)
+        items = list(repo_mapping.items())
+        self.assertEqual(len(items), 3)
+        self.assertIn((("", "my_workspace"), "_main"), items)
+        
+        keys = list(repo_mapping.keys())
+        values = list(repo_mapping.values())
+        self.assertEqual(len(keys), 3)
+        self.assertEqual(len(values), 3)
+        self.assertIn("_main", values)
+
+    # TODO: Add manifest-based test for compact repo mapping
     # def testManifestBasedRlocationWithCompactRepoMapping(self) -> None:
     #     """Test that compact repo mapping also works with manifest-based runfiles."""
     #     pass

From f471b7f7b7bdbe37f4008c9bb1ea4d93ad88742d Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 16:51:10 -0400
Subject: [PATCH 03/20] More fully pythonic mapping

---
 python/runfiles/runfiles.py     | 82 ++++++++++++---------------------
 tests/runfiles/runfiles_test.py | 78 ++++++++++++++++++++++---------
 2 files changed, 84 insertions(+), 76 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 587b9773d9..10f09d43e9 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -87,19 +87,28 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
 
         return _RepositoryMapping(exact_mappings, prefixed_mappings)
 
-    def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
-        """Look up the canonical repository name for a target apparent repo name.
+    # Mapping protocol implementation
+    def __getitem__(self, key: Tuple[str, str]) -> str:
+        """Get repository mapping for (source_canonical, target_apparent) key.
+        
+        This handles both exact mappings and prefix-based mappings introduced by the 
+        --incompatible_compact_repo_mapping_manifest flag. Exact mappings are tried
+        first, followed by prefix-based mappings where order matters.
         
         Args:
-            source_repo: The canonical name of the source repository
-            target_apparent: The apparent name of the target repository to resolve
+            key: Tuple of (source_canonical, target_apparent)
             
         Returns:
-            The canonical name of the target repository, or None if no mapping found
+            target_canonical repository name
+            
+        Raises:
+            KeyError: if no mapping exists for the key
         """
+        source_repo, target_apparent = key
+        
         # Try exact mapping first
-        if (source_repo, target_apparent) in self._exact_mappings:
-            return self._exact_mappings[(source_repo, target_apparent)]
+        if key in self._exact_mappings:
+            return self._exact_mappings[key]
 
         # Try prefixed mapping if no exact match found
         for (prefix_source, target_app), target_canonical in self._prefixed_mappings.items():
@@ -107,53 +116,18 @@ def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
                 return target_canonical
 
         # No mapping found
-        return None
-
-    def is_empty(self) -> bool:
-        """Check if the repository mapping is empty."""
-        return not self._exact_mappings and not self._prefixed_mappings
-
-    # Mapping protocol implementation (for exact mappings only)
-    def __getitem__(self, key: Tuple[str, str]) -> str:
-        """Get exact mapping for (source_canonical, target_apparent) key.
-        
-        Note: This only handles exact mappings, not prefix-based mappings.
-        For prefix-aware lookup, use the lookup() method instead.
-        
-        Args:
-            key: Tuple of (source_canonical, target_apparent)
-            
-        Returns:
-            target_canonical repository name
-            
-        Raises:
-            KeyError: if no exact mapping exists for the key
-        """
-        return self._exact_mappings[key]
+        raise KeyError(key)
 
     def __iter__(self) -> Iterator[Tuple[str, str]]:
-        """Iterate over exact mapping keys."""
-        return iter(self._exact_mappings)
+        """Iterate over all mapping keys (exact first, then prefixed)."""
+        # First yield all exact mapping keys
+        yield from self._exact_mappings.keys()
+        # Then yield all prefixed mapping keys
+        yield from self._prefixed_mappings.keys()
 
     def __len__(self) -> int:
-        """Return the number of exact mappings."""
-        return len(self._exact_mappings)
-
-    # Additional mapping methods for convenience
-    def get(self, key: Tuple[str, str], default: Optional[str] = None) -> Optional[str]:
-        """Get exact mapping with default value.
-        
-        Note: This only handles exact mappings, not prefix-based mappings.
-        For prefix-aware lookup, use the lookup() method instead.
-        
-        Args:
-            key: Tuple of (source_canonical, target_apparent)
-            default: Default value to return if key not found
-            
-        Returns:
-            target_canonical repository name or default
-        """
-        return self._exact_mappings.get(key, default)
+        """Return the total number of mappings (exact + prefixed)."""
+        return len(self._exact_mappings) + len(self._prefixed_mappings)
 
 
 class _ManifestBased:
@@ -312,7 +286,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if os.path.isabs(path):
             return path
 
-        if source_repo is None and not self._repo_mapping.is_empty():
+        if source_repo is None and self._repo_mapping:
             # Look up runfiles using the repository mapping of the caller of the
             # current method. If the repo mapping is empty, determining this
             # name is not necessary.
@@ -328,9 +302,11 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
 
         # Look up the target repository using the repository mapping
         if source_repo is not None:
-            target_canonical = self._repo_mapping.lookup(source_repo, target_repo)
-            if target_canonical is not None:
+            try:
+                target_canonical = self._repo_mapping[(source_repo, target_repo)]
                 return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
+            except KeyError:
+                pass  # No mapping found, continue to fallback
 
         # No mapping found - assume target_repo is already canonical or
         # we're not using Bzlmod
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 44f85b6c3a..cb95012bb5 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -637,7 +637,7 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
             )
 
     def testRepositoryMappingImplementsMappingProtocol(self) -> None:
-        """Test that RepositoryMapping implements the Mapping protocol correctly."""
+        """Test that RepositoryMapping implements the Mapping protocol with unified lookup."""
         from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
         
         # Create a RepositoryMapping with both exact and prefixed mappings
@@ -655,54 +655,86 @@ def testRepositoryMappingImplementsMappingProtocol(self) -> None:
         
         # Test Mapping protocol methods
         
-        # Test __len__ - should only count exact mappings
-        self.assertEqual(len(repo_mapping), 3)
+        # Test __len__ - should count both exact and prefixed mappings
+        self.assertEqual(len(repo_mapping), 5)  # 3 exact + 2 prefixed
         
         # Test __getitem__ - should work for exact mappings only
         self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
         self.assertEqual(repo_mapping[("", "config_lib")], "config_lib~1.0.0")
         self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
         
-        # Test KeyError for non-existent exact mapping
+        # Test KeyError for non-existent mapping (neither exact nor prefix match)
         with self.assertRaises(KeyError):
             _ = repo_mapping[("nonexistent", "repo")]
         
-        # Test iteration - should iterate over exact mapping keys only
+        # Test iteration - should iterate over all mapping keys (exact first, then prefixed)
         keys = list(repo_mapping)
-        self.assertEqual(len(keys), 3)
-        self.assertIn(("", "my_workspace"), keys)
-        self.assertIn(("", "config_lib"), keys)
-        self.assertIn(("deps+specific_repo", "external_dep"), keys)
+        self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
         
-        # Test 'in' operator (via __contains__)
+        # Check that exact mappings come first
+        exact_keys = [("", "my_workspace"), ("", "config_lib"), ("deps+specific_repo", "external_dep")]
+        prefixed_keys = [("deps+", "external_dep"), ("test_deps+", "test_lib")]
+        
+        for exact_key in exact_keys:
+            self.assertIn(exact_key, keys)
+        
+        for prefixed_key in prefixed_keys:
+            self.assertIn(prefixed_key, keys)
+        
+        # Verify order: exact keys should come before prefixed keys
+        first_three_keys = keys[:3]
+        last_two_keys = keys[3:]
+        
+        for exact_key in exact_keys:
+            self.assertIn(exact_key, first_three_keys)
+        
+        for prefixed_key in prefixed_keys:
+            self.assertIn(prefixed_key, last_two_keys)
+        
+        # Test 'in' operator (via __contains__) - works for both exact and prefix-based
         self.assertTrue(("", "my_workspace") in repo_mapping)
         self.assertFalse(("nonexistent", "repo") in repo_mapping)
+        # Prefix-based lookups also work with 'in' operator (consistent with [])
+        self.assertTrue(("deps+some_repo", "external_dep") in repo_mapping)
         
-        # Test get() method
-        self.assertEqual(repo_mapping.get(("", "my_workspace")), "_main")
-        self.assertIsNone(repo_mapping.get(("nonexistent", "repo")))
-        self.assertEqual(repo_mapping.get(("nonexistent", "repo"), "default"), "default")
-        
-        # Test that lookup() still works for both exact and prefix-based
+        # Test that the mapping interface works for both exact and prefix-based
         # Exact lookup
-        self.assertEqual(repo_mapping.lookup("", "my_workspace"), "_main")
+        self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
         
-        # Prefix-based lookup (not available via Mapping protocol)
-        self.assertEqual(repo_mapping.lookup("deps+some_repo", "external_dep"), "external_dep~prefix")
+        # Prefix-based lookup
+        self.assertEqual(repo_mapping[("deps+some_repo", "external_dep")], "external_dep~prefix")
         
         # Exact takes precedence over prefix
-        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
+        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
+        
+        # Test get() method with both exact and prefix-based
+        self.assertEqual(repo_mapping.get(("", "my_workspace")), "_main")
+        self.assertEqual(repo_mapping.get(("deps+some_repo", "external_dep")), "external_dep~prefix")
+        self.assertIsNone(repo_mapping.get(("nonexistent", "repo")))
+        self.assertEqual(repo_mapping.get(("nonexistent", "repo"), "default"), "default")
         
         # Test items(), keys(), values() (inherited from Mapping)
         items = list(repo_mapping.items())
-        self.assertEqual(len(items), 3)
+        self.assertEqual(len(items), 5)  # 3 exact + 2 prefixed
         self.assertIn((("", "my_workspace"), "_main"), items)
+        self.assertIn((("deps+", "external_dep"), "external_dep~prefix"), items)
         
         keys = list(repo_mapping.keys())
         values = list(repo_mapping.values())
-        self.assertEqual(len(keys), 3)
-        self.assertEqual(len(values), 3)
+        self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
+        self.assertEqual(len(values), 5)  # 3 exact + 2 prefixed
         self.assertIn("_main", values)
+        self.assertIn("external_dep~prefix", values)
+        
+        # Test pythonic truthiness (no need for is_empty method)
+        self.assertTrue(repo_mapping)  # Non-empty mapping is truthy
+        self.assertTrue(bool(repo_mapping))
+        
+        # Test empty mapping
+        empty_mapping = _RepositoryMapping({}, {})
+        self.assertFalse(empty_mapping)  # Empty mapping is falsy
+        self.assertFalse(bool(empty_mapping))
+        self.assertEqual(len(empty_mapping), 0)
 
     # TODO: Add manifest-based test for compact repo mapping
     # def testManifestBasedRlocationWithCompactRepoMapping(self) -> None:

From ab34ab7572a62ea275de523de172d30d611091d5 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 16:55:53 -0400
Subject: [PATCH 04/20] Add changelog

---
 CHANGELOG.md                | 6 ++++++
 python/runfiles/runfiles.py | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 382096f826..ec13e733ea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -92,6 +92,12 @@ END_UNRELEASED_TEMPLATE
 
 {#v0-0-0-added}
 ### Added
+* (runfiles) The Python runfiles library now supports Bazel's
+  `--incompatible_compact_repo_mapping_manifest` flag, which uses prefix-based
+  repository mappings to reduce memory usage for large dependency graphs under
+  bzlmod. This allows the repository mapping manifest to be significantly
+  smaller (from tens of megabytes to much less) while maintaining full
+  functionality.
 * (bootstrap) {obj}`--bootstrap_impl=system_python` now supports the
   {obj}`main_module` attribute.
 * (bootstrap) {obj}`--bootstrap_impl=system_python` now supports the
diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 10f09d43e9..8fe65770e6 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -15,6 +15,12 @@
 """Runfiles lookup library for Bazel-built Python binaries and tests.
 
 See @rules_python//python/runfiles/README.md for usage instructions.
+
+:::{versionadded} VERSION_NEXT_FEATURE
+Support for Bazel's `--incompatible_compact_repo_mapping_manifest` flag was added.
+This enables prefix-based repository mappings to reduce memory usage for large
+dependency graphs under bzlmod.
+:::
 """
 import collections.abc
 import inspect

From 7009cefd2a6ffe2aaeaa9836db3ceb36b4cb4a8f Mon Sep 17 00:00:00 2001
From: Jeff Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 17:09:42 -0400
Subject: [PATCH 05/20] Update python/runfiles/runfiles.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---
 python/runfiles/runfiles.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 8fe65770e6..e11c863ba1 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -36,8 +36,8 @@ class _RepositoryMapping(collections.abc.Mapping[Tuple[str, str], str]):
     Handles both exact mappings and prefix-based mappings introduced by the 
     --incompatible_compact_repo_mapping_manifest flag.
     
-    Implements the Mapping protocol for exact lookups, while providing a separate
-    lookup() method for prefix-aware resolution.
+    Implements the Mapping protocol, providing a unified lookup for both exact
+    and prefix-based mappings.
     """
 
     def __init__(

From 0ab9cf68763cc0bcf6cce5c7d618bbcf2a954a27 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 17:44:46 -0400
Subject: [PATCH 06/20] Refactor mapping tests

---
 tests/runfiles/runfiles_test.py | 99 ++++++++++++++++++++++-----------
 1 file changed, 68 insertions(+), 31 deletions(-)

diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index cb95012bb5..3cfd78064c 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -636,11 +636,10 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
                 dir + "/lib~general/foo/file",
             )
 
-    def testRepositoryMappingImplementsMappingProtocol(self) -> None:
-        """Test that RepositoryMapping implements the Mapping protocol with unified lookup."""
+    def _createTestRepositoryMapping(self):
+        """Helper method to create a test RepositoryMapping with both exact and prefixed mappings."""
         from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
         
-        # Create a RepositoryMapping with both exact and prefixed mappings
         exact_mappings = {
             ("", "my_workspace"): "_main",
             ("", "config_lib"): "config_lib~1.0.0",
@@ -651,27 +650,45 @@ def testRepositoryMappingImplementsMappingProtocol(self) -> None:
             ("test_deps+", "test_lib"): "test_lib~2.1.0",
         }
         
-        repo_mapping = _RepositoryMapping(exact_mappings, prefixed_mappings)
-        
-        # Test Mapping protocol methods
-        
-        # Test __len__ - should count both exact and prefixed mappings
+        return _RepositoryMapping(exact_mappings, prefixed_mappings)
+
+    def testRepositoryMappingLen(self) -> None:
+        """Test __len__ method - should count both exact and prefixed mappings."""
+        repo_mapping = self._createTestRepositoryMapping()
         self.assertEqual(len(repo_mapping), 5)  # 3 exact + 2 prefixed
         
-        # Test __getitem__ - should work for exact mappings only
+        # Test empty mapping
+        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
+        empty_mapping = _RepositoryMapping({}, {})
+        self.assertEqual(len(empty_mapping), 0)
+
+    def testRepositoryMappingGetItem(self) -> None:
+        """Test __getitem__ method - should work for both exact and prefix-based mappings."""
+        repo_mapping = self._createTestRepositoryMapping()
+        
+        # Test exact lookup
         self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
         self.assertEqual(repo_mapping[("", "config_lib")], "config_lib~1.0.0")
         self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
         
+        # Test prefix-based lookup
+        self.assertEqual(repo_mapping[("deps+some_repo", "external_dep")], "external_dep~prefix")
+        
+        # Test that exact takes precedence over prefix
+        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
+        
         # Test KeyError for non-existent mapping (neither exact nor prefix match)
         with self.assertRaises(KeyError):
             _ = repo_mapping[("nonexistent", "repo")]
+
+    def testRepositoryMappingIter(self) -> None:
+        """Test __iter__ method - should iterate over all mapping keys (exact first, then prefixed)."""
+        repo_mapping = self._createTestRepositoryMapping()
         
-        # Test iteration - should iterate over all mapping keys (exact first, then prefixed)
         keys = list(repo_mapping)
         self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
         
-        # Check that exact mappings come first
+        # Check that all expected keys are present
         exact_keys = [("", "my_workspace"), ("", "config_lib"), ("deps+specific_repo", "external_dep")]
         prefixed_keys = [("deps+", "external_dep"), ("test_deps+", "test_lib")]
         
@@ -690,51 +707,71 @@ def testRepositoryMappingImplementsMappingProtocol(self) -> None:
         
         for prefixed_key in prefixed_keys:
             self.assertIn(prefixed_key, last_two_keys)
+
+    def testRepositoryMappingContains(self) -> None:
+        """Test __contains__ method (in operator) - should work for both exact and prefix-based mappings."""
+        repo_mapping = self._createTestRepositoryMapping()
         
-        # Test 'in' operator (via __contains__) - works for both exact and prefix-based
+        # Test exact lookups
         self.assertTrue(("", "my_workspace") in repo_mapping)
-        self.assertFalse(("nonexistent", "repo") in repo_mapping)
-        # Prefix-based lookups also work with 'in' operator (consistent with [])
-        self.assertTrue(("deps+some_repo", "external_dep") in repo_mapping)
+        self.assertTrue(("deps+specific_repo", "external_dep") in repo_mapping)
         
-        # Test that the mapping interface works for both exact and prefix-based
-        # Exact lookup
-        self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
-        
-        # Prefix-based lookup
-        self.assertEqual(repo_mapping[("deps+some_repo", "external_dep")], "external_dep~prefix")
+        # Test prefix-based lookups (consistent with __getitem__)
+        self.assertTrue(("deps+some_repo", "external_dep") in repo_mapping)
         
-        # Exact takes precedence over prefix
-        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
+        # Test non-existent mapping
+        self.assertFalse(("nonexistent", "repo") in repo_mapping)
+
+    def testRepositoryMappingGet(self) -> None:
+        """Test get() method - should work for both exact and prefix-based mappings."""
+        repo_mapping = self._createTestRepositoryMapping()
         
-        # Test get() method with both exact and prefix-based
+        # Test exact lookups
         self.assertEqual(repo_mapping.get(("", "my_workspace")), "_main")
+        self.assertEqual(repo_mapping.get(("deps+specific_repo", "external_dep")), "external_dep~exact")
+        
+        # Test prefix-based lookups
         self.assertEqual(repo_mapping.get(("deps+some_repo", "external_dep")), "external_dep~prefix")
+        
+        # Test non-existent mapping
         self.assertIsNone(repo_mapping.get(("nonexistent", "repo")))
         self.assertEqual(repo_mapping.get(("nonexistent", "repo"), "default"), "default")
+
+    def testRepositoryMappingKeysValuesItems(self) -> None:
+        """Test keys(), values(), and items() methods (inherited from Mapping)."""
+        repo_mapping = self._createTestRepositoryMapping()
         
-        # Test items(), keys(), values() (inherited from Mapping)
+        # Test items()
         items = list(repo_mapping.items())
         self.assertEqual(len(items), 5)  # 3 exact + 2 prefixed
         self.assertIn((("", "my_workspace"), "_main"), items)
         self.assertIn((("deps+", "external_dep"), "external_dep~prefix"), items)
         
+        # Test keys()
         keys = list(repo_mapping.keys())
-        values = list(repo_mapping.values())
         self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
+        self.assertIn(("", "my_workspace"), keys)
+        self.assertIn(("deps+", "external_dep"), keys)
+        
+        # Test values()
+        values = list(repo_mapping.values())
         self.assertEqual(len(values), 5)  # 3 exact + 2 prefixed
         self.assertIn("_main", values)
         self.assertIn("external_dep~prefix", values)
+
+    def testRepositoryMappingTruthiness(self) -> None:
+        """Test pythonic truthiness - non-empty mapping is truthy, empty mapping is falsy."""
+        repo_mapping = self._createTestRepositoryMapping()
         
-        # Test pythonic truthiness (no need for is_empty method)
-        self.assertTrue(repo_mapping)  # Non-empty mapping is truthy
+        # Test non-empty mapping is truthy
+        self.assertTrue(repo_mapping)
         self.assertTrue(bool(repo_mapping))
         
-        # Test empty mapping
+        # Test empty mapping is falsy
+        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
         empty_mapping = _RepositoryMapping({}, {})
-        self.assertFalse(empty_mapping)  # Empty mapping is falsy
+        self.assertFalse(empty_mapping)
         self.assertFalse(bool(empty_mapping))
-        self.assertEqual(len(empty_mapping), 0)
 
     # TODO: Add manifest-based test for compact repo mapping
     # def testManifestBasedRlocationWithCompactRepoMapping(self) -> None:

From 0497f17613bf04298675a4a632896a7db3e0861d Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 19:19:18 -0400
Subject: [PATCH 07/20] Use splitlines

---
 python/runfiles/runfiles.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index e11c863ba1..d70cdf0d5c 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -78,10 +78,7 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
 
         exact_mappings = {}
         prefixed_mappings = {}
-        for line in content.split("\n"):
-            if not line:
-                # Empty line following the last line break
-                break
+        for line in content.splitlines():
             source_canonical, target_apparent, target_canonical = line.split(",")
             if source_canonical.endswith("*"):
                 # This is a prefixed mapping - remove the '*' for prefix matching

From 268f00b1aee5afe7f03e3a0ab7bd59febcca782a Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Thu, 18 Sep 2025 19:22:46 -0400
Subject: [PATCH 08/20] grouped prefix optimization

---
 python/runfiles/runfiles.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index d70cdf0d5c..7ac53123eb 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -23,6 +23,7 @@
 :::
 """
 import collections.abc
+from collections import defaultdict
 import inspect
 import os
 import posixpath
@@ -53,6 +54,11 @@ def __init__(
         """
         self._exact_mappings = exact_mappings
         self._prefixed_mappings = prefixed_mappings
+        
+        # Group prefixed mappings by target_apparent for faster lookups
+        self._grouped_prefixed_mappings = defaultdict(list)
+        for (prefix_source, target_app), target_canonical in self._prefixed_mappings.items():
+            self._grouped_prefixed_mappings[target_app].append((prefix_source, target_canonical))
 
     @staticmethod
     def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
@@ -114,9 +120,10 @@ def __getitem__(self, key: Tuple[str, str]) -> str:
             return self._exact_mappings[key]
 
         # Try prefixed mapping if no exact match found
-        for (prefix_source, target_app), target_canonical in self._prefixed_mappings.items():
-            if target_app == target_apparent and source_repo.startswith(prefix_source):
-                return target_canonical
+        if target_apparent in self._grouped_prefixed_mappings:
+            for prefix_source, target_canonical in self._grouped_prefixed_mappings[target_apparent]:
+                if source_repo.startswith(prefix_source):
+                    return target_canonical
 
         # No mapping found
         raise KeyError(key)

From 79b36c5612fc9f922ee953163eb966ce23c98c0d Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Fri, 19 Sep 2025 07:40:01 -0400
Subject: [PATCH 09/20] Fix python 3.8.20 type issue

---
 python/runfiles/runfiles.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 7ac53123eb..1c7c70c091 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -28,17 +28,14 @@
 import os
 import posixpath
 import sys
-from typing import Dict, Iterator, List, Optional, Tuple, Union
+from typing import Dict, Iterator, List, Mapping, Optional, Tuple, Union
 
 
-class _RepositoryMapping(collections.abc.Mapping[Tuple[str, str], str]):
+class _RepositoryMapping(collections.abc.Mapping):
     """Repository mapping for resolving apparent repository names to canonical ones.
     
     Handles both exact mappings and prefix-based mappings introduced by the 
     --incompatible_compact_repo_mapping_manifest flag.
-    
-    Implements the Mapping protocol, providing a unified lookup for both exact
-    and prefix-based mappings.
     """
 
     def __init__(

From fe77b24af1b30fe0073eadf3240247bf80d2b98a Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Fri, 19 Sep 2025 07:42:44 -0400
Subject: [PATCH 10/20] Remove unused imports

---
 python/runfiles/runfiles.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 1c7c70c091..0f03c078b8 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -28,7 +28,7 @@
 import os
 import posixpath
 import sys
-from typing import Dict, Iterator, List, Mapping, Optional, Tuple, Union
+from typing import Dict, Iterator, Optional, Tuple, Union
 
 
 class _RepositoryMapping(collections.abc.Mapping):

From c4b89d671314af7d2b8691480c0691688ff87821 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Fri, 19 Sep 2025 07:48:54 -0400
Subject: [PATCH 11/20] Avoid KeyError

---
 python/runfiles/runfiles.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 0f03c078b8..9db6b4ba64 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -309,11 +309,9 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
 
         # Look up the target repository using the repository mapping
         if source_repo is not None:
-            try:
-                target_canonical = self._repo_mapping[(source_repo, target_repo)]
+            target_canonical = self._repo_mapping.get((source_repo, target_repo))
+            if target_canonical is not None:
                 return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
-            except KeyError:
-                pass  # No mapping found, continue to fallback
 
         # No mapping found - assume target_repo is already canonical or
         # we're not using Bzlmod

From ffda0afc664d27d3f9f033b8f42698242c363eaf Mon Sep 17 00:00:00 2001
From: Jeff Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 09:25:22 -0400
Subject: [PATCH 12/20] Update CHANGELOG.md

Co-authored-by: Richard Levasseur <richardlev@gmail.com>
---
 CHANGELOG.md | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ec13e733ea..8498ad32d8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -93,11 +93,7 @@ END_UNRELEASED_TEMPLATE
 {#v0-0-0-added}
 ### Added
 * (runfiles) The Python runfiles library now supports Bazel's
-  `--incompatible_compact_repo_mapping_manifest` flag, which uses prefix-based
-  repository mappings to reduce memory usage for large dependency graphs under
-  bzlmod. This allows the repository mapping manifest to be significantly
-  smaller (from tens of megabytes to much less) while maintaining full
-  functionality.
+  `--incompatible_compact_repo_mapping_manifest` flag.
 * (bootstrap) {obj}`--bootstrap_impl=system_python` now supports the
   {obj}`main_module` attribute.
 * (bootstrap) {obj}`--bootstrap_impl=system_python` now supports the

From 0e14bcaaadf352e71df51815d41530e8ca43f8ff Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 09:25:54 -0400
Subject: [PATCH 13/20] Remove TODO

---
 tests/runfiles/runfiles_test.py | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 3cfd78064c..9322ede720 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -773,11 +773,6 @@ def testRepositoryMappingTruthiness(self) -> None:
         self.assertFalse(empty_mapping)
         self.assertFalse(bool(empty_mapping))
 
-    # TODO: Add manifest-based test for compact repo mapping
-    # def testManifestBasedRlocationWithCompactRepoMapping(self) -> None:
-    #     """Test that compact repo mapping also works with manifest-based runfiles."""
-    #     pass
-
     def testCurrentRepository(self) -> None:
         # Under bzlmod, the current repository name is the empty string instead
         # of the name in the workspace file.

From ca067865a8dcfbf0604d624563ffd5869f38e659 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 09:32:13 -0400
Subject: [PATCH 14/20] Remove Mapping compat

---
 python/runfiles/runfiles.py     |  36 +++------
 tests/runfiles/runfiles_test.py | 131 ++++----------------------------
 2 files changed, 26 insertions(+), 141 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 9db6b4ba64..c4fa2a2733 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -28,10 +28,10 @@
 import os
 import posixpath
 import sys
-from typing import Dict, Iterator, Optional, Tuple, Union
+from typing import Dict, Optional, Tuple, Union
 
 
-class _RepositoryMapping(collections.abc.Mapping):
+class _RepositoryMapping:
     """Repository mapping for resolving apparent repository names to canonical ones.
     
     Handles both exact mappings and prefix-based mappings introduced by the 
@@ -93,24 +93,21 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
 
         return _RepositoryMapping(exact_mappings, prefixed_mappings)
 
-    # Mapping protocol implementation
-    def __getitem__(self, key: Tuple[str, str]) -> str:
-        """Get repository mapping for (source_canonical, target_apparent) key.
+    def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
+        """Look up repository mapping for the given source and target.
         
         This handles both exact mappings and prefix-based mappings introduced by the 
         --incompatible_compact_repo_mapping_manifest flag. Exact mappings are tried
         first, followed by prefix-based mappings where order matters.
         
         Args:
-            key: Tuple of (source_canonical, target_apparent)
+            source_repo: Source canonical repository name
+            target_apparent: Target apparent repository name
             
         Returns:
-            target_canonical repository name
-            
-        Raises:
-            KeyError: if no mapping exists for the key
+            target_canonical repository name, or None if no mapping exists
         """
-        source_repo, target_apparent = key
+        key = (source_repo, target_apparent)
         
         # Try exact mapping first
         if key in self._exact_mappings:
@@ -123,18 +120,7 @@ def __getitem__(self, key: Tuple[str, str]) -> str:
                     return target_canonical
 
         # No mapping found
-        raise KeyError(key)
-
-    def __iter__(self) -> Iterator[Tuple[str, str]]:
-        """Iterate over all mapping keys (exact first, then prefixed)."""
-        # First yield all exact mapping keys
-        yield from self._exact_mappings.keys()
-        # Then yield all prefixed mapping keys
-        yield from self._prefixed_mappings.keys()
-
-    def __len__(self) -> int:
-        """Return the total number of mappings (exact + prefixed)."""
-        return len(self._exact_mappings) + len(self._prefixed_mappings)
+        return None
 
 
 class _ManifestBased:
@@ -293,7 +279,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if os.path.isabs(path):
             return path
 
-        if source_repo is None and self._repo_mapping:
+        if source_repo is None and self._repo_mapping is not None:
             # Look up runfiles using the repository mapping of the caller of the
             # current method. If the repo mapping is empty, determining this
             # name is not necessary.
@@ -309,7 +295,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
 
         # Look up the target repository using the repository mapping
         if source_repo is not None:
-            target_canonical = self._repo_mapping.get((source_repo, target_repo))
+            target_canonical = self._repo_mapping.lookup(source_repo, target_repo)
             if target_canonical is not None:
                 return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
 
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 9322ede720..01af925a9b 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -636,8 +636,8 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
                 dir + "/lib~general/foo/file",
             )
 
-    def _createTestRepositoryMapping(self):
-        """Helper method to create a test RepositoryMapping with both exact and prefixed mappings."""
+    def testRepositoryMappingLookup(self) -> None:
+        """Test _RepositoryMapping.lookup() method for both exact and prefix-based mappings."""
         from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
         
         exact_mappings = {
@@ -650,128 +650,27 @@ def _createTestRepositoryMapping(self):
             ("test_deps+", "test_lib"): "test_lib~2.1.0",
         }
         
-        return _RepositoryMapping(exact_mappings, prefixed_mappings)
-
-    def testRepositoryMappingLen(self) -> None:
-        """Test __len__ method - should count both exact and prefixed mappings."""
-        repo_mapping = self._createTestRepositoryMapping()
-        self.assertEqual(len(repo_mapping), 5)  # 3 exact + 2 prefixed
-        
-        # Test empty mapping
-        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
-        empty_mapping = _RepositoryMapping({}, {})
-        self.assertEqual(len(empty_mapping), 0)
-
-    def testRepositoryMappingGetItem(self) -> None:
-        """Test __getitem__ method - should work for both exact and prefix-based mappings."""
-        repo_mapping = self._createTestRepositoryMapping()
-        
-        # Test exact lookup
-        self.assertEqual(repo_mapping[("", "my_workspace")], "_main")
-        self.assertEqual(repo_mapping[("", "config_lib")], "config_lib~1.0.0")
-        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
-        
-        # Test prefix-based lookup
-        self.assertEqual(repo_mapping[("deps+some_repo", "external_dep")], "external_dep~prefix")
-        
-        # Test that exact takes precedence over prefix
-        self.assertEqual(repo_mapping[("deps+specific_repo", "external_dep")], "external_dep~exact")
-        
-        # Test KeyError for non-existent mapping (neither exact nor prefix match)
-        with self.assertRaises(KeyError):
-            _ = repo_mapping[("nonexistent", "repo")]
-
-    def testRepositoryMappingIter(self) -> None:
-        """Test __iter__ method - should iterate over all mapping keys (exact first, then prefixed)."""
-        repo_mapping = self._createTestRepositoryMapping()
-        
-        keys = list(repo_mapping)
-        self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
-        
-        # Check that all expected keys are present
-        exact_keys = [("", "my_workspace"), ("", "config_lib"), ("deps+specific_repo", "external_dep")]
-        prefixed_keys = [("deps+", "external_dep"), ("test_deps+", "test_lib")]
-        
-        for exact_key in exact_keys:
-            self.assertIn(exact_key, keys)
-        
-        for prefixed_key in prefixed_keys:
-            self.assertIn(prefixed_key, keys)
-        
-        # Verify order: exact keys should come before prefixed keys
-        first_three_keys = keys[:3]
-        last_two_keys = keys[3:]
-        
-        for exact_key in exact_keys:
-            self.assertIn(exact_key, first_three_keys)
-        
-        for prefixed_key in prefixed_keys:
-            self.assertIn(prefixed_key, last_two_keys)
-
-    def testRepositoryMappingContains(self) -> None:
-        """Test __contains__ method (in operator) - should work for both exact and prefix-based mappings."""
-        repo_mapping = self._createTestRepositoryMapping()
+        repo_mapping = _RepositoryMapping(exact_mappings, prefixed_mappings)
         
         # Test exact lookups
-        self.assertTrue(("", "my_workspace") in repo_mapping)
-        self.assertTrue(("deps+specific_repo", "external_dep") in repo_mapping)
-        
-        # Test prefix-based lookups (consistent with __getitem__)
-        self.assertTrue(("deps+some_repo", "external_dep") in repo_mapping)
-        
-        # Test non-existent mapping
-        self.assertFalse(("nonexistent", "repo") in repo_mapping)
-
-    def testRepositoryMappingGet(self) -> None:
-        """Test get() method - should work for both exact and prefix-based mappings."""
-        repo_mapping = self._createTestRepositoryMapping()
-        
-        # Test exact lookups
-        self.assertEqual(repo_mapping.get(("", "my_workspace")), "_main")
-        self.assertEqual(repo_mapping.get(("deps+specific_repo", "external_dep")), "external_dep~exact")
+        self.assertEqual(repo_mapping.lookup("", "my_workspace"), "_main")
+        self.assertEqual(repo_mapping.lookup("", "config_lib"), "config_lib~1.0.0")
+        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
         
         # Test prefix-based lookups
-        self.assertEqual(repo_mapping.get(("deps+some_repo", "external_dep")), "external_dep~prefix")
+        self.assertEqual(repo_mapping.lookup("deps+some_repo", "external_dep"), "external_dep~prefix")
+        self.assertEqual(repo_mapping.lookup("test_deps+another_repo", "test_lib"), "test_lib~2.1.0")
         
-        # Test non-existent mapping
-        self.assertIsNone(repo_mapping.get(("nonexistent", "repo")))
-        self.assertEqual(repo_mapping.get(("nonexistent", "repo"), "default"), "default")
-
-    def testRepositoryMappingKeysValuesItems(self) -> None:
-        """Test keys(), values(), and items() methods (inherited from Mapping)."""
-        repo_mapping = self._createTestRepositoryMapping()
-        
-        # Test items()
-        items = list(repo_mapping.items())
-        self.assertEqual(len(items), 5)  # 3 exact + 2 prefixed
-        self.assertIn((("", "my_workspace"), "_main"), items)
-        self.assertIn((("deps+", "external_dep"), "external_dep~prefix"), items)
-        
-        # Test keys()
-        keys = list(repo_mapping.keys())
-        self.assertEqual(len(keys), 5)  # 3 exact + 2 prefixed
-        self.assertIn(("", "my_workspace"), keys)
-        self.assertIn(("deps+", "external_dep"), keys)
-        
-        # Test values()
-        values = list(repo_mapping.values())
-        self.assertEqual(len(values), 5)  # 3 exact + 2 prefixed
-        self.assertIn("_main", values)
-        self.assertIn("external_dep~prefix", values)
-
-    def testRepositoryMappingTruthiness(self) -> None:
-        """Test pythonic truthiness - non-empty mapping is truthy, empty mapping is falsy."""
-        repo_mapping = self._createTestRepositoryMapping()
+        # Test that exact takes precedence over prefix
+        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
         
-        # Test non-empty mapping is truthy
-        self.assertTrue(repo_mapping)
-        self.assertTrue(bool(repo_mapping))
+        # Test non-existent mapping
+        self.assertIsNone(repo_mapping.lookup("nonexistent", "repo"))
+        self.assertIsNone(repo_mapping.lookup("unknown+repo", "missing"))
         
-        # Test empty mapping is falsy
-        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
+        # Test empty mapping
         empty_mapping = _RepositoryMapping({}, {})
-        self.assertFalse(empty_mapping)
-        self.assertFalse(bool(empty_mapping))
+        self.assertIsNone(empty_mapping.lookup("any", "repo"))
 
     def testCurrentRepository(self) -> None:
         # Under bzlmod, the current repository name is the empty string instead

From f0558a165002f48855433453d6852dc6c11440f0 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 09:36:59 -0400
Subject: [PATCH 15/20] Run precommit

---
 python/runfiles/runfiles.py     | 49 +++++++++++++++++++--------------
 tests/runfiles/runfiles_test.py | 36 ++++++++++++++++--------
 2 files changed, 53 insertions(+), 32 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index c4fa2a2733..c4bc7c35e8 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -23,47 +23,52 @@
 :::
 """
 import collections.abc
-from collections import defaultdict
 import inspect
 import os
 import posixpath
 import sys
+from collections import defaultdict
 from typing import Dict, Optional, Tuple, Union
 
 
 class _RepositoryMapping:
     """Repository mapping for resolving apparent repository names to canonical ones.
-    
-    Handles both exact mappings and prefix-based mappings introduced by the 
+
+    Handles both exact mappings and prefix-based mappings introduced by the
     --incompatible_compact_repo_mapping_manifest flag.
     """
 
     def __init__(
-        self, 
-        exact_mappings: Dict[Tuple[str, str], str], 
-        prefixed_mappings: Dict[Tuple[str, str], str]
+        self,
+        exact_mappings: Dict[Tuple[str, str], str],
+        prefixed_mappings: Dict[Tuple[str, str], str],
     ) -> None:
         """Initialize repository mapping with exact and prefixed mappings.
-        
+
         Args:
             exact_mappings: Dict mapping (source_canonical, target_apparent) -> target_canonical
             prefixed_mappings: Dict mapping (source_prefix, target_apparent) -> target_canonical
         """
         self._exact_mappings = exact_mappings
         self._prefixed_mappings = prefixed_mappings
-        
+
         # Group prefixed mappings by target_apparent for faster lookups
         self._grouped_prefixed_mappings = defaultdict(list)
-        for (prefix_source, target_app), target_canonical in self._prefixed_mappings.items():
-            self._grouped_prefixed_mappings[target_app].append((prefix_source, target_canonical))
+        for (
+            prefix_source,
+            target_app,
+        ), target_canonical in self._prefixed_mappings.items():
+            self._grouped_prefixed_mappings[target_app].append(
+                (prefix_source, target_canonical)
+            )
 
     @staticmethod
     def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
         """Create RepositoryMapping from a repository mapping manifest file.
-        
+
         Args:
             repo_mapping_path: Path to the repository mapping file, or None if not available
-            
+
         Returns:
             RepositoryMapping instance with parsed mappings
         """
@@ -72,7 +77,7 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
         # In this case, just apply empty repo mappings.
         if not repo_mapping_path:
             return _RepositoryMapping({}, {})
-        
+
         try:
             with open(repo_mapping_path, "r", encoding="utf-8", newline="\n") as f:
                 content = f.read()
@@ -95,27 +100,29 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
 
     def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
         """Look up repository mapping for the given source and target.
-        
-        This handles both exact mappings and prefix-based mappings introduced by the 
+
+        This handles both exact mappings and prefix-based mappings introduced by the
         --incompatible_compact_repo_mapping_manifest flag. Exact mappings are tried
         first, followed by prefix-based mappings where order matters.
-        
+
         Args:
             source_repo: Source canonical repository name
             target_apparent: Target apparent repository name
-            
+
         Returns:
             target_canonical repository name, or None if no mapping exists
         """
         key = (source_repo, target_apparent)
-        
+
         # Try exact mapping first
         if key in self._exact_mappings:
             return self._exact_mappings[key]
 
         # Try prefixed mapping if no exact match found
         if target_apparent in self._grouped_prefixed_mappings:
-            for prefix_source, target_canonical in self._grouped_prefixed_mappings[target_apparent]:
+            for prefix_source, target_canonical in self._grouped_prefixed_mappings[
+                target_apparent
+            ]:
                 if source_repo.startswith(prefix_source):
                     return target_canonical
 
@@ -297,7 +304,9 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if source_repo is not None:
             target_canonical = self._repo_mapping.lookup(source_repo, target_repo)
             if target_canonical is not None:
-                return self._strategy.RlocationChecked(target_canonical + "/" + remainder)
+                return self._strategy.RlocationChecked(
+                    target_canonical + "/" + remainder
+                )
 
         # No mapping found - assume target_repo is already canonical or
         # we're not using Bzlmod
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 01af925a9b..d7db527990 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -638,8 +638,10 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
 
     def testRepositoryMappingLookup(self) -> None:
         """Test _RepositoryMapping.lookup() method for both exact and prefix-based mappings."""
-        from python.runfiles.runfiles import _RepositoryMapping  # buildifier: disable=bzl-visibility
-        
+        from python.runfiles.runfiles import (
+            _RepositoryMapping,  # buildifier: disable=bzl-visibility
+        )
+
         exact_mappings = {
             ("", "my_workspace"): "_main",
             ("", "config_lib"): "config_lib~1.0.0",
@@ -649,25 +651,35 @@ def testRepositoryMappingLookup(self) -> None:
             ("deps+", "external_dep"): "external_dep~prefix",
             ("test_deps+", "test_lib"): "test_lib~2.1.0",
         }
-        
+
         repo_mapping = _RepositoryMapping(exact_mappings, prefixed_mappings)
-        
+
         # Test exact lookups
         self.assertEqual(repo_mapping.lookup("", "my_workspace"), "_main")
         self.assertEqual(repo_mapping.lookup("", "config_lib"), "config_lib~1.0.0")
-        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
-        
+        self.assertEqual(
+            repo_mapping.lookup("deps+specific_repo", "external_dep"),
+            "external_dep~exact",
+        )
+
         # Test prefix-based lookups
-        self.assertEqual(repo_mapping.lookup("deps+some_repo", "external_dep"), "external_dep~prefix")
-        self.assertEqual(repo_mapping.lookup("test_deps+another_repo", "test_lib"), "test_lib~2.1.0")
-        
+        self.assertEqual(
+            repo_mapping.lookup("deps+some_repo", "external_dep"), "external_dep~prefix"
+        )
+        self.assertEqual(
+            repo_mapping.lookup("test_deps+another_repo", "test_lib"), "test_lib~2.1.0"
+        )
+
         # Test that exact takes precedence over prefix
-        self.assertEqual(repo_mapping.lookup("deps+specific_repo", "external_dep"), "external_dep~exact")
-        
+        self.assertEqual(
+            repo_mapping.lookup("deps+specific_repo", "external_dep"),
+            "external_dep~exact",
+        )
+
         # Test non-existent mapping
         self.assertIsNone(repo_mapping.lookup("nonexistent", "repo"))
         self.assertIsNone(repo_mapping.lookup("unknown+repo", "missing"))
-        
+
         # Test empty mapping
         empty_mapping = _RepositoryMapping({}, {})
         self.assertIsNone(empty_mapping.lookup("any", "repo"))

From 76e04d05534869a8a9b33fcc7075ac2a58d1efc8 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 09:50:58 -0400
Subject: [PATCH 16/20] Bring back the assertion

---
 python/runfiles/runfiles.py     | 25 +++++++++++++++++++++----
 tests/runfiles/runfiles_test.py |  4 ++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index c4bc7c35e8..f12e52c333 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -129,6 +129,14 @@ def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
         # No mapping found
         return None
 
+    def is_empty(self) -> bool:
+        """Check if this repository mapping is empty (no exact or prefixed mappings).
+
+        Returns:
+            True if there are no mappings, False otherwise
+        """
+        return len(self._exact_mappings) == 0 and len(self._prefixed_mappings) == 0
+
 
 class _ManifestBased:
     """`Runfiles` strategy that parses a runfiles-manifest to look up runfiles."""
@@ -286,7 +294,7 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         if os.path.isabs(path):
             return path
 
-        if source_repo is None and self._repo_mapping is not None:
+        if source_repo is None and not self._repo_mapping.is_empty():
             # Look up runfiles using the repository mapping of the caller of the
             # current method. If the repo mapping is empty, determining this
             # name is not necessary.
@@ -295,11 +303,20 @@ def Rlocation(self, path: str, source_repo: Optional[str] = None) -> Optional[st
         # Split off the first path component, which contains the repository
         # name (apparent or canonical).
         target_repo, _, remainder = path.partition("/")
-        if not remainder:
-            # path did not contain a slash and referred to a root symlink,
-            # which should not be mapped.
+        if not remainder or self._repo_mapping.lookup(source_repo, target_repo) is None:
+            # One of the following is the case:
+            # - not using Bzlmod, so the repository mapping is empty and
+            #   apparent and canonical repository names are the same
+            # - target_repo is already a canonical repository name and does not
+            #   have to be mapped.
+            # - path did not contain a slash and referred to a root symlink,
+            #   which also should not be mapped.
             return self._strategy.RlocationChecked(path)
 
+        assert (
+            source_repo is not None
+        ), "BUG: if the `source_repo` is None, we should never go past the `if` statement above"
+
         # Look up the target repository using the repository mapping
         if source_repo is not None:
             target_canonical = self._repo_mapping.lookup(source_repo, target_repo)
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index d7db527990..4db8193d9f 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -683,6 +683,10 @@ def testRepositoryMappingLookup(self) -> None:
         # Test empty mapping
         empty_mapping = _RepositoryMapping({}, {})
         self.assertIsNone(empty_mapping.lookup("any", "repo"))
+        
+        # Test is_empty() method
+        self.assertFalse(repo_mapping.is_empty())  # Should have mappings
+        self.assertTrue(empty_mapping.is_empty())  # Should be empty
 
     def testCurrentRepository(self) -> None:
         # Under bzlmod, the current repository name is the empty string instead

From ba410b7fce363afb8ee754c7233f2be2ea71d574 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 10:02:34 -0400
Subject: [PATCH 17/20] Address mypy error

---
 python/runfiles/runfiles.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index f12e52c333..15459d5cd0 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -98,7 +98,7 @@ def create_from_file(repo_mapping_path: Optional[str]) -> "_RepositoryMapping":
 
         return _RepositoryMapping(exact_mappings, prefixed_mappings)
 
-    def lookup(self, source_repo: str, target_apparent: str) -> Optional[str]:
+    def lookup(self, source_repo: Optional[str], target_apparent: str) -> Optional[str]:
         """Look up repository mapping for the given source and target.
 
         This handles both exact mappings and prefix-based mappings introduced by the

From 48ff63fa7fe5615a45d1f7c8c467340844307800 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 10:28:30 -0400
Subject: [PATCH 18/20] Additional type fix

---
 python/runfiles/runfiles.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index 15459d5cd0..847d9cbfd7 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -112,6 +112,9 @@ def lookup(self, source_repo: Optional[str], target_apparent: str) -> Optional[s
         Returns:
             target_canonical repository name, or None if no mapping exists
         """
+        if source_repo is None:
+            return None
+
         key = (source_repo, target_apparent)
 
         # Try exact mapping first

From 13c756ca53ea41026f536ca56afdc9af9583f274 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 14:48:33 -0400
Subject: [PATCH 19/20] Remove bzl-visibility comment

---
 tests/runfiles/runfiles_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 4db8193d9f..9d8fad9e50 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -639,7 +639,7 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
     def testRepositoryMappingLookup(self) -> None:
         """Test _RepositoryMapping.lookup() method for both exact and prefix-based mappings."""
         from python.runfiles.runfiles import (
-            _RepositoryMapping,  # buildifier: disable=bzl-visibility
+            _RepositoryMapping,
         )
 
         exact_mappings = {

From 8f9b0f78e364874a3ac36d5a9c86cc9c5a6da218 Mon Sep 17 00:00:00 2001
From: Klukas <jklukas@netflix.com>
Date: Mon, 22 Sep 2025 14:52:56 -0400
Subject: [PATCH 20/20] Move import to top

---
 tests/runfiles/runfiles_test.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index 9d8fad9e50..b8a3d5f7b7 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -18,6 +18,7 @@
 from typing import Any, List, Optional
 
 from python.runfiles import runfiles
+from python.runfiles.runfiles import _RepositoryMapping
 
 
 class RunfilesTest(unittest.TestCase):
@@ -638,10 +639,6 @@ def testDirectoryBasedRlocationWithCompactRepoMappingOrderMatters(self) -> None:
 
     def testRepositoryMappingLookup(self) -> None:
         """Test _RepositoryMapping.lookup() method for both exact and prefix-based mappings."""
-        from python.runfiles.runfiles import (
-            _RepositoryMapping,
-        )
-
         exact_mappings = {
             ("", "my_workspace"): "_main",
             ("", "config_lib"): "config_lib~1.0.0",
@@ -683,7 +680,7 @@ def testRepositoryMappingLookup(self) -> None:
         # Test empty mapping
         empty_mapping = _RepositoryMapping({}, {})
         self.assertIsNone(empty_mapping.lookup("any", "repo"))
-        
+
         # Test is_empty() method
         self.assertFalse(repo_mapping.is_empty())  # Should have mappings
         self.assertTrue(empty_mapping.is_empty())  # Should be empty