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.

Sign up for GitHub

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

Jump to bottom

os: fix Windows (*os.File).Seek on directory handle #36019

Closed
bradfitz opened this issue Dec 6, 2019 · 16 comments
Closed

os: fix Windows (*os.File).Seek on directory handle #36019

bradfitz opened this issue Dec 6, 2019 · 16 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Dec 6, 2019

Half of our Windows builders (windows-386-2008, windows-amd64-2016, windows-amd64-longtest) fail to seek an *os.File opened on a directory.

But windows-amd64-2008 and windows-amd64-2012 work.

Figure this out and re-enable the test on Windows. (The test was added late in the Go 1.14 cycle, just before beta)
@bradfitz bradfitz added help wanted OS-Windows NeedsFix The path to resolution is known, but the work has not been done. labels Dec 6, 2019
@bradfitz bradfitz added this to the Go1.15 milestone Dec 6, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/210283 mentions this issue: os: skip a new failing test on Windows

gopherbot pushed a commit that referenced this issue Dec 6, 2019
@bradfitz
os: skip a new failing test on Windows
a6c8fac 
This test was recently added in CL 209961.

Apparently Windows can't seek a directory filehandle?

And move the test from test/fixedbugs (which is mostly for compiler bugs) to
an os package test.

Updates #36019

Change-Id: I626b69b0294471014901d0ccfeefe5e2c7651788
Reviewed-on: https://go-review.googlesource.com/c/go/+/210283
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
@networkimprov
Copy link

cc @alexbrainman @mattn @zx2c4

@alexbrainman
Copy link
Member

Go use FindFirstFile, FindNextFile and FindClose Windows API to implement directory reading. The API does not provide functionality to seek to random position of the directory. I don't know what the alternatives are, if any.

Alex

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/213439 mentions this issue: os: document that File.Seek works on directories, but not portably

gopherbot pushed a commit that referenced this issue Jan 6, 2020
@bradfitz
os: document that File.Seek works on directories, but not portably
6b1a3f7 
Updates #36019

Change-Id: I9fea2c3c5138e2233290979e4724f6e7b91da652
Reviewed-on: https://go-review.googlesource.com/c/go/+/213439
Reviewed-by: Ian Lance Taylor <[email protected]>
@networkimprov
Copy link

Could File.Seek(0, os.SeekStart) be mapped to FindFirstFile() ?

@mattn
Copy link
Member

mattn commented Jan 7, 2020

man 3 fdopendir say:

fd is used internally by the implementation, and should not otherwise be used by the application.

https://linux.die.net/man/3/opendir

As far as I can see the doc, Go should not support Seek for directory.

@zx2c4
Copy link
Contributor

zx2c4 commented Jan 7, 2020

NtQueryDirectoryFile may be of use.

@alexbrainman
Copy link
Member

Could File.Seek(0, os.SeekStart) be mapped to FindFirstFile() ?

Yes. But people will still complain that File.Seel(1, os.SeekStart) fails. So, lets not pretend.

NtQueryDirectoryFile

Yes, it might work. But is all extra new code worth ability to File.Seek?

Alex

@networkimprov
Copy link

Could File.Seek(0, os.SeekStart) be mapped to FindFirstFile() ?

Yes. But people will still complain that File.Seel(1, os.SeekStart) fails. So, lets not pretend.

Well you could FindNextFile() in a loop for i < seek_pos?

@alexbrainman
Copy link
Member

Well you could FindNextFile() in a loop for i < seek_pos?

I don't understand what you are proposing. Please, send a CL with the change, if you know what to do.

Thank you.

Alex

@networkimprov

This comment has been minimized.

Sign in to view
@alexbrainman

This comment has been minimized.

Sign in to view
@networkimprov

This comment has been minimized.

Sign in to view
@odeke-em odeke-em modified the milestones: Go1.15, Go1.16 May 23, 2020
@odeke-em
Copy link
Member

Punting to Go1.17 as we didn’t get much action for the Go1.16 development cycle.

@odeke-em odeke-em modified the milestones: Go1.16, Go1.17 Dec 25, 2020
@ianlancetaylor ianlancetaylor modified the milestones: Go1.17, Backlog Apr 21, 2021
@jingxu97 jingxu97 mentioned this issue May 18, 2021
Merged
@qmuntal qmuntal mentioned this issue May 10, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/405275 mentions this issue: os,syscall: File.Stat to use file handle for directories on Windows

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/458336 mentions this issue: os: reenable TestReaddirSmallSeek on windows

gopherbot pushed a commit that referenced this issue Dec 19, 2022
@qmuntal @neild
os: reenable TestReaddirSmallSeek on windows
a5a4744 
TestReaddirSmallSeek should have been reenabled as part of
CL 405275, but didn't. Do it now.

Updates #36019

Change-Id: I5676eee4e63675d30e9d48ac708e72bd036b6aee
Reviewed-on: https://go-review.googlesource.com/c/go/+/458336
Reviewed-by: Alex Brainman <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Quim Muntal <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: David Chase <[email protected]>
@ferrmin ferrmin mentioned this issue Dec 20, 2022
Merged
@qmuntal qmuntal mentioned this issue Apr 5, 2023
Closed
@golang golang locked and limited conversation to collaborators Dec 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@bradfitz @mattn @zx2c4 @networkimprov @ianlancetaylor @odeke-em @gopherbot @alexbrainman