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: ReadDir fails on file systems without File ID support on Windows #61907

Closed
hajimehoshi opened this issue Aug 9, 2023 · 32 comments
Closed
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@hajimehoshi
Copy link
Member

hajimehoshi commented Aug 9, 2023

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

$ go version
go version go1.21.0 windows/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
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\hajimehoshi\AppData\Local\go-build
set GOENV=C:\Users\hajimehoshi\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\hajimehoshi\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\hajimehoshi\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.21.0
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=\\Mac\Home\unctest\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\HAJIME~1\AppData\Local\Temp\go-build3663691949=/tmp/go-build -gno-record-gcc-switches

What did you do?

Prepare these files in a UNC directory starting with \\ like \\Mac\Home\unctest. Here, I assume the current directory is \\Mac\Home\unctest:

\\Mac\Home\unctest\go.mod:

module foo

go 1.21

\\Mac\Home\unctest\main.go

package main

import (
        "foo/bar"
)

func main() {
        bar.Hello()
}

\\Mac\Home\unctest\bar\hello.go

package bar                                                                                                                                                                                                                                                                           
                                                                                                                                                                                                                                                                                      
func Hello() {                                                                                                                                                                                                                                                                        
        println("bar")                                                                                                                                                                                                                                                                
} 

And run go run . at \\Mac\Home\unctest.

What did you expect to see?

bar is printed

What did you see instead?

Running the program fails:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run .
GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.
@hajimehoshi
Copy link
Member Author

I have confirmed Go 1.20 worked:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go1.20.7 run .
bar

So, this is a regression.

@hajimehoshi
Copy link
Member Author

I could minimize the test case:

\\Mac\Home\unctest\go.mod:

module foo

go 1.21

\\Mac\Home\unctest\main.go:

package main

func main() {
        println("hello")
}

and then run go run .:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run .
GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.

@hajimehoshi hajimehoshi changed the title cmd/go: importing a package whose files are in a UNC path fails cmd/go: compiling fails when the current directory is a UNC path Aug 9, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Aug 9, 2023

CC @bcmills maybe?

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 9, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 9, 2023
@hajimehoshi
Copy link
Member Author

go run ./main.go

succeeded, and

go mod tidy

failed. I think using something related to a module fails when the current directory is a UNC path.

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

I wasn't aware that it was even possible for the current working directory to be a UNC path. (To my knowledge it still isn't possible with cmd.exe.) 😵‍💫

I suspect that this may be a side effect of https://go.dev/cl/452995 (attn @qmuntal).

(CC @golang/windows)

@bcmills bcmills modified the milestones: Backlog, Go1.22 Aug 9, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

@hajimehoshi, does os.ReadDir(`\\Mac\Home\unctest`) succeed for that directory? What kind of filesystem is serving that path?

@hajimehoshi
Copy link
Member Author

@bcmills

does os.ReadDir(\\Mac\Home\unctest) succeed for that directory?

Apparently no:

package main

import (
        "os"
)

func main() {
        _, err := os.ReadDir(`\\Mac\Home\unctest`)
        if err != nil {
                println("os.ReadDir failed!: ", err.Error())
        }
}
PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run .\main.go
os.ReadDir failed!:  GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.

What kind of filesystem is serving that path?

I'm using Parallels. The host is macOS and the guest is Windows. \\Mac\Home is a path for my home directory of macOS from Windows.

@bcmills bcmills changed the title cmd/go: compiling fails when the current directory is a UNC path os: ReadDir fails for UNC paths shared with Windows using Parallels Aug 9, 2023
@bcmills bcmills removed the GoCommand cmd/go label Aug 9, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

@gopherbot, please backport to Go 1.21. This is a significant regression in ReadDir.

@gopherbot
Copy link

Backport issue(s) opened: #61910 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@hajimehoshi
Copy link
Member Author

hajimehoshi commented Aug 9, 2023

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

Makes sense. Do you happen to know whether your share is using SMB or AFP?

@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

And, another question: if you mount the UNC path as a drive letter, and refer to it by the drive letter instead of the UNC path, does ReadDir still fail?

@hajimehoshi
Copy link
Member Author

I'm not familiar with this, but smb, maybe?

if you mount the UNC path as a drive letter, and refer to it by the drive letter instead of the UNC path, does ReadDir still fail?

I'll try tomorrow.

@hajimehoshi
Copy link
Member Author

hajimehoshi commented Aug 10, 2023

Z: is the drive letter for the same home directory.

package main

import (
        "os"
)

