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

Bazelisk-Go: Version history from GCS is incorrect wrt release candidates #171

Closed
fweikert opened this issue Sep 27, 2020 · 0 comments · Fixed by #176
Closed

Bazelisk-Go: Version history from GCS is incorrect wrt release candidates #171

fweikert opened this issue Sep 27, 2020 · 0 comments · Fixed by #176

Comments

@fweikert
Copy link
Member

Problem:

When setting the Bazel version to latest, latest-1 etc. or no value at all, Bazelisk-go cannot handle Bazel versions that have a release candidate, but no release yet.

Explanation:

This is a bug in the code that retrieves available Bazel versions from GCS: The code currently fetches the names of all available directories in the bucket (e.g. '3.4.0', '3.4.1', '3.5.0', ...), then sorts them and checks whether the last element has a release/ subdirectory. If not, the last element is removed from the version history before returning the shorter list.

There are two problems with this approach:

  1. latest does not work if both the last version and the second-to-last one don't have a full release yet, since the second-to-last version is never examined in detail (example breakage: Bazelisk is pulling a non existing release ( 3.5.1 ) when invoked w/o .bazelversion #170).
  2. latest-n returns an incorrect result if any of the latest N versions (except the very last one) does not have a release yet.

Problem 1 can be fixed easily by looping until the last version has a release. Problem 2 is harder to fix since we would have to check every existing version for whether a corresponding release exists (i.e. starting with 0.1.0). This change would have severe performance implications, which is especially bad since this code path is executed very frequently. One idea would be to change the GetReleaseVersions() function of the ReleaseRepo interface by adding an additional "lastN" parameter, which would reduce the number of GCS queries.

fweikert added a commit that referenced this issue Sep 27, 2020
If `latest-n` was requested, Bazelisk previously only examined the most recent version to see whether it had a release (and not just candidates).
If there was no release, Bazelisk simply returned the second-to-last without checking whether that version had a release, too.

Now Bazelisk checks the latest versions until it finds one with a release.

Fixes #170
Progress towards #171
fweikert added a commit that referenced this issue Oct 1, 2020
If `latest-n` was requested, Bazelisk previously only examined the most recent version to see whether it had a release (and not just candidates).
If there was no release, Bazelisk simply returned the second-to-last without checking whether that version had a release, too.

Now Bazelisk checks the latest versions until it finds one with a release.

Fixes #170
Progress towards #171
fweikert added a commit to fweikert/bazelisk that referenced this issue Oct 2, 2020
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
fweikert added a commit that referenced this issue Oct 5, 2020
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 #171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant