proposal: os: add Touch to set access/mod times to current time

[sybrenstuvel]

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

$ go version
go version go1.12.5 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/flamencoadmin/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/flamencoadmin/go"
GOPROXY=""
GORACE=""
GOROOT="/opt/go"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/flamencoadmin/gotouch/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build400479428=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Call os.Chtimes(filepath, now, now) where filepath is poining to a regular file on an SMB share and now := time.Now().

What did you expect to see?

No error, and the mtime of the file changed to 'now'.

What did you see instead?

An os.PathError is returned as follows:

Error type: *os.PathError
Error msg: chtimes /mnt/flamenco-input/file-store/stored/d6/2d13ed80d57784d9c5113a4bb32541c2b23a1e7ef02117f49084985bee144d/8271557.blob: operation not permitted
    op  : chtimes
    path: /mnt/flamenco-input/file-store/stored/d6/2d13ed80d57784d9c5113a4bb32541c2b23a1e7ef02117f49084985bee144d/8271557.blob
    err : operation not permitted

The SMB share is served from a Microsoft SMB server (an Azure Files share), which apparently doesn't support setting the modification/access time to a specific timestamp. However, what it does support is setting the mtime to "now" using the Linux touch CLI application. By using strace touch thefile I found that it actually calls utimensat() with NULL as timestamp, rather than passing an explicit timestamp.

To test whether passing NULL would work, I added the following function to src/syscall/syscall_linux.go:

func UtimesNanoNow(path string) (err error) {
	err = utimensat(_AT_FDCWD, path, nil, 0)
	if err != ENOSYS {
		return err
	}
	return utimes(path, nil)
}

The following now works fine on the SMB share:

func ChtimesNow(name string) error {
	if e := syscall.UtimesNanoNow(name); e != nil {
		return &os.PathError{"chtimes", name, e}
	}
	return nil
}

I tested this on Linux (4.18.0-1014-azure) on Ubuntu (18.04.2). The SMB share was mounted with the following options, as per the Microsoft documentation:

//${ACCOUNT_NAME}.file.core.windows.net/flamenco-resources on /mnt/flamenco-resources type cifs (rw,relatime,vers=3.0,sec=ntlmssp,cache=strict,username=${ACCOUNT_NAME},password=${ACCOUNT_PASSWORD},dir_mode=0770,file_mode=0775,gid=flamenco,forcegid,sec=ntlmssp,mfsymlinks)

[ianlancetaylor]

I guess the question is whether we should add os.ChtimesNow to support this edge case, or whether we should expect programs to call unix.UtimesNanoAt, from the golang.org/x/sys/unix package, instead.

[sybrenstuvel]

Touching a file is a common operation. I suspect it is even more common than setting the timestamps to a different time, although that's just my gut feeling.

I vote for os.ChtimesNow (or simply call it os.Touch), as the operation is at the same level of abstraction as os.Chtimes. Just setting a file's mtime to "now" should IMHO not require any platform-specific branches in your code.

unix.UtimesNanoAt doesn't accept nil, so I doubt that it'll fix this issue. Or did you mean my UtimesNanoNow function @ianlancetaylor ?

[ianlancetaylor]

When I look at the implementation of unix.UtimesNanoAt I see that it does accept nil as the []Timespec argument, and that it does the right thing. This dates back to https://golang.org/cl/12690.

[ianlancetaylor]

I turned this issue into a proposal for os.Touch.

[beoran]

If this is named Touch(), then will it also create the file if it does not exist, as per the touch unix command?

[sybrenstuvel]

@beoran for me personally that wouldn't be necessary, but I wouldn't object (unless it delays the availability of the function). All I need is a Chtimes(now) call that works for my use case.

[ianlancetaylor]

It seems that there are two special cases for os.Chtimes: leave the time unchanged, and set the time to "now". Adding os.Touch will help with the latter but not the former.

[bradfitz]

Other gross/weird idea: add sentinel time.Time values for UTIME_NOW and UTIME_OMIT, perhaps within a sentinel *time.Location.

[ianlancetaylor]

I kind of like that, actually. We could add os.ChtimesUnchanged and os.ChtimesNow of type time.Time.

[sybrenstuvel]

It seems that there are two special cases for os.Chtimes: leave the time unchanged, and set the time to "now".

Would you mind linking the source of that information?

We could add os.ChtimesUnchanged

What would the normal English interpretation be of a name like ChtimesUnchanged? To me, "Change time unchanged" sounds cryptic at best.

What is the use of calling a "change times" function with argument "do not change"?

[ianlancetaylor]

The source for the observation that there are two special cases is 1) examination of actual calls to os.Chtimes in Google's code, and 2) the observation that the Linux kernel supports two special cases UTIME_NOW and UTIME_OMIT (see http://man7.org/linux/man-pages/man2/utimensat.2.html).

I agree that os.ChtimesUnchanged is not the best name, happy to hear alternatives. Note that it is not a function, but a value of type time.Time.

The use of the argument is that os.Chtimes takes two arguments for two different times, and some code wants to only change one of them. Currently that code must first call Stat to get the time to pass to os.Chtimes; an os.ChtimesUnchanged, whatever we call it, would simplify that code.

[bradfitz]

We don't need Chtimes in the name. It'd look pretty stuttery with them:

os.Chtimes(path, os.ChtimesUnchanged, os.ChtimesNow)

How about:

package os

// Special time.Time values for os.Chtimes:
var (
    // KeepTime is recognized by Chtimes to mean that the access or modification
    // time should not be changed. It is not a valid Time value otherwise.
    KeepTime  = time.Unix(0, 1).In(utimeLocation)
    // TouchTime is recognized by Chtimes to mean that the access or modification
    // time should be set to the current time. It is not a valid Time value otherwise.
    TouchTime = time.Unix(0, 2).In(utimeLocation)
)

[sybrenstuvel]

@bradfitz that looks good to me!

[rsc]

The special time location bothers me but a sentinel time seems OK.
We already have the zero time for 'don't update this', so really we just need a sentinel for "now".
Maybe time.Date(10000, 1, 1, 0, 0, 0, 0, time.UTC)?

[beoran]

I already use such a value as a sentinel for "forever" in a large professional application. I don't see how it could mean "now". We already have time.Now() for this use case. Op di 28 mei 2019 22:51 schreef Russ Cox <notifications@github.com>:

…

The special time location bothers me but a sentinel time seems OK. We already have the zero time for 'don't update this', so really we just need a sentinel for "now". Maybe time.Date(10000, 1, 1, 0, 0, 0, 0, time.UTC)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31880?email_source=notifications&email_token=AAARM6IC32AEO5VQZ4BHPFDPXWLNHA5CNFSM4HLHNDM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWNMWSY#issuecomment-496683851>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAARM6O5UNTEZGU7HLAKOGLPXWLNHANCNFSM4HLHNDMQ> .

[bradfitz]

time.Now isn't a sentinel value. It's a function that returns a time value.

[ianlancetaylor]

Also kind of the point of this issue is that there is at least one case where using time.Now() doesn't work.

[rsc]

Last week we talked about a sentinel for meaning "tell the FS 'now'". One problem with this approach is that it only works when people think to use it, and more importantly not doing it only breaks on Azure Files or other SMB servers. So most code will work everywhere except Azure and it will be hard to get everyone to fix their code for a use case they may not directly need.

One possible alternative, following a bit @beoran's suggestion, is to say that if the time passed to Chtimes is "within epsilon" of the current time, then Chtimes just automatically tells the file system to use the special "now" form instead of an explicit time. For example you could define "within epsilon" to mean "has monotonic time at most 100ms earlier than now". That would have an unfortunate effect if you did os.Chtimes(time.Now()) and the code was interrupted between those calls. But it would work most of the time without requiring everyone to learn a new way to spell that call.

On balance it is probably still better to have the sentinel instead?

[rsc]

Does anyone know more about the underlying issue with Azure Files / SMB? Is this a fundamental limitation of the SMB protocol that there is no "set mtime" RPC, or is it a problem specific to Azure Files not implementing that call?

/cc @ashleymcnamara

[sybrenstuvel]

Last week we talked about a sentinel for meaning "tell the FS 'now'". One problem with this approach is that it only works when people think to use it, and more importantly not doing it only breaks on Azure Files or other SMB servers. So most code will work everywhere except Azure and it will be hard to get everyone to fix their code for a use case they may not directly need.

This is a good point. I think it can be partially solved by good documentation, making the correct choice (using the sentinel value) more obvious than using the probably-works choice. This is also partially why my initial suggestion had a different, parameterless function; when ChTimes() and ChTimesNow() appear next to each other in a module function list, it's easier to just pick the ChTimesNow() and harder to think "I can use time.Now() and pass its return value to ChTimes()".

I would certainly add a warning to ChTimes() that states that setting an arbitrary mtime/atime isn't supported on all filesystems, and that the sentinel value for 'now' has wider filesystem support than passing an explicit time.Now().

One possible alternative, following a bit @beoran's suggestion, is to say that if the time passed to Chtimes is "within epsilon" of the current time, then Chtimes just automatically tells the file system to use the special "now" form instead of an explicit time. For example you could define "within epsilon" to mean "has monotonic time at most 100ms earlier than now". That would have an unfortunate effect if you did os.Chtimes(time.Now()) and the code was interrupted between those calls. But it would work most of the time without requiring everyone to learn a new way to spell that call.

I wouldn't like this approach; I don't like code that should work most of the time. Sure, in the point you make above we also have code that works most of the time. The distribution of failure is different though: it reliably never works on those Microsoft SMB servers, and it reliably always works on other filesystems. The "epsilon approach" would unreliably work most of the time, which I think is much worse.

