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