Skip to content
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: building withing lower parallelism than dependency group size #2051

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

wesbillman
Copy link
Collaborator

Fixes #2042

Before this change, we would get build errors because the go.work file for a given module would include all the built modules including modules that were in the same build group layer as a given module.

For example, if we had 4 modules all in the same group [a, b, c, d] and we build with a -j2 option (2 parallel builds), we would run into an issue where a and b would build first, in parallel. a would finish and we would start to build c. But, when we looked up the schema to get shared modules for the go.work file for c, that list would now include a even though they are in the same build group and should never depend on each other.

This fix changes the build func to not re-lookup built modules and instead just use the set of builtModules that were available at the start of the build group.

With this change we should be able to safely support any -j option higher or lower than the number of modules. This probably would have eventually happened either way once we had a system with more modules than the number of cpus on a given machine.

@wesbillman wesbillman requested a review from alecthomas as a code owner July 11, 2024 16:29
@wesbillman wesbillman requested review from a team and worstell and removed request for a team July 11, 2024 16:29
@ftl-robot ftl-robot mentioned this pull request Jul 11, 2024
@wesbillman wesbillman merged commit d16ef79 into main Jul 11, 2024
51 checks passed
@wesbillman wesbillman deleted the fix-running-buildengine-with-low-parallelism branch July 11, 2024 16:36
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.

-j2 option breaking builds
2 participants