golang / go

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

x/tools/internal: panic, "GOROOT not set in evaluated environment" #40670

Closed kevinburke1 closed 4 years ago

kevinburke1 commented 4 years ago

I put the following in a file named filename.go (filename doesn't matter):

 $ cat filename.go
package zip

// Zip provides facilities for operating ZIP archives.
// See https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT.
type Zip struct {
    zw             *zip.Writer
    zr             *zip.Reader
}

I then run GOROOT=/Users/kevin/go goimports . I expect that the "archive/zip" import will be added to the import section. Instead I get a panic:

$ goimports .
panic: GOROOT not set in evaluated environment

goroutine 1 [running]:
golang.org/x/tools/internal/imports.(*ProcessEnv).mustGetEnv(...)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:796
golang.org/x/tools/internal/imports.(*ProcessEnv).goroot(0x1385a00, 0xc0000b4200, 0x3)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:786 +0x119
golang.org/x/tools/internal/imports.addStdlibCandidates.func1(0x1214a6b, 0xb)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:898 +0x209
golang.org/x/tools/internal/imports.addStdlibCandidates(0xc0000f6630, 0xc0000909c0)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:915 +0x135
golang.org/x/tools/internal/imports.getFixes(0xc0000b2980, 0xc0000e2480, 0xc0000b41c0, 0xb, 0x1385a00, 0x0, 0x0, 0x1, 0x40, 0x11f0260)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:562 +0x1fc
golang.org/x/tools/internal/imports.fixImportsDefault(0xc0000b2980, 0xc0000e2480, 0xc0000b41c0, 0xb, 0x1385a00, 0x600, 0x1381ec0)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:522 +0x65
golang.org/x/tools/internal/imports.Process(0xc0000b41c0, 0xb, 0xc00014c000, 0xd4, 0x600, 0x1381ec0, 0x0, 0x0, 0xc0000d5ae8, 0x105078f, ...)
    /Users/kevin/src/golang.org/x/tools/internal/imports/imports.go:56 +0x19e
main.processFile(0xc0000b41c0, 0xb, 0x0, 0x0, 0x1262ce0, 0xc0000ae008, 0x2, 0x0, 0x0)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:142 +0x1b1
main.visitFile(0xc0000b41c0, 0xb, 0x1267120, 0xc000093110, 0x0, 0x0, 0xb, 0xc0000d5cf0)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:189 +0xea
path/filepath.walk(0xc0000b41c0, 0xb, 0x1267120, 0xc000093110, 0x1232d30, 0x0, 0x0)
    /Users/kevin/go/src/path/filepath/path.go:360 +0x423
path/filepath.walk(0x7ffeefbff612, 0x1, 0x1267120, 0xc000093040, 0x1232d30, 0x0, 0x1267120)
    /Users/kevin/go/src/path/filepath/path.go:384 +0x2fe
path/filepath.Walk(0x7ffeefbff612, 0x1, 0x1232d30, 0xc000092f70, 0x0)
    /Users/kevin/go/src/path/filepath/path.go:406 +0x105
main.walkDir(...)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:198
main.gofmtMain()
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:289 +0x2c9
main.main()
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:207 +0x32

Note I have explicitly set GOROOT in the environment. Adding a print statement to the relevant line in fix.go, it looks like e.Env is nil at the point where it expects to be reading GOROOT.

The package name must match the imports, I think, you can also reproduce if you replace zip with io in all three places, but you can't reproduce if you replace io in one place and keep zip in the other place.

I'm able to reproduce this on x/tools tip as well as on the commit tagged gopls/v0.4.4.

kevinburke1 commented 4 years ago

I'm using Go tip as well but I can also reproduce if I rebuild goimports using Go 1.14 latest.

stamblerre commented 4 years ago

/cc @heschik

heschi commented 4 years ago

Please run goimports -v and attach the logs. Something is probably wrong with your go installation.

kevinburke1 commented 4 years ago
 $ GOROOT=~/go goimports -v .
2020/08/10 12:03:17.471712 fixImports(filename="filename.go"), abs="/Users/kevin/src/github.com/kevinburke/zip-goimports-repro/filename.go", srcDir="/Users/kevin/src/github.com/kevinburke/zip-goimports-repro" ...
panic: GOROOT not set in evaluated environment

goroutine 1 [running]:
golang.org/x/tools/internal/imports.(*ProcessEnv).mustGetEnv(...)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:796
golang.org/x/tools/internal/imports.(*ProcessEnv).goroot(0x1425880, 0xc00001a2d0, 0x3)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:786 +0x119
golang.org/x/tools/internal/imports.addStdlibCandidates.func1(0x12466cb, 0xb)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:898 +0x21c
golang.org/x/tools/internal/imports.addStdlibCandidates(0xc0001465a0, 0xc000010ae0)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:915 +0x131
golang.org/x/tools/internal/imports.getFixes(0xc0000209c0, 0xc00012a480, 0xc00001a290, 0xb, 0x1425880, 0x0, 0x0, 0x1, 0x40, 0x121e540)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:562 +0x1f8
golang.org/x/tools/internal/imports.fixImportsDefault(0xc0000209c0, 0xc00012a480, 0xc00001a290, 0xb, 0x1425880, 0x600, 0x1421d40)
    /Users/kevin/src/golang.org/x/tools/internal/imports/fix.go:522 +0x60
golang.org/x/tools/internal/imports.Process(0xc00001a290, 0xb, 0xc000186000, 0xd4, 0x600, 0x1421d40, 0x0, 0x0, 0xc00010fae8, 0x104de0f, ...)
    /Users/kevin/src/golang.org/x/tools/internal/imports/imports.go:56 +0x19e
main.processFile(0xc00001a290, 0xb, 0x0, 0x0, 0x12a1540, 0xc00000e018, 0x2, 0x0, 0x0)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:142 +0x1a6
main.visitFile(0xc00001a290, 0xb, 0x12a6200, 0xc000061450, 0x0, 0x0, 0xb, 0xc00010fcf0)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:189 +0xcc
path/filepath.walk(0xc00001a290, 0xb, 0x12a6200, 0xc000061450, 0x1265580, 0x0, 0x0)
    /Users/kevin/go1.14/src/path/filepath/path.go:360 +0x425
path/filepath.walk(0x7ffeefbff62d, 0x1, 0x12a6200, 0xc000061380, 0x1265580, 0x0, 0x12a6200)
    /Users/kevin/go1.14/src/path/filepath/path.go:384 +0x2ff
path/filepath.Walk(0x7ffeefbff62d, 0x1, 0x1265580, 0xc0000612b0, 0x0)
    /Users/kevin/go1.14/src/path/filepath/path.go:406 +0xff
main.walkDir(...)
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:198
main.gofmtMain()
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:289 +0x2b8
main.main()
    /Users/kevin/src/golang.org/x/tools/cmd/goimports/goimports.go:207 +0x32
heschi commented 4 years ago

Huh, I really expected that to be more useful. Okay.

Don't bother setting GOROOT, that's only confusing the situation. Please run go env -json GO111MODULE GOFLAGS GOINSECURE GOMOD GOMODCACHE GONOPROXY GONOSUMDB GOPATH GOPROXY GOROOT GOSUMDB and show the results.

kevinburke1 commented 4 years ago
$ go env -json GO111MODULE GOFLAGS GOINSECURE GOMOD GOMODCACHE GONOPROXY GONOSUMDB GOPATH GOPROXY GOROOT GOSUMDB
{
    "GO111MODULE": "off",
    "GOFLAGS": "",
    "GOINSECURE": "",
    "GOMOD": "",
    "GOMODCACHE": "/Users/kevin/pkg/mod",
    "GONOPROXY": "github.com/meterup",
    "GONOSUMDB": "github.com/meterup",
    "GOPATH": "/Users/kevin",
    "GOPROXY": "direct",
    "GOROOT": "/Users/kevin/go",
    "GOSUMDB": "sum.golang.org"
}
kevinburke1 commented 4 years ago

Did you try to reproduce it using the example above? Can you reproduce it?

gopherbot commented 4 years ago

Change https://golang.org/cl/247797 mentions this issue: internal/imports: fix crash when adding stdlib imports

heschi commented 4 years ago

Apologies, I didn't give much credit to the idea that goimports had been broken for over a month or however long it's been. Fix above.

kevinburke1 commented 4 years ago

Well, I could only trigger it if the package name matched the stdlib package I was trying to import, if I named the package something else there wouldn't be a panic.