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

cmd/vet, cmd/go: vet crashes for long paths in Windows, affecting go test #46299

Open
ajgajg1134 opened this issue May 20, 2021 · 12 comments
Open

cmd/vet, cmd/go: vet crashes for long paths in Windows, affecting go test #46299

ajgajg1134 opened this issue May 20, 2021 · 12 comments

Comments

@ajgajg1134
Copy link

@ajgajg1134 ajgajg1134 commented May 20, 2021

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

$ go version
go version go1.16.2 windows/amd64

Does this issue reproduce with the latest release?

Yes, confirmed with go1.16.4 as well.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Andrew\AppData\Local\go-build
set GOENV=C:\Users\Andrew\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Andrew\go\pkg\mod
set GONOPROXY=gitlab.com/vistaprint-org/*
set GONOSUMDB=gitlab.com/vistaprint-org/*
set GOOS=windows
set GOPATH=C:\Users\Andrew\go
set GOPRIVATE=gitlab.com/vistaprint-org/*
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\Andrew\go\go1.16.2
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Users\Andrew\go\go1.16.2\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.16.2
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
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=C:\Users\Andrew\AppData\Local\Temp\go-build1429628315=/tmp/go-build -gno-record-gcc-switches

What did you do?

Have a test file with a long path import something with a very long path.
For example: Have a file at:
C:\Users\Andrew\go\src\somegitplace.test\mycompany-org\manufacturing-software\availability\material-location-availability-listener
and in that file import something like:
somegitplace.test/mycompany-org/manufacturing-software/availability/material-location-availability-service/domain/types/endoflifestatus

Run go test ./...

What did you expect to see?

Tests passing (or failing)

What did you see instead?

"C:\Users\Andrew\go\go1.16.4\pkg\tool\windows_amd64\vet.exe: fork/exec C:\Users\Andrew\go\go1.16.4\pkg\tool\windows_amd64\vet.exe: The directory name is invalid."

@networkimprov
Copy link

@networkimprov networkimprov commented May 21, 2021

@egonelbre
Copy link
Contributor

@egonelbre egonelbre commented May 21, 2021

I'm unable to reproduce the issue.

@ajgajg1134 which version of Windows are you using? Some Windows versions have a MAX_PATH limit of 260 (see https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation).

Which filesystem are you using? e.g. FAT32 has a 255 character limit.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented May 21, 2021

I'm unable to reproduce the issue.

@egonelbre if you are checking on current tip, you might be affected by

https://go-review.googlesource.com/c/go/+/291291

Alex

@egonelbre
Copy link
Contributor

@egonelbre egonelbre commented May 21, 2021

@alexbrainman I was using Go 1.16.4 1.16.3

@ajgajg1134
Copy link
Author

@ajgajg1134 ajgajg1134 commented May 21, 2021

I'm on Windows 10 Home, Version 10.0.19042 Build 19042.
I was also able to reproduce this issue on a colleague's machine and he is running Windows 10 Pro OS build 19042.985

As for the path length limit, couldn't the tools work around that if they provided a \\?\ UNC path? If that's not possible could the error messages be improved so that it's obvious the failure is due to path length?

To help with reproduction of this issue I created a test repository where I can consistently force failure by running go test ./...: https://github.com/ajgajg1134/test-golang-failure-with-long-path-windows EDIT: I'm having trouble making a consistently reproducible github repo. It seems like the issue is easier to reproduce when the path becomes too long after being vendored locally. I'll update here once I get a solid repro in github

One interesting bit here that might help is the it seems this issue is confined to vet. If I run go test -vet=off ./... then I can successfully complete the tests.

@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented May 21, 2021

From src/cmd/go/internal/work/exec.go:

        runErr := b.run(a, p.Dir, p.ImportPath, env, cfg.BuildToolexec, tool, vetFlags, a.Objdir+"vet.cfg")

I wonder if p.Dir needs to go through the fixupLongPath routine?

@ajgajg1134
Copy link
Author

@ajgajg1134 ajgajg1134 commented May 21, 2021

Update: I've managed to make a repo that can consistently fail(for real this time): https://github.com/ajgajg1134/test-golang-fail2

Using the vendor folder makes it possible / much more likely that a user will hit this path length limit.

@dmitshur dmitshur added this to the Backlog milestone May 21, 2021
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented May 21, 2021

@dmitshur dmitshur changed the title cmd/vet (and therefore go test) crashes for long paths in Windows cmd/vet, cmd/go: vet crashes for long paths in Windows, affecting go test May 21, 2021
@networkimprov
Copy link

@networkimprov networkimprov commented May 21, 2021

If vet calls filepath.Walk then this may be the problem described in #21782.

A fix for that and a related bug is described in #36375 (comment)

It's a simple fix. I don't know why it hasn't been tried.

@zpavlinovic
Copy link

@zpavlinovic zpavlinovic commented May 25, 2021

From src/cmd/go/internal/work/exec.go:

        runErr := b.run(a, p.Dir, p.ImportPath, env, cfg.BuildToolexec, tool, vetFlags, a.Objdir+"vet.cfg")

I wonder if p.Dir needs to go through the fixupLongPath routine?

My current examination of the issue suggests that would fix the issue:

builder.run(..., p.Dir, ...) --> builder.runOut(..., p.Dir, ...) --> exec.Cmd.Dir = p.Dir

I am testing this hypothesis now. However, fixLongPath is a private member of os package. Not sure how can we use it directly but I am currently interested if that would actually solve it.

@networkimprov
Copy link

@networkimprov networkimprov commented May 25, 2021

What stdlib call is p.Dir passed to ultimately? The fix probably belongs there.

@zpavlinovic
Copy link

@zpavlinovic zpavlinovic commented May 26, 2021

It looks to me that it is ultimately passed to syscall.StartProcess. I didn't go further than that.

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