Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
kevinburke1 opened this issue Aug 10, 2020 · 10 comments
Closed

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

kevinburke1 opened this issue Aug 10, 2020 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@kevinburke1
Copy link

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.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Aug 10, 2020
@gopherbot gopherbot added this to the Unreleased milestone Aug 10, 2020
@kevinburke1
Copy link
Author

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

@stamblerre
Copy link
Contributor

/cc @heschik

@heschi
Copy link
Contributor

heschi commented Aug 10, 2020

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

@kevinburke1
Copy link
Author 

 $ 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
Copy link
Contributor

heschi commented Aug 10, 2020

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
Copy link
Author 

$ 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
Copy link
Author

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

@gopherbot
Copy link
Contributor

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

@heschi
Copy link
Contributor

heschi commented Aug 10, 2020

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
Copy link
Author

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.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 11, 2020
@golang golang locked and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@andybons @stamblerre @gopherbot @heschi @kevinburke1