func main() {
        if _, err := os.ReadDir(`\\Mac\Home\unctest`); err != nil {
                println("os.ReadDir with UNC failed!: ", err.Error())
        }
        if _, err := os.ReadDir(`Z:\`); err != nil {
                println("os.ReadDir with Z: failed!: ", err.Error())
        }
}

and

PS Z:\unctest> go run .
GetFileInformationByHandleEx Z:\unctest: The parameter is incorrect.
PS Z:\unctest> go run .\main.go
os.ReadDir with UNC failed!:  GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.
os.ReadDir with Z: failed!:  GetFileInformationByHandleEx Z:\: The parameter is incorrect.

So, whether the path is UNC or not doesn't matter!

EDIT: Go 1.20 worked (after modifying go.mod for Go 1.20):

PS Z:\unctest> go1.20.7 run .
PS Z:\unctest> go1.20.7 run .\main.go

@qmuntal
Copy link
Contributor

qmuntal commented Aug 10, 2023

Thanks for the detailed report @hajimehoshi. I suspect that your file system doesn't support traversing directories using FILE_ID_BOTH_DIR_INFO, probably because it can't generate unique 64-bit file IDs.

I'll prepare a CL to fallback to FILE_FULL_DIR_INFO if FILE_ID_BOTH_DIR_INFO fails, which doesn't require file IDs.

@gopherbot
Copy link

Change https://go.dev/cl/518195 mentions this issue: os: support file systems without file IDs when reading directores on windows

@qmuntal
Copy link
Contributor

qmuntal commented Aug 10, 2023

@hajimehoshi can you test if CL 452995 fixes your issue?

You need to follow these steps to run the Go toolchain at CL 452995:

go install golang.org/dl/gotip@latest
gotip download 518195
gotip run .

Edit: fix CL number

@hajimehoshi
Copy link
Member Author

hajimehoshi commented Aug 10, 2023

@qmuntal This was not fixed unfortunately:

PS C:\Users\hajimehoshi> go install golang.org/dl/gotip@latest
This will download and execute code from golang.org/cl/452995, continue? [y/n] y
Fetching CL 452995, Patch Set 15...
remote: Finding sources: 100% (25940/25940)
remote: Total 25940 (delta 16811), reused 22125 (delta 16811)

Resolving deltas: 100% (16811/16811), completed with 564 local objects.
From https://go.googlesource.com/go
 * branch                  refs/changes/95/452995/15 -> FETCH_HEAD
Updating files: 100% (4355/4355), done.
Previous HEAD position was 0a1a092c4b Revert "cmd/cgo: use --no-gc-sections if available"
HEAD is now at 1951857ec0 os: use handle based APIs to read directories on windows
Building Go cmd/dist using C:\Program Files\Go. (go1.21.0 windows/amd64)
Building Go toolchain1 using C:\Program Files\Go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for windows/amd64.
---
Installed Go for windows/amd64 in C:\Users\hajimehoshi\sdk\gotip
Installed commands in C:\Users\hajimehoshi\sdk\gotip\bin
Success. You may now run 'gotip'!
PS Z:\unctest> gotip version
go version devel go1.21-1951857ec0 Tue Jan 24 13:26:00 2023 +0000 windows/amd64
PS Z:\unctest> gotip run .
GetFileInformationByHandleEx Z:\unctest: The parameter is incorrect.
PS Z:\unctest> gotip run ./main.go
os.ReadDir with UNC failed!:  GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.
os.ReadDir with Z: failed!:  GetFileInformationByHandleEx Z:\: The parameter is incorrect.

@hajimehoshi
Copy link
Member Author

@hajimehoshi
Copy link
Member Author

PS C:\Users\hajimehoshi> gotip download 518195
This will download and execute code from golang.org/cl/518195, continue? [y/n] y
Fetching CL 518195, Patch Set 2...
From https://go.googlesource.com/go
 * branch                  refs/changes/95/518195/2 -> FETCH_HEAD
HEAD is now at e17e4a5e3f os: support file systems without file IDs when reading directories on windows
ERROR: Cannot find C:\Users\hajimehoshi\Go1.4\bin\go.exe
Set GOROOT_BOOTSTRAP to a working Go tree >= Go 1.20.6.
Success. You may now run 'gotip'!

So, should I install go at ~/Go1.4...?

@gopherbot
Copy link

gopherbot commented Aug 10, 2023

Change https://go.dev/cl/518196 mentions this issue: [release-branch.go1.21] os: support file systems without file IDs when reading directories on windows

@qmuntal
Copy link
Contributor

qmuntal commented Aug 10, 2023

Wasn't the fix https://go-review.googlesource.com/c/go/+/518195, not https://go-review.googlesource.com/c/go/+/452995 by the way?

Yep, my fault.

So, should I install go at ~/Go1.4...?

CL 518195 is based on master, which needs at least go1.20 to build. You probably don't have it installed.

I've just cherry-picked it into go1.21 release branch: https://go-review.googlesource.com/c/go/+/518196. Please try with gotip download 518196.

@hajimehoshi
Copy link
Member Author

hajimehoshi commented Aug 10, 2023

@qmuntal Yep, the patch seems to work now.

PS C:\Users\hajimehoshi> gotip download 518196
This will download and execute code from golang.org/cl/518196, continue? [y/n] y
Fetching CL 518196, Patch Set 4...
remote: Finding sources: 100% (47719/47719)
Receiving objects: 100% (47719/47719), 58.93 MiB | 6.33 MiB/s, done.
remote: Total 47719 (delta 31888), reused 42826 (delta 31888)
Resolving deltas: 100% (31888/31888), completed with 675 local objects.
From https://go.googlesource.com/go
 * branch                  refs/changes/96/518196/4 -> FETCH_HEAD
Warning: you are leaving 262 commits behind, not connected to
any of your branches:

  e17e4a5e3f os: support file systems without file IDs when reading directories on windows
  d9f7e1dc73 runtime: fix asan asm on amd64
  e6637f3293 os: test that copying to append-only files doesn't fail on Linux
  6c5fc6d7ce all: update vendored dependencies
 ... and 258 more.

HEAD is now at ddd112b9f7 [release-branch.go1.21] os: support file systems without file IDs when reading directories on windows
Building Go cmd/dist using C:\Program Files\Go. (go1.21.0 windows/amd64)
Building Go toolchain1 using C:\Program Files\Go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for windows/amd64.
---
Installed Go for windows/amd64 in C:\Users\hajimehoshi\sdk\gotip
Installed commands in C:\Users\hajimehoshi\sdk\gotip\bin
Success. You may now run 'gotip'!
PS Z:\unctest> gotip version
go version go1.21.0 windows/amd64
PS Z:\unctest> gotip run .
PS Z:\unctest> gotip run .\main.go

@qmuntal qmuntal changed the title os: ReadDir fails for UNC paths shared with Windows using Parallels os: ReadDir fails on fyle systems without File ID support on Windows Aug 10, 2023
@hajimehoshi hajimehoshi changed the title os: ReadDir fails on fyle systems without File ID support on Windows os: ReadDir fails on file systems without File ID support on Windows Aug 10, 2023
@qmuntal
Copy link
Contributor

qmuntal commented Aug 11, 2023

@hajimehoshi could you check if os.SameFile returns true for two different files under \\Mac\Home\ using go1.21? I don't have a file system without file ID support myself, and I'm wondering if os.SameFile has always been broken in that situation.

@hajimehoshi
Copy link
Member Author

@qmuntal This seems to work as expected, even with the current Go 1.21.0.

package main

import (
        "fmt"
        "os"
)

func main() {
        // \\Mac\Home\ and Z:\ are the same path.

        f1, err := os.Stat(`\\Mac\Home\unctest\main.go`)
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        f2, err := os.Stat(`Z:\unctest\main.go`)
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        fmt.Printf("os.SameFile(f1, f1): %t\n", os.SameFile(f1, f1))
        fmt.Printf("os.SameFile(f2, f2): %t\n", os.SameFile(f2, f2))
        fmt.Printf("os.SameFile(f1, f2): %t\n", os.SameFile(f1, f2))
}

Not patched:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run ./main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f1, f2): true

Patched:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> gotip run ./main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f1, f2): true

@hajimehoshi
Copy link
Member Author

Oh two different files? Wait a sec.

@hajimehoshi
Copy link
Member Author

package main

import (
        "fmt"
        "os"
)

func main() {
        f1, err := os.Stat(`\\Mac\Home\unctest\main.go`)
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        f2, err := os.Stat(`\\Mac\Home\unctest\go.mod`)
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        fmt.Printf("os.SameFile(f1, f1): %t\n", os.SameFile(f1, f1))
        fmt.Printf("os.SameFile(f2, f2): %t\n", os.SameFile(f2, f2))
        fmt.Printf("os.SameFile(f1, f2): %t\n", os.SameFile(f1, f2))
}

Not patched:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run ./main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f1, f2): false

Patched:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> gotip run ./main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f1, f2): false

Is this expected?

@qmuntal
Copy link
Contributor

qmuntal commented Aug 11, 2023

Is this expected?

I was expecting it to always return true, because your file system doesn't seem to support file IDs, and os.SameFile doesn't handle this case properly.

Here is another experiment:

package main

import (
        "fmt"
        "os"
)

func main() {
       files, err := os.ReadDir(`\\Mac\Home\unctest`)
       if err != nil {
                println("os.ReadDir with UNC failed!: ", err.Error())
        }
        f1, _ := files[0].Info()
        f2, _ := files[1].Info()
        f3, err := os.Stat(`\\Mac\Home\unctest\` + f1.Name())
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        f4, err := os.Stat(`\\Mac\Home\unctest\` + f2.Name())
        if err != nil {
                fmt.Fprintf(os.Stderr, "%v\n", err)
        }
        fmt.Printf("os.SameFile(f1, f1): %t\n", os.SameFile(f1, f1))
        fmt.Printf("os.SameFile(f2, f2): %t\n", os.SameFile(f2, f2))
        fmt.Printf("os.SameFile(f3, f3): %t\n", os.SameFile(f3, f3))
        fmt.Printf("os.SameFile(f4, f4): %t\n", os.SameFile(f4, f4))
        fmt.Printf("os.SameFile(f1, f2): %t\n", os.SameFile(f1, f2))
        fmt.Printf("os.SameFile(f1, f3): %t\n", os.SameFile(f1, f3))
        fmt.Printf("os.SameFile(f2, f3): %t\n", os.SameFile(f2, f3))
        fmt.Printf("os.SameFile(f2, f4): %t\n", os.SameFile(f2, f4))
}

Use go1.20 and go1.21 patched please. There might has been os.SameFile regression that is not fixed by CL 518196.

@hajimehoshi
Copy link
Member Author

Go 1.20:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go1.20.7 run .\main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f3, f3): true
os.SameFile(f4, f4): true
os.SameFile(f1, f2): false
os.SameFile(f1, f3): true
os.SameFile(f2, f3): false
os.SameFile(f2, f4): true

Go 1.21:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go version
go version go1.21.0 windows/amd64
PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> go run .\main.go
os.ReadDir with UNC failed!:  GetFileInformationByHandleEx \\Mac\Home\unctest: The parameter is incorrect.
panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
main.main()
        //Mac/Home/unctest/main.go:13 +0x605
exit status 2

Go 1.21 patched:

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> gotip run .\main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f3, f3): true
os.SameFile(f4, f4): true
os.SameFile(f1, f2): true
os.SameFile(f1, f3): false
os.SameFile(f2, f3): false
os.SameFile(f2, f4): false

@qmuntal
Copy link
Contributor

qmuntal commented Aug 11, 2023

Thanks! There is clearly something weird in go1.21 that hasn't been fixed in CL 518196. I'll give it another pass to see what's happening.

@qmuntal
Copy link
Contributor

qmuntal commented Aug 11, 2023

Done. @hajimehoshi can you updated the patched go1.21 toolchain(run gotip download 518196), and try again the previous experiment?

@qmuntal qmuntal added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 11, 2023
@hajimehoshi
Copy link
Member Author

PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> gotip download 518196
This will download and execute code from golang.org/cl/518196, continue? [y/n] y
Fetching CL 518196, Patch Set 5...
remote: Finding sources: 100% (47720/47720)
Receiving objects: 100% (47720/47720), 58.92 MiB | 6.34 MiB/s, done.

Resolving deltas: 100% (31893/31893), completed with 675 local objects.
From https://go.googlesource.com/go
 * branch                  refs/changes/96/518196/5 -> FETCH_HEAD
Warning: you are leaving 1 commit behind, not connected to
any of your branches:

  ddd112b9f7 [release-branch.go1.21] os: support file systems without file IDs when reading directories on windows

HEAD is now at 7ba8a3b518 [release-branch.go1.21] os: support file systems without file IDs when reading directories on windows
Building Go cmd/dist using C:\Program Files\Go. (go1.21.0 windows/amd64)
Building Go toolchain1 using C:\Program Files\Go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for windows/amd64.
---
Installed Go for windows/amd64 in C:\Users\hajimehoshi\sdk\gotip
Installed commands in C:\Users\hajimehoshi\sdk\gotip\bin
Success. You may now run 'gotip'!
PS Microsoft.PowerShell.Core\FileSystem::\\Mac\Home\unctest> gotip run .\main.go
os.SameFile(f1, f1): true
os.SameFile(f2, f2): true
os.SameFile(f3, f3): true
os.SameFile(f4, f4): true
os.SameFile(f1, f2): false
os.SameFile(f1, f3): true
os.SameFile(f2, f3): false
os.SameFile(f2, f4): true

This seems to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Development

No branches or pull requests

5 participants