Make auto upgrade careful about breaking changes (fixes #1047)

cmd/syncthing/main.go

@@ -1274,7 +1274,8 @@ func autoUpgrade() {
 			continue
 		}
 
-		if upgrade.CompareVersions(rel.Tag, Version) <= 0 {
+		if upgrade.CompareVersions(rel.Tag, Version) != upgrade.Newer {
+			// Skip equal, older or majorly newer (incompatible) versions
 			continue
 		}
 

internal/upgrade/upgrade_common.go

@@ -87,8 +87,18 @@ func ToURL(url string) error {
 	}
 }
 
-// Returns 1 if a>b, -1 if a<b and 0 if they are equal
-func CompareVersions(a, b string) int {
+type Relation int
+
+const (
+	MajorOlder Relation = -2 // Older by a major version (x in x.y.z or 0.x.y).
+	Older               = -1 // Older by a minor version (y or z in x.y.z, or y in 0.x.y)
+	Equal               = 0  // Versions are semantically equal
+	Newer               = 1  // Newer by a minor version (y or z in x.y.z, or y in 0.x.y)
+	MajorNewer          = 2  // Newer by a major version (x in x.y.z or 0.x.y).
+)
+
+// Returns a relation describing how a compares to b.
+func CompareVersions(a, b string) Relation {
 	arel, apre := versionParts(a)
 	brel, bpre := versionParts(b)
 
@@ -100,27 +110,39 @@ func CompareVersions(a, b string) int {
 	// First compare major-minor-patch versions
 	for i := 0; i < minlen; i++ {
 		if arel[i] < brel[i] {
-			return -1
+			if i == 0 {
+				return MajorOlder
+			}
+			if i == 1 && arel[0] == 0 {
+				return MajorOlder
+			}
+			return Older
 		}
 		if arel[i] > brel[i] {
-			return 1
+			if i == 0 {
+				return MajorNewer
+			}
+			if i == 1 && arel[0] == 0 {
+				return MajorNewer
+			}
+			return Newer
 		}
 	}
 
 	// Longer version is newer, when the preceding parts are equal
 	if len(arel) < len(brel) {
-		return -1
+		return Older
 	}
 	if len(arel) > len(brel) {
-		return 1
+		return Newer
 	}
 
 	// Prerelease versions are older, if the versions are the same
 	if len(apre) == 0 && len(bpre) > 0 {
-		return 1
+		return Newer
 	}
 	if len(apre) > 0 && len(bpre) == 0 {
-		return -1
+		return Older
 	}
 
 	minlen = len(apre)
@@ -135,24 +157,24 @@ func CompareVersions(a, b string) int {
 			switch bv := bpre[i].(type) {
 			case int:
 				if av < bv {
-					return -1
+					return Older
 				}
 				if av > bv {
-					return 1
+					return Newer
 				}
 			case string:
-				return -1
+				return Older
 			}
 		case string:
 			switch bv := bpre[i].(type) {
 			case int:
-				return 1
+				return Newer
 			case string:
 				if av < bv {
-					return -1
+					return Older
 				}
 				if av > bv {
-					return 1
+					return Newer
 				}
 			}
 		}
@@ -160,14 +182,14 @@ func CompareVersions(a, b string) int {
 
 	// If all else is equal, longer prerelease string is newer
 	if len(apre) < len(bpre) {
-		return -1
+		return Older
 	}
 	if len(apre) > len(bpre) {
-		return 1
+		return Newer
 	}
 
 	// Looks like they're actually the same
-	return 0
+	return Equal
 }
 
 // Split a version into parts.

internal/upgrade/upgrade_test.go

@@ -19,33 +19,37 @@ import "testing"
 
 var testcases = []struct {
 	a, b string
-	r    int
+	r    Relation
 }{
-	{"0.1.2", "0.1.2", 0},
-	{"0.1.3", "0.1.2", 1},
-	{"0.1.1", "0.1.2", -1},
-	{"0.3.0", "0.1.2", 1},
-	{"0.0.9", "0.1.2", -1},
-	{"1.1.2", "0.1.2", 1},
-	{"0.1.2", "1.1.2", -1},
-	{"0.1.10", "0.1.9", 1},
-	{"0.10.0", "0.2.0", 1},
-	{"30.10.0", "4.9.0", 1},
-	{"0.9.0-beta7", "0.9.0-beta6", 1},
-	{"1.0.0-alpha", "1.0.0-alpha.1", -1},
-	{"1.0.0-alpha.1", "1.0.0-alpha.beta", -1},
-	{"1.0.0-alpha.beta", "1.0.0-beta", -1},
-	{"1.0.0-beta", "1.0.0-beta.2", -1},
-	{"1.0.0-beta.2", "1.0.0-beta.11", -1},
-	{"1.0.0-beta.11", "1.0.0-rc.1", -1},
-	{"1.0.0-rc.1", "1.0.0", -1},
-	{"1.0.0+45", "1.0.0+23-dev-foo", 0},
-	{"1.0.0-beta.23+45", "1.0.0-beta.23+23-dev-foo", 0},
-	{"1.0.0-beta.3+99", "1.0.0-beta.24+0", -1},
+	{"0.1.2", "0.1.2", Equal},
+	{"0.1.3", "0.1.2", Newer},
+	{"0.1.1", "0.1.2", Older},
+	{"0.3.0", "0.1.2", MajorNewer},
+	{"0.0.9", "0.1.2", MajorOlder},
+	{"1.3.0", "1.1.2", Newer},
+	{"1.0.9", "1.1.2", Older},
+	{"2.3.0", "1.1.2", MajorNewer},
+	{"1.0.9", "2.1.2", MajorOlder},
+	{"1.1.2", "0.1.2", MajorNewer},
+	{"0.1.2", "1.1.2", MajorOlder},
+	{"0.1.10", "0.1.9", Newer},
+	{"0.10.0", "0.2.0", MajorNewer},
+	{"30.10.0", "4.9.0", MajorNewer},
+	{"0.9.0-beta7", "0.9.0-beta6", Newer},
+	{"1.0.0-alpha", "1.0.0-alpha.1", Older},
+	{"1.0.0-alpha.1", "1.0.0-alpha.beta", Older},
+	{"1.0.0-alpha.beta", "1.0.0-beta", Older},
+	{"1.0.0-beta", "1.0.0-beta.2", Older},
+	{"1.0.0-beta.2", "1.0.0-beta.11", Older},
+	{"1.0.0-beta.11", "1.0.0-rc.1", Older},
+	{"1.0.0-rc.1", "1.0.0", Older},
+	{"1.0.0+45", "1.0.0+23-dev-foo", Equal},
+	{"1.0.0-beta.23+45", "1.0.0-beta.23+23-dev-foo", Equal},
+	{"1.0.0-beta.3+99", "1.0.0-beta.24+0", Older},
 
-	{"v1.1.2", "1.1.2", 0},
-	{"v1.1.2", "V1.1.2", 0},
-	{"1.1.2", "V1.1.2", 0},
+	{"v1.1.2", "1.1.2", Equal},
+	{"v1.1.2", "V1.1.2", Equal},
+	{"1.1.2", "V1.1.2", Equal},
 }
 
 func TestCompareVersions(t *testing.T) {