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: Chtimes fails on FAT32 #33335

Closed
bep opened this issue Jul 28, 2019 · 10 comments
Closed

os: Chtimes fails on FAT32 #33335

bep opened this issue Jul 28, 2019 · 10 comments

Comments

@bep
Copy link
Contributor

@bep bep commented Jul 28, 2019

This is a refurbished version of #33334

I some times refer Hugo users to this issue tracker when I'm 99.9% certain that this is the right place for such an issue.

I don't have the bandwidth to track down all the Go issues I find on that project. I do when it hurts me enough, and I do maintain a large test suite that I test on 3x3 platforms/Go versions, which sometimes finds bugs/regressions in Go itself. Which is a win/win in this big dugnad project we call open source.

Which is why I would appreciate if you don't just brush it off as a non-issue if you see some unrelated package in the bug report.

gohugoio/hugo#6127

/cc @gj52

@dominikh

This comment has been minimized.

Copy link
Member

@dominikh dominikh commented Jul 28, 2019

@bep I'm sorry if I've caused you additional, unnecessary work, but I hope you can appreciate how "chtimes fails on FAT32" as a bug report is vastly different from asking "why is there a colon in the error message of this software I am using".

I did not close the original issue because it was referred from Hugo, I closed it because it provided no useful information, had a straightforward answer to the asked question, and made no indication that it was originally filed elsewhere. Rather, it looked like one of many instances of "I'm using a program written in Go and it displays an error I don't understand". I'm sure you can see that such a description doesn't imply a bug in Go itself, and rather looks like difficulty in using some software written in Go. And it is natural to refer users to the issue tracker of the software they are having difficulty with.

@dominikh

This comment has been minimized.

Copy link
Member

@dominikh dominikh commented Jul 28, 2019

And, of course, to be remotely able to debug this issue, the user needs to fill in the issue template, and provide a minimal example that reproduces the issue. Which implies they need to be able to write and run Go code. They also need to dig into Hugo to figure out how exactly it is calling chtimes.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Jul 31, 2019

@bep I have no idea what your problem is, because you did not explain. So I just tried few things.

In particular we have TestChtimes test in os package. And it tests os.Chtimes. I inserted USB flash disk into my computer (as D: drive), and then did this

C:\>set TMP=D:\temp

C:\>set TEMP=D:\temp

C:\>test -test.v -test.run=TestChtimes$
=== RUN TestChtimes
--- FAIL: TestChtimes (0.05s)
    os_test.go:1178: AccessTime didn't go backwards; was=2019-07-31 00:00:00 +1000 AEST, after=2019-07-31 00:00:00 +1000 AEST
    os_test.go:1183: ModTime didn't go backwards; was=2019-07-31 05:31:42 +1000 AEST, after=2019-07-31 05:31:42 +1000 AEST
FAIL

C:\>

So the TestChtimes indeed fails when FAT is used.

But os.Chtimes uses Windows API SetFileTime to implement whatever it does. And according to https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfiletime

Not all file systems can record creation and last access times and not all file systems record them in the same manner. For example, on FAT, create time has a resolution of 10 milliseconds, write time has a resolution of 2 seconds, and access time has a resolution of 1 day (really, the access date).

And, indeed, if I change TestChtimes

diff --git a/src/os/os_test.go b/src/os/os_test.go
index c101281258..c01f4d8f94 100644
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -1144,7 +1144,7 @@ func testChtimes(t *testing.T, name string) {
  // Move access and modification time back a second
  at := Atime(preStat)
  mt := preStat.ModTime()
-	err = Chtimes(name, at.Add(-time.Second), mt.Add(-time.Second))
+	err = Chtimes(name, at.Add(-2*time.Second), mt.Add(-2*time.Second))
  if err != nil {
   t.Fatalf("Chtimes %s: %s", name, err)
  }

it passes. So it is hit and miss when you use FAT.

Let me know, if you want us to do anything else for this issue.

Thank you.

Alex

@gj52

This comment has been minimized.

Copy link

@gj52 gj52 commented Jul 31, 2019

sound good like a solution :-)
The colon seems a part of the error message and not of the parameter / path.
change it to t.Fatalf("Chtimes '%s': %s", name, err) ???

@agnivade

This comment has been minimized.

Copy link
Contributor

@agnivade agnivade commented Aug 9, 2019

@gj52 - From the comment above, it seems like it is WAI. So there is no solution here, because there is no bug. The t.Fatalf line is in a test and will not appear when you use the os package. What you want to be changed is essentially the String method of PathError

func (e *PathError) Error() string { return e.Op + " " + e.Path + ": " + e.Err.Error() }

You are certainly welcome to improve the error message, but this issue is specifically about Chtimes failing on FAT32.

@gj52

This comment has been minimized.

Copy link

@gj52 gj52 commented Aug 9, 2019

@agnivade Hi, I would change the parameter sequence, first the error and than the wrong parameters
Don't know if GOLANG has guidelines for error messages ;-)

@dominikh

This comment has been minimized.

Copy link
Member

@dominikh dominikh commented Aug 11, 2019

We're not going to change the format of this error. It has been stable for years, and is consistent with other errors.

Also, please, let's stay on topic. This issue is about Chtimes failing, not the styling of the error.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Aug 28, 2019

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this Aug 28, 2019
@mshade

This comment has been minimized.

Copy link

@mshade mshade commented Jan 13, 2020

Wanted to comment and mention that this issue also appears to affect exFAT. The following is encountered during a docker build when Docker's storage resides on an exFAT formatted partition on Windows:

ERROR: chtimes \\?\D:\ProgramData\Docker\tmp\docker-builder907137105\.git: The parameter is incorrect.

@VRandme

This comment has been minimized.

Copy link

@VRandme VRandme commented Mar 18, 2020

I can also replicate this error for exfat.
I encountered this when Hugo tried to build from an exfat source
Error: Error copying static files: chtimes : The parameter is incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.