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.

Sign up for GitHub

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

Jump to bottom

x/build: builder issues bringing up new ports #61925

Closed
4a6f656c opened this issue Aug 10, 2023 · 10 comments
Closed

x/build: builder issues bringing up new ports #61925

4a6f656c opened this issue Aug 10, 2023 · 10 comments
Assignees
@dmitshur
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@4a6f656c
Copy link
Contributor

The openbsd/ppc64 port is being merged (per #56001) - however, CL 482115 (d0099ef) marked the port as "broken" since it is incomplete. This has a flow on effect that the builder is now effectively useless - regular build runs now immediately fail with:

go tool dist: build stopped because the port openbsd/ppc64 is marked as broken

Use the -force flag to build anyway.

https://build.golang.org/log/1022f466e1aa71c4dbb2b23702588ecd39d50594

And attempting to use it via TRY=openbsd-ppc64 results in the same thing:

https://go-review.googlesource.com/c/go/+/475638/15
https://storage.googleapis.com/go-build-log/989e94bc/openbsd-ppc64-n2vi_89268ab2.log

The only way to get the builder to actually run, appears to be to run it with a CL that removes broken for the port.

This largely defeats the point of having a builder online prior to a merge starting - while it has been normal to mark the builder has having known issues, in the past one has been able to identify what state the port is in (i.e. where it is currently failing) based on the normal https://build.golang.org dashboard. Likewise, testing an actual work-in-progress port and ensuring that a change works as expected (i.e. progresses) on the port builder has been possible in the past.

I'm not sure what is expected here - are we able to remove broken for in progress ports? Or should the dashboard pass -force unconditionally so that we are actually able to use the build dashboard and try bots?
@4a6f656c
Copy link
Contributor Author

cc @ianlancetaylor @heschi

@heschi
Copy link
Contributor

heschi commented Aug 10, 2023

I marked them broken because the release process is now packaging all ports regardless of whether they pass tests, and I used broken-ness to indicate that we shouldn't try. From my perspective there's basically no difference between a port that simply fails to compile and one that fails because it's marked broken, which is why I didn't think too hard about doing it.

I think you should feel free to mark them unbroken as soon as they compile. (I assume you don't need the trybots to tell you when that is, but maybe I'm wrong?)

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 10, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 10, 2023
@mknyszek mknyszek changed the title all: builder issues bringing up new ports x/build: builder issues bringing up new ports Aug 10, 2023
@mknyszek mknyszek added the Builders x/build issues (builders, bots, dashboards) label Aug 10, 2023
@mknyszek mknyszek modified the milestones: Backlog, Unreleased Aug 10, 2023
@ianlancetaylor
Copy link
Contributor

I'm inclined to think that the builders should always pass -force.

@4a6f656c
Copy link
Contributor Author

I marked them broken because the release process is now packaging all ports regardless of whether they pass tests, and I used broken-ness to indicate that we shouldn't try. From my perspective there's basically no difference between a port that simply fails to compile and one that fails because it's marked broken, which is why I didn't think too hard about doing it.

Understood. And I tend to agree with that.

I think you should feel free to mark them unbroken as soon as they compile. (I assume you don't need the trybots to tell you when that is, but maybe I'm wrong?)

The issue here is the lack of data from the dashboard and trybots - for example, compare:

https://build.golang.org/log/c8c6cb7587ac74600879a5799d7fb2537b88f54a

to:

https://build.golang.org/log/8582a472e399d242790b395b8bdf2a0b6a6ba7f9

The openbsd/ppc64 port is further along, however there's no way to identify that. There can also be a large number of changes prior to a port compiling (especially when bringing up a new architecture such as linux/riscv64). If you're reviewing changes, you're also not able to tell if they actually compile, etc. Another case is when issues are showing up under the builder/trybot, but not when you run it on a different environment.

@4a6f656c
Copy link
Contributor Author

I'm inclined to think that the builders should always pass -force.

Thanks, this seems to be the best way forward. I think we may be able to add this to makeScriptArgs and allScriptArgs for the specific builders...

@gopherbot
Copy link

Change https://go.dev/cl/518655 mentions this issue: dashboard: add -force to script invocations for broken/incomplete ports

@dmitshur
Copy link
Contributor

I think the general expectation is that the broken flag is removed at the top of a CL stack to get a port working, and the builders begin to work then. If it is needed to see builder results before that point, adding the -broken flag to its configuration, along with marking it with a known issue, makes sense.

gopherbot pushed a commit to golang/build that referenced this issue Aug 13, 2023
@4a6f656c
dashboard: add -force to script invocations for broken/incomplete ports
1054d53 
This should allow the dashboard and trybots to still provide useful
results. Also add known issues for openbsd/mips64.

Updates golang/go#58110
Updates golang/go#61925

Change-Id: I102f4e913cddc38dc617972d1da8cc291cda0a0b
Reviewed-on: https://go-review.googlesource.com/c/build/+/518655
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/521356 mentions this issue: dashboard: always include -force in all and make script arguments

@dmitshur
Copy link
Contributor

dmitshur commented Oct 3, 2023

@4a6f656c Friendly ping (trying on GitHub too) on CL 521356. Otherwise I can try to carry it forward next week. Thanks.

@dmitshur dmitshur self-assigned this Oct 10, 2023
@gopherbot
Copy link

Change https://go.dev/cl/535395 mentions this issue: internal/buildgo: always prepend -force to make script arguments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitshur dmitshur

Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@mknyszek @dmitshur @ianlancetaylor @4a6f656c @gopherbot @heschi