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

x/build: run platform/arch specific builders when platform/arch constrained files are changed #71396

Open
rolandshoemaker opened this issue Jan 22, 2025 · 2 comments
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rolandshoemaker
Copy link
Member

When making changes to platform or arch constrained files (.go/.s) it would be nice for LUCI to automatically include builders of those types in the set that is run. Currently this has to be done manually, which is not always particularly obvious, and can lead to changes appearing to pass builders when they would fail in the specific builders were included.

This doesn't seem to be easily done with location filters regexps in the luci main.star, since we want to be more specific when there is a platform suffix (e.g. if example_linux_amd64.go changes, we only want to run the linux-amd64 builder), and less specific when there is only an arch suffix (e.g. if example_amd64.go changes, we want to run all of the amd64 builders). I fiddled around with this and couldn't find an obvious way to accomplish it.

One possible option is to add a special mode to golangbuild which scans the file names changed in a CL, and does this matching itself, spawning an additional set of builders.

Ideally we'd also do this for build constrained files which use the //go:build syntax, but this seems a lot more complex, as it's not particularly easy to extract this information via the gerrit API (as far as I can tell). For a first pass, it'd probably be fine to just do the file name based approach, since that covers the majority of use cases.

cc @golang/release

@rolandshoemaker rolandshoemaker added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 22, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 22, 2025
@rolandshoemaker
Copy link
Member Author

rolandshoemaker commented Jan 22, 2025

One note, when I say "we want to run all of the amd64 builders" for the example_amd64.go case, we may actually want to only run a slightly expanded subset over the defaults, but where to strike the balance between testing things haven't broken, and reducing the cost of testing a CL, is hard to determine.

@ianlancetaylor ianlancetaylor changed the title x/build: run platform/arch specific builders when platform/arc constrained files are changed x/build: run platform/arch specific builders when platform/arch constrained files are changed Jan 22, 2025
@dmitshur
Copy link
Contributor

To connect some dots: in #66009, @mknyszek has a paragraph that mentions a "proxy" builder, which is similar (if not identical) to the "spawning an additional set of builders" description here.

At a high level, this issue sounds like "partially automate SlowBot requests". One important consideration of doing SlowBot selection automatically is deciding whether the change warrants potentially waiting a long time for a low-capacity (or possibly missing) slowbot, or whether it should only consider high-capacity builders in scope.

One more consideration: depending on whether this is intended for CLs we're mostly sending ourselves (and we can rely more on client-side-only helpers), or more broadly any CL anyone makes (where we can only rely on server-side checks), another option to consider might be something in git-codereview's pre-commit hooks. That hook is already written in Go, so it could look at the .go files being modified and compute a Cq-Include-Trybots footer to add next to the Change-Id footer it already adds. This has some upsides like being able to modify the Cq-Include-Trybots as part of the code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Status: No status
Development

No branches or pull requests

3 participants