x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567

Helflym · 2019-04-19T09:08:24Z

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).

bradfitz · 2019-04-19T14:56:38Z

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:

# internal/cpu
compile: version "devel c8aaec2f70c5ccbca1ec2152c57d19981ac09133" does not match go tool version "go1.12.1"

So I don't know why it's expecting Go 1.12.1 or where that comes from.

/cc @ianlancetaylor @bcmills @dmitshur

bcmills · 2019-04-19T15:11:02Z

There are two PATH entries in the command line logged at the top:

"PATH=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go/bin:/opt/freeware/bin:/usr/bin:/etc:/usr/sbin:/usr/ucb:/usr/bin/X11:/sbin:/usr/java7_64/jre/bin:/usr/java7_64/bin"

and

"PATH=/opt/freeware/bin:/usr/bin:/etc:/usr/sbin:/usr/ucb:/usr/bin/X11:/sbin:/usr/java7_64/jre/bin:/usr/java7_64/bin"

The latter does not include $GOROOT/bin, so that's presumably where the go at 1.12.1 is coming from.

I don't know why compile is coming from the devel version, though.

Helflym · 2019-04-19T15:38:01Z

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.

bradfitz · 2019-04-19T15:51:13Z

I don't know why compile is coming from the devel version, though.

Because this is a build that's running repo "go" at master (rev c8aaec2f70c5ccbca1ec2152c57d19981ac09133) against x/net master.

bradfitz · 2019-04-19T15:51:38Z

But, yeah, double/unmerged PATH definitely looks like the problem here. Thanks for noticing that and for the CL link.

gopherbot · 2019-08-29T16:38:47Z

Change https://golang.org/cl/192327 mentions this issue: dashboard: disable failing repos on misconfigured builders

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>

gopherbot · 2019-08-29T19:05:38Z

Change https://golang.org/cl/192335 mentions this issue: dashboard: extend aix-ppc64 skip to the website repo

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>

gopherbot · 2019-08-30T13:55:06Z

Change https://golang.org/cl/192559 mentions this issue: go.mod: add go version directive

