fix: spill module mapping args to a file#2644
Merged
aignas merged 1 commit intobazel-contrib:mainfrom Mar 3, 2025
Merged
Conversation
mattem
force-pushed
the
modules_mapping_perf
branch
2 times, most recently
from
d69abcd to
17313cc
Compare
aignas
reviewed
Mar 3, 2025
Collaborator
aignas
left a comment
aignas
left a comment
There was a problem hiding this comment.
In general LGTM, but would love to have some explanation of how things are wired. It took me a few minutes to look at it and google official docs to understand how it is actually wired.
mattem
force-pushed
the
modules_mapping_perf
branch
from
17313cc to
3919282
Compare
mattem
force-pushed
the
modules_mapping_perf
branch
from
3919282 to
15b5cce
Compare
dcode
pushed a commit
to dcode/rules_python
that referenced
this pull request
Mar 3, 2025
Calls to the modules mapping rule contains very long command line args due to the use of the full `wheels` parameter. This change adds support for spilling the args into a file as needed. In addition, it improves the performance of the `modules_mapping` rule: * Remove the calls `to_list` that are unnecessary on the depset. * Remove the iteration over the depset when passing to `args`, and other calls to `.path`, and instead let args do this lazily.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Calls to the modules mapping rule contains very long command line args due to the use of the full
wheelsparameter. This change adds support for spilling the args into a file as needed.In addition, it improves the performance of the
modules_mappingrule:to_listthat are unnecessary on the depset.args, and other calls to.path, and instead let args do this lazily.