Merge pull request #1378 from Zillode/draft-upgrade

Do not consider draft releases or releases with emtpy assets

cmd/syncthing/gui.go

@@ -618,7 +618,7 @@ func restGetUpgrade(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), 500)
 		return
 	}
-	rel, err := upgrade.LatestRelease(strings.Contains(Version, "-beta"))
+	rel, err := upgrade.LatestGithubRelease(Version)
 	if err != nil {
 		http.Error(w, err.Error(), 500)
 		return
@@ -660,7 +660,7 @@ func restGetLang(w http.ResponseWriter, r *http.Request) {
 }
 
 func restPostUpgrade(w http.ResponseWriter, r *http.Request) {
-	rel, err := upgrade.LatestRelease(strings.Contains(Version, "-beta"))
+	rel, err := upgrade.LatestGithubRelease(Version)
 	if err != nil {
 		l.Warnln("getting latest release:", err)
 		http.Error(w, err.Error(), 500)

cmd/syncthing/main.go

@@ -50,7 +50,6 @@ var (
 	BuildHost   = "unknown"
 	BuildUser   = "unknown"
 	IsRelease   bool
-	IsBeta      bool
 	LongVersion string
 )
 
@@ -79,9 +78,6 @@ func init() {
 	exp := regexp.MustCompile(`^v\d+\.\d+\.\d+(-beta[\d\.]+)?$`)
 	IsRelease = exp.MatchString(Version)
 
-	// Check for a beta build
-	IsBeta = strings.Contains(Version, "-beta")
-
 	stamp, _ := strconv.Atoi(BuildStamp)
 	BuildDate = time.Unix(int64(stamp), 0)
 
@@ -327,7 +323,7 @@ func main() {
 	}
 
 	if doUpgrade || doUpgradeCheck {
-		rel, err := upgrade.LatestRelease(IsBeta)
+		rel, err := upgrade.LatestGithubRelease(Version)
 		if err != nil {
 			l.Fatalln("Upgrade:", err) // exits 1
 		}
@@ -999,7 +995,7 @@ func autoUpgrade() {
 		case <-timer.C:
 		}
 
-		rel, err := upgrade.LatestRelease(IsBeta)
+		rel, err := upgrade.LatestGithubRelease(Version)
 		if err == upgrade.ErrUpgradeUnsupported {
 			events.Default.Unsubscribe(sub)
 			return

internal/upgrade/upgrade_supported.go

@@ -23,11 +23,12 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strings"
 )
 
 // Returns the latest release, including prereleases or not depending on the argument
-func LatestRelease(prerelease bool) (Release, error) {
+func LatestGithubRelease(version string) (Release, error) {
 	resp, err := http.Get("https://api.github.com/repos/syncthing/syncthing/releases?per_page=10")
 	if err != nil {
 		return Release{}, err
@@ -40,19 +41,47 @@ func LatestRelease(prerelease bool) (Release, error) {
 	json.NewDecoder(resp.Body).Decode(&rels)
 	resp.Body.Close()
 
+	return LatestRelease(version, rels)
+}
+
+type SortByRelease []Release
+
+func (s SortByRelease) Len() int {
+	return len(s)
+}
+func (s SortByRelease) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+func (s SortByRelease) Less(i, j int) bool {
+	return CompareVersions(s[i].Tag, s[j].Tag) > 0
+}
+
+func LatestRelease(version string, rels []Release) (Release, error) {
 	if len(rels) == 0 {
 		return Release{}, ErrVersionUnknown
 	}
 
-	if prerelease {
-		// We are a beta version. Use the latest.
-		return rels[0], nil
-	}
+	sort.Sort(SortByRelease(rels))
+	// Check for a beta build
+	beta := strings.Contains(version, "-beta")
 
-	// We are a regular release. Only consider non-prerelease versions for upgrade.
 	for _, rel := range rels {
-		if !rel.Prerelease {
-			return rel, nil
+		if rel.Prerelease && !beta {
+			continue
+		}
+		for _, asset := range rel.Assets {
+			assetName := path.Base(asset.Name)
+			// Check for the architecture
+			expectedRelease := releaseName(rel.Tag)
+			if debug {
+				l.Debugf("expected release asset %q", expectedRelease)
+			}
+			if debug {
+				l.Debugln("considering release", assetName)
+			}
+			if strings.HasPrefix(assetName, expectedRelease) {
+				return rel, nil
+			}
 		}
 	}
 	return Release{}, ErrVersionUnknown

internal/upgrade/upgrade_test.go

@@ -6,9 +6,13 @@
 
 package upgrade
 
-import "testing"
+import (
+	"encoding/json"
+	"os"
+	"testing"
+)
 
-var testcases = []struct {
+var versions = []struct {
 	a, b string
 	r    Relation
 }{
@@ -27,6 +31,7 @@ var testcases = []struct {
 	{"0.10.0", "0.2.0", MajorNewer},
 	{"30.10.0", "4.9.0", MajorNewer},
 	{"0.9.0-beta7", "0.9.0-beta6", Newer},
+	{"0.9.0-beta7", "1.0.0-alpha", MajorOlder},
 	{"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},
@@ -44,9 +49,39 @@ var testcases = []struct {
 }
 
 func TestCompareVersions(t *testing.T) {
-	for _, tc := range testcases {
-		if r := CompareVersions(tc.a, tc.b); r != tc.r {
-			t.Errorf("compareVersions(%q, %q): %d != %d", tc.a, tc.b, r, tc.r)
+	for _, v := range versions {
+		if r := CompareVersions(v.a, v.b); r != v.r {
+			t.Errorf("compareVersions(%q, %q): %d != %d", v.a, v.b, r, v.r)
+		}
+	}
+}
+
+var upgrades = map[string]string{
+	"v0.10.21":                        "v0.10.30",
+	"v0.10.29":                        "v0.10.30",
+	"v0.10.31":                        "v0.10.30",
+	"v0.10.0-alpha":                   "v0.10.30",
+	"v0.10.0-beta":                    "v0.11.0-beta0",
+	"v0.11.0-beta0+40-g53cb66e-dirty": "v0.11.0-beta0",
+}
+
+func TestRelease(t *testing.T) {
+	fd, err := os.Open("testdata/github-releases.json")
+	if err != nil {
+		t.Errorf("Missing github-release test data")
+	}
+	defer fd.Close()
+
+	var rels []Release
+	json.NewDecoder(fd).Decode(&rels)
+
+	for old, target := range upgrades {
+		upgrade, err := LatestRelease(old, rels)
+		if err != nil {
+			t.Errorf("error retrieving latest version", err)
+		}
+		if upgrade.Tag != target {
+			t.Errorf("Invalid upgrade release: %v -> %v, but got %v", old, target, upgrade.Tag)
 		}
 	}
 }