x/sys/unix: Stat_t.Atim is named Atimespec on darwin #31735

guonaihong · 2019-04-29T01:25:01Z

What version of Go are you using (`go version`)?

$ go version
go version go1.11.1 linux/amd64

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/guo/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/guo/go"
GOPROXY=""
GORACE=""
GOROOT="/home/guo/go-version/go1.11.1"
GOTMPDIR=""
GOTOOLDIR="/home/guo/go-version/go1.11.1/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build480480625=/tmp/go-build -gno-record-gcc-switches"

What did you do?

cat test.go

package main

import (
	"fmt"
	"golang.org/x/sys/unix"
	"os"
	"time"
)

func statTimes(name string) (atime, mtime, ctime time.Time, err error) {
	var stat unix.Stat_t
	err = unix.Stat(name, &stat)
	if err != nil {
		return
	}

	atime = time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec))
	mtime = time.Unix(int64(stat.Mtim.Sec), int64(stat.Mtim.Nsec))
	ctime = time.Unix(int64(stat.Ctim.Sec), int64(stat.Ctim.Nsec))
	return
}

func main() {

	fmt.Println(statTimes(os.Args[0]))
}

linux

env GOPATH=`pwd` CGO_ENABLED=0 GOOS=linux GOARCH=amd64  go run test.go

output

2019-04-29 09:16:40.531596746 +0800 CST 2019-04-29 09:16:40.52759687 +0800 CST 2019-04-29 09:16:40.52759687 +0800 CST <nil>

darwin

env GOPATH=`pwd` CGO_ENABLED=0 GOOS=darwin GOARCH=amd64  go run test.go

output

./test.go:17:30: stat.Atim undefined (type unix.Stat_t has no field or method Atim)
./test.go:18:30: stat.Mtim undefined (type unix.Stat_t has no field or method Mtim)
./test.go:19:30: stat.Ctim undefined (type unix.Stat_t has no field or method Ctim)

What did you expect to see?

Mac platform is the same as linux output

What did you see instead?

Mac platform error

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

bradfitz · 2019-04-29T05:38:06Z

@tklauser, @ianlancetaylor, IIRC this is named differently in x/sys/unix because it was in syscall? Is that enough of an argument? Renaming it would break more people, though.

I think the best option would probably be to add some accessors methods (both get & set) for those fields that are portable.

ianlancetaylor · 2019-04-29T22:00:35Z

The names in the syscall package vary on various systems.

js, nacl: two fields: Atime, AtimeNsec, both int64
aix: Atim with type StTimespec_t
darwin, freebsd, netbsd: Atimespec with type Timespec
dragonfly, linux, openbsd, solaris: Atim with type Timespec

The names in the x/sys/unix package also vary, and are slightly different:

aix: Atim with type StTimespec
darwin, netbsd: Atimespec with type Timespec
dragonfly, freebsd, linux, openbsd, solaris: Atim with type Timespec

freebsd/386 also has an Atim_ext field, I'm not sure what that is.

These are all of course a result of different paths that the operating systems have taken to expand the traditional st_atime field beyond holding just seconds.

