GO: Fix latest-n release history from GCS (bazelbuild#176)

Previously the `latest-n` version identifier could point at incorrect Bazel versions since unreleased versions (i.e. release candidates) were not always discarded from the version history.

Since checking the complete Bazel history (starting with 0.1.0) is costly, this change also adds the lastN parameter to core.ReleaseRepo.GetReleaseVersions(). As a result, the code only examines as few versions as possible.

Fixes bazelbuild#171

bazelisk_version_test.go

@@ -65,7 +65,7 @@ func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T
 	listBody := buildGCSResponseOrFail(t, []string{"4.0.0/", "5.0.0/", "6.0.0/"}, []interface{}{})
 	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 200, listBody)
 
-	v4ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{"fake_release_item__since_this_is_a_release"})
+	v4ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{"fake_release_item_since_this_is_a_release"})
 	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=4.0.0/release/", 200, v4ReleaseBucket)
 
 	v5ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{})
@@ -87,6 +87,58 @@ func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T
 	}
 }
 
+func TestResolveLatestVersion_FilterReleaseCandidates(t *testing.T) {
+	listBody := buildGCSResponseOrFail(t, []string{"3.0.0", "4.0.0/", "5.0.0/", "6.0.0/"}, []interface{}{})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 200, listBody)
+
+	v3ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{"fake_release_item_since_this_is_a_release"})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=3.0.0/release/", 200, v3ReleaseBucket)
+
+	v4ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=4.0.0/release/", 200, v4ReleaseBucket)
+
+	v5ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=5.0.0/release/", 200, v5ReleaseBucket)
+
+	v6ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{"fake_release_item_since_this_is_a_release"})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=6.0.0/release/", 200, v6ReleaseBucket)
+
+	gcs := &repositories.GCSRepo{}
+	repos := core.CreateRepositories(gcs, nil, nil, nil, false)
+	version, _, err := repos.ResolveVersion(tmpDir, versions.BazelUpstream, "latest-1")
+
+	if err != nil {
+		t.Fatalf("Version resolution failed unexpectedly: %v", err)
+	}
+	expectedVersion := "3.0.0"
+	if version != expectedVersion {
+		t.Fatalf("Expected version %s, but got %s", expectedVersion, version)
+	}
+}
+
+func TestResolveLatestVersion_ShouldFailIfNotEnoughReleases(t *testing.T) {
+	listBody := buildGCSResponseOrFail(t, []string{"3.0.0/", "4.0.0/"}, []interface{}{})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 200, listBody)
+
+	v3ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{"fake_release_item_since_this_is_a_release"})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=3.0.0/release/", 200, v3ReleaseBucket)
+
+	v4ReleaseBucket := buildGCSResponseOrFail(t, []string{}, []interface{}{})
+	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/&prefix=4.0.0/release/", 200, v4ReleaseBucket)
+
+	gcs := &repositories.GCSRepo{}
+	repos := core.CreateRepositories(gcs, nil, nil, nil, false)
+	_, _, err := repos.ResolveVersion(tmpDir, versions.BazelUpstream, "latest-1")
+
+	if err == nil {
+		t.Fatal("Expected ResolveVersion() to fail.")
+	}
+	expectedError := "unable to determine latest version: requested 2 latest releases, but only found 1"
+	if err.Error() != expectedError {
+		t.Fatalf("Expected error message '%s', but got '%v'", expectedError, err)
+	}
+}
+
 func TestResolveLatestVersion_GCSIsDown(t *testing.T) {
 	transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 500, "")
 

core/repositories.go

@@ -18,8 +18,8 @@ type DownloadFunc func(destDir, destFile string) (string, error)
 
 // ReleaseRepo represents a repository that stores Bazel releases.
 type ReleaseRepo interface {
-	// GetReleaseVersions returns a list of all available release versions.
-	GetReleaseVersions(bazeliskHome string) ([]string, error)
+	// GetReleaseVersions returns a list of all available release versions. If lastN is smaller than 1, all available versions are being returned.
+	GetReleaseVersions(bazeliskHome string, lastN int) ([]string, error)
 
 	// DownloadRelease downloads the given Bazel version into the specified location and returns the absolute path.
 	DownloadRelease(version, destDir, destFile string) (string, error)
@@ -102,7 +102,10 @@ func (r *Repositories) resolveFork(bazeliskHome string, vi *versions.Info) (stri
 }
 
 func (r *Repositories) resolveRelease(bazeliskHome string, vi *versions.Info) (string, DownloadFunc, error) {
-	version, err := resolvePotentiallyRelativeVersion(bazeliskHome, r.Releases.GetReleaseVersions, vi)
+	lister := func(bazeliskHome string) ([]string, error) {
+		return r.Releases.GetReleaseVersions(bazeliskHome, vi.LatestOffset+1)
+	}
+	version, err := resolvePotentiallyRelativeVersion(bazeliskHome, lister, vi)
 	if err != nil {
 		return "", nil, err
 	}
@@ -212,7 +215,7 @@ type noReleaseRepo struct {
 	Error error
 }
 
-func (nrr *noReleaseRepo) GetReleaseVersions(bazeliskHome string) ([]string, error) {
+func (nrr *noReleaseRepo) GetReleaseVersions(bazeliskHome string, lastN int) ([]string, error) {
 	return []string{}, nrr.Error
 }
 

repositories/gcs.go

@@ -36,38 +36,29 @@ type GCSRepo struct{}
 // ReleaseRepo
 
 // GetReleaseVersions returns the versions of all available Bazel releases in this repository.
-func (gcs *GCSRepo) GetReleaseVersions(bazeliskHome string) ([]string, error) {
-	return getVersionHistoryFromGCS(true)
+func (gcs *GCSRepo) GetReleaseVersions(bazeliskHome string, lastN int) ([]string, error) {
+	history, err := getVersionHistoryFromGCS()
+	if err != nil {
+		return []string{}, err
+	}
+	releases, err := gcs.removeCandidates(history, lastN)
+	if err != nil {
+		return []string{}, err
+	}
+	if len(releases) == 0 {
+		return []string{}, errors.New("there are no releases available")
+	}
+	return releases, nil
 }
 
-func getVersionHistoryFromGCS(onlyFullReleases bool) ([]string, error) {
+func getVersionHistoryFromGCS() ([]string, error) {
 	prefixes, _, err := listDirectoriesInReleaseBucket("")
 	if err != nil {
 		return []string{}, fmt.Errorf("could not list Bazel versions in GCS bucket: %v", err)
 	}
 
 	available := getVersionsFromGCSPrefixes(prefixes)
 	sorted := versions.GetInAscendingOrder(available)
-
-	// TODO(#171): This algorithm is incorrect if version == 'latest-n' and any of the last n versions (except the last one) does not have a release yet.
-	if onlyFullReleases {
-		for len(sorted) > 0 {
-			lastIndex := len(sorted) - 1
-			latestVersion := sorted[lastIndex]
-			_, isRelease, err := listDirectoriesInReleaseBucket(latestVersion + "/release/")
-			if err != nil {
-				return []string{}, fmt.Errorf("could not list available releases for %v: %v", latestVersion, err)
-			}
-			if isRelease {
-				break
-			}
-			sorted = sorted[:lastIndex]
-		}
-		if len(sorted) == 0 {
-			return []string{}, errors.New("there are no releases available")
-		}
-	}
-
 	return sorted, nil
 }
 
@@ -117,21 +108,54 @@ func (gcs *GCSRepo) DownloadRelease(version, destDir, destFile string) (string,
 	return httputil.DownloadBinary(url, destDir, destFile)
 }
 
+func (gcs *GCSRepo) removeCandidates(history []string, lastN int) ([]string, error) {
+	var resolvedLimit int
+	if lastN < 1 {
+		resolvedLimit = len(history)
+	} else {
+		resolvedLimit = lastN
+	}
+
+	descendingReleases := make([]string, 0)
+	for hpos := len(history) - 1; hpos >= 0 && len(descendingReleases) < resolvedLimit; hpos-- {
+		latestVersion := history[hpos]
+		_, isRelease, err := listDirectoriesInReleaseBucket(latestVersion + "/release/")
+		if err != nil {
+			return []string{}, fmt.Errorf("could not list available releases for %v: %v", latestVersion, err)
+		}
+		if isRelease {
+			descendingReleases = append(descendingReleases, latestVersion)
+		}
+	}
+
+	if lastN > 0 && len(descendingReleases) < lastN {
+		return []string{}, fmt.Errorf("requested %d latest releases, but only found %d", lastN, len(descendingReleases))
+	}
+	return reverseInPlace(descendingReleases), nil
+}
+
+func reverseInPlace(values []string) []string {
+	for i := 0; i < len(values)/2; i++ {
+		j := len(values) - 1 - i
+		values[i], values[j] = values[j], values[i]
+	}
+	return values
+}
+
 // CandidateRepo
 
-// GetCandidateVersions returns all versions of available release candidates in this repository.
+// GetCandidateVersions returns all versions of available release candidates for the latest release in this repository.
 func (gcs *GCSRepo) GetCandidateVersions(bazeliskHome string) ([]string, error) {
-	available, err := getVersionHistoryFromGCS(false)
+	history, err := getVersionHistoryFromGCS()
 	if err != nil {
 		return []string{}, err
 	}
 
-	if len(available) == 0 {
+	if len(history) == 0 {
 		return []string{}, errors.New("could not find any Bazel versions")
 	}
 
-	sorted := versions.GetInAscendingOrder(available)
-	latestVersion := sorted[len(sorted)-1]
+	latestVersion := history[len(history)-1]
 
 	// Append slash to match directories
 	rcPrefixes, _, err := listDirectoriesInReleaseBucket(latestVersion + "/")