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/cmd/release: darwin-amd64 release tests aren't passing, cmd/go's TestScript/cgo_stale_precompiled fails #50892

Closed
mknyszek opened this issue Jan 28, 2022 · 8 comments
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@mknyszek
Copy link
Contributor

When working on the go1.18beta2 release via releasebot (releasebot -mode=prepare go1.18beta2) I discovered two consecutive failures of the form:

go test proxy running at GOPROXY=http://127.0.0.1:51540/mod
--- FAIL: TestScript (0.02s)
    --- FAIL: TestScript/cgo_stale_precompiled (0.66s)
        script_test.go:257:
            # Regression test for https://go.dev/issue/47215 and https://go.dev/issue/50183:
            # A mismatched $GOROOT_FINAL or missing $CC caused the C dependencies of the net
            # package to appear stale, and it could not be rebuilt due to a missing $CC. (0.000s)
            # Control case: net must not already be stale. (0.654s)
            > ! stale net
            FAIL: testdata/script/cgo_stale_precompiled.txt:8: net is unexpectedly stale

I tried to get a third failure, but instead the buildlet hung along with others as per #50890. That issue also has the complete output of both failures produced by releasebot.

This issue seems potentially related to #48319 which included a fix that included a whole bunch of the same keywords that appear in the failure (https://golang.org/cl/380915).

CC @dmitshur @toothrot @bcmills

@mknyszek mknyszek added this to the Go1.18 milestone Jan 28, 2022
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 28, 2022
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Jan 28, 2022
@mknyszek
Copy link
Contributor Author

The failure just reproduced a third time in a row for a complete run. I think this is now blocking the beta.

@dmitshur
Copy link
Contributor

dmitshur commented Jan 28, 2022

Reproduced this locally just by setting GOROOT_FINAL to a non-empty string while running the test. Filed #50893 with details for that.

Update: #50893 (comment) suggests it may not be the same problem as what x/build/cmd/release tests are running into.

@gopherbot
Copy link

Change https://golang.org/cl/381834 mentions this issue: internal/testenv: add a test for the GoTool function

@gopherbot
Copy link

Change https://golang.org/cl/381835 mentions this issue: cmd/go: temporarily skip TestScript/cgo_stale_precompiled

@gopherbot
Copy link

Change https://golang.org/cl/381836 mentions this issue: cmd/go: add detail to test failures

@bcmills
Copy link
Member

bcmills commented Jan 28, 2022

This is another failure due to the same root cause as #46347. I'll rewrite the test to use the same approach as in https://go-review.googlesource.com/c/go/+/322629/5/src/cmd/go/testdata/script/cgo_stale.txt, and follow up with one or more issues to align either the default behavior of make.bash or the dashboard builders with the behavior required by cmd/release.

@bcmills bcmills self-assigned this Jan 28, 2022
@gopherbot
Copy link

Change https://golang.org/cl/381854 mentions this issue: cmd/go: rewrite TestScript/cgo_stale_precompiled to be agnostic to staleness

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 31, 2022
@gopherbot
Copy link

Change https://golang.org/cl/382055 mentions this issue: dashboard: test with -mmacosx-version-min set on darwin-amd64-10_15

gopherbot pushed a commit to golang/build that referenced this issue Feb 1, 2022
Bryan and I discussed that it might be a good idea to move the logic
that sets -mmacosx-version-min=nnn via CGO_CFLAGS on GOOS=darwin from
x/build/cmd/release to the cmd/go command. That way, if a problem
happens, it's reported not only during tests that the release script
runs, but also during normal pre/post-submit builders, thus sooner.

Until that happens (and in case it doesn't), set this environment
variable on at least one of our darwin/amd64 builders to improve
our test coverage for this edge case. This is inexpensive to do
compared to adding an entire new builder, and can be done quickly.

This is one of minor differences in environment between the tests
that run during by builders and during the release, and the long
term plan to catch all of them is still via nightly releases.

For golang/go#50892.
Updates golang/go#29205.

Change-Id: I5dd9a2f1dd457f54db6590d4ee181f81df8f7c38
Reviewed-on: https://go-review.googlesource.com/c/build/+/382055
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 1, 2022
For #50892

Change-Id: I14ff1c43b39687a0ba5e668ee962cecfb49e4beb
Reviewed-on: https://go-review.googlesource.com/c/go/+/381836
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@rsc rsc unassigned bcmills Jun 22, 2022
gopherbot pushed a commit that referenced this issue Aug 17, 2022
GoTool was added in CL 20967, and revised in CL 21292, for #14901.

I don't fully understand what problem the GoTool function was added to
solve: the discussion on that issue was pretty sparse, but it seems
like when we run tests of GOROOT packages they always know their own
location relative to GOROOT (and thus always know where to find the
'go' tool).

Lacking that understanding, I don't want to change its behavior, but I
do at least want to verify that it resolves to the real 'go' tool in
the common case (running 'go test' on a package in GOROOT/src).

For #50892
For #50893
Updates #14901

Change-Id: I06d831e6765be631dfc4854d7fddc3d27fc1de34
Reviewed-on: https://go-review.googlesource.com/c/go/+/381834
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants