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

go uses lstat instead of stat to test date of files #5830

Closed
gopherbot opened this issue Jul 2, 2013 · 15 comments
Closed

go uses lstat instead of stat to test date of files #5830

gopherbot opened this issue Jul 2, 2013 · 15 comments

Comments

@gopherbot
Copy link

by richard.wm.jones:

go uses lstat instead of stat to test the date of some files when decided what to
recompile.  As a result, it is broken if symlinks are used in packaged installs.

We are trying to diagnose this bug in Fedora:

go install errors: open /usr/lib64/golang/pkg/linux_amd64/errors.a: permission denied

When stracing the go process we see:

stat("/usr/lib64/golang/pkg/linux_amd64/errors.a", {st_mode=S_IFREG|0644,
st_size=6940, ...}) = 0

^ this is correct, but later ...

stat("/usr/lib64/golang/src/pkg/errors", {st_mode=S_IFDIR|0755, st_size=4096,
..
.}) = 0
open("/usr/lib64/golang/src/pkg/errors", O_RDONLY|O_CLOEXEC) = 3
getdents64(3, /* 5 entries */, 4096)    = 160
getdents64(3, /* 0 entries */, 4096)    = 0
lstat("/usr/lib64/golang/src/pkg/errors/errors.go", {st_mode=S_IFLNK|0777,
st_size=52, ...}) = 0
lstat("/usr/lib64/golang/src/pkg/errors/example_test.go",
{st_mode=S_IFLNK|0777, st_size=58, ...}) = 0
lstat("/usr/lib64/golang/src/pkg/errors/errors_test.go",
{st_mode=S_IFLNK|0777, st_size=57, ...}) = 0

^ these are wrong

close(3)                                = 0
open("/usr/lib64/golang/src/pkg/errors/errors.go", O_RDONLY|O_CLOEXEC) = 3
read(3, "// Copyright 2011 The Go Authors"..., 4096) = 499
close(3)                                = 0
open("/usr/lib64/golang/src/pkg/errors/errors_test.go", O_RDONLY|O_CLOEXEC) = 3
read(3, "// Copyright 2011 The Go Authors"..., 4096) = 1271
close(3)                                = 0
open("/usr/lib64/golang/src/pkg/errors/example_test.go", O_RDONLY|O_CLOEXEC) =
3
read(3, "// Copyright 2012 The Go Authors"..., 4096) = 692
close(3)                                = 0

In the Fedora package, the source files are replaced by symlinks
to an architecture-neutral /usr/share directory:

$ ls -l /usr/lib64/golang/src/pkg/errors/
total 0
lrwxrwxrwx. 1 root root 52 Jul  1 12:04 errors.go ->
../../../../../share/golang/src/pkg/errors/errors.go
lrwxrwxrwx. 1 root root 57 Jul  1 12:04 errors_test.go ->
../../../../../share/golang/src/pkg/errors/errors_test.go
lrwxrwxrwx. 1 root root 58 Jul  1 12:04 example_test.go ->
../../../../../share/golang/src/pkg/errors/example_test.go

Unfortunately because the symlinks are "new" (the actual source files
are dated fine), go always tries (and fails) to recompile them.

Using stat instead of lstat would fix this.
@minux
Copy link
Member

minux commented Jul 2, 2013

Comment 1:

why not just symlink the src directory?

@agoode
Copy link

agoode commented Jul 2, 2013

Comment 2:

There are some build-dependent files that are generated and put into src during the
uild, so just symlinking src won't work. Those generated files have to be in /usr/lib or
/usr/lib64.

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 3:

cmd/go will definitely use syscall.Stat to determine the modification time of source
files,
it uses Lstat just to do filepath.EvalSymlinks.
i tried to reproduce the problem like this, but it doesn't reproduce.
$ ../bin/go install -v std
$ mv pkg/errors/errors.go pkg/errors/errors2.go
$ ln -s errors2.go pkg/errors/errors.go
$ ../bin/go install -v std
$
i even tried to move the source file to another partition, but as long as i perserve the
mtime, go won't try to reinstall the packages.
are you sure that you have preserved the correct mtime in /usr/share/golang/src/pkg/
directory tree?

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 4:

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

Comment 5 by richard.wm.jones:

There is no place in the strace output where it uses stat [not lstat] on 'errors.go', so
I don't see how it could even test the mtime of the target file.  This is with go 1.1.1
in Fedora.

@gopherbot
Copy link
Author

Comment 6 by richard.wm.jones:

Attached is the strace output.

Attachments:

  1. strace.txt (244625 bytes)

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 7:

