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

go/build: doesn't properly use backslashes on Windows #36861

Closed
al45tair opened this issue Jan 29, 2020 · 3 comments
Closed

go/build: doesn't properly use backslashes on Windows #36861

al45tair opened this issue Jan 29, 2020 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows

Comments

@al45tair
Copy link

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

$ go version
go version go1.13.5 windows/amd64

Does this issue reproduce with the latest release?

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\alastair.houghton\AppData\Local\go-build
set GOENV=C:\Users\alastair.houghton\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\work\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\ALASTA~1.HOU\AppData\Local\Temp\go-build003328866=/tmp/go-build -gno-record-gcc-switches

What did you do?

I was trying to use operator-sdk on Windows. I know, I know, I'm on Windows...

The operator-sdk generate k8s command was putting files in the wrong place. The reason for this is that, on Windows, the IsLocalImport() function in src/go/build/build.go gets passed a path that looks like this:

.\\pkg\\apis\\foobar\\v1alpha1

whereupon it returns false, because it's looking for forward slashes.

What did you expect to see?

Everything in build.go should probably work on Windows. Right now, I can see a number of other things that won't as well.

IsLocalImport() should presumably just use filepath.IsAbs()?

What did you see instead?

There's code in src/go/build/build.go that is clearly not right for Windows because it relies on pathnames containing slashes where on Windows they're going to contain backslashes instead.

@agnivade agnivade changed the title src/go/build/build.go doesn't properly use backslashes on Windows go/build: doesn't properly use backslashes on Windows Jan 29, 2020
@agnivade agnivade added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Jan 29, 2020
@al45tair
Copy link
Author

OK, so using filepath.IsAbs() didn't work. The following change, however, does appear to fix things:

diff --git a/src/go/build/build.go b/src/go/build/build.go
index e89aa7708d..4b77cba901 100644
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -1829,8 +1829,12 @@ var ToolDir = getToolDir()
 // IsLocalImport reports whether the import path is
 // a local import path, like ".", "..", "./foo", or "../foo".
 func IsLocalImport(path string) bool {
-       return path == "." || path == ".." ||
-               strings.HasPrefix(path, "./") || strings.HasPrefix(path, "../")
+       if path == "." || path == ".." {
+               return true
+       }
+       slashPath := filepath.ToSlash(path)
+       return strings.HasPrefix(slashPath, "./") ||
+               strings.HasPrefix(slashPath, "../")
 }

 // ArchChar returns "?" and an error.

I'm not sure this is necessarily the correct fix, and building a new version of Go on Windows was... fun. (Maybe the docs for that need clarifying a bit; some of the tests appear to fail, at least on my box here, but nothing relating to this change - they fail with the original code too.)

@heschi
Copy link
Contributor

heschi commented Jan 29, 2020

IsLocalImport reports whether the import path is...

Import paths are slash separated. The downstream project shouldn't be using IsLocalImport on filesystem paths.

@al45tair
Copy link
Author

al45tair commented Feb 3, 2020

OK, in which case the bug would seem to be upstream in k8s.io/gengo. I'll fix it there instead. Thanks :-)

@al45tair al45tair closed this as completed Feb 3, 2020
@golang golang locked and limited conversation to collaborators Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

4 participants