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

Go: Fix build scripts running for each workspace #16704

Merged
merged 5 commits into from
Jun 11, 2024

Conversation

mbg
Copy link
Member

@mbg mbg commented Jun 7, 2024

Summary

This PR fixes a bug in the Go autobuilder which we became aware of in #16448: if a repository contains multiple logical workspaces and a build script (such as a Makefile), then that build script is erroneously executed for each logical workspace.

The fix is mainly to only execute build scripts once, but this PR also adjusts some other, related parts of the autobuild process to ensure that they run for the correct workspace, rather than in the root of the repository.

Evaluation

I have a run a QA experiment for this PR which shows only one regression, nine progressions, and various timing improvements that we would expect to see from not running build scripts multiple times.

Further, I have tested the repository linked to in the above issue with this fix and can confirm that the build time has improved considerably while still successfully extracting all 49 modules.

@mbg mbg self-assigned this Jun 7, 2024
@github-actions github-actions bot added the Go label Jun 7, 2024
@mbg mbg changed the base branch from main to mbg/go/improve-version-selection-v2 June 7, 2024 12:34
@mbg mbg force-pushed the mbg/go/fix/build-scripts-running-more-than-once branch from 2f5441b to e7a60b7 Compare June 7, 2024 16:22
Base automatically changed from mbg/go/improve-version-selection-v2 to main June 10, 2024 16:03
@mbg mbg marked this pull request as ready for review June 11, 2024 16:41
@mbg mbg requested a review from a team as a code owner June 11, 2024 16:41
@mbg mbg force-pushed the mbg/go/fix/build-scripts-running-more-than-once branch from 5c1f68c to e7a60b7 Compare June 11, 2024 17:08
Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but might want @smowton to review it as well.

Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

LGTM

@mbg mbg merged commit e9bd85e into main Jun 11, 2024
13 checks passed
@mbg mbg deleted the mbg/go/fix/build-scripts-running-more-than-once branch June 11, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants