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

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
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Backlog

Comments

@ajgajg1134
Copy link

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

cc @alexbrainman @rasky @zx2c4 @mattn

@gopherbot add OS-Windows

@egonelbre
Copy link
Contributor

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

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 commented May 21, 2021 •
edited

@alexbrainman I was using Go 1.16.4 1.16.3

@ajgajg1134
Copy link
Author

ajgajg1134 commented May 21, 2021 •
edited

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 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

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 dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 21, 2021
@dmitshur
Copy link
Contributor

dmitshur commented May 21, 2021 •
edited

CC @matloob, @timothy-king via owners.

Also CC @bufflig.

@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

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
Contributor

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.

@zpavlinovic zpavlinovic added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label May 25, 2021
@networkimprov
Copy link

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

@zpavlinovic
Copy link
Contributor

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
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@zx2c4 @egonelbre @networkimprov @zpavlinovic @dmitshur @ajgajg1134 @gopherbot @alexbrainman