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

os: allow Chtimes with time.Time{} to avoid setting time #32558

Open
rsc opened this issue Jun 11, 2019 · 8 comments
Open

os: allow Chtimes with time.Time{} to avoid setting time #32558

rsc opened this issue Jun 11, 2019 · 8 comments

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Jun 11, 2019

os.Chtimes(f, a, m) sets the access and modification times of the file f to a, m.
The underlying call can usually say "leave this one unset" to set just one of the two.
We could support that by defining that time.Time{} means "don't set this one".
We probably should do that.

Thoughts?

Related to but different from #31880.

@beoran

This comment has been minimized.

Copy link

@beoran beoran commented Jun 12, 2019

While this would be an improvement, it might have some issues with backwards compatibility.

Therefore I would propose, as I did in the related issue to have a new function os.Chfiletimes that takes os.FileTime in stead of time.Time for the last two arguments, where os.FileTime could be something like type FileTime struct { time.Time ; Now bool ; Ignore bool }.

@rsc rsc changed the title proposal: os: allow Chtimes with time.Time to avoid setting time proposal: os: allow Chtimes with time.Time{} to avoid setting time Jun 25, 2019
@rsc

This comment has been minimized.

Copy link
Contributor Author

@rsc rsc commented Jun 25, 2019

Need to understand #31880 before we can reasonably decide something here - maybe more is needed and we can make one change instead of two. But #31880 is blocked on knowing more about the Windows-induced constraints.

@rsc

This comment has been minimized.

Copy link
Contributor Author

@rsc rsc commented Aug 27, 2019

Re @beoran's comment, we confirmed that Chtimes with the zero time right now ends up in 1754 on a Linux file system. So if we adopted the zero time convention then use on older versions would not recognize or reject the zero time and would end up in 1754 instead.

@rsc

This comment has been minimized.

Copy link
Contributor Author

@rsc rsc commented Aug 27, 2019

Given that it looks like #31880 may be declined, we'll probably want to decide this next. It seems like the main question is whether anyone needs this functionality. Does anyone need this?

(The backwards compatibility issue can be solved by just waiting a couple releases before starting to use a change.)

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 4, 2019

I looked at 150 uses of os.Chtimes in Google code. I found 20 cases where it appeared that it would be convenient to leave one of the times unchanged.

Coincidentally I found 21 cases that set one or both times to "now".

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Nov 7, 2019

I've mentioned a half dozen times to @ianlancetaylor and @griesemer that I thought we already did this and I've tried to find old CLs (that were never submitted?) or old threads on this, but all along it seems I was remembering https://golang.org/pkg/os/#Chown instead which says:

A uid or gid of -1 means to not change that value

The fact that Chown already does this pattern and I assumed we did the same for Chtimes seems like a vote in favor of this proposal.

@rsc

This comment has been minimized.

Copy link
Contributor Author

@rsc rsc commented Nov 13, 2019

Matching os.Chown's approach for ignoring part of the operation seems like a decent argument for doing the same in os.Chtimes.

This seems like a likely accept in the absence of any objections or arguments against it.

Leaving open for a week for any final comments.

@rsc

This comment has been minimized.

Copy link
Contributor Author

@rsc rsc commented Nov 20, 2019

No change in consensus. Accepted.

@rsc rsc modified the milestones: Proposal, Go1.15 Nov 20, 2019
@rsc rsc changed the title proposal: os: allow Chtimes with time.Time{} to avoid setting time os: allow Chtimes with time.Time{} to avoid setting time Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.