feat(runfiles): support for --incompatible_compact_repo_mapping_manifest #3277

jklukas · 2025-09-18T20:58:17Z

Under bzlmod, the repo mapping can become quite large (i.e. tens of megabytes) because its size scales as a factor of the number of repos in the transitive dependencies.

To address this, the --incompatible_compact_repo_mapping_manifest flag was introduced. This changes the repo mapping formation to use prefixes (instead of exact repo names) for mapping things.

To make this work with the runfiles library, the code has to be updated to handle these prefixes instead of just exact strings.

gemini-code-assist · 2025-09-18T20:58:32Z

Summary of Changes

Hello @jklukas, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates support for Bazel's --incompatible_compact_repo_mapping_manifest flag into the Python runfiles library. This is a critical enhancement for Bazel users leveraging bzlmod, as it addresses the issue of excessively large repository mapping manifests by introducing prefix-based mapping resolution. The change ensures that the runfiles library can correctly locate files even when the repository mapping is in its compact, memory-optimized format, thereby improving the efficiency and scalability of Bazel builds with extensive dependency trees.

Highlights

Compact Repository Mapping Support: The Python runfiles library now supports Bazel's --incompatible_compact_repo_mapping_manifest flag.
Memory Optimization: This feature enables prefix-based repository mappings, significantly reducing the size of the repo mapping manifest (from tens of megabytes to much less) for large bzlmod dependency graphs.
New _RepositoryMapping Class: A new internal class _RepositoryMapping was introduced to intelligently parse and resolve both exact and prefix-based repository mappings, ensuring correct runfiles resolution.
Enhanced Rlocation Method: The Rlocation method in the Runfiles class has been updated to utilize the new _RepositoryMapping logic, handling the complexities of prefix-based lookups with proper precedence.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

jklukas · 2025-09-18T20:59:03Z

cc @fmeum for review

gemini-code-assist

Code Review

This pull request adds support for Bazel's --incompatible_compact_repo_mapping_manifest flag, which is a great feature for improving performance with bzlmod. The implementation is solid and includes a comprehensive set of tests. My review includes suggestions for a performance optimization in the new mapping logic, a fix for an outdated docstring, and some refactoring opportunities in both the implementation and the tests to improve maintainability and readability.

python/runfiles/runfiles.py

tests/runfiles/runfiles_test.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

jklukas · 2025-09-18T23:27:27Z

python/runfiles/runfiles.py


-        assert (
-            source_repo is not None
-        ), "BUG: if the `source_repo` is None, we should never go past the `if` statement above"


TODO: This assertion may still be important. I need to reassess above whether we need to match the previous conditional.

I don't think an equivalent assertion is needed with the new logic, but I'm open to more thoughts on it.

Please keep the assert. I'm not entirely sure if we have test coverage of all the cases.

Looking at the code, I don't see why the comment wouldn't still apply. source_repo can end up None here if the repo mapping is empty. (which, according to comments, occurs for workspace mode)

rickeylev

I was going to push some changes, but noticed that "maintainer can modify" isn't set.

Mostly LGTM, but please remove all the Mapping API compatibility code. As an internal class, there's no need for it to support the wide range of dict operations.

CHANGELOG.md

python/runfiles/runfiles.py

rickeylev · 2025-09-20T21:39:49Z

python/runfiles/runfiles.py

+        return _RepositoryMapping(exact_mappings, prefixed_mappings)
+
+    # Mapping protocol implementation
+    def __getitem__(self, key: Tuple[str, str]) -> str:


Please rename this to e.g. get_canonical or similar, just not __getitem__. This is an internal custom class, so there's not need to add the abstraction or complications of making it look dict-like.

python/runfiles/runfiles.py

tests/runfiles/runfiles_test.py

rickeylev · 2025-09-20T22:06:17Z

python/runfiles/runfiles.py


-        assert (
-            source_repo is not None
-        ), "BUG: if the `source_repo` is None, we should never go past the `if` statement above"


Please keep the assert. I'm not entirely sure if we have test coverage of all the cases.

Looking at the code, I don't see why the comment wouldn't still apply. source_repo can end up None here if the repo mapping is empty. (which, according to comments, occurs for workspace mode)

Co-authored-by: Richard Levasseur <[email protected]>

