Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Docs: Version upgrade sorting confusion #1528

Closed
mattayes opened this issue Jan 14, 2018 · 1 comment · Fixed by #1538
Closed

Docs: Version upgrade sorting confusion #1528

mattayes opened this issue Jan 14, 2018 · 1 comment · Fixed by #1538

Comments

@mattayes
Copy link
Contributor

mattayes commented Jan 14, 2018

What version of dep are you using (dep version)?

v0.3.2

What's the problem?

I'm having a hard time understand the How does dep decide what version of a dependency to use docs. Here's my interpretation of the ordering rules:

  1. Non pre-release semver versions, sorted according to the semver 2.0 spec.
  2. Pre-release semver versions, sorted according to the semver 2.0 spec.
  3. The default branch(es) (master for Git, default for Mercurial, etc.), sorted lexicographically(?).
  4. All other branches, sorted lexicographically.
  5. All non-semver versions (tags), sorted lexicographically.
  6. Revisions, sorted lexicographically.

What did you expect to see?

Given the slice example from the docs:

So, given a slice of the following versions:

Branch: master, devel
Semver tags: v1.0.0, v1.1.0, v1.1.0-alpha1
Non-semver tags: footag
Revision: f6e74e8

I'd expect the upgrade order to look like this:

[v1.1.0, v1.0.0, v1.1.0-alpha1, master, devel, footag, f6e74e8d]

What did you see instead?

[v1.1.0, v1.0.0, v1.1.0-alpha1, footag, devel, master, f6e74e8d]

Notice how the tag comes before the non-default branch, which itself comes before the default branch.

The tests relating to version ordering appear to match my interpretation (eup is the ordering):

func TestVersionSorts(t *testing.T) {
rev := Revision("flooboofoobooo")
v1 := NewBranch("master").Pair(rev)
v2 := NewBranch("test").Pair(rev)
v3 := NewVersion("1.0.0").Pair(rev)
v4 := NewVersion("1.0.1").Pair(rev)
v5 := NewVersion("v2.0.5").Pair(rev)
v6 := NewVersion("2.0.5.2").Pair(rev)
v7 := newDefaultBranch("unwrapped").Pair(rev)
v8 := NewVersion("20.0.5.2").Pair(rev)
v9 := NewVersion("v1.5.5-beta.4").Pair(rev)
v10 := NewVersion("v3.0.1-alpha.1").Pair(rev)
start := []Version{
v1,
v2,
v3,
v4,
v5,
v6,
v7,
v8,
v9,
v10,
rev,
}
down := make([]Version, len(start))
copy(down, start)
up := make([]Version, len(start))
copy(up, start)
edown := []Version{
v3, v4, v5, // semvers
v9, v10, // prerelease semver
v7, v1, v2, // floating/branches
v6, v8, // plain versions
rev, // revs
}
eup := []Version{
v5, v4, v3, // semvers
v10, v9, // prerelease semver
v7, v1, v2, // floating/branches
v6, v8, // plain versions
rev, // revs
}
SortForUpgrade(up)
var wrong []int
for k, v := range up {
if eup[k] != v {
wrong = append(wrong, k)
t.Errorf("Expected version %s in position %v on upgrade sort, but got %s", eup[k], k, v)
}
}
if len(wrong) > 0 {
// Just helps with readability a bit
t.Errorf("Upgrade sort positions with wrong versions: %v", wrong)
}

I'm happy to do the PR!

@sdboyer
Copy link
Member

sdboyer commented Jan 14, 2018

Ugh. yes, your interpretation (and the tests) are correct. i must've fat-fingered those docs when i wrote them. sorry, and good catch! a PR would be awesome 🎉 🎉 🙇

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants