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: CreateFile fails on Windows/exFAT after CL 143578 #29214

Closed
egonelbre opened this issue Dec 13, 2018 · 13 comments
Closed

os: CreateFile fails on Windows/exFAT after CL 143578 #29214

egonelbre opened this issue Dec 13, 2018 · 13 comments

Comments

@egonelbre
Copy link
Contributor

@egonelbre egonelbre commented Dec 13, 2018

On a fairly clean Windows system running make.bat with master (944a9c7) fails when using exFAT filesystem for the temporary / work directory.

Building Go cmd/dist using C:\Go
Building Go toolchain1 using C:\Go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
# sync/atomic
GetFileInformationByHandleEx $WORK\b015\asm.o: The parameter is incorrect.
# internal/cpu
GetFileInformationByHandleEx $WORK\b006\cpu_x86.o: The parameter is incorrect.
# runtime/internal/atomic
GetFileInformationByHandleEx $WORK\b012\asm_amd64.o: The parameter is incorrect.
go tool dist: FAILED: c:\Go.tip\pkg\tool\windows_amd64\go_bootstrap install -gcflags=all= -ldflags=all= -i cmd/asm cmd/cgo cmd/compile cmd/link: exit status 2
The system cannot find the batch label specified - fail

I bisected the issue to f108158.

It seems

err = windows.GetFileInformationByHandleEx(h, windows.FileAttributeTagInfo, (*byte)(unsafe.Pointer(&ti)), uint32(unsafe.Sizeof(ti)))
is failing for exFAT.

Sidenote The system cannot find the batch label specified - fail seems to be due to .bat files not using windows line endings

@alexbrainman

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

go1.11.2 windows/amd64 for bootstrap.

Windows 10 Pro, Version 1809 (OS Build 117763.194)

go.tip env Output
c:\Go.tip\src>go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=c:\Go.tip\pkg\obj\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\egone\go
set GOPROXY=
set GORACE=
set GOROOT=c:\Go.tip
set GOTMPDIR=
set GOTOOLDIR=c:\Go.tip\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=Z:\go-build032936922=/tmp/go-build -gno-record-gcc-switches

Bisect results

bisect good d154ef6
c:\Go.tip\src>git co d154ef60a0c88be98c70bbe
Previous HEAD position was a1ee0a21cf runtime, time: refactor startNano handling
HEAD is now at d154ef60a0 path/filepath: change IsAbs("NUL") to return true
...
c:\Go.tip\src>make.bat
Building Go cmd/dist using C:\Go
Building Go toolchain1 using C:\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:\Go.tip
Installed commands in c:\Go.tip\bin
bisect bad f108158
c:\Go.tip\src>git co f10815898c0732e2e6cdb697d6f95f33f865
Previous HEAD position was d154ef60a0 path/filepath: change IsAbs("NUL") to return true
HEAD is now at f10815898c os: use CreateFile for Stat of symlinks
...
c:\Go.tip\src>make.bat
Building Go cmd/dist using C:\Go
Building Go toolchain1 using C:\Go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
# sync/atomic
GetFileInformationByHandleEx $WORK\b015\asm.o: The parameter is incorrect.
# runtime/internal/atomic
GetFileInformationByHandleEx $WORK\b012\asm_amd64.o: The parameter is incorrect.
# internal/cpu
GetFileInformationByHandleEx $WORK\b006\cpu_x86.o: The parameter is incorrect.
go tool dist: FAILED: c:\Go.tip\pkg\tool\windows_amd64\go_bootstrap install -gcflags=all= -ldflags=all= -i cmd/asm cmd/cgo cmd/compile cmd/link: exit status 2
The system cannot find the batch label specified - fail
@egonelbre egonelbre changed the title cmd/go: master fails to build on Windows cmd/go: master fails to build on Windows with exFAT Dec 13, 2018
@bcmills bcmills changed the title cmd/go: master fails to build on Windows with exFAT os: CreateFile fails on Windows/exFAT after CL 143578 Dec 13, 2018
@bcmills bcmills added the OS-Windows label Dec 13, 2018
@bcmills bcmills added this to the Go1.12 milestone Dec 13, 2018
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Dec 13, 2018

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 15, 2018

Change https://golang.org/cl/154377 mentions this issue: os: make Stat work on FAT file system

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Dec 15, 2018

@egonelbre thank you for creating this issue. Please try https://go-review.googlesource.com/c/go/+/154377 to see if it fixes your problem.

I am not sure how to test my change - we need disk with FAT file system on it. Maybe we can attach disk with FAT to our builders.

Alex

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

@egonelbre egonelbre commented Dec 15, 2018

It's possible to setup virtual disks with VHD: https://docs.microsoft.com/en-us/powershell/module/hyper-v/mount-vhd?view=win10-ps

Building Go master now works.

However, here's output from tests running on all different file-systems with drive as the temp (e.g. Z:\) or subdir as the temp (e.g. Z:\Temp):

  1. Windows Default NTFS TEMP=%USERPROFILE%\AppData\Local\Temp https://gist.github.com/egonelbre/69c48199f391be6b545468f88d72e8f5#file-default-txt
  2. NTFS https://gist.github.com/egonelbre/69c48199f391be6b545468f88d72e8f5#file-ntfs-txt
  3. FAT https://gist.github.com/egonelbre/69c48199f391be6b545468f88d72e8f5#file-fat-txt
  4. FAT32 https://gist.github.com/egonelbre/69c48199f391be6b545468f88d72e8f5#file-fat32-txt
  5. exFAT https://gist.github.com/egonelbre/69c48199f391be6b545468f88d72e8f5#file-exfat-txt

Also, due to a typo I noticed that filepath.Join("Z:", "test") returns "Z:test". So if you set TEMP=Z: lokt of tests start failing. And I suspect some of the tests, when you specify TEMP=Z:\, fail for the same reason.

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

@egonelbre egonelbre commented Dec 15, 2018

Found #26953 with regards to filepath.Join("Z:", "test").

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

@egonelbre egonelbre commented Dec 15, 2018

Ok, seems that when you have TEMP=Z:\ then os.TempDir() returns "Z:" instead of "Z:\", which causes tests to fail.

PS: having temp dir as the root drive is definitely a rare occasion and can be worked around by the user.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Dec 16, 2018

It's possible to setup virtual disks with VHD: https://docs.microsoft.com/en-us/powershell/module/hyper-v/mount-vhd?view=win10-ps

Yes. We can, probably, create FAT volumes and mount them before builder boots. And have whatever we want tested there. We could, probably, use similar strategy for #28547 too.

However, here's output from tests running on all different file-systems with drive as the temp (e.g. Z:\) or subdir as the temp (e.g. Z:\Temp):

I am not surprised that some tests fail in that environment. But FAT file system does not provide many functions that we all expect. So I am not worried that some tests fail.

I don't use FAT file system myself. If you think, there are some important failures there to fix, please, create new issues for them, and than we can decide what to do.

Ok, seems that when you have TEMP=Z:\ then os.TempDir() returns "Z:" instead of "Z:\", which causes tests to fail.

PS: having temp dir as the root drive is definitely a rare occasion and can be worked around by the user.

I think it is an obvious bug. I filled #29291 to track it.

Alex

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

@egonelbre egonelbre commented Dec 16, 2018

Basic code for VHD manipulation:

$vhd = ".\fat32.vhd"

New-VHD -Path $vhd -SizeBytes 128MB
Mount-VHD -Path $vhd -Passthru |
    Initialize-Disk -Passthru |
    New-Partition -DriveLetter W -UseMaximumSize |
    Format-Volume -FileSystem FAT32 -Confirm:$false -Force

Dismount-VHD -Path $vhd

As for the failures, they are expected:

  1. Links don't work on FAT (TestHardLink, TestDirectoryJunction, TestNTNamespaceSymlink)
  2. Long paths don't work on FAT (TestLongPath, TestMkdirAllExtendedLength)
  3. AccessTime has 1 hour resolution, ModTime has 2 second resolution on FAT (TestChtimes, TestChtimesDir)

So they behave on FAT as I would expect. Tests could skip those, but not sure whether it's important enough. (Detection using GetVolumeInformationA)

For me using FAT filesystem was partly an accident.

@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Dec 17, 2018

Kindly paging @bradfitz @dmitshur for perhaps some ideas on attaching FAT filesystems to the builders as requested by @alexbrainman's #29214 (comment)

@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 17, 2018

What @egonelbre posted above looks good. Can a test just do that? Or t.Skip if permissions are lacking?

If you want something more, please file a builder bug and cc @dmitshur.

@egonelbre

This comment has been minimized.

Copy link
Contributor Author

@egonelbre egonelbre commented Dec 17, 2018

Can a test just do that?

@bradfitz Not sure which part of it.

For VHD setup you do need Administrator access. I suspect setting it up with PowerShell prior to launching tests, would be much easier to manage. But, it is possible to use Windows API for it (https://stackoverflow.com/questions/24396644/programmatically-mount-a-microsoft-virtual-hard-drive-vhd), but that still would require admin permissions for running them (and implementing the API).

So you would additionally need https://stackoverflow.com/a/8196291 to check prior to invoking these. Otherwise people would start getting a UAC request every time they run Go tests.

For checking what you can do with a drive, GetVolumeInformation returns info such as FILE_SUPPORTS_HARD_LINKS, lpMaximumComponentLength. I don't think they require any special permissions, other than being able to read/write the disk. For access/modified time I didn't find the API that returns time granularity, but the test could also use larger offset than 1s to make it pass.

I would probably setup builder with it, rather than changing it in tests. Then re-run go test os with TEMP set to that drive.

@gopherbot gopherbot closed this in e453577 Dec 17, 2018
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Dec 17, 2018

IIRC our builders run as Administrator. If they wanted to, they could create the FAT-on-RAMDISK as needed, running either Powershell or Windows APIs... whatever's easiest.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Dec 29, 2018

Basic code for VHD manipulation:

I tried that, but it does not work on my computer. Apparently you need Hyper-V installed for that. And I cannot install Hyper-V, because it is incompatible with VirtualBox that I use. But if someone can try to stick these commands into $GOPATH/golang.org/x/build/env/windows/startup.ps1 and recreate our builders system with it, we could try and write some tests that use them.

Alex

@golang golang locked and limited conversation to collaborators Dec 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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