Since the fields in x/sys/unix are all type Timespec or an equivalent, I think we could seriously consider renaming all of them to Atim with type Timespec. But accessor methods would also work. We would need six methods (get and set for each of Atim, Mtim, Ctim`), but by using build tags we would only need three implementations.

bradfitz · 2019-04-29T23:12:41Z

I'd support a breaking change if you do. People using modules wouldn't get a surprise break until an update anyway.

tklauser · 2019-05-02T10:13:00Z

I'd be fine with a breaking change as well. FWIW, we already did the Atimespec -> Atim rename on freebsd (as part of the ino64support by @paulzhol in https://golang.org/cl/136816).

paulzhol · 2019-05-02T10:30:24Z

We've reverted syscall back to Atimespec in http://golang.org/cl/155958 though.

bradfitz · 2019-05-02T14:40:55Z

@paulzhol, std/syscall has stricter compatibility rules, though. We can do more cleanups in x/sys/unix.

paulzhol · 2019-05-02T17:46:52Z

@bradfitz it should be [ACM]time according to posix.
I'm just worried that we'd to end up with something like m4/stat-time.m4 and lib/stat-time.h the more systems we add.

Also I was under the assumption that with the syscall freeze, x/sys/unix would be a replacement which would allow more rapid changes independent of the standard compiler and library. Not to provide a unified low-level view of all unix systems.

ianlancetaylor · 2019-05-02T17:59:32Z

Yes, st_atim should work in C on all Unix systems. The problem is that Go is underlying at the underlying structs, and they have different names on different systems. C programmers don't normally see those names. It seems worthwhile to arrange for the x/sys/unix package to provide consistent names across systems, just as C programmers see. The only downside is breaking compatibility on some systems. I don't think this as taking a position on whether x/sys/unix should provide a unified low-level view of all Unix systems; it's just the appropriate response for this specific case.

gopherbot · 2019-05-02T18:46:35Z

Change https://golang.org/cl/175037 mentions this issue: unix: add portable Stat_t time accessors

bradfitz · 2019-05-02T18:49:05Z

Here's the other option of adding accessors instead of renaming fields: https://go-review.googlesource.com/c/sys/+/175037

Thoughts?

paulzhol · 2019-05-02T18:53:07Z

Can we pre-process cgo -godefs input with cpp with a _POSIX_C_SOURCE=1 to get the translated names in ztypes_*?
edit: nevermind, it might work for darwin, but at least on FreeBSD the field members are different regardless (st_atim). The _POSIX_C_SOURCE just defines the accessors to be at_atime or at_atimespec.

tklauser · 2019-05-03T06:24:54Z

Here's the other option of adding accessors instead of renaming fields: https://go-review.googlesource.com/c/sys/+/175037

Thoughts?

Nice. Now that I see how straightforward the implementation is, I slightly prefer this to renaming the fields. I think it is less brittle than some preprocessor magic and/or ugly regex based postprocessing in mkpost.go.

Also, this would in principle be backportable to package syscall.

gopherbot · 2019-05-03T09:34:28Z

Change https://golang.org/cl/175157 mentions this issue: unix: rename Stat_t time fields to [AMC]time and Birthtime

paulzhol · 2019-05-03T09:39:45Z

The rename version is at https://golang.org/cl/175157. Please run the try bots I'm sure stuff are broken.

tklauser · 2019-05-03T09:55:58Z

The rename version is at https://golang.org/cl/175157. Please run the try bots I'm sure stuff are broken.

TryBots seem happy but they wont test on many of the affected geese (e.g. most BSDs, solaris, aix).

paulzhol · 2019-05-03T10:07:35Z

I also think the accessors CL is elegant, but we already have two versions of it due to syscall differences:

https://github.com/golang/go/blob/master/src/os/stat_unix.go
https://github.com/golang/go/blob/master/src/os/stat_linux.go
https://github.com/golang/go/blob/master/src/os/stat_freebsd.go
plus many more

and

https://github.com/golang/go/blob/master/src/archive/tar/stat_unix.go
https://github.com/golang/go/blob/master/src/archive/tar/stat_actime1.go
https://github.com/golang/go/blob/master/src/archive/tar/stat_actime2.go

bradfitz · 2019-05-03T14:55:00Z

@paulzhol, we can ignore std for the purpose of deciding x/sys's API. If anything, perhaps we can even make std use x/sys/unix instead of std's syscall. Russ said he didn't want two syscall packages in std, but he never said the one couldn't be x/sys/unix. So perhaps we clean up x/sys/unix and then switch std to using it.

ianlancetaylor · 2019-05-14T20:47:10Z

Let's rename the fields to the names used on GNU/Linux.

gopherbot · 2019-05-15T21:01:44Z

Change https://golang.org/cl/177437 mentions this issue: unix: fix TestStatFieldNames on aix and TestUtimesNanoAt on darwin

Following CL 175157 which renames Stat_t time fields to [AMCB]tim. Updates golang/go#31735 Change-Id: I0791c59bab307d237b315c1b919265902f7d9917 Reviewed-on: https://go-review.googlesource.com/c/sys/+/177437 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

bradfitz changed the title ~~Unix.stat_t structure cannot be compiled on mac platform~~ x/sys/unix: Stat_t.Atim is named Atimespec on darwin Apr 29, 2019

gopherbot added this to the Unreleased milestone Apr 29, 2019

bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 29, 2019

guonaihong mentioned this issue May 14, 2019

add AmcTime get last access,modify,change time golang/sys#33

Closed

ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label May 14, 2019

gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 14, 2019

gopherbot closed this as completed in golang/sys@87c8727 May 15, 2019

golang locked and limited conversation to collaborators May 14, 2020

gopherbot added the FrozenDueToAge label May 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/sys/unix: Stat_t.Atim is named Atimespec on darwin #31735

x/sys/unix: Stat_t.Atim is named Atimespec on darwin #31735

guonaihong commented Apr 29, 2019 •

edited

bradfitz commented Apr 29, 2019

ianlancetaylor commented Apr 29, 2019

bradfitz commented Apr 29, 2019

tklauser commented May 2, 2019

paulzhol commented May 2, 2019

bradfitz commented May 2, 2019

paulzhol commented May 2, 2019 •

edited

ianlancetaylor commented May 2, 2019

gopherbot commented May 2, 2019

bradfitz commented May 2, 2019

paulzhol commented May 2, 2019 •

edited

tklauser commented May 3, 2019 •

edited

gopherbot commented May 3, 2019

paulzhol commented May 3, 2019

tklauser commented May 3, 2019

paulzhol commented May 3, 2019

bradfitz commented May 3, 2019

ianlancetaylor commented May 14, 2019

gopherbot commented May 15, 2019

x/sys/unix: Stat_t.Atim is named Atimespec on darwin #31735

x/sys/unix: Stat_t.Atim is named Atimespec on darwin #31735

Comments

guonaihong commented Apr 29, 2019 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

linux

output

darwin

output

What did you expect to see?

What did you see instead?

bradfitz commented Apr 29, 2019

ianlancetaylor commented Apr 29, 2019

bradfitz commented Apr 29, 2019

tklauser commented May 2, 2019

paulzhol commented May 2, 2019

bradfitz commented May 2, 2019

paulzhol commented May 2, 2019 • edited

ianlancetaylor commented May 2, 2019

gopherbot commented May 2, 2019

bradfitz commented May 2, 2019

paulzhol commented May 2, 2019 • edited

tklauser commented May 3, 2019 • edited

gopherbot commented May 3, 2019

paulzhol commented May 3, 2019

tklauser commented May 3, 2019

paulzhol commented May 3, 2019

bradfitz commented May 3, 2019

ianlancetaylor commented May 14, 2019

gopherbot commented May 15, 2019

guonaihong commented Apr 29, 2019 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

paulzhol commented May 2, 2019 •

edited

paulzhol commented May 2, 2019 •

edited

tklauser commented May 3, 2019 •

edited