Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GO: Return latest version from GCS that is a full release #172

Merged
merged 3 commits into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .bazelci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ platforms:
ubuntu1604:
test_targets:
- //:go_bazelisk_test
- //:go_default_test
- //:go_version_test
- //:py_bazelisk_test
- //:py3_bazelisk_test
test_flags:
Expand All @@ -11,6 +13,8 @@ platforms:
ubuntu1804:
test_targets:
- //:go_bazelisk_test
- //:go_default_test
- //:go_version_test
- //:py_bazelisk_test
- //:py3_bazelisk_test
test_flags:
Expand All @@ -19,6 +23,8 @@ platforms:
macos:
test_targets:
- //:go_bazelisk_test
- //:go_default_test
- //:go_version_test
- //:py_bazelisk_test
- //:py3_bazelisk_test
test_flags:
Expand All @@ -27,6 +33,8 @@ platforms:
windows:
test_targets:
- //:go_bazelisk_test
- //:go_default_test
- //:go_version_test
- //:py_bazelisk_test
test_flags:
- --flaky_test_attempts=1
Expand Down
4 changes: 4 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ go_test(
embed = [":go_default_library"],
importpath = "github.com/bazelbuild/bazelisk",
deps = [
"//core",
"//httputil",
"//repositories",
"//versions",
"@io_bazel_rules_go//go/tools/bazel:go_default_library",
],
)
Expand Down
26 changes: 26 additions & 0 deletions bazelisk_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,32 @@ func TestResolveLatestRcVersion(t *testing.T) {
}
}

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"})
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{}{})
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")

if err != nil {
t.Fatalf("Version resolution failed unexpectedly: %v", err)
}
expectedVersion := "4.0.0"
if version != expectedVersion {
t.Fatalf("Expected version %s, but got %s", expectedVersion, version)
}
}

func TestResolveLatestVersion_GCSIsDown(t *testing.T) {
transport.AddResponse("https://www.googleapis.com/storage/v1/b/bazel/o?delimiter=/", 500, "")

Expand Down
1 change: 0 additions & 1 deletion core/repositories.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Package core contains interfaces to work with Bazel repositories.
package core

import (
Expand Down
22 changes: 15 additions & 7 deletions repositories/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,22 @@ func getVersionHistoryFromGCS(onlyFullReleases bool) ([]string, error) {
available := getVersionsFromGCSPrefixes(prefixes)
sorted := versions.GetInAscendingOrder(available)

if onlyFullReleases && len(sorted) > 0 {
latestVersion := sorted[len(sorted)-1]
_, isRelease, err := listDirectoriesInReleaseBucket(latestVersion + "/release/")
if err != nil {
return []string{}, fmt.Errorf("could not list release candidates for latest release: %v", err)
// 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my understanding, what we would actually want to do here is to basically filter the sorted array and remove all entries for which no release version exists, right? But we don't do that, because you think it's too slow due to the number of HTTP requests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the problem. But we can work around this by specifying how many of the latest releases we need: #171

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 !isRelease {
sorted = sorted[:len(sorted)-1]
if len(sorted) == 0 {
return []string{}, errors.New("there are no releases available")
}
}

Expand Down