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

Jump to bottom

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

Open
bradfitz opened this issue Dec 6, 2019 · 14 comments
Open

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

bradfitz opened this issue Dec 6, 2019 · 14 comments
Labels
help wanted NeedsFix OS-Windows
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

@bradfitz 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 this to the Go1.15 milestone Dec 6, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 6, 2019

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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@networkimprov
Copy link

@networkimprov networkimprov commented Dec 11, 2019

cc @alexbrainman @mattn @zx2c4

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Dec 21, 2019

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

@gopherbot gopherbot commented Jan 6, 2020

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 <iant@golang.org>
@networkimprov
Copy link

@networkimprov networkimprov commented Jan 6, 2020

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

@mattn
Copy link
Member

@mattn 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 zx2c4 commented Jan 7, 2020

NtQueryDirectoryFile may be of use.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Jan 10, 2020

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

@networkimprov networkimprov commented Jan 10, 2020

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

@alexbrainman alexbrainman commented Jan 10, 2020

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 was marked as off-topic.

Sign in to view
@alexbrainman

This comment was marked as off-topic.

Sign in to view
@networkimprov

This comment was marked as off-topic.

Sign in to view
@odeke-em odeke-em removed this from the Go1.15 milestone May 23, 2020
@odeke-em odeke-em added this to the Go1.16 milestone May 23, 2020
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 25, 2020

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

@odeke-em odeke-em removed this from the Go1.16 milestone Dec 25, 2020
@odeke-em odeke-em added this to the Go1.17 milestone Dec 25, 2020
@ianlancetaylor ianlancetaylor removed this from the Go1.17 milestone Apr 21, 2021
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Apr 21, 2021
@jingxu97 jingxu97 mentioned this issue May 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix OS-Windows
Projects
None yet
Milestone
Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
@bradfitz @mattn @zx2c4 @networkimprov @ianlancetaylor @odeke-em @gopherbot @alexbrainman