panic on gopkg.in .v0 imports

[daenney]

What version of Go (go version) and dep (git describe --tags) are you using?

[I] 13:21:22 ~/D/g/s/g/d/testboom $ go version
go version go1.8.3 darwin/amd64

[I] 13:23:56 ~/D/g/s/g/g/dep (master) $ git describe --tags
v0.1.0-313-g44a454b

What dep command did you run?

dep init -v

Root project is "github.com/daenney/testboom"
 1 transitively valid internal packages
 1 external packages imported from 1 projects
(0)   ✓ select (root)
(1)	? attempt gopkg.in/go-ldap/ldif.v0 with 1 pkgs; 0 versions to try
panic: version queue is empty, should not happen

goroutine 1 [running]:
github.com/golang/dep/internal/gps.(*solver).findValidVersion(0xc42016eb00, 0xc4201de1e0, 0xc420153fe0, 0x1, 0x1, 0x0, 0xc420153fe0)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:845 +0x5fa
github.com/golang/dep/internal/gps.(*solver).createVersionQueue(0xc42016eb00, 0xc420180b01, 0x18, 0x0, 0x0, 0xc420153fe0, 0x1, 0x1, 0x0, 0x0, ...)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:832 +0xace
github.com/golang/dep/internal/gps.(*solver).solve(0xc42016eb00, 0x0, 0x0, 0xc42019a0f0)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:449 +0x471
github.com/golang/dep/internal/gps.(*solver).Solve(0xc42016eb00, 0x3d, 0x16ce440, 0xc42019a030, 0xc4200140c6)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:399 +0xb1
main.(*initCommand).Run(0xc420153a6a, 0xc420015950, 0xc420010470, 0x0, 0x0, 0x0, 0x0)
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/init.go:172 +0x710
main.(*Config).Run(0xc42001b080, 0xc42001b080)
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/main.go:158 +0x876
main.main()
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/main.go:45 +0x253

What did you expect to see?

An error informing me that .v0 URLs aren't really useful and just use https://github.com/go-ldap/ldif directly or it working as expected.

What did you see instead?

A panic.

Reproduce:

[I] 13:24:17 ~/D/g/s/g/d/testboom $ pwd
/Users/daenney/Development/go/src/github.com/daenney/testboom

[I] 13:25:24 ~/D/g/s/g/d/testboom $ cat main.go
package main

import (
	"gopkg.in/go-ldap/ldif.v0"
)

func main() {
	l := &ldif.LDIF{}
	print(l)
}

[I] 13:25:36 ~/D/g/s/g/d/testboom $ dep init -v
Root project is "github.com/daenney/testboom"
...

[daenney]

The problems stem from this:

Version zero (v0) is reserved for packages that are so immature that offering any kind of API stability guarantees would be unreasonable. This is equivalent to labeling the package as alpha or beta quality, and as such the use of these packages as dependencies of stable packages and applications is discouraged.
Packages should not remain in v0 for too long, as the lack of API stability hinders their adoption, and hurts the stability of packages and applications that depend on them.
Repositories in GitHub that have no version tag or branch matching the pattern described above are also considered unstable, and thus gopkg.in takes their master branch as v0. This should only be used once the package maintainers encourage the use of gopkg.in, though.

http://labix.org/gopkg.in#VersionZero

[carolynvs]

Yeah we don't handle gopkg.in perfectly yet. Originally reported in #776 (comment)

[sdboyer]

hi! thanks for the issue report.

so the quote from labix is related, but the real issue here is that the guarantees gopkg.in tries to create don't translate terribly well to a dep world. as a result, we had to implement a layer for mapping the semantics gopkg.in creates into dep's model. dep actually never talks to gopkg.in servers at all - it more or less reimplements the server protocol locally.

this issue is one we've seen a few times (as @carolynvs noted) - i actually thought we'd fixed it, but apparently not. basically, the problem is that the handling we wrote sometimes allows a zero-length list of versions to come back, and the solver expects an invariant that, if a source exists, there is at least one version for it.

[daenney]

Argh, I tried searching for a similar issue, feel a bit silly for missing the one @carolynvs surfaced.

Considering what dep does it could be argued that gopkg.in doesn't really need to be a thing anymore, but I guess that's a pretty big hurdle to overcome for dep adoption in that case.

Could either of you give me an indication of where in the code I can start looking? For now I've just changed my import paths to avoid the .v0 mapping which solves the problem.

[sdboyer]

Argh, I tried searching for a similar issue, feel a bit silly for missing the one @carolynvs surfaced.

don't worry about it 😄

Considering what dep does it could be argued that gopkg.in doesn't really need to be a thing anymore, but I guess that's a pretty big hurdle to overcome for dep adoption in that case.

i do think what gopkg.in offers will likely be subsumed by dep eventually, but our job is 💯 to make this process as smooth as possible. so, we do things like this!

Could either of you give me an indication of where in the code I can start looking?

Sure! the bug's somewhere in here.

[daenney]

Do you have any suggestion as to how to trigger it? I was hoping to just pop into Gogland/IntelliJ and fire it up but that doesn't work, as I need to run that code from a different directory entirely. CLI dlv is pretty much voodoo to me 😞.

[daenney]

Never mind, figured it out, I can set a working directory in the run configurations to a location that'll cause dep init to blow up.

[daenney]

Alright, so stepping through this, a few tidbits that might be informative.

It manages to determine the commit alright, ovlist ([]gps.PairedVersion) looks like this:

name = "master"
isDefault = true
r = 6cf99803a771ebb53cab20894908aca4a9a591a2

The gps.gopkginSource:

major = 0
unstable = false

Eventually we step in and hit the case branchVersion, tv has name = master and isDefault = true, and sv (semver.Version) gets us:

major = 0
minor = 0
tiny = 0
pre = ""
metadata = ""
original = ""
special = 0

So far this seems to make sense to me and err now contains Invalid Semantic Version, which seems fair. As such we hit the continue in the error checking condition and since there's nothing else in ovlist it exits the loop.

What I'm a bit surprised at now though is that bsv looks like it did get populated with sv, but I don't think it actually is. It seems that way at first glance, as it has the same major, minor, tiny, special, metadata, pre and original values but those are the null values for the fields on semver.Version. So by happenstance these two things end up looking the same. I found this rather confusing since a bit further down the line there is a bsv = sv assignment but we never hit that part, but since it's a var bsv semver.Version this makes sense.

As a consequence if bsv != (semver.Version{}) evaluates to false, we never add anything to vlist and then return that, empty. Which I suspect is what ends up causing the panic.

I'm not quite sure if this helps?

I think the trick here is probably that if you end up with a gopkgin.Source with major = 0 to bail out early and just tell it to track master? That seems to be what gopkg is doing, or at least that's what I'm making of that quote about Version Zero.

[sdboyer]

Which I suspect is what ends up causing the panic.

💯 - returning zero-length list is definitely the cause of the panic

I think the trick here is probably that if you end up with a gopkgin.Source with major = 0 to bail out early and just tell it to track master?

it's fine to have semver versions with leading zeroes, and if such tags do exist, we should be letting them through. so, it's not quite as simple as just "fall back to master" - but that is the action of last resort.

[daenney]

I agree that if semver tags of 0.x.y nature exist we should normally use them, I'm just not convinced Gopkg.in actually does. I find the documentation a bit fuzzy on the exact behaviour of version 0 but looking at the version parsing they do it seems they don't unconditionally map .v0 to -unstable.

How do you suppose we could fix this? Knowing how to treat this seems to require knowing if any 0.x.y release tags exist on the source repository.

[sdboyer]

but looking at the version parsing they do it seems they don't unconditionally map .v0 to -unstable.

IIRC from when i last looked (maybe six months ago?), gopkg.in treats -unstable only as an actual suffix. it's entirely separate from the leading zero.

How do you suppose we could fix this? Knowing how to treat this seems to require knowing if any 0.x.y release tags exist on the source repository.

oh, that part's easy 😄 the full, unfiltered list from github is what's returned from the first line:

	ovlist, err := s.gitSource.listVersions(ctx)

we never actually talk to a gopkg.in server, at all - we jump straight to github, so we can do the remapping cleanly.

[sdboyer]

i'd swear we got this fixed?

[daenney]

Doesn't look like it:

[I] 20:08:28 ~/D/g/s/g/d/testboom $ dep init -v
Getting direct dependencies...
Checked 1 directories for packages.
Found 1 direct dependencies.
Root project is "github.com/daenney/testboom"
 1 transitively valid internal packages
 1 external packages imported from 1 projects
(0)   ✓ select (root)
(1)	? attempt gopkg.in/go-ldap/ldif.v0 with 1 pkgs; 0 versions to try
panic: version queue is empty, should not happen

goroutine 1 [running]:
github.com/golang/dep/internal/gps.(*solver).findValidVersion(0xc4200d6400, 0xc42024c000, 0xc42007de80, 0x1, 0x1, 0x0, 0xc42007de80)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:884 +0x53f
github.com/golang/dep/internal/gps.(*solver).createVersionQueue(0xc4200d6400, 0xc420190d81, 0x18, 0x0, 0x0, 0xc42007de80, 0x1, 0x1, 0x0, 0x0, ...)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:871 +0x955
github.com/golang/dep/internal/gps.(*solver).solve(0xc4200d6400, 0x0, 0x0, 0x0)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:493 +0x4cf
github.com/golang/dep/internal/gps.(*solver).Solve(0xc4200d6400, 0x3d, 0x17697a0, 0xc420194d80, 0xc4200140c6)
	/Users/daenney/Development/go/src/github.com/golang/dep/internal/gps/solver.go:443 +0xaa
main.(*initCommand).Run(0xc420173772, 0xc42009af50, 0xc4200821a0, 0x0, 0x0, 0x0, 0x0)
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/init.go:186 +0x881
main.(*Config).Run(0xc42009cea0, 0xc42009cea0)
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/main.go:160 +0x7f6
main.main()
	/Users/daenney/Development/go/src/github.com/golang/dep/cmd/dep/main.go:45 +0x1fc

[I] 20:06:49 ~/D/g/s/g/g/dep (master) $ dep version
dep:
 version     : devel
 build date  :
 git hash    :
 go version  : go1.9
 go compiler : gc
 platform    : darwin/amd64

[I] 20:06:52 ~/D/g/s/g/g/dep (master) $ git describe --tags
v0.3.1-70-ga445d4d

[michael-go]

yep, confirm it isn't fixed. just happened to me with the master dep (v0.3.1-79-g0edac3f)

[carolynvs]

I think the original bug (can't find it right now) was marked as fixed with the missing HEAD PR? Looking into this today.

UPDATE: Found the original #776 (comment), I did verify the fix so I'm going to check for a regression or other edge case.