Skip to content

misc/cgo/testshared fails on fast enough machines that don't have fine-grained file timestamps #10724

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

Closed
siebenmann opened this issue May 6, 2015 · 8 comments
Milestone

Comments

@siebenmann
Copy link

Ever since it was introduced, one of my machines fails in misc/cgo/testshared when building git master. The failure message:

##### ../misc/cgo/testshared
ldd: warning: you do not have execution permission for `/data/code/go-lang/go/misc/cgo/testshared/pkg/linux_amd64_RaFaEwVC_dynlink/libdep.so'
/data/code/go-lang/go/misc/cgo/testshared/pkg/linux_amd64_RaFaEwVC_dynlink/dep.a was not rebuilt
2015/05/06 14:21:36 Failed: exit status 1

The ultimate cause is that this machine is fast enough to build (or finish building by writing the files out), touch a file, copy files, and rebuild, all in the same second, and my copy of the git repo is on a filesystem that only has timestamps with a one second granularity. In this situation you need to insert artificial delays of at least a second both before and after the touch-and-copy code in testshared:

diff --git a/misc/cgo/testshared/test.bash b/misc/cgo/testshared/test.bash
index 21004ad..e471403 100755
--- a/misc/cgo/testshared/test.bash
+++ b/misc/cgo/testshared/test.bash
@@ -96,15 +96,19 @@ assert_not_rebuilt () {
 }

 # If the source is newer than both the .a file and the .so, both are rebuilt.
+sleep 1
 touch src/dep/dep.go
 will_check_rebuilt $rootdir/libdep.so $rootdir/dep.a
+sleep 1
 go install -installsuffix="$mysuffix" -linkshared exe
 assert_rebuilt $rootdir/dep.a
 assert_rebuilt $rootdir/libdep.so

 # If the .a file is newer than the .so, the .so is rebuilt (but not the .a)
+sleep 1
 touch $rootdir/dep.a
 will_check_rebuilt $rootdir/libdep.so $rootdir/dep.a
+sleep 1
 go install  -installsuffix="$mysuffix" -linkshared exe
 assert_not_rebuilt $rootdir/dep.a
 assert_rebuilt $rootdir/libdep.so

The machine that this fails on is a 64-bit Fedora 21 machine using an ext3 filesystem (mounted with relatime, but I don't believe that makes any difference). On Linux, ext3 does not support nanosecond timestamps but ext4 and various other filesystems do to some extent.

@bradfitz
Copy link
Contributor

bradfitz commented May 6, 2015

Per #10571, let's not add 4 more seconds of sleep here. Death by a thousand cuts, straw camel back, etc.

I'd prefer to move this into Go code in src/cmd/dist/test.go and then it would be much easier to do things like sleep up to 1 second (measuring how long the previous commands took), and do things like check the inode for other changes, if applicable first. It's also easier to parallelize things in Go.

@mwhudson
Copy link
Contributor

mwhudson commented May 6, 2015

Most machines are fast enough to do all these steps in a second, I guess most people are using ext4 or something by now. I agree with @bradfitz that sleeps are not the answer. Maybe use https://godoc.org/os#Chtimes to push the times into the past?

@siebenmann
Copy link
Author

I think and agree that in general moving file times backwards is the best guaranteed reliable way of doing this. You get full control without delays and without worrying about timestamp precision or the speed of operations.

@mwhudson
Copy link
Contributor

mwhudson commented May 7, 2015

But it turns out you can't push them too far into the past because then they are older than the compilers and everything gets rebuilt anyway :) Can you try the change at https://golang.org/cl/9768?

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/9768 mentions this issue.

@siebenmann
Copy link
Author

This change works for me. However, 'date -d ...' is not portable; it's a GNU date extension and is missing on (for example) FreeBSD, where 'date -d' means something completely different.

(You may already know this and just be trying out the approach to make sure it works.)

@bradfitz
Copy link
Contributor

bradfitz commented May 7, 2015

Can we please just move this to Go code in src/cmd/dist/test.go and not worry about portability of shell code? We already have a portable language.

@mwhudson
Copy link
Contributor

mwhudson commented May 8, 2015

@bradfitz wins, I rewrote it in Go.

mwhudson added a commit to mwhudson/go that referenced this issue May 11, 2015
And fix to work on filesystems with only 1s resolution.

Fixes golang#10724

Change-Id: Ia07463f090b4290fc27f5953fa94186463d7afc7
mwhudson added a commit to mwhudson/go that referenced this issue May 18, 2015
And fix to work on filesystems with only 1s resolution.

Fixes golang#10724

Change-Id: Ia07463f090b4290fc27f5953fa94186463d7afc7
mwhudson added a commit to mwhudson/go that referenced this issue May 18, 2015
And fix to work on filesystems with only 1s resolution.

Fixes golang#10724

Change-Id: Ia07463f090b4290fc27f5953fa94186463d7afc7
@mikioh mikioh added this to the Go1.5 milestone May 23, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants