-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: windows-*-longtest builders have no Git binary in %PATH% #46693
Comments
Per #46692, I think we should configure |
Per #49841, we should also install any other VCS tools supported by the intersection of |
Change https://go.dev/cl/398855 mentions this issue: |
When we added VCS stamping in the Go 1.18 release, we defaulted to -buildvcs=true, on the theory that most folks will actually want VCS information stamped. We also made -buildvcs=true error out if a VCS directory is found and no VCS tool is available, on the theory that a user who builds with '-buildvcs=true' will be very surprised if the VCS metadata is silently missing. However, that causes a problem for CI environments that don't have the appropriate VCS tool installed. (And we know that's a common situation because we're in that situation ourselves — see #46693!) The new '-buildvcs=auto' setting provides a middle ground: it stamps VCS information by default when the tool is present (and reports explicit errors if the tool errors out), but omits the metadata when the tool isn't present at all. Fixes #51748. Updates #51999. Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4 Reviewed-on: https://go-review.googlesource.com/c/go/+/398855 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Change https://go.dev/cl/400454 mentions this issue: |
…s the default When we added VCS stamping in the Go 1.18 release, we defaulted to -buildvcs=true, on the theory that most folks will actually want VCS information stamped. We also made -buildvcs=true error out if a VCS directory is found and no VCS tool is available, on the theory that a user who builds with '-buildvcs=true' will be very surprised if the VCS metadata is silently missing. However, that causes a problem for CI environments that don't have the appropriate VCS tool installed. (And we know that's a common situation because we're in that situation ourselves — see #46693!) The new '-buildvcs=auto' setting provides a middle ground: it stamps VCS information by default when the tool is present (and reports explicit errors if the tool errors out), but omits the metadata when the tool isn't present at all. Updates #51748. Updates #51999. Fixes #51798. Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4 Reviewed-on: https://go-review.googlesource.com/c/go/+/398855 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> (cherry picked from commit 4569fe6) Reviewed-on: https://go-review.googlesource.com/c/go/+/400454 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Change https://go.dev/cl/505895 mentions this issue: |
It wasn't caught by x/build's Windows builders because they don't have git and skip these tests. But LUCI ones do have git, and caught this. For golang/go#46693. Change-Id: I50f3b69909344f4657e5c737e3b434a2538e4939 Reviewed-on: https://go-review.googlesource.com/c/review/+/505895 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
LUCI bots have Git installed. |
I got a shiny new Windows laptop for personal use, and out of curiosity I decided to install the Go toolchain and run
go test cmd/go
on it.Unfortunately, that immediately uncovered two existing test failures (#46692 and #46691) that had not shown up in the Go project's builders — even the
longtest
builders.The common, relevant factor between the two failing tests is this line:
That seems to imply that either the Go
windows-amd64-longtest
builder does not have agit
binary installed, or thatgit
binary is omitted from%PATH%
when thecmd/go
tests are run.This seems like a pretty substantial (and unexpected) gap in test coverage, but hopefully it is pretty easy to fix. However, it may reveal issues that need to be fixed in previous releases. (For a start, we may need to backport the
GCM_INTERACTIVE
portion of CL 300157.)CC @golang/release
The text was updated successfully, but these errors were encountered: