cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin #37573

bcmills · 2020-02-28T20:53:35Z

2020-02-28T20:15:38-964fac3/linux-amd64-longtest

test_race_install_cgo was converted to a script test in CL 214426. The old test was not run in parallel, and asserted that go test -race -i runtime/race did not reinstall cmd/cgo.

I suspect that this flake was caused by some other test somehow overwriting cmd/cgo when it should not have.

The text was updated successfully, but these errors were encountered:

bcmills · 2020-03-02T14:47:09Z

2020-03-01T05:45:39-1e9665d/windows-amd64-longtest

dmitshur · 2020-03-08T00:17:41Z

Another failure on windows-amd64-longtest at a7f918c1 from today.

bcmills · 2020-03-17T01:26:17Z

2020-03-16T22:31:39-2fbca94/windows-amd64-longtest
2020-03-15T00:17:52-dad94e7/windows-amd64-longtest
2020-03-13T19:06:00-be72e3c/linux-amd64-longtest
2020-03-11T19:59:39-f6a0d72/windows-amd64-longtest
2020-03-09T20:19:25-5fac45a/linux-amd64-longtest

I think this is a pairwise-incompatibility with some other test, but so far I haven't been able to track it down running each cmd/go test pairwise with test_race_install_cgo with a high -count.

gopherbot · 2020-03-17T15:09:35Z

Change https://golang.org/cl/223745 mentions this issue: cmd/go: replace TestCgoDependsOnSyscall with a simpler script test

The existing test attempted to remove '_race' binaries from GOROOT/pkg, which could not only fail if GOROOT is read-only, but also interfere with other tests run in parallel. Updates #30316 Updates #37573 Updates #17751 Change-Id: Id7e2286ab67f8333baf4d52244b7f4476aa93a46 Reviewed-on: https://go-review.googlesource.com/c/go/+/223745 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>

gopherbot · 2020-03-17T17:11:32Z

Change https://golang.org/cl/223746 mentions this issue: cmd/go: add a "don't care about success" operator to script_test

bcmills · 2020-03-17T17:12:08Z

I'm hoping that CL 223745 does the trick, but there may be other interactions lurking.

bcmills · 2020-03-17T18:09:14Z

That CL didn't do it: https://build.golang.org/log/d9ad32ca35be46c7fac61bf609655f3e3100a803

At least the feedback was quick!

gopherbot · 2020-03-18T01:33:08Z

Change https://golang.org/cl/223755 mentions this issue: cmd/dist: run a checkNotStale on the builders before running tests

bcmills · 2020-03-18T03:58:00Z

FINALLY found it!

Doesn't seem to reproduce at all when running as a not-root user. That's why it was so [redacted] hard to track down.

For some reason, get_update_all adjusts file attributes (but not content!) on ~everything in GOROOT/pkg/tool, but only when running as root.

~/bisect/src$ sudo ../bin/go test cmd/go -run='TestScript/(get_update_all|test_race_install_cgo)'
go test proxy running at GOPROXY=http://127.0.0.1:41653/mod
--- FAIL: TestScript (0.02s)
    --- FAIL: TestScript/test_race_install_cgo (0.95s)
        script_test.go:202:
            # Tests Issue #10500 (0.938s)
            > [!race] skip
            > ! stale cmd/cgo
            > env GOBIN=$WORK/bin
            > go install mtime sametime
            > go tool -n cgo
            [stdout]
            /usr/local/google/home/bcmills/bisect/pkg/tool/linux_amd64/cgo
            > cp stdout cgopath.txt
            > exec $GOBIN/mtime cgopath.txt # get the mtime of the file whose name is in cgopath.txt
            [stdout]
            "2020-03-17T23:53:40.190152563-04:00"
            > cp stdout cgotime_before.txt
            > ? go test -race -i runtime/race
            > exec $GOBIN/mtime cgopath.txt # get the mtime of the file whose name is in cgopath.txt
            [stdout]
            "2020-03-17T23:54:37.402358823-04:00"
            > cp stdout cgotime_after.txt
            > exec $GOBIN/sametime cgotime_before.txt cgotime_after.txt
            [stderr]
            time in cgotime_before.txt (2020-03-17 23:53:40.190152563 -0400 EDT) is not the same as time in cgotime_after.txt (2020-03-17 23:54:37.402358823 -0400 EDT)[exit status 1]
            FAIL: testdata/script/test_race_install_cgo.txt:22: unexpected command failure

FAIL
FAIL    cmd/go  2.813s
FAIL

bcmills · 2020-03-18T04:14:36Z

I think I've figured out what's going on.

Our own test from CL 12174 (2015) was rendered unparallelizable by our own intentional breakage of the non-intrusive behavior that test was relying on in CL 76590 (2017).

go/src/cmd/go/internal/work/exec.go

Lines 1465 to 1476 in 938ad55

    
           // Whether we're smart enough to avoid a complete rebuild 
        
           // depends on exactly what the staleness and rebuild algorithms 
        
           // are, as well as potentially the state of the Go build cache. 
        
           // We don't really want users to be able to infer (or worse start depending on) 
        
           // those details from whether the modification time changes during 
        
           // "go install", so do a best-effort update of the file times to make it 
        
           // look like we rewrote a.Target even if we did not. Updating the mtime 
        
           // may also help other mtime-based systems that depend on our 
        
           // previous mtime updates that happened more often. 
        
           // This is still not perfect - we ignore the error result, and if the file was 
        
           // unwritable for some reason then pretending to have written it is also 
        
           // confusing - but it's probably better than not doing the mtime update.

gopherbot · 2020-03-18T15:54:27Z

Change https://golang.org/cl/223919 mentions this issue: cmd/go: make 'go get' and 'go install' avoid updating mtimes of binaries that are not stale

bcmills · 2020-03-18T16:02:51Z

Retitled for the diagnosed root cause.

For golang/go#37573

Use that operator to make test_race_install_cgo agnostic to whether GOROOT/pkg is writable. Updates #37573 Updates #30316 Change-Id: I018c63b3c369209345069f917bbb3a52179e2b58 Reviewed-on: https://go-review.googlesource.com/c/go/+/223746 Reviewed-by: Jay Conrod <jayconrod@google.com>

gopherbot · 2020-03-18T18:21:52Z

Change https://golang.org/cl/223925 mentions this issue: cmd/go: rebuild cmd/cgo in test_race_install_cgo instead of asserting that it is not stale

Some of the darwin-amd64 builders are providing a stale environment. Let's un-break them while we investigate. Updates #37573 Updates #33598 Change-Id: I8b79778fe4d5aa916557c1ba89fa9c776d130b01 Reviewed-on: https://go-review.googlesource.com/c/go/+/223925 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>

bcmills · 2020-03-26T02:55:21Z

Retitled again. After discussion with @rsc, we decided that the mtime change is correct and should be kept, and instead we should prohibit the test version of the cmd/go binary from attempting to change metadata in GOROOT/bin.

gopherbot · 2020-03-27T14:50:43Z

Change https://golang.org/cl/225897 mentions this issue: cmd/go/internal/work: disallow testgo binary from installing to GOROOT

bcmills assigned matloob Feb 28, 2020

bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure. labels Feb 28, 2020

bcmills added this to the Go1.15 milestone Feb 28, 2020

bcmills changed the title ~~cmd/go: flake in TestScript/test_race_install_cgo~~ cmd/go: TestScript/test_race_install_cgo is flaky Mar 2, 2020

bcmills mentioned this issue Mar 12, 2020

cmd/go: tests fail when run with -count=2 #37820

Closed

bcmills assigned bcmills and unassigned matloob Mar 17, 2020

bcmills added NeedsFix The path to resolution is known, but the work has not been done. and removed Testing An issue that has been verified to require only test changes, not just a test failure. labels Mar 18, 2020

gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 18, 2020

bcmills changed the title ~~cmd/go: TestScript/test_race_install_cgo is flaky~~ cmd/go: 'go get ...' spuriously updates the mtime of cmd/cgo when it was not stale Mar 18, 2020

bcmills pushed a commit to bcmills/watchdir that referenced this issue Mar 18, 2020

watchdir: add command

4b87c10

For golang/go#37573

bcmills changed the title ~~cmd/go: 'go get ...' spuriously updates the mtime of cmd/cgo when it was not stale~~ cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin Mar 26, 2020

bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Mar 27, 2020

gopherbot closed this as completed in d22086e Mar 27, 2020

golang locked and limited conversation to collaborators Mar 27, 2021

gopherbot added the FrozenDueToAge label Mar 27, 2021

rsc unassigned bcmills Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin #37573

cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin #37573

bcmills commented Feb 28, 2020 •

edited by dmitshur

bcmills commented Mar 2, 2020

dmitshur commented Mar 8, 2020

bcmills commented Mar 17, 2020

gopherbot commented Mar 17, 2020

gopherbot commented Mar 17, 2020

bcmills commented Mar 17, 2020

bcmills commented Mar 17, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 18, 2020

bcmills commented Mar 18, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 18, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 26, 2020

gopherbot commented Mar 27, 2020

cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin #37573

cmd/go: TestScript/get_update_all changes mtimes in GOROOT/bin #37573

Comments

bcmills commented Feb 28, 2020 • edited by dmitshur

bcmills commented Mar 2, 2020

dmitshur commented Mar 8, 2020

bcmills commented Mar 17, 2020

gopherbot commented Mar 17, 2020

gopherbot commented Mar 17, 2020

bcmills commented Mar 17, 2020

bcmills commented Mar 17, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 18, 2020

bcmills commented Mar 18, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 18, 2020

gopherbot commented Mar 18, 2020

bcmills commented Mar 26, 2020

gopherbot commented Mar 27, 2020

bcmills commented Feb 28, 2020 •

edited by dmitshur