x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567
Comments
|
I don't know what's going on there. Per the first line of that first URL, it should only be using Go built at c8aaec2, which you see later in the log with: So I don't know why it's expecting Go 1.12.1 or where that comes from. |
bradfitz
added
Builders
|
There are two
and
The latter does not include I don't know why |
|
I think the second path comes from https://go-review.googlesource.com/c/build/+/152457. However, they are supposed to be merged together or at least $GOROOT/bin should be added to the $PATH coming from the dashboard. |
Because this is a build that's running repo "go" at master (rev |
|
But, yeah, double/unmerged PATH definitely looks like the problem here. Thanks for noticing that and for the CL link. |
|
Change https://golang.org/cl/192327 mentions this issue: |
Updates golang/go#32836 Updates golang/go#31567 Updates golang/go#11811 Change-Id: I5443b61cf7732abf906ce2e93eca5408579a55c8 Reviewed-on: https://go-review.googlesource.com/c/build/+/192327 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
bcmills
changed the title
|
Change https://golang.org/cl/192335 mentions this issue: |
Updates golang/go#31567 Change-Id: Ia8d996a70166c4395393f4674af87ad755104fe1 Reviewed-on: https://go-review.googlesource.com/c/build/+/192335 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
Change https://golang.org/cl/192559 mentions this issue: |
(I mostly just wanted to make a trivial change to this repo to clear out the failure cells for misconfigured builders on the landing page of https://build.golang.org.) Updates golang/go#31567 Change-Id: I2d779a143c711e86fb2f34451d7398d3514623ae Reviewed-on: https://go-review.googlesource.com/c/website/+/192559 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
dmitshur
added
NeedsFix
dmitshur
changed the title
Updates golang/go#32836 Updates golang/go#31567 Updates golang/go#11811 Change-Id: I5443b61cf7732abf906ce2e93eca5408579a55c8 Reviewed-on: https://go-review.googlesource.com/c/build/+/192327 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
Updates golang/go#31567 Change-Id: Ia8d996a70166c4395393f4674af87ad755104fe1 Reviewed-on: https://go-review.googlesource.com/c/build/+/192335 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
https://build.golang.org/log/18084f24be9cc1f31dbef14b8e398003e21045e0 |
|
As of CL 353549 the It's still failing, but it's now at least failing in a much more coherent way that indicates a completely-stale |
|
Change https://golang.org/cl/354754 mentions this issue: |
|
Change https://golang.org/cl/354755 mentions this issue: |
|
@bcmills Based on a recent test failure (e.g., https://build.golang.org/log/3d50d2e41477382548e0b84905d5dff6ea05f2f2) and the test code, to me it looks like the problem may be that the test code tries to run "go" from PATH, but the Go toolchain that was built and is being tested (the one in GOROOT/bin) is not in PATH. I'm only seeing one PATH entry in https://build.golang.org/log/3d50d2e41477382548e0b84905d5dff6ea05f2f2, so though CL 354754 looks like a good change to me, it may not be enough. |
I agree — that's my assessment of the problem too.
My evidence so far is:
I don't have request logs, but my hypothesis is that what's happening here is:
The fix is to change step (3) to edit the actual path key-value instead of the out-of-date one. |
If both the process environment and the "env" post field included entries for "PATH", the setPathEnv helper function would erroneously use the first one instead of the last. (In current versions of Go, os/exec always uses the last entry for each variable.) This change removes much of the complexity of setPathEnv, pushing some of the less obvious parts of the logic (skipping empty slices and suppressing no-op changes) to the caller side. For golang/go#31567 Change-Id: I7460bf65c61073a71f0ea11d4d69a16f3e9b7c16 Reviewed-on: https://go-review.googlesource.com/c/build/+/354754 Trust: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
|
buildlet.aix-ppc64 is now rebuilt with CL 354754, and should be used in future builds. |
|
The (https://build.golang.org/log/5d976685f5d214a02a754b4327387f9fa101f256) |
bcmills
changed the title
(I mostly just wanted to make a trivial change to this repo to clear out the failure cells for misconfigured builders on the landing page of https://build.golang.org.) Updates golang/go#31567 Change-Id: I2d779a143c711e86fb2f34451d7398d3514623ae Reviewed-on: https://go-review.googlesource.com/c/website/+/192559 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
|
Change https://go.dev/cl/452678 mentions this issue: |
|
Change https://go.dev/cl/452776 mentions this issue: |
Also remove existing special cases that transform "go" into gorootBinGo, because they make debugging and code-reviews more difficult: log messages that don't include the full path can mask bugs like #31567, and the reader of the code has to trace through the various call chains to verify that the correct "go" is being used. Instead, we can make the use of the correct "go" command plainly obvious in the code by using one consistent name for it. (Prior to this CL, we had three different names for it: gorootBinGo, "go", and cmdGo. Now we have only one. Updates #31567. Change-Id: Ia9ff27e5e800c79af5a4e9f2803c9ea5ccafbf35 Reviewed-on: https://go-review.googlesource.com/c/go/+/452678 Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
This should help to prevent bugs from unintended use of system tools, especially the system or bootstrap "go" command. (Suggested by Austin on CL 452678.) Updates #31567. Change-Id: I71809ee30d06eda4b5ff8f90656d4f1a462d35dd Reviewed-on: https://go-review.googlesource.com/c/go/+/452776 Reviewed-by: Austin Clements <austin@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com>
The tests of x/tools and x/tour are failing on aix/ppc64, because the builder seems to use an hybrid version between 1.12.1 and master. Any idea of what must be changed/fixed ?
https://build.golang.org/log/b7b160e7a7e733fbe768d45af70cf1c626ac4289
https://build.golang.org/log/7e28abbbd860c0ea1f212805b15ce5f3e8bf5d14
/cc @bradfitz
Edit by @dmitshur: The root cause of this problem is described in #31567 (comment).
The text was updated successfully, but these errors were encountered: