cmd/version: factor version number parsing routines into fs/version

cmd/version/version.go

@@ -4,13 +4,12 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"regexp"
-	"strconv"
 	"strings"
 	"time"
 
 	"github.com/ncw/rclone/cmd"
 	"github.com/ncw/rclone/fs"
+	"github.com/ncw/rclone/fs/version"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
@@ -66,63 +65,8 @@ Or
 	},
 }
 
-var parseVersion = regexp.MustCompile(`^(?:rclone )?v(\d+)\.(\d+)(?:\.(\d+))?(?:-(\d+)(?:-(g[\wβ-]+))?)?$`)
-
-type version []int
-
-func newVersion(in string) (v version, err error) {
-	r := parseVersion.FindStringSubmatch(in)
-	if r == nil {
-		return v, errors.Errorf("failed to match version string %q", in)
-	}
-	atoi := func(s string) int {
-		i, err := strconv.Atoi(s)
-		if err != nil {
-			fs.Errorf(nil, "Failed to parse %q as int from %q: %v", s, in, err)
-		}
-		return i
-	}
-	v = version{
-		atoi(r[1]), // major
-		atoi(r[2]), // minor
-	}
-	if r[3] != "" {
-		v = append(v, atoi(r[3])) // patch
-	} else if r[4] != "" {
-		v = append(v, 0) // patch
-	}
-	if r[4] != "" {
-		v = append(v, atoi(r[4])) // dev
-	}
-	return v, nil
-}
-
-// String converts v to a string
-func (v version) String() string {
-	var out []string
-	for _, vv := range v {
-		out = append(out, fmt.Sprint(vv))
-	}
-	return strings.Join(out, ".")
-}
-
-// cmp compares two versions returning >0, <0 or 0
-func (v version) cmp(o version) (d int) {
-	n := len(v)
-	if n > len(o) {
-		n = len(o)
-	}
-	for i := 0; i < n; i++ {
-		d = v[i] - o[i]
-		if d != 0 {
-			return d
-		}
-	}
-	return len(v) - len(o)
-}
-
 // getVersion gets the version by checking the download repository passed in
-func getVersion(url string) (v version, vs string, date time.Time, err error) {
+func getVersion(url string) (v version.Version, vs string, date time.Time, err error) {
 	resp, err := http.Get(url)
 	if err != nil {
 		return v, vs, date, err
@@ -144,26 +88,17 @@ func getVersion(url string) (v version, vs string, date time.Time, err error) {
 	if err != nil {
 		return v, vs, date, err
 	}
-	v, err = newVersion(vs)
+	v, err = version.New(vs)
 	return v, vs, date, err
 }
 
 // check the current version against available versions
 func checkVersion() {
 	// Get Current version
-	currentVersion := fs.Version
+	vCurrent, err := version.New(fs.Version)
-	currentIsGit := strings.HasSuffix(currentVersion, "-DEV")
-	if currentIsGit {
-		currentVersion = currentVersion[:len(currentVersion)-4]
-	}
-	vCurrent, err := newVersion(currentVersion)
 	if err != nil {
 		fs.Errorf(nil, "Failed to get parse version: %v", err)
 	}
-	if currentIsGit {
-		vCurrent = append(vCurrent, 999, 999)
-	}
-
 	const timeFormat = "2006-01-02"
 
 	printVersion := func(what, url string) {
@@ -177,7 +112,7 @@ func checkVersion() {
 			v,
 			"(released "+t.Format(timeFormat)+")",
 		)
-		if v.cmp(vCurrent) > 0 {
+		if v.Cmp(vCurrent) > 0 {
 			fmt.Printf("  upgrade: %s\n", url+vs)
 		}
 	}
@@ -190,7 +125,7 @@ func checkVersion() {
 		"beta",
 		"https://beta.rclone.org/",
 	)
-	if currentIsGit {
+	if vCurrent.IsGit() {
 		fmt.Println("Your version is compiled from git so comparisons may be wrong.")
 	}
 }

cmd/version/version_test.go

@@ -1,7 +1,6 @@
 package version
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"runtime"
@@ -46,65 +45,3 @@ func TestVersionWorksWithoutAccessibleConfigFile(t *testing.T) {
 	// 	assert.NoError(t, cmd.Root.Execute())
 	// })
 }
-
-func TestVersionNew(t *testing.T) {
-	for _, test := range []struct {
-		in      string
-		want    version
-		wantErr bool
-	}{
-		{"v1.41", version{1, 41}, false},
-		{"rclone v1.41", version{1, 41}, false},
-		{"rclone v1.41.23", version{1, 41, 23}, false},
-		{"rclone v1.41.23-100", version{1, 41, 23, 100}, false},
-		{"rclone v1.41-100", version{1, 41, 0, 100}, false},
-		{"rclone v1.41.23-100-g12312a", version{1, 41, 23, 100}, false},
-		{"rclone v1.41-100-g12312a", version{1, 41, 0, 100}, false},
-		{"rclone v1.42-005-g56e1e820β", version{1, 42, 0, 5}, false},
-		{"rclone v1.42-005-g56e1e820-feature-branchβ", version{1, 42, 0, 5}, false},
-
-		{"v1.41s", nil, true},
-		{"rclone v1-41", nil, true},
-		{"rclone v1.41.2c3", nil, true},
-		{"rclone v1.41.23-100 potato", nil, true},
-		{"rclone 1.41-100", nil, true},
-		{"rclone v1.41.23-100-12312a", nil, true},
-	} {
-		what := fmt.Sprintf("in=%q", test.in)
-		got, err := newVersion(test.in)
-		if test.wantErr {
-			assert.Error(t, err, what)
-		} else {
-			assert.NoError(t, err, what)
-		}
-		assert.Equal(t, test.want, got, what)
-	}
-
-}
-
-func TestVersionCmp(t *testing.T) {
-	for _, test := range []struct {
-		a, b version
-		want int
-	}{
-		{version{1}, version{1}, 0},
-		{version{1}, version{2}, -1},
-		{version{2}, version{1}, 1},
-		{version{2}, version{2, 1}, -1},
-		{version{2, 1}, version{2}, 1},
-		{version{2, 1}, version{2, 1}, 0},
-		{version{2, 1}, version{2, 2}, -1},
-		{version{2, 2}, version{2, 1}, 1},
-	} {
-		got := test.a.cmp(test.b)
-		if got < 0 {
-			got = -1
-		} else if got > 0 {
-			got = 1
-		}
-		assert.Equal(t, test.want, got, fmt.Sprintf("%v cmp %v", test.a, test.b))
-		// test the reverse
-		got = -test.b.cmp(test.a)
-		assert.Equal(t, test.want, got, fmt.Sprintf("%v cmp %v", test.b, test.a))
-	}
-}

fs/version/version.go

@@ -0,0 +1,86 @@
+package version
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+
+	"github.com/ncw/rclone/fs"
+	"github.com/pkg/errors"
+)
+
+// Version represents a parsed rclone version number
+type Version []int
+
+var parseVersion = regexp.MustCompile(`^(?:rclone )?v(\d+)\.(\d+)(?:\.(\d+))?(?:-(\d+)(?:-(g[\wβ-]+))?)?$`)
+
+// New parses a version number from a string
+//
+// This will be returned with up to 4 elements for major, minor,
+// patch, subpatch release.
+//
+// If the version number represents a compiled from git version
+// number, then it will be returned as major, minor, 999, 999
+func New(in string) (v Version, err error) {
+	isGit := strings.HasSuffix(in, "-DEV")
+	if isGit {
+		in = in[:len(in)-4]
+	}
+	r := parseVersion.FindStringSubmatch(in)
+	if r == nil {
+		return v, errors.Errorf("failed to match version string %q", in)
+	}
+	atoi := func(s string) int {
+		i, err := strconv.Atoi(s)
+		if err != nil {
+			fs.Errorf(nil, "Failed to parse %q as int from %q: %v", s, in, err)
+		}
+		return i
+	}
+	v = Version{
+		atoi(r[1]), // major
+		atoi(r[2]), // minor
+	}
+	if r[3] != "" {
+		v = append(v, atoi(r[3])) // patch
+	} else if r[4] != "" {
+		v = append(v, 0) // patch
+	}
+	if r[4] != "" {
+		v = append(v, atoi(r[4])) // dev
+	}
+	if isGit {
+		v = append(v, 999, 999)
+	}
+	return v, nil
+}
+
+// String converts v to a string
+func (v Version) String() string {
+	var out []string
+	for _, vv := range v {
+		out = append(out, fmt.Sprint(vv))
+	}
+	return strings.Join(out, ".")
+}
+
+// Cmp compares two versions returning >0, <0 or 0
+func (v Version) Cmp(o Version) (d int) {
+	n := len(v)
+	if n > len(o) {
+		n = len(o)
+	}
+	for i := 0; i < n; i++ {
+		d = v[i] - o[i]
+		if d != 0 {
+			return d
+		}
+	}
+	return len(v) - len(o)
+}
+
+// IsGit returns true if the current version was compiled from git
+func (v Version) IsGit() bool {
+	return len(v) >= 4 && v[2] == 999 && v[3] == 999
+}

fs/version/version_test.go

@@ -0,0 +1,89 @@
+package version
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNew(t *testing.T) {
+	for _, test := range []struct {
+		in      string
+		want    Version
+		wantErr bool
+	}{
+		{"v1.41", Version{1, 41}, false},
+		{"rclone v1.41", Version{1, 41}, false},
+		{"rclone v1.41.23", Version{1, 41, 23}, false},
+		{"rclone v1.41.23-100", Version{1, 41, 23, 100}, false},
+		{"rclone v1.41-100", Version{1, 41, 0, 100}, false},
+		{"rclone v1.41.23-100-g12312a", Version{1, 41, 23, 100}, false},
+		{"rclone v1.41-100-g12312a", Version{1, 41, 0, 100}, false},
+		{"rclone v1.42-005-g56e1e820β", Version{1, 42, 0, 5}, false},
+		{"rclone v1.42-005-g56e1e820-feature-branchβ", Version{1, 42, 0, 5}, false},
+
+		{"v1.41s", nil, true},
+		{"rclone v1-41", nil, true},
+		{"rclone v1.41.2c3", nil, true},
+		{"rclone v1.41.23-100 potato", nil, true},
+		{"rclone 1.41-100", nil, true},
+		{"rclone v1.41.23-100-12312a", nil, true},
+
+		{"v1.41-DEV", Version{1, 41, 999, 999}, false},
+	} {
+		what := fmt.Sprintf("in=%q", test.in)
+		got, err := New(test.in)
+		if test.wantErr {
+			assert.Error(t, err, what)
+		} else {
+			assert.NoError(t, err, what)
+		}
+		assert.Equal(t, test.want, got, what)
+	}
+
+}
+
+func TestCmp(t *testing.T) {
+	for _, test := range []struct {
+		a, b Version
+		want int
+	}{
+		{Version{1}, Version{1}, 0},
+		{Version{1}, Version{2}, -1},
+		{Version{2}, Version{1}, 1},
+		{Version{2}, Version{2, 1}, -1},
+		{Version{2, 1}, Version{2}, 1},
+		{Version{2, 1}, Version{2, 1}, 0},
+		{Version{2, 1}, Version{2, 2}, -1},
+		{Version{2, 2}, Version{2, 1}, 1},
+	} {
+		got := test.a.Cmp(test.b)
+		if got < 0 {
+			got = -1
+		} else if got > 0 {
+			got = 1
+		}
+		assert.Equal(t, test.want, got, fmt.Sprintf("%v cmp %v", test.a, test.b))
+		// test the reverse
+		got = -test.b.Cmp(test.a)
+		assert.Equal(t, test.want, got, fmt.Sprintf("%v cmp %v", test.b, test.a))
+	}
+}
+
+func TestString(t *testing.T) {
+	v, err := New("v1.44.1-2")
+	assert.NoError(t, err)
+
+	assert.Equal(t, "1.44.1.2", v.String())
+}
+
+func TestIsGit(t *testing.T) {
+	v, err := New("v1.44")
+	assert.NoError(t, err)
+	assert.Equal(t, false, v.IsGit())
+
+	v, err = New("v1.44-DEV")
+	assert.NoError(t, err)
+	assert.Equal(t, true, v.IsGit())
+}