jklukas · 2025-09-22T13:54:40Z

Thank you for the quick review! I believe all comments are now addressed. I removed the Mapping compat; now we just have a lookup method and an is_empty method as needed for the rest of the logic.

jklukas · 2025-09-22T18:54:14Z

/gemini review

gemini-code-assist

Code Review

This pull request adds support for Bazel's --incompatible_compact_repo_mapping_manifest flag, which uses prefix-based repository mappings. The implementation introduces a _RepositoryMapping class to handle both exact and new prefix-based mappings, which is a clean way to encapsulate the logic. The accompanying tests are thorough and cover various scenarios including precedence and ordering.

I've found a couple of areas for improvement. First, in Rlocation, there's an opportunity to refactor the logic to avoid a redundant lookup and remove some unreachable code, making it more efficient and maintainable. Second, in _RepositoryMapping.create_from_file, adding a check for empty lines in the manifest file would make the parsing more robust.

Overall, this is a great addition that addresses an important performance issue with large dependency graphs in Bazel.

gemini-code-assist · 2025-09-22T18:57:36Z

python/runfiles/runfiles.py

+        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
+                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
+


The loop here doesn't account for empty or whitespace-only lines that content.splitlines() might produce. This could lead to a ValueError when trying to unpack the result of line.split(',') on the next line. It's safer to strip each line and skip it if it's empty.

Suggested change

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

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

for line in content.splitlines():

line = line.strip()

if not line:

continue

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

It may be best to error out anyway if repo mappings diverged from expected format in the ways described here.

jklukas added 4 commits September 18, 2025 15:49

Support compact repo mapping format

2759525

Refactor into _RepositoryMapping

590637f

More fully pythonic mapping

f471b7f

Add changelog

ab34ab7

jklukas requested review from rickeylev and aignas as code owners September 18, 2025 20:58

jklukas mentioned this pull request Sep 18, 2025

Support incompatible_compact_repo_mapping_manifest in runfiles library #3022

Open

gemini-code-assist bot reviewed Sep 18, 2025

View reviewed changes

jklukas and others added 4 commits September 18, 2025 17:09

Update python/runfiles/runfiles.py

7009cef

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Refactor mapping tests

0ab9cf6

Use splitlines

0497f17

grouped prefix optimization

268f00b

jklukas commented Sep 18, 2025

View reviewed changes

jklukas added 3 commits September 19, 2025 07:40

Fix python 3.8.20 type issue

79b36c5

Remove unused imports

fe77b24

Avoid KeyError

c4b89d6

rickeylev requested changes Sep 20, 2025

View reviewed changes

jklukas and others added 5 commits September 22, 2025 09:25

Update CHANGELOG.md

ffda0af

Co-authored-by: Richard Levasseur <[email protected]>

Remove TODO

0e14bca

Remove Mapping compat

ca06786

Run precommit

f0558a1

Bring back the assertion

76e04d0

jklukas requested a review from rickeylev September 22, 2025 13:53

jklukas added 3 commits September 22, 2025 09:54

Merge branch 'main' into feat/runfiles-compact-repo-manifest

b18cd54

Address mypy error

ba410b7

Additional type fix

48ff63f

jklukas added 2 commits September 22, 2025 14:48

Remove bzl-visibility comment

13c756c

Move import to top

8f9b0f7

gemini-code-assist bot reviewed Sep 22, 2025

View reviewed changes

jklukas added 2 commits September 23, 2025 08:58

Merge branch 'main' into feat/runfiles-compact-repo-manifest

7491694

Merge branch 'main' into feat/runfiles-compact-repo-manifest

f158d87

Uh oh!

feat(runfiles): support for --incompatible_compact_repo_mapping_manifest #3277

Are you sure you want to change the base?

feat(runfiles): support for --incompatible_compact_repo_mapping_manifest #3277

Uh oh!

Conversation

jklukas commented Sep 18, 2025

Uh oh!

gemini-code-assist bot commented Sep 18, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

jklukas commented Sep 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jklukas Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

jklukas Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

rickeylev Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

jklukas Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

rickeylev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rickeylev Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rickeylev Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

jklukas commented Sep 22, 2025

Uh oh!

jklukas commented Sep 22, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

jklukas Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!