did you pass -f to strace?
my output of "strace -f -olog ../bin/go install -v std" clearly show this (this is
output of "grep -A2 -B2 errors.go log"):
26033 getdents64(3, /* 6 entries */, 4096) = 200
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors_test.go", {st_mode=S_IFREG|0644,
st_size=1271, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors.go", {st_mode=S_IFREG|0644,
st_size=499, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/example_test.go", {st_mode=S_IFREG|0644,
st_size=692, ...}) = 0
26033 close(3)                          = 0
26033 open("/home/minux/s/go/src/pkg/errors/errors.go", O_RDONLY|O_CLOEXEC) = 3
26033 read(3, "// Copyright 2011 The Go Authors"..., 4096) = 499
26033 close(3)                          = 0
--
26033 getdents64(3, /* 6 entries */, 4096) = 200
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors_test.go", {st_mode=S_IFREG|0644,
st_size=1271, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors.go", {st_mode=S_IFREG|0644,
st_size=499, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/example_test.go", {st_mode=S_IFREG|0644,
st_size=692, ...}) = 0
26033 close(3)                          = 0
--
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner_test.go",
{st_mode=S_IFREG|0644, st_size=18676, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/errors.go", {st_mode=S_IFREG|0644,
st_size=3081, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/example_test.go",
{st_mode=S_IFREG|0644, st_size=1071, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner.go", {st_mode=S_IFREG|0644,
st_size=17022, ...}) = 0
26033 close(3)                          = 0
26033 open("/home/minux/s/go/src/pkg/go/scanner/errors.go", O_RDONLY|O_CLOEXEC) = 3
26033 read(3, "// Copyright 2009 The Go Authors"..., 4096) = 3081
26033 close(3)                          = 0
--
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner_test.go",
{st_mode=S_IFREG|0644, st_size=18676, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/errors.go", {st_mode=S_IFREG|0644,
st_size=3081, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/example_test.go",
{st_mode=S_IFREG|0644, st_size=1071, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner.go", {st_mode=S_IFREG|0644,
st_size=17022, ...}) = 0
--
26033 getdents64(3, /* 6 entries */, 4096) = 200
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors_test.go", {st_mode=S_IFREG|0644,
st_size=1271, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/errors.go", {st_mode=S_IFREG|0644,
st_size=499, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/errors/example_test.go", {st_mode=S_IFREG|0644,
st_size=692, ...}) = 0
26033 close(3)                          = 0
26033 open("/home/minux/s/go/src/pkg/errors/errors.go", O_RDONLY|O_CLOEXEC) = 3
26033 read(3, "// Copyright 2011 The Go Authors"..., 4096) = 499
26033 close(3)                          = 0
--
26033 getdents64(3, /* 0 entries */, 4096) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner_test.go",
{st_mode=S_IFREG|0644, st_size=18676, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/errors.go", {st_mode=S_IFREG|0644,
st_size=3081, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/example_test.go",
{st_mode=S_IFREG|0644, st_size=1071, ...}) = 0
26033 lstat("/home/minux/s/go/src/pkg/go/scanner/scanner.go", {st_mode=S_IFREG|0644,
st_size=17022, ...}) = 0
26033 close(3)                          = 0
26033 open("/home/minux/s/go/src/pkg/go/scanner/errors.go", O_RDONLY|O_CLOEXEC) = 3
26033 read(3, "// Copyright 2009 The Go Authors"..., 4096) = 3081
26033 close(3)                          = 0
--
26033 stat("/home/minux/s/go/pkg/linux_amd64/errors.a", {st_mode=S_IFREG|0644,
st_size=7100, ...}) = 0
26033 stat("/home/minux/s/go/pkg/linux_amd64/runtime.a", {st_mode=S_IFREG|0644,
st_size=1087872, ...}) = 0
26033 stat("/home/minux/s/go/src/pkg/errors/errors.go", {st_mode=S_IFREG|0644,
st_size=499, ...}) = 0
26033 stat("/home/minux/s/go/pkg/linux_amd64/sync/atomic.a", {st_mode=S_IFREG|0644,
st_size=8530, ...}) = 0
26033 stat("/home/minux/s/go/pkg/linux_amd64/runtime.a", {st_mode=S_IFREG|0644,
st_size=1087872, ...}) = 0
--
26033 stat("/home/minux/s/go/pkg/linux_amd64/unicode.a", {st_mode=S_IFREG|0644,
st_size=585092, ...}) = 0
26033 stat("/home/minux/s/go/pkg/linux_amd64/unicode/utf8.a", {st_mode=S_IFREG|0644,
st_size=22640, ...}) = 0
26033 stat("/home/minux/s/go/src/pkg/go/scanner/errors.go", {st_mode=S_IFREG|0644,
st_size=3081, ...}) = 0
26033 stat("/home/minux/s/go/src/pkg/go/scanner/scanner.go", {st_mode=S_IFREG|0644,
st_size=17022, ...}) = 0
26033 stat("/home/minux/s/go/pkg/linux_amd64/go/ast.a", {st_mode=S_IFREG|0644,
st_size=945492, ...}) = 0