(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>

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>

bcmills · 2020-02-20T14:21:20Z

https://build.golang.org/log/18084f24be9cc1f31dbef14b8e398003e21045e0

--- FAIL: TestHookChangeGofmt (2.94s)
    hook_test.go:267: invoking commit hook explicitly
    hook_test.go:268: git-codereview hook-invoke pre-commit
    hook_test.go:271: change without hook installed
    hook_test.go:273: git-codereview change
    hook_test.go:276: change with hook installed
    hook_test.go:493: in git-codereview/, ran [go build -o /ramdisk8GB/workdir-host-aix-ppc64-osuosl/tmp/git-codereview-test399063055/git-client/git-codereview]: exit status 2
        # unicode
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # crypto/internal/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/internal/sys
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # math/bits
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # unicode/utf8
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # vendor/golang.org/x/crypto/internal/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/cpu
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # encoding
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # crypto/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/nettrace
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # container/list
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # unicode/utf16
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/race
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # vendor/golang.org/x/crypto/cryptobyte/asn1
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/internal/atomic
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # sync/atomic
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/cgo
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
--- FAIL: TestHookCommitMsgFromGit (2.54s)
    hook_test.go:493: in git-codereview/, ran [go build -o /ramdisk8GB/workdir-host-aix-ppc64-osuosl/tmp/git-codereview-test792055703/git-client/git-codereview]: exit status 2
        # vendor/golang.org/x/crypto/internal/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/cpu
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # unicode/utf16
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # crypto/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # container/list
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # vendor/golang.org/x/crypto/cryptobyte/asn1
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # encoding
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/nettrace
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # unicode/utf8
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/internal/sys
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # crypto/internal/subtle
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # internal/race
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # math/bits
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # unicode
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/internal/atomic
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # sync/atomic
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
        # runtime/cgo
        compile: version "devel 49f8d45994b6782e4925057909e4896832334a26" does not match go tool version "go1.13"
FAIL
FAIL	golang.org/x/review/git-codereview	72.131s

CC @cagedmantis @toothrot

bcmills · 2021-10-08T19:08:38Z

As of CL 353549 the PATH is no longer doubled and the failure mode of this builder no longer mixes go components from different parts of the path.

It's still failing, but it's now at least failing in a much more coherent way that indicates a completely-stale go toolchain rather than just a partly-stale one (https://build.golang.org/log/f18e14391e30e30a150c41a4dd79ec9e6a22f02b):

:: Running /ramdisk8GB/workdir-host-aix-ppc64-osuosl/go/bin/go with args ["/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go/bin/go" "test" "-short" "golang.org/x/review/..."] and env ["TERM=dumb" "AUTHSTATE=compat" "SHELL=/usr/bin/ksh" "HOME=/" "USER=root" "PATH=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go/bin:/opt/freeware/bin:/usr/bin:/etc:/usr/sbin:/usr/ucb:/usr/bin/X11:/sbin:/usr/java7_64/jre/bin:/usr/java7_64/bin" "TZ=CST6CDT" "LANG=en_US" "LOCPATH=/usr/lib/nls/loc" "LC__FASTMSG=true" "ODMDIR=/etc/objrepos" "CLCMD_PASSTHRU=1" "LOGNAME=root" "LOGIN=root" "META_BUILDLET_BINARY_URL=https://storage.googleapis.com/go-builder-data/buildlet.aix-ppc64" "GO_STAGE0_NET_DELAY=400ms" "GO_STAGE0_DL_DELAY=100ms" "NLSPATH=/usr/lib/nls/msg/%L/%N:/usr/lib/nls/msg/%L/%N.cat:/usr/lib/nls/msg/%l.%c/%N:/usr/lib/nls/msg/%l.%c/%N.cat" "WORKDIR=/ramdisk8GB/workdir-host-aix-ppc64-osuosl" "GOROOT_BOOTSTRAP=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go1.4" "GO_BUILDER_NAME=aix-ppc64" "GOROOT_BOOTSTRAP=/opt/freeware/lib/golang" "PATH=/opt/freeware/bin:/usr/bin:/etc:/usr/sbin:/usr/ucb:/usr/bin/X11:/sbin:/usr/java7_64/jre/bin:/usr/java7_64/bin" "GOROOT=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/go" "GOPATH=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/gopath" "GOPROXY=http://gk3-services-nap-yh2ma9qv-7a84e634-8cll.c.symbolic-datum-552.internal:30157" "GOPROXY=https://proxy.golang.org" "TMPDIR=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/tmp" "GOCACHE=/ramdisk8GB/workdir-host-aix-ppc64-osuosl/gocache"] in dir /ramdisk8GB/workdir-host-aix-ppc64-osuosl/gopath/src/golang.org/x/review
…
--- FAIL: TestHookCommitMsgFromGit (0.69s)
    hook_test.go:493: in git-codereview/, ran [go build -o /ramdisk8GB/workdir-host-aix-ppc64-osuosl/tmp/git-codereview-test3736945410/git-client/git-codereview]: exit status 1
        api.go:15:2: //go:build comment without // +build comment
FAIL
FAIL	golang.org/x/review/git-codereview	72.903s

gopherbot · 2021-10-08T19:41:36Z

Change https://golang.org/cl/354754 mentions this issue: cmd/buildlet: in setPathEnv, override the last PATH entry, not the first

gopherbot · 2021-10-08T19:41:37Z

Change https://golang.org/cl/354755 mentions this issue: dashboard: re-enable default repos on aix

dmitshur · 2021-10-20T15:02:30Z

@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.

bcmills · 2021-11-01T17:47:15Z

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 agree — that's my assessment of the problem too.

I'm only seeing one PATH entry …, so though CL 354754 looks like a good change to me, it may not be enough.

My evidence so far is:

There is a PATH entry in dashboard/builders.go, and that entry matches what is currently showing in the build logs.
The coordinator explicitly adds $WORKDIR/go/bin to PATH when it executes subrepo tests (cmd/coordinator/coordinator.go:2815).
There were duplicated paths in previous logs for the aix-ppc64 builder.
The only other builder that specifies a PATH in dashboard/builders.go is js-wasm, but that builder doesn't run tests that run go commands because js doesn't support os/exec at all.

I don't have request logs, but my hypothesis is that what's happening here is:

cmd/coordinator sends the PATH entry from dashboard/builders.go in the builder's environment, and sends the Path field from the ExecOpts as a separate query parameter.
cmd/buildlet appends the PATH entry to the environment (after the PATH entry that was already present in the process's environment) then calls setPathEnv to add in the values from the Path field.
setPathEnv edits the former PATH (from the process's environment) instead of the one added in step (1).
envutil.Dedup keeps the PATH value added in step (1) instead of the one edited in step (3).

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>

dmitshur · 2021-11-01T18:59:51Z

buildlet.aix-ppc64 is now rebuilt with CL 354754, and should be used in future builds.

bcmills · 2021-11-01T20:37:20Z

The aix-ppc64 builder is now passing for x/review!

(https://build.golang.org/log/5d976685f5d214a02a754b4327387f9fa101f256)

(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>

gopherbot · 2022-11-22T03:50:37Z

Change https://go.dev/cl/452678 mentions this issue: cmd/dist: consistently use $GOROOT/bin/go instead of just "go"

gopherbot · 2022-11-22T18:52:46Z

Change https://go.dev/cl/452776 mentions this issue: cmd/dist: require absolute path to executable in flattenCommandLine

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>

gopherbot added this to the Unreleased milestone Apr 19, 2019

bradfitz added Builders x/build issues (builders, bots, dashboards) help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-AIX labels Apr 19, 2019

bcmills changed the title ~~x/tools, x/tour: wrong builder version on aix/ppc64~~ x/build: 'go' subprocesses invoke mismatched versions on aix-ppc64 builder Aug 29, 2019

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 21, 2019

dmitshur changed the title ~~x/build: 'go' subprocesses invoke mismatched versions on aix-ppc64 builder~~ x/build: aix-ppc64 builder has double/unmerged PATH causing wrong 'go' binary to be found Sep 21, 2019

bcmills mentioned this issue Aug 31, 2020

x/build: freebsd-arm-paulzhol builder is failing cmd/link TestDWARF #41101

Closed

bcmills changed the title ~~x/build: aix-ppc64 builder has double/unmerged PATH causing wrong 'go' binary to be found~~ x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 Nov 1, 2021

bcmills self-assigned this Nov 1, 2021

bcmills removed help wanted OS-AIX labels Nov 1, 2021

gopherbot closed this as completed in golang/build@37fcb2f Nov 4, 2021

bcmills mentioned this issue Nov 8, 2021

x/tools/go/internal/gccgoimporter: TestInstallationImporter failures with "bad magic number" on aix-ppc64 #49445

Closed

bcmills mentioned this issue Jan 28, 2022

x/build/cmd/coordinator: failures are sometimes missing error output #39349

Closed

rsc unassigned bcmills Jun 23, 2022

golang locked and limited conversation to collaborators Nov 22, 2023

gopherbot added the FrozenDueToAge label Nov 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567

x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567

Helflym commented Apr 19, 2019 •

edited by dmitshur

Loading

bradfitz commented Apr 19, 2019

bcmills commented Apr 19, 2019 •

edited

Loading

Helflym commented Apr 19, 2019

bradfitz commented Apr 19, 2019

bradfitz commented Apr 19, 2019

gopherbot commented Aug 29, 2019

gopherbot commented Aug 29, 2019

gopherbot commented Aug 30, 2019

bcmills commented Feb 20, 2020 •

edited

Loading

bcmills commented Oct 8, 2021

gopherbot commented Oct 8, 2021

gopherbot commented Oct 8, 2021

dmitshur commented Oct 20, 2021

bcmills commented Nov 1, 2021

dmitshur commented Nov 1, 2021 •

edited

Loading

bcmills commented Nov 1, 2021

gopherbot commented Nov 22, 2022

gopherbot commented Nov 22, 2022

x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567

x/build/cmd/buildlet: $PATH miscomputed when set in "env" parameter, causing test failures on aix-ppc64 #31567

Comments

Helflym commented Apr 19, 2019 • edited by dmitshur Loading

bradfitz commented Apr 19, 2019

bcmills commented Apr 19, 2019 • edited Loading

Helflym commented Apr 19, 2019

bradfitz commented Apr 19, 2019

bradfitz commented Apr 19, 2019

gopherbot commented Aug 29, 2019

gopherbot commented Aug 29, 2019

gopherbot commented Aug 30, 2019

bcmills commented Feb 20, 2020 • edited Loading

bcmills commented Oct 8, 2021

gopherbot commented Oct 8, 2021

gopherbot commented Oct 8, 2021

dmitshur commented Oct 20, 2021

bcmills commented Nov 1, 2021

dmitshur commented Nov 1, 2021 • edited Loading

bcmills commented Nov 1, 2021

gopherbot commented Nov 22, 2022

gopherbot commented Nov 22, 2022

Helflym commented Apr 19, 2019 •

edited by dmitshur

Loading

bcmills commented Apr 19, 2019 •

edited

Loading

bcmills commented Feb 20, 2020 •

edited

Loading

dmitshur commented Nov 1, 2021 •

edited

Loading