golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.38k stars 17.71k forks source link

test/fixedbugs: bug248 and bug369 fail when the absolute path contains a space #10197

Open dr2chase opened 9 years ago

dr2chase commented 9 years ago

Go version:

go version devel +6ffed30 Wed Mar 18 15:14:37 2015 +0000 darwin/amd64

I did:

cd ~/"Google Drive"
git clone https://go.googlesource.com/go
cd go/src
./all.bash

I expected to see a successful build and test.

Instead I saw:

...
##### ../test
# go run run.go -- fixedbugs/bug248.go
exit status 1
bug2.go:8: import path contains space character: "/Users/drchase/Google Drive/work/go/test/fixedbugs/bug248.dir/bug0"
bug2.go:9: import path contains space character: "/Users/drchase/Google Drive/work/go/test/fixedbugs/bug248.dir/bug1"

and a similar failure with bug369.go . My workaround is to rename "Google Drive" to "GoogleDrive" (and correct external references to the original name).

Small reproducer (can copy and paste into shell):

cd;  mkdir -p tmp ; cd tmp ; mkdir "t m p" ; cd "t m p"

cat > main.go <<EOF
package main
import (
  p0 "./x"
  "fmt"
 )
func main() {
  fmt.Println(p0.Id("Hello, playground"))
}
EOF

mkdir x
cat > x/bug0.go <<EOF
package x
func Id (s string) string {
  return s
}
EOF

go tool 6g main.go
# end of copy and paste

This produces the error:

main.go:3: import path contains space character: "/Users/drchase/tmp/t m p/x"
main.go:7: undefined: p0 in p0.Id

Ordinary compilation works fine:

$ go run main.go
Hello, playground
ianlancetaylor commented 9 years ago

For background on this change see http://golang.org/issue/3021 and http://golang.org/cl/5674043 . In the latter, Russ said "We can exclude them now and relent if people complain. I propose to exclude control characters, spaces, invalid UTF-8, and backslash and see what happens."

@rsc

dr2chase commented 9 years ago

True, but note that there are no spaces in the import path of the Go test case itself, and the user may not have any control over that name. We've also taken care to avoid this problem in the usual case -- notice that "go run main.go" processes the import without complaint. This looks more like an artifact than a policy, but it's an artifact that appears if someone attempts a standard go clone+build at a path name with a space in it.

Even if it is a policy, I don't think it makes sense to exclude spaces. Windows/Linux/Mac treat spaces unexceptionally. I ran a find command on the root of my (relatively fresh Mac laptop) file system and found over 1000 directories with a space in their name, while #$@^%~:;=+?|"`'{}[]<>,*\ combined appeared in only 44 directory names. 28 had ( and 28 had ) -- probably the same 28.

rsc commented 9 years ago

The bug only affects people or programs that:

  1. invoke the compiler directly
  2. do so in directories containing spaces
  3. have local imports (import "./foo") in their programs
  4. do not specify a -D flag to set the local import prefix

That's a lot of conditions. It's true that fixedbugs/bug248.go runs into this, as does fixedbugs/bug369.go, but probably the fix should be to correct those tests, not to make larger changes.

Note that the go command does work fine, because it arranges to use a sanitized version of the local directory in the path to the compiled artifacts, and it instructs the compiler to use that sanitized version with -D.

We should still fix the tests, but this is not blocking any release.

bcmills commented 5 years ago

probably the fix should be to correct those tests, not to make larger changes.

Retitled accordingly.