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/cgo: ${SRCDIR} doesn't properly handle paths with spaces #11868

Closed
jtsylve opened this issue Jul 25, 2015 · 12 comments

Comments

@jtsylve
Copy link
Contributor

commented Jul 25, 2015

Given a cgo directive like the following

//#cgo CFLAGS: -I${SRCDIR}/../../include

I get the following error

malformed #cgo argument: -IC:/Users/Joe Sylve/go/src/go4n6/disk/vmdk/../../include

Neither of the following seem to make much of a difference

//#cgo CFLAGS: -I"${SRCDIR}/../../include"
//#cgo CFLAGS: -I\"${SRCDIR}/../../include\"

Tested on go1.5beta2 on both 32 and 64 bit Windows.

@ianlancetaylor ianlancetaylor changed the title cgo ${SRCDIR} doesn't properly handle paths with spaces cmd/cgo: ${SRCDIR} doesn't properly handle paths with spaces Jul 25, 2015

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jul 25, 2015

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Jul 28, 2015

Please, provide instructions to reproduce your issue. Thank you.

Alex

@mattn

This comment has been minimized.

Copy link
Member

commented Jul 28, 2015

C:\>mkdir "foo bar"

C:\>cd "foo bar"

C:\foo bar>type con > main.go
package main

//#cgo CFLAGS: -I${SRCDIR}/../../include
import "C"

func main() {
}
^Z

C:\foo bar>go build
can't load package: package .: c:\foo bar\main.go: malformed
#cgo argument: -IC:/foo bar/../../include
@mattn

This comment has been minimized.

Copy link
Member

commented Jul 28, 2015

This is depend on behavior of escape arguments on unix-shell or cmd.exe.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Jul 29, 2015

Thank you @mattn. This is how it works on linux too:

# pwd
/root/src/issue 11868
# cat main.go
package main
//#cgo CFLAGS: -I${SRCDIR}/../../include
import "C"
func main() {
}
# go build
can't load package: package issue 11868: /root/src/issue 11868/main.go: malformed #cgo argument: -I/root/src/issue 11868/../../include
#

It appears we don't allow space inside of #cgo line parameters. I don't know why is that. Leaving for others to explain.

Alex

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2015

It's a bug.

I don't know if changing safeCgoName in go/build/build.go is sufficient, or whether we need to add quoting somewhere.

@mattn

This comment has been minimized.

Copy link
Member

commented Jul 29, 2015

gcc doesn't allows "-I/path/to directory". it should be -I"/path/to directory". (at least, on windows)

@jtsylve

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2015

Have there been any more thoughts on how this can be fixed? If someone could point me in the right direction, I'd be happy to try to fix it myself.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2015

@jtsylve Thanks--I would start with my suggestion above: look at safeCgoName in go/build/build.go. Fix that to accept spaces. Then make sure any necessary quoting is done.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

It is important that an attempted fix here does not allow spaces in other places. Changing safeCgoName sounds dangerous.

It does seem reasonable that ${SRCDIR}'s expansion should be properly handled even when that variable contains spaces.

@jtsylve

This comment has been minimized.

Copy link
Contributor Author

commented Oct 16, 2015

The issue is more complicated than just changing safeCgoName, unfortunately. In order to make it work correctly with gcc we need to allow quotes. splitQuoted tends to affect this negatively. I have yet to come up with a reasonable solution.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 25, 2015

CL https://golang.org/cl/16302 mentions this issue.

@jtsylve

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2015

Under which cases is splitQuoted needed and when should we avoid spaces as @rsc mentioned?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.