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: improve trybot aliases for ppc64 #42067

Open
laboger opened this issue Oct 19, 2020 · 11 comments
Open

x/build: improve trybot aliases for ppc64 #42067

laboger opened this issue Oct 19, 2020 · 11 comments

Comments

@laboger
Copy link
Contributor

@laboger laboger commented Oct 19, 2020

I frequently see changes being made to code related to PPC64 which only use TRY=ppc64 for testing those changes. While I understand that this seems like the right default for code related to ppc64, this actually runs only the linux-ppc64 trybot, which tests only big endian and has limited linking capabilities (internal linking only, no cgo, no others like plug-ins or pie). So this selection is not the best default and does not provide good coverage for PPC64 testing.

I understand the naming is unfortunate because in the Go source code PPC64 usually used as a general term for code generation that applies to linux-ppc64, linux-ppc64le, linux-ppc64le-power9, and aix-ppc64.

Could we change the ppc64 alias in build/dashboard/builders.go to run the linux-ppc64le buildlet? This is really the most commonly used Power target for Go and provides the most linking capability. If someone REALLY wanted to run ppc64 big endian they could still use the linux-ppc64 alias.

I'd also like to have an alias for ppc64le on power9, maybe ppc64lep9.

I could make the change but I don't know how to test it. @dmitshur

@gopherbot gopherbot added the Builders label Oct 19, 2020
@gopherbot gopherbot added this to the Unreleased milestone Oct 19, 2020
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 19, 2020

I frequently see changes being made to code related to PPC64 which only use TRY=ppc64 for testing those changes. [...] Could we change the ppc64 alias in build/dashboard/builders.go to run the linux-ppc64le buildlet?

We currently have the following aliases related to ppc64:

"aix":            "aix-ppc64",
"linux-ppc64":    "linux-ppc64-buildlet",
"linux-ppc64le":  "linux-ppc64le-buildlet",
"ppc64":          "linux-ppc64-buildlet",
"ppc64le":        "linux-ppc64le-buildlet",

I think longer aliases need to be precise, and "linux-ppc64", "linux-ppc64le" should get the expected builder. But a short alias such as "ppc64" can be modified to point to the builder we think is most fitting. (Ideally, in the future, "ppc64" should expand to multiple builders to provide sufficient coverage, and people can use a longer alias if they want a very specific single builder.)

We should remove "ppc64le" to avoid or making it seem like "ppc64" gets the big endian one.

I'd also like to have an alias for ppc64le on power9, maybe ppc64lep9.

Given that it's an alias for a specific target, how about "linux-ppc64le-power9" to be more clear and consistent?

@laboger Do you mind sending a mail to golang-dev@ to notify people who rely on PPC64 aliases about this change and direct them to this issue if they want to express support for it or share concerns? It'd be helpful to get some more feedback from others before we make a change to slowbot aliases.

Also CC @golang/release.

@laboger
Copy link
Contributor Author

@laboger laboger commented Oct 19, 2020

Thanks, I agree with all your suggestions, especially at some point to have one keyword to run all 4 variations. I will send a note to golang-dev to gather more opinions.

  • ppc64 alias will run the ppc64le buildlet
  • long names will specify the exact buildlet and there will be a new name for power9 that is easier to remember: aix-ppc64, linux-ppc64, linux-ppc64le,linux-ppc64le-power9
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 19, 2020

Sound good, thanks for doing that.

I've remembered that we have a test that ensures each GOOS, GOARCH term has some slowbot alias. But given how similar ppc64{,le} are, I think it's fine for us to modify the test so that a slowbot alias for "ppc64" also satisfies the requirement for "ppc64le".

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 21, 2020

Maybe we could add "ppc64x"?

@laboger
Copy link
Contributor Author

@laboger laboger commented Oct 23, 2020

Maybe we could add "ppc64x"?

And what would ppc64x run?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 23, 2020

And what would ppc64x run?

BE and LE. Basically, *ppc64*.

@laboger
Copy link
Contributor Author

@laboger laboger commented Oct 27, 2020

BE and LE. Basically, ppc64.

Yes, that sounds great once we get multiple trybots to run for a single alias.

@laboger
Copy link
Contributor Author

@laboger laboger commented Oct 29, 2020

One other suggestion, although maybe this belongs in a different issue. If I have a CL and specify a new TRY list, it doesn't rerun the trybots unless the CL has changed since the last TRY (at least that appears to be the way it works.) It seems that if a new TRY list is specified it should rerun it?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 29, 2020

I think it actually runs, but Go Bot doesn't print a message when it starts. It will print a message when it finishes.

@laboger
Copy link
Contributor Author

@laboger laboger commented Oct 29, 2020

Look at this CL: https://go-review.googlesource.com/c/go/+/266202. Shouldn't the message say which slowbots ran on the last one? Or maybe it is still using the first set of keywords which were not correct.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 29, 2020

This appears to be issues #42084 and #38235. Please add a comment there so we have more data points.

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
4 participants
You can’t perform that action at this time.