Skip to content

Conversation

@dhellmann
Copy link
Member

Change the way we determine whether a package is ready to be built in
build-parallel so that we build its installation dependencies before we
build it.

Logically it should not be possible for package A installation
dependencies to depend in any way on A, so this ordering should be
safe. The change ensures that when we mark A as built, it is actually
ready to be consumed in build environments for other packages.

Addresses #755

Change the way we determine whether a package is ready to be built in
build-parallel so that we build its installation dependencies before we
build it.

Logically it should not be possible for package A installation
dependencies to depend in any way on A, so this ordering should be
safe. The change ensures that when we mark A as built, it is actually
ready to be consumed in build environments for other packages.

Addresses python-wheel-build#755

Signed-off-by: Doug Hellmann <[email protected]>
@dhellmann dhellmann requested a review from a team as a code owner September 20, 2025 17:22
@dhellmann dhellmann requested a review from tiran September 20, 2025 17:22
@dhellmann
Copy link
Member Author

@tiran This is a simpler fix than the rewrite in #759 and given where we are in the release schedule downstream I'm more comfortable trying a small change like this now and coming back to the rewrite later. What do you think?

@dhellmann dhellmann marked this pull request as draft September 20, 2025 18:04
@dhellmann
Copy link
Member Author

I think the assumption about dependencies is not accurate. I have a graph file for vLLM that can be built in sequence but not with this change.
graph.json

@dhellmann
Copy link
Member Author

I've tested the latest graph for the original collection and I am able to build it all from scratch when I force. I can then re-run the build with a fully populated cache and it works. Finally, I can clear the local cache and use the partially filled cache server and that works.

I'm testing another complex graph in the same way but those builds take longer.

@dhellmann dhellmann marked this pull request as ready for review September 20, 2025 21:06
Copy link
Collaborator

@tiran tiran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doug and I discussed the changes today.

We agree that this change is fixing the build issues we were seeing in downstream. It's not the optimal solution for the problem. Parallel build is now waiting for all dependencies of a package to become ready before it starts building a package. The optimal solution would only wait until all build dependencies and their install dependencies are ready.

This is the simplest fix that resolves the bug and gives reasonable performance. It's better than serialized builds for sure.

@mergify mergify bot merged commit 712a0a9 into python-wheel-build:main Sep 23, 2025
111 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants