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/go: CC variable misparsed when set to a path containing spaces #41400

Closed
XamarinPOC opened this issue Sep 15, 2020 · 38 comments
Closed

cmd/go: CC variable misparsed when set to a path containing spaces #41400

XamarinPOC opened this issue Sep 15, 2020 · 38 comments
Assignees
Milestone

Comments

@XamarinPOC
Copy link

@XamarinPOC XamarinPOC commented Sep 15, 2020

go build -buildmode=c-shared -o C:\Users\XAMARI~1\AppData\Local\Temp\gomobile-work-285666087\lib\armeabi-v7a\libbasic.so golang.org/x/mobile/example/basic failed: exit status 2

runtime/cgo

exec: "C:\Program": file does not exist

E:\GoWorkDir>go env

set GO111MODULE=auto
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Xamarin Developer\AppData\Local\go-build
set GOENV=C:\Users\Xamarin Developer\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=E:\GoWorkDir\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=E:\GoWorkDir
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\XAMARI~1\AppData\Local\Temp\go-build823121506=/tmp/go-build -gno-record-gcc-switches

@davecheney
Copy link
Contributor

@davecheney davecheney commented Sep 15, 2020

Your go installation is corrupt and/or the time stamps have been disrupted, possibly by unpacking one version of go over another

Please delete c:\go and reinstall from a fresh copy

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented Sep 15, 2020

I am actually able to repro this.

exact command: CC="C:\\path with spaces\\bin\\gcc" go build or CC="C:/path with spaces/bin/gcc" go build
works with any source file that uses cgo

It's specific to the CC/CXX class of variables so looks like something is wrong with how go parses them.

cc @ianlancetaylor

@chaitanyaevoke

This comment has been minimized.

@bcmills bcmills changed the title # runtime/cgo exec: "C:\\Program": file does not exist cmd/go: exec error with cgo when CC variable contains spaces on Windows Sep 15, 2020
@bcmills bcmills added this to the Go1.16 milestone Sep 15, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Sep 15, 2020

@AlexRouSg, thanks for the detail. I can even reproduce it on Linux:

example.com/bin with spaces$ echo "$CC"
/tmp/tmp.jTseVJjT2L/example.com/bin with spaces/gcc

example.com/bin with spaces$ "$CC" --version
gcc (Debian 9.3.0-13) 9.3.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


example.com/bin with spaces$ go env CC
/tmp/tmp.jTseVJjT2L/example.com/bin

@bcmills bcmills changed the title cmd/go: exec error with cgo when CC variable contains spaces on Windows cmd/go: CC variable misparsed when set to a path containing spaces Sep 15, 2020
@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented Sep 15, 2020

So looks like there are multiple places that needs fixing ...

I've found

func (p *Package) gccBaseCmd() []string {

which only affects running the cgo binary direcly

go build and go tool cgo seems to have a different issue and I can't seem to figure out where that is. But likely it's related to calling strings.Fields or something along those lines.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 15, 2020

Yes, there are several places where we're calling strings.Fields to split this sort of command, which seems clearly incorrect to me.

But I think we also try to follow the usual C toolchain behavior for the CC environment variable, so I'm trying to figure out if there is a standard format for escaping paths containing spaces. (Perhaps @ianlancetaylor or @jayconrod knows?)

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented Sep 15, 2020

@XamarinPOC @chaitanyaevoke In the meantime for gomobile, you'll have to install the SDK to a path without spaces

@XamarinPOC
Copy link
Author

@XamarinPOC XamarinPOC commented Sep 15, 2020

I just uninstalled and installed fresh Go version 1.15.2 but no luck.

I'm trying to run build command => gomobile build -target=android golang.org/x/.............. to generate apk file

@XamarinPOC
Copy link
Author

@XamarinPOC XamarinPOC commented Sep 15, 2020

@AlexRouSg Which Sdk?? Can you . please explain it .

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented Sep 15, 2020

@XamarinPOC
You need to install the android sdk/ndk to a path without spaces like C:\androidsdk
Make sure you changed the variable ANDROID_HOME too

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 15, 2020

Change https://golang.org/cl/255018 mentions this issue: cmd/go: allow the CC path to contain spaces

@XamarinPOC
Copy link
Author

@XamarinPOC XamarinPOC commented Sep 15, 2020

Yup ,after removing spaces its working now. Can we get this fix in next release.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 15, 2020

Wait. Right now I can run

CC="gcc -O3" go build

and all my cgo files will be built with -O3.

Now, maybe that is a mistake and maybe we should change it. But we definitely shouldn't change that behavior casually. We should discuss it on the mailing list, we should find out whether it will break anybody.

In particular there is no documented way to pass C compiler flags when running make.bash other than setting CC to include the compiler and the flags. And the value of the CC environment variable used when running make.bash because the default value of the C compiler used when building programs.

In other words, we can't just treat this a minor bug that we should just fix. We have to think about how this has worked in the past and how it should work going forward.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 15, 2020

The CL that I mailed preserves the property that space-separated arguments following the executable name are treated as arguments: it uses exec.LookPath to identify the first space that could plausibly indicate a break between the command and its arguments, and falls back to the old strings.Fields behavior if none of the possible prefixes resolves to an executable.

I believe that preserves the existing behavior for all commands that would successfully invoke a compiler, and also fixes some number of cases that previously would have failed.

@bcmills bcmills self-assigned this Sep 15, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 15, 2020

OK, thanks. I'm a bit worried about the possibility of confusion. I guess it should work but it seems pretty complicated.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 16, 2020

Agreed. I was kind of hoping that there would be some existing standard or extension for quoting within CC, but I couldn't find one.

As an alternative, I suppose that we could allow shell-style quoting within the CC variable. (Do you think that would be better or worse?)

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented Sep 16, 2020

Shell style quoting sounds better to me. I can imagine people having args like -I/path with spaces/ which wouldn't be covered by the current CL. Tho that makes me wonder if cgo handles args with spaces correctly for #cgo CFLAGS family of comments or pkgconfig provided flags ...

EDIT:
Looks like #cgo comments understands shell quoting while pkgconfig is kinda weird.
pkgconfig escapes spaces with \ so it looks like -L/path\ with\ spaces which then causes cgo to error

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 16, 2020

Shell quoting sounds a little better to me, I guess.

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Jan 21, 2021

I'm looking for a narrow fix of #43808 that can be backported, and it will probably affect the eventual solution to this issue.

What I'm thinking right now is that we should support shell quoting using the code in go/build.splitQuoted. That's what we use for #cgo directives today, so it seems reasonable to reuse it here (in the absence of any broader standard).

For #43808, we could quote the CC and GCCGO variables passed from cmd/go to cmd/cgo if needed, then unquote them in cmd/cgo. For 1.17, we could fix this issue by expanding that to cmd/go and anywhere else we deal with CC. We should fix #43078 at the same time.

@justinfx
Copy link

@justinfx justinfx commented Apr 17, 2021

I seem to be hitting this same problem with CGO_FLAGS and CGO_LDFLAGS, where because it is trying to point at a python installation, it is getting values like:

-I"C:/Program Files/Python39/include"
-L"C:/Program Files/Python39"

and then ends up splitting "Program" and "Files". Is this a more general problem than only the CC var?

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 19, 2021

@justinfx, I would not be surprised if the problem is more general than just CC, but the fix is probably specific to each variable. I suggest filing a separate issue for CGO_FLAGS parsing so that we don't lose track of it.

@ChristianOConnor
Copy link

@ChristianOConnor ChristianOConnor commented May 15, 2021

I'm having the same problem. I have a little bit of an unconventional setup. I use Windows 10 with Cmder set to bash as admin. I wanted to use the Visual Studio C compiler to build and run a project so I tried go env -w CC="/mnt/c/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/Llvm/bin/clang-cl.exe" and whenever I try to build and run anything using cgo I get this error cgo: exec C:/tools/Cmder/vendor/git-for-windows/mnt/c/Program: exec: "C:/tools/Cmder/vendor/git-for-windows/mnt/c/Program": file does not exist

Any workarounds yet?

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented May 15, 2021

@ChristianOConnor msvc/clang on windows is not supported so whether this gets resolved won't help you there.

@ChristianOConnor
Copy link

@ChristianOConnor ChristianOConnor commented May 15, 2021

@ChristianOConnor msvc/clang on windows is not supported so whether this gets resolved won't help you there.

Oh okay got you. What is the recommended c compiler for go? MinGW-w64? I'm just worried that this will mess up my build path for compiling C programs in Visual Studio. Is it safe to have MinGW-w64 installed alongside Visual Studio C build tools?

@AlexRouSg
Copy link
Contributor

@AlexRouSg AlexRouSg commented May 15, 2021

@ChristianOConnor

The only (as least the only I could find that is documented to work) C compiler on windows is mingw-w64.

Is it safe to have MinGW-w64 installed alongside Visual Studio C build tools?

As long as you setup your PATH variable correctly it should be fine. Go will look for gcc, g++, ar but you can always use the more specific program name of CC=x86_64-w64-mingw32-gcc etc...

@ChristianOConnor
Copy link

@ChristianOConnor ChristianOConnor commented May 15, 2021

@ChristianOConnor

The only (as least the only I could find that is documented to work) C compiler on windows is mingw-w64.

Is it safe to have MinGW-w64 installed alongside Visual Studio C build tools?

As long as you setup your PATH variable correctly it should be fine. Go will look for gcc, g++, ar but you can always use the more specific program name of CC=x86_64-w64-mingw32-gcc etc...

Okay awesome thanks!

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 15, 2021

Change https://golang.org/cl/334732 mentions this issue: [dev.cmdgo] cmd: support space and quotes in CC and CXX

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 15, 2021

Change https://golang.org/cl/334730 mentions this issue: [dev.cmdgo] cmd/internal/str: move package from cmd/go/internal/str

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 15, 2021

Change https://golang.org/cl/334731 mentions this issue: [dev.cmdgo] cmd/internal/str: add functions for quoting and splitting args

@bcmills bcmills assigned jayconrod and unassigned bcmills Jul 15, 2021
@bcmills bcmills removed this from the Go1.17 milestone Jul 15, 2021
@bcmills bcmills added this to the Go1.18 milestone Jul 15, 2021
gopherbot pushed a commit that referenced this issue Jul 30, 2021
This will let cmd/cgo and cmd/link use this package for argument parsing.

For #41400

Change-Id: I12ee21151bf3f00f3e8d427faaaab2453c823117
Reviewed-on: https://go-review.googlesource.com/c/go/+/334730
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopherbot pushed a commit that referenced this issue Jul 30, 2021
… args

JoinAndQuoteFields does the inverse of SplitQuotedFields: it joins a
list of arguments with spaces into one string, quoting arguments that
contain spaces or quotes.

QuotedStringListFlag uses SplitQuotedFields and JoinAndQuoteFields
together to define new flags that accept lists of arguments.

For #41400

Change-Id: I4986b753cb5e6fabb5b489bf26aedab889f853f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/334731
Trust: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopherbot pushed a commit that referenced this issue Jul 30, 2021
The CC and CXX environment variables now support spaces and quotes
(both double and single). This fixes two issues: first, if CC is a
single path that contains spaces (like 'c:\Program
Files\gcc\bin\gcc.exe'), that should now work if the space is quoted
or escaped (#41400). Second, if CC or CXX has multiple arguments (like
'gcc -O2'), they are now split correctly, and the arguments are passed
before other arguments when invoking the C compiler. Previously,
strings.Fields was used to split arguments, and the arguments were
placed later in the command line. (#43078).

Fixes #41400
Fixes #43078

Change-Id: I2d5d89ddb19c94adef65982a8137b01f037d5c11
Reviewed-on: https://go-review.googlesource.com/c/go/+/334732
Trust: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 12, 2021

Change https://golang.org/cl/341934 mentions this issue: cmd/internal/str: move package from cmd/go/internal/str

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 12, 2021

Change https://golang.org/cl/341935 mentions this issue: cmd/internal/str: add utilities for quoting and splitting args

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 12, 2021

Change https://golang.org/cl/341936 mentions this issue: cmd: support space and quotes in CC and CXX

gopherbot pushed a commit that referenced this issue Aug 13, 2021
This will let cmd/cgo and cmd/link use this package for argument parsing.

For #41400

Change-Id: I12ee21151bf3f00f3e8d427faaaab2453c823117
Reviewed-on: https://go-review.googlesource.com/c/go/+/334730
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/341934
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Aug 16, 2021
JoinAndQuoteFields does the inverse of SplitQuotedFields: it joins a
list of arguments with spaces into one string, quoting arguments that
contain spaces or quotes.

QuotedStringListFlag uses SplitQuotedFields and JoinAndQuoteFields
together to define new flags that accept lists of arguments.

For #41400

Change-Id: I4986b753cb5e6fabb5b489bf26aedab889f853f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/334731
Trust: Jay Conrod <jayconrod@google.com>
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/341935
@gopherbot gopherbot closed this in 742dcba Aug 16, 2021
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