[rsc]

Right now we have:

• os.Chtimes(a, m) - sets a, m

We might want:

• os.Chtimes(_, m) - set only m
• os.Chtimes(a, _) - set only a
• os.Chtimes(_, NOW) - set only m to NOW
• os.Chtimes(NOW, _) - set only a to NOW
• os.Chtimes(NOW, NOW) - set both to NOW

We can handle the "don't set this" case with time.Time{}. That seems unobjectionable. What's left is the NOW cases.

We could add os.ChtimesNow but with no arguments it would only cover one of those three. To cover all three we'd need arguments, like os.Chtimes(false, true), which is mysterious. We could add os.ChtimesModNow and os.ChtimesAccessNow but that would leave no way to do the "NOW, NOW" and guarantee they got the same time. None of these sound good. We need a better idea.

I am still confused about why it is not possible to set the times to anything at all. I looked briefly at the latest CIFS spec and can't see where it says "you can't set atime/mtime to anything but right now" nor do I even see "here is how to set them to an abstract right now". Or is Azure imposing the constraint? If so, where is that documented? I'd really like to hear something authoritative from Microsoft before we start adding workarounds that show up in package os's exported, long-term API. (/cc @erikstmartin)

It seems like maybe Go is not using the right Windows call inside Chtimes. If that's the case, let's fix that.

/cc @spf13

[rsc]

Started #32558 for the _ (don't set me) case. This issue will be about time.Now() vs NOW vs Azure.

[spf13]

I was working on a program recently that encountered a similar issue. Specifically that from Go, SMB operations don't respect calls to change the mtime of files.

[beoran]

I think os.Chtimes is doing too much at once. I would split it into os.Chatime and os.Chmtime, and give it a (time.Time, is.Chtimeflags) signature, where the flags are used to indicate special features such as time.Now.

[ianlancetaylor]

@beoran That doesn't provide a clear way to set both times to "now" and to the same "now". Also it's worth noting that on Unix and Windows the underlying system call takes both times. And also there is really only one flag--set the time to "now"--and it's hard to see why there would ever be another one.

[beoran]

Yes, you are right. I checked the system calls SetFileTime() and utimens(), and I realized that now. It makes sense because if two system calls were needed, it would be impossible to synchronize both times. But I do notice that on both OS, the system calls do no take a wall clock time like LPSYSTEMTIME or struct time, but a specific file-related time, FILETIME or struct timespec, which allows more easily to specify special values than time.Time does.

So, I would say that this is then the problem with os.Chtimes, it takes plain time.Time, but it should probably take a file system specific time, e.g, os.FileTime. For backwards compatibility, os.Chtimes can be left as it it, and the new function that takes the os.FileTime could be named something like os.Chfiletimes. The os.FileTime could be something like type FileTime struct { time.Time ; Now bool ; Ignore bool }.

I think this would be better than making an arbitrary time.Time{} "magical" in the sense that it will set the time to NOW, since that arbitrary time.Time risks to have already being used in existing code for other purposes.

[sybrenstuvel]

os.FileTime could be something like type FileTime struct { time.Time ; Now bool ; Ignore bool }

To me this looks like a hassle, compared to just passing a time.Time instance. Furthermore, it allows for the invalid combination Now=true; Ignore=true; those two are mutually exclusive.

[rsc]

I don't think we should make any decisions here without some kind of definitive clarification about what the Azure- or SMB-imposed limitations are. We are designing around something and we don't even know what shape it is. Let's not kick around any new APIs until we understand that better.

See #31880 (comment) and #31880 (comment).

/cc @ashleymcnamara @erikstmartin

[rsc]

The original report claims that using Chtimes with any specific time fails on Linux talking to a Microsoft Azure Files server over SMB. We do not understand why this restriction would exist - the CIFS protocol clearly supports sending a time in the various calls that change file modification times.

We asked for details about what the specific CIFS/SMB/WinAPI/Azure restrictions might be, in #31880 (comment) and #31880 (comment) and #31880 (comment). No answers here.

In the absence of information about what the underlying constraints are, it is essentially impossible to decide what an appropriate Go API change would be. Without new information, this issue seems like a likely decline (not enough information).

Leaving open for a week for final comments.

[ianlancetaylor]

Marked this last week as likely decline w/ call for last comments (#31880 (comment)).
Declining now.

[sybrenstuvel]

I couldn't answer earlier as I was on holiday.

We asked for details about what the specific CIFS/SMB/WinAPI/Azure restrictions might be, in #31880 (comment) and #31880 (comment) and #31880 (comment). No answers here.

I'm far from a CIFS/SMB expert, I just know that one thing worked and another thing didn't. I was hoping someone else from the Golang community could chip in.