panic windows: should never call ListPackages on root project

[jessfraz]

happening on solve when packages are nested in root project:

Build started
git config --global core.autocrlf input
git clone -q git@github.com:golang/hoard.git c:\gopath\src\github.com\golang\hoard
Warning: Permanently added the RSA host key for IP address '192.30.253.113' to the list of known hosts.
git fetch -q origin +refs/pull/96/merge:
git checkout -qf FETCH_HEAD
Running Install scripts
rmdir c:\go /s /q
appveyor DownloadFile https://storage.googleapis.com/golang/go%GOVERSION%.windows-amd64.msi
Downloading go1.7.windows-amd64.msi (74,977,280 bytes)...100%
msiexec /i go%GOVERSION%.windows-amd64.msi /q
set Path=c:\go\bin;c:\gopath\bin;%Path%
go version
go version go1.7 windows/amd64
go env
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=c:\gopath
set GORACE=
set GOROOT=C:\go
set GOTOOLDIR=C:\go\pkg\tool\windows_amd64
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
go build
go test
--- FAIL: TestInit (3.50s)
	hoard_test.go:262: git [checkout v0.8.0] standard error:
	hoard_test.go:263: Note: checking out 'v0.8.0'.
		
		You are in 'detached HEAD' state. You can look around, make experimental
		changes and commit them, and you can discard any commits you make in this
		state without impacting any branches by performing another checkout.
		
		If you want to create a new branch to retain commits you create, you may
		do so (now or later) by using -b with the checkout command again. Example:
		
		  git checkout -b <new-branch-name>
		
		HEAD is now at 645ef00... doc tweaks
		
	hoard_test.go:262: git [checkout 42b84f9ec624953ecbf81a94feccb3f5935c5edf] standard error:
	hoard_test.go:263: Note: checking out '42b84f9ec624953ecbf81a94feccb3f5935c5edf'.
		
		You are in 'detached HEAD' state. You can look around, make experimental
		changes and commit them, and you can discard any commits you make in this
		state without impacting any branches by performing another checkout.
		
		If you want to create a new branch to retain commits you create, you may
		do so (now or later) by using -b with the checkout command again. Example:
		
		  git checkout -b <new-branch-name>
		
		HEAD is now at 42b84f9... Merge pull request #384 from mnzt/master
		
	hoard_test.go:167: running testhoard [init]
	hoard_test.go:187: standard error:
	hoard_test.go:188: hoard: Finding dependencies for "github.com/golang/notexist"...
		hoard: Found 3 dependencies.
		hoard: Building dependency graph...
		hoard: no buildable Go source files in C:\Users\appveyor\AppData\Local\Temp\1\gotest447160834\src\github.com\golang\notexist
		hoard: Package "github.com/golang/notexist/foo", analyzing...
		hoard: Package "github.com/golang/notexist/foo" has import "github.com/Sirupsen/logrus", analyzing...
		hoard: Package "github.com/golang/notexist/foo" has import "github.com/pkg/errors", analyzing...
		hoard: Package "github.com/golang/notexist/foo/bar", analyzing...
		hoard: Analyzing transitive imports...
		hoard: Analyzing "github.com/pkg/errors"...
		hoard: Analyzing "github.com/Sirupsen/logrus"...
		hoard: Analyzing "golang.org/x/sys/unix"...
		hoard: Solving...
		Root project is "github.com/golang/notexist"
		 2 transitively valid internal packages
		 3 external packages imported from 3 projects
		✓ select (root)
		| ? revisit github.com/golang/notexist to add 1 pkgs
		panic: should never call ListPackages on root project
		
		goroutine 1 [running]:
		panic(0x787fc0, 0xc0421adde0)
			C:/go/src/runtime/panic.go:500 +0x1af
		github.com/golang/hoard/vendor/github.com/sdboyer/gps.(*bridge).ListPackages(0xc042422ff0, 0xc04240bae0, 0x1a, 0x0, 0x0, 0x9b9460, 0xc0421adc10, 0x30, 0x12, 0x0, ...)
			c:/gopath/src/github.com/golang/hoard/vendor/github.com/sdboyer/gps/bridge.go:283 +0x22d
		github.com/golang/hoard/vendor/github.com/sdboyer/gps.(*solver).checkRequiredPackagesExist(0xc0424245a0, 0xc04240bae0, 0x1a, 0x0, 0x0, 0x9b9460, 0xc0421adc10, 0xc0421add30, 0x1, 0x1, ...)
			c:/gopath/src/github.com/golang/hoard/vendor/github.com/sdboyer/gps/satisfy.go:111 +0xa7
		github.com/golang/hoard/vendor/github.com/sdboyer/gps.(*solver).check(0xc0424245a0, 0xc04240bae0, 0x1a, 0x0, 0x0, 0x9b9460, 0xc0421adc10, 0xc0421add30, 0x1, 0x1, ...)
			c:/gopath/src/github.com/golang/hoard/vendor/github.com/sdboyer/gps/satisfy.go:28 +0x149
		github.com/golang/hoard/vendor/github.com/sdboyer/gps.(*solver).solve(0xc0424245a0, 0x0, 0x0, 0x1)
			c:/gopath/src/github.com/golang/hoard/vendor/github.com/sdboyer/gps/solver.go:427 +0x2d9
		github.com/golang/hoard/vendor/github.com/sdboyer/gps.(*solver).Solve(0xc0424245a0, 0x55, 0xc042129cc0, 0x1a, 0xc042156840)
			c:/gopath/src/github.com/golang/hoard/vendor/github.com/sdboyer/gps/solver.go:330 +0x95
		main.(*initCommand).Run(0x9f2570, 0xc042037d90, 0x0, 0x0, 0x0, 0x0)
			c:/gopath/src/github.com/golang/hoard/init.go:151 +0x1025
		main.main()
			c:/gopath/src/github.com/golang/hoard/main.go:95 +0x652
		
	hoard_test.go:197: go [init] failed unexpectedly: exit status 2
	hoard_test.go:84: remove C:\Users\appveyor\AppData\Local\Temp\1\gotest447160834\src\github.com\golang\notexist: The process cannot access the file because it is being used by another process.
FAIL
exit status 1
FAIL	github.com/golang/hoard	27.523s
Command exited with code 1

[sdboyer]

yeah, i need to create a regression test to specifically cover this. i just wish the trace output here:

| ? revisit github.com/golang/notexist to add 1 pkgs

indicated which package it was trying add :/

[sdboyer]

We figured out what the problem is - there's a filepath.Clean() call in the bowels of gps.PackageTree.ExternalReach(), which causes slashes to get normalized back to os.Separator. It's ephemeral, so we don't see the results emerge anywhere outside of the method call, but the normalized path is used to do prefix checking.

Thus, on a windows machine, github.com/golang/notexist is mistaken as NOT being a prefix of github.com/golang/notexist/foo/bar, and the solver ends up trying to revisit something in the root project. 💥 💥

I need to figure out if that filepath.Clean() call is even necessary. IIRC, my original intent was to use it to deal with weird relative import path situations, but I'm not actually sure if that's really possible anymore at that point, or if it's even worth dealing with the situation when it arises.

[sdboyer]

should be all solved now