@gopherbot
Copy link
Author

Comment 8 by richard.wm.jones:

What makes you think -f will help?  There are no other PIDs shown in your extract.
Try moving the go file to another directory entirely, so the symlink crosses a directory
boundary.

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 9:

because go programs are multithreaded.
$ strace -olog ../bin/go install -v std
$ ls -l log
-rw-r--r-- 1 minux minux 881546 Jul  3 21:39 log
$ strace -f -olog ../bin/go install -v std
$ ls -l log
-rw-r--r-- 1 minux minux 1952415 Jul  3 21:39 log
try it yourself to see the difference.
as i've explained, i even moved the source to another partition, and still couldn't
reproduce the issue.

@gopherbot
Copy link
Author

Comment 10 by richard.wm.jones:

Here's the strace -f output, but it's still not calling stat on errors.go.
Also the stat on various files that appear to be involved:
$ stat /usr/lib64/golang/src/pkg/errors/errors.go
  File: ‘/usr/lib64/golang/src/pkg/errors/errors.go’ -> ‘../../../../../share/golang/src/pkg/errors/errors.go’
  Size: 52          Blocks: 0          IO Block: 4096   symbolic link
Device: fd02h/64770d    Inode: 18588       Links: 1
Access: (0777/lrwxrwxrwx)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:lib_t:s0
Access: 2013-07-03 15:11:22.042166200 +0100
Modify: 2013-07-03 15:11:09.367183206 +0100
Change: 2013-07-03 15:11:09.368183204 +0100
 Birth: -
$ stat /usr/share/golang/src/pkg/errors/errors.go 
  File: ‘/usr/share/golang/src/pkg/errors/errors.go’
  Size: 499         Blocks: 8          IO Block: 4096   regular file
Device: fd02h/64770d    Inode: 21386       Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:usr_t:s0
Access: 2013-07-03 15:11:22.042166200 +0100
Modify: 2013-06-13 04:08:08.000000000 +0100
Change: 2013-07-03 15:11:12.155179500 +0100
 Birth: -
$ stat /usr/lib64/golang/pkg/linux_amd64/errors.a 
  File: ‘/usr/lib64/golang/pkg/linux_amd64/errors.a’
  Size: 6940        Blocks: 16         IO Block: 4096   regular file
Device: fd02h/64770d    Inode: 15567       Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:lib_t:s0
Access: 2013-07-03 15:11:22.095166128 +0100
Modify: 2013-06-19 16:18:50.000000000 +0100
Change: 2013-07-03 15:11:07.938185098 +0100
 Birth: -

Attachments:

  1. strace-f.txt (673302 bytes)

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 11:

what if you run "strace -f -olog go install -v std"?
please also show output of "stat /usr/lib64/golang/pkg/tool/linux_amd64/6g".

@minux
Copy link
Member

minux commented Jul 3, 2013

Comment 12:

the downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=973842
i've verified that downstream doesn't apply any patches to cmd/go and its dependencies.

@gopherbot
Copy link
Author

Comment 13 by richard.wm.jones:

File: ‘/usr/lib64/golang/pkg/tool/linux_amd64/6g’
  Size: 569731      Blocks: 1120       IO Block: 4096   regular file
Device: fd02h/64770d    Inode: 16914       Links: 1
Access: (0755/-rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Context: system_u:object_r:lib_t:s0
Access: 2013-07-03 15:11:22.071166161 +0100
Modify: 2013-06-19 16:18:49.000000000 +0100
Change: 2013-07-03 15:11:08.712184074 +0100
 Birth: -

Attachments:

  1. strace.log (2141574 bytes)

@minux
Copy link
Member

minux commented Jul 7, 2013

Comment 14:

i suspect the problem is that somehow runtime.a is newer than errors.a.
at line 23354, cmd/go is indeed using stat(2) to test modification time
of sources under pkg/runtime, and then go on to stat *.a, then it
determined that it must rebuild the errors package.

@minux
Copy link
Member

minux commented Jul 7, 2013

Comment 15:

the issue really isn't cmd/go's problem (cmd/go is using the correct syscall),
please report it back to the downstream packager.

Status changed to Invalid.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
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

3 participants