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: add builder for ios/arm64 #41610

Closed
dmitshur opened this issue Sep 24, 2020 · 12 comments
Closed

x/build: add builder for ios/arm64 #41610

dmitshur opened this issue Sep 24, 2020 · 12 comments

Comments

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 24, 2020

CL 254740 has added GOOS=ios to the output of go tool dist list. TestTryBotsCompileAllPorts in x/build/dashobard is failing because we don't yet have a builder (real or misc-compile one) covering the new ios/arm64 pair.

This is the tracking issue to add a builder. We may want to repurpose or clone the existing darwin-arm64-corellium builder for this. Need to think about how to best make this work for Go 1.16 and Go 1.15+1.14 (or maybe we don't try to support 1.15+1.14 because iOS is an experimental port).

/cc @cagedmantis @toothrot @cherrymui

@dmitshur dmitshur changed the title x/build: x/build: add builder for ios/arm64 Sep 24, 2020
@gopherbot gopherbot added this to the Unreleased milestone Sep 24, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 24, 2020

Change https://golang.org/cl/257117 mentions this issue: dashboard: skip known missing ios/arm64 builder

gopherbot pushed a commit to golang/build that referenced this issue Sep 24, 2020
We are aware that a builder is missing, and have the tracking issue
golang.org/issue/41610 for resolving that. Add a skip to the tests
so that the build dashboard is green until the work of adding the
builder is complete.

For golang/go#41610.

Change-Id: I69dbeb4c28d5567f24b4570f44715d8b5443365f
Reviewed-on: https://go-review.googlesource.com/c/build/+/257117
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 25, 2020

Change https://golang.org/cl/257617 mentions this issue: dashboard: create misc-compile-ios builder

gopherbot pushed a commit to golang/build that referenced this issue Sep 25, 2020
Create the builder as a post-submit builder with a non-zero KnownIssue
value first. Once we know it works well, we'll have the confidence to
be able to safely promote it to a normal TryBot, without the risk of
causing disruption for contributors.

For golang/go#41610.

Change-Id: If871addf325b7728109afbe094b72d40e27ceec1
Reviewed-on: https://go-review.googlesource.com/c/build/+/257617
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 28, 2020

Change https://golang.org/cl/258057 mentions this issue: dashboard: remove misc-compile-ios builder

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 28, 2020

Change https://golang.org/cl/257962 mentions this issue: src/buildall.bash: remove mobile filter

gopherbot pushed a commit to golang/build that referenced this issue Sep 28, 2020
Since sending CL 257617, I've learned that iOS was never a supported
target for misc-compile. It was always filtered out unconditionally
in GOROOT/src/buildall.bash, and the comment suggesting that we test
iOS targets was inaccurate.

Remove the naive misc-compile-ios builder since it's not expected to
start working soon, and update misc-builder architecture filtering
code and comments to match the current reality better.

After this change, it will be viable to remove mobile target filtering
from GOROOT/src/buildall.bash. If that's done, then x/build/dashboard
will become a centralized place for all misc-builder target selection.

Ignore darwin-arm in TestTryBotsCompileAllPorts so the test doesn't
fail when executed with Go 1.14. Not having that builder is intended.

For golang/go#41610.
Updates golang/go#25963.

Change-Id: Ic6f5dc95edb67f8e6ebc0a77979e55eb0cc325cb
Reviewed-on: https://go-review.googlesource.com/c/build/+/258057
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@dmitshur
Copy link
Member Author

@dmitshur dmitshur commented Sep 28, 2020

We may want to repurpose or clone the existing darwin-arm64-corellium builder for this.

This seems to be our best bet so far. @eliasnaur Are you able to assist with this transition?

@steeve Following up on your offer at #40523 (comment), are you okay with giving access to the the Corellium dashboard to 3 people on the Go team (myself, @cagedmantis and @toothrot)?

Need to think about how to best make this work for Go 1.16 and Go 1.15+1.14 (or maybe we don't try to support 1.15+1.14 because iOS is an experimental port).

The current build policy for the darwin-arm64-corellium builder is to build only the master branch on the main Go repository. So, we don't need to do anything to keep 1.15 and 1.14 working since they're out of scope.

Ideally we should modify the host and builder names to use the new GOOS value for iOS. However, changing the host name means a new builder key will become required. @eliasnaur Does supplying a different builder key require rebuilding device snapshots? If it's a lot of trouble, maybe we can keep the host name as is and rename the builder only.

@steeve
Copy link
Contributor

@steeve steeve commented Sep 29, 2020

@dmitshur Absolutely. I sent invites to you all on your golang.org addresses.

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Sep 29, 2020

We may want to repurpose or clone the existing darwin-arm64-corellium builder for this.

This seems to be our best bet so far. @eliasnaur Are you able to assist with this transition?

Sure, let me know what you want me to do. Have you seen https://github.com/golang/build/tree/master/env/corellium, in particular the ios directory?

I believe there are two iOS builders at the moment, which means you should be able to experiment with the other. It may be enough to scp an updated https://github.com/golang/build/blob/master/env/corellium/ios/files/builder.sh and updated builder key (see https://github.com/golang/build/blob/da0cd803ac9a4447a7ef245838cc6ac90b012a4c/env/corellium/ios/install.sh#L33).

@dmitshur
Copy link
Member Author

@dmitshur dmitshur commented Oct 2, 2020

@steeve Thanks for the answer and sorry that my comment was possibly misleading. There are more steps we need to take on our side before we'd be able to offer help with directly managing the dashboard, and I don't expect that will be resolved quickly.

@eliasnaur I should be able to help review CLs for x/build if you're open to take on this work. It's no problem and completely understandable if you don't have the bandwidth to get to this.

@steeve
Copy link
Contributor

@steeve steeve commented Oct 2, 2020

@steeve Thanks for the answer and sorry that my comment was possibly misleading. There are more steps we need to take on our side before we'd be able to offer help with directly managing the dashboard, and I don't expect that will be resolved quickly.

No worries, let me know once you folks are ready.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 3, 2020

Change https://golang.org/cl/259358 mentions this issue: dashboard,env: move iOS builders to GOOS=ios

gopherbot pushed a commit to golang/build that referenced this issue Oct 5, 2020
Updates golang/go#41610

Change-Id: Ie8cb3294da5566e09fc06c01e679b8b773a439cf
Reviewed-on: https://go-review.googlesource.com/c/build/+/259358
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Oct 5, 2020
Mobile targets are not supported by misc-compile trybots, as tracked in
golang.org/issue/25963, and need to be filtered out. The buildall.bash
script was created in CL 9438, back when it was a single all-compile
builder, and it was easier to filter out mobile targets in the script
than to come up with a pattern that matches all non-mobile targets.

As of CL 254740, all mobile targets (Android and iOS) have unique GOOS
values. That makes it it easy to filter them out in x/build/dashboard.
This was done in CL 258057. As a result, it's now viable to simplify
this script and perform all misc-compile target selection in x/build,
rather than having it spread it across two places.

Also, as of CL 10750, the all-compile builder has turned into multiple
misc-compile builders, so update the script description accordingly.

Updates #41610.

Change-Id: I1e33260ac18cf0a70bb68cd8e3db5587100c7e87
Reviewed-on: https://go-review.googlesource.com/c/go/+/257962
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 6, 2020

Change https://golang.org/cl/259579 mentions this issue: dashboard: remove known issue for iOS builders

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 6, 2020

Change https://golang.org/cl/259580 mentions this issue: cmd/buildlet: fix bash wrapper check for iOS

gopherbot pushed a commit to golang/build that referenced this issue Oct 6, 2020
Updates golang/go#41610

Change-Id: I9a2e151eaf8104e2f3a1a8d36f8dc72a6351fd4a
Reviewed-on: https://go-review.googlesource.com/c/build/+/259580
Trust: Elias Naur <mail@eliasnaur.com>
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants