Skip to content

Commit

Permalink
Fix bug in last_rc logic (bazelbuild#237)
Browse files Browse the repository at this point in the history
We're going to upload rolling releases to GCS, too. While Bazelisk continues to download rolling releases from GitHub, the presence of rolling releases on GCS actually broke the resolution logic for "last_rc". This commit fixes the problem and adds test coverage for similar scenarios.
  • Loading branch information
fweikert authored May 3, 2021
1 parent 1e42403 commit 97a0d60
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 32 deletions.
49 changes: 30 additions & 19 deletions bazelisk_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ func TestMain(m *testing.M) {

func TestResolveLatestRcVersion(t *testing.T) {
s := setUp(t)
s.AddVersion("4.0.0", false)
s.AddVersion("10.0.0", false)
s.AddVersion("11.0.0", true)
s.AddVersion("11.11.0", false, 1, 2)
s.AddVersion("4.0.0", false, nil, nil)
s.AddVersion("10.0.0", false, nil, nil)
s.AddVersion("11.0.0", true, nil, nil)
s.AddVersion("11.11.0", false, []int{1, 2}, nil)
s.AddVersion("12.0.0", false, nil, []string{"12.0.0-pre.20210504.1"})
s.Finish()

gcs := &repositories.GCSRepo{}
Expand All @@ -65,7 +66,7 @@ func TestResolveLatestRcVersion(t *testing.T) {

func TestResolveLatestRcVersion_WithFullRelease(t *testing.T) {
s := setUp(t)
s.AddVersion("4.0.0", true, 1, 2, 3)
s.AddVersion("4.0.0", true, []int{1, 2, 3}, nil)
s.Finish()

gcs := &repositories.GCSRepo{}
Expand All @@ -83,9 +84,9 @@ func TestResolveLatestRcVersion_WithFullRelease(t *testing.T) {

func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T) {
s := setUp(t)
s.AddVersion("4.0.0", true)
s.AddVersion("5.0.0", false)
s.AddVersion("6.0.0", false)
s.AddVersion("4.0.0", true, nil, nil)
s.AddVersion("5.0.0", false, nil, nil)
s.AddVersion("6.0.0", false, nil, nil)
s.Finish()

gcs := &repositories.GCSRepo{}
Expand All @@ -101,12 +102,13 @@ func TestResolveLatestVersion_TwoLatestVersionsDoNotHaveAReleaseYet(t *testing.T
}
}

func TestResolveLatestVersion_FilterReleaseCandidates(t *testing.T) {
func TestResolveLatestVersion_ShouldOnlyReturnStableReleases(t *testing.T) {
s := setUp(t)
s.AddVersion("3.0.0", true)
s.AddVersion("4.0.0", false)
s.AddVersion("5.0.0", false)
s.AddVersion("6.0.0", true)
s.AddVersion("3.0.0", true, []int{1}, nil)
s.AddVersion("4.0.0", false, nil, nil)
s.AddVersion("5.0.0", false, nil, nil)
s.AddVersion("6.0.0", true, []int{1, 2}, nil)
s.AddVersion("7.0.0", false, nil, []string{"12.0.0-pre.20210504.1"})
s.Finish()

gcs := &repositories.GCSRepo{}
Expand All @@ -124,8 +126,8 @@ func TestResolveLatestVersion_FilterReleaseCandidates(t *testing.T) {

func TestResolveLatestVersion_ShouldFailIfNotEnoughReleases(t *testing.T) {
s := setUp(t)
s.AddVersion("3.0.0", true)
s.AddVersion("4.0.0", false)
s.AddVersion("3.0.0", true, nil, nil)
s.AddVersion("4.0.0", false, nil, nil)
s.Finish()

gcs := &repositories.GCSRepo{}
Expand Down Expand Up @@ -239,13 +241,22 @@ type gcsSetup struct {
test *testing.T
}

func (g *gcsSetup) AddVersion(version string, hasRelease bool, rcs ...int) *gcsSetup {
func (g *gcsSetup) AddVersion(version string, hasRelease bool, rcs []int, rolling []string) *gcsSetup {
g.versionPrefixes = append(g.versionPrefixes, fmt.Sprintf("%s/", version))
prefixes := make([]string, 0)

register := func(subDir string) {
path := fmt.Sprintf("%s/%s/", version, subDir)
prefixes = append(prefixes, path)
g.addURL(path, false)
}

for _, rc := range rcs {
prefix := fmt.Sprintf("%s/rc%d/", version, rc)
prefixes = append(prefixes, prefix)
g.addURL(prefix, false)
register(fmt.Sprintf("rc%d", rc))
}

for _, r := range rolling {
register(r)
}

// The /release/ URLs have to exist, even if there is no release. In this case GCS returns no items, though.
Expand Down
32 changes: 19 additions & 13 deletions repositories/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,22 +156,28 @@ func (gcs *GCSRepo) GetCandidateVersions(bazeliskHome string) ([]string, error)
return []string{}, errors.New("could not find any Bazel versions")
}

latestVersion := history[len(history)-1]

// Append slash to match directories
rcPrefixes, _, err := listDirectoriesInReleaseBucket(latestVersion + "/")
if err != nil {
return []string{}, fmt.Errorf("could not list release candidates for latest release: %v", err)
}
// Find most recent directory that contains any release candidates.
// Typically it should be the last or second-to-last, depending on whether there are new rolling releases.
for pos := len(history) - 1; pos >= 0; pos-- {
// Append slash to match directories
bucket := fmt.Sprintf("%s/", history[pos])
rcPrefixes, _, err := listDirectoriesInReleaseBucket(bucket)
if err != nil {
return []string{}, fmt.Errorf("could not list release candidates for latest release: %v", err)
}

rcs := make([]string, 0)
for _, v := range getVersionsFromGCSPrefixes(rcPrefixes) {
// Remove full releases
if strings.Contains(v, "rc") {
rcs = append(rcs, v)
rcs := make([]string, 0)
for _, v := range getVersionsFromGCSPrefixes(rcPrefixes) {
// Remove full and rolling releases
if strings.Contains(v, "rc") {
rcs = append(rcs, v)
}
}
if len(rcs) > 0 {
return rcs, nil
}
}
return rcs, nil
return nil, nil
}

// DownloadCandidate downloads the given release candidate into the specified location and returns the absolute path.
Expand Down

0 comments on commit 97a0d60

Please sign in to comment.