-
Couldn't load subscription status.
- Fork 31
fix: fix build-parallel to include installation dependencies #778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix build-parallel to include installation dependencies #778
Conversation
af9ce5c to
0b62fa8
Compare
build-parallel needs to build the installation dependencies of a package before declaring that package as ready. This change uses graphlib to build a toplogical sorter for the dependency graph and then works through the iterations of ready nodes. For each set of ready nodes, any exclusive nodes are built one at a time and then all of the remaining ready nodes are built together in parallel. Fixes python-wheel-build#755 Signed-off-by: Doug Hellmann <[email protected]>
0b62fa8 to
63d853e
Compare
| sorter: graphlib.TopologicalSorter[DependencyNode] = ( | ||
| graphlib.TopologicalSorter() | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this give correct values at all? Graphlib requires hashable input type. The version of DependencyNode in this PR does neither implement __eq__ nor __hash__. My PR #763 changes the classes to frozen, hashable dataclasses to get correct behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have split off the dataclass changes from PR 763 and created #780 .
| logger.info( | ||
| f"{node.key}: requires exclusive build, running it alone" | ||
| ) | ||
| _build_some_nodes([node]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about you call _build_parallel() here? You don't need the pool worker to build a single node in serial.
| # A node can be built when all of its build dependencies are built | ||
| entries: list[BuildSequenceEntry] = [] | ||
| # invalidate uv cache | ||
| wkctx.uv_clean_cache(*reqs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uv has published a new version that supports concurrent use of uv cache clean and uv pip install. With the new version we can move the cleanup into the build wheel process again and simplify this code.
|
We merged #771 and will do a more complete rewrite of this logic when we are not in a critical release period downstream. |
build-parallel needs to build the installation dependencies of a package before declaring that package as ready. This change uses graphlib to build a toplogical sorter for the dependency graph and then works through the iterations of ready nodes. For each set of ready nodes, any exclusive nodes are built one at a time and then all of the remaining ready nodes are built together in parallel.
Fixes #755