Improve last green commit support (#614)

Author: meteorcloudy

README.md

@@ -66,11 +66,12 @@ Bazelisk currently understands the following formats for version labels:
 Additionally, a few special version names are supported for our official releases only (these formats do not work when using a fork):
 - `last_green` refers to the Bazel binary that was built at the most recent commit that passed [Bazel CI](https://buildkite.com/bazel/bazel-bazel).
   Ideally this binary should be very close to Bazel-at-head.
-- `last_downstream_green` points to the most recent Bazel binary that builds and tests all [downstream projects](https://buildkite.com/bazel/bazel-at-head-plus-downstream) successfully.
 - `last_rc` points to the most recent release candidate.
   If there is no active release candidate, Bazelisk uses the latest Bazel release instead.
 - `rolling` refers to the latest rolling release (even if there is a newer LTS release).
 
+Note: `last_downstream_green` support has been removed, please use `last_green` instead.
+
 ## Where does Bazelisk get Bazel from?
 
 By default Bazelisk retrieves Bazel releases, release candidates and binaries built at green commits from Google Cloud Storage. The downloaded artifacts are validated against the SHA256 value recorded in `BAZELISK_VERIFY_SHA256` if this variable is set in the configuration file.
@@ -219,7 +220,7 @@ Additionally, the Bazelisk home directory is also evaluated in precedence order.
 
 For ease of use, the Python version of Bazelisk is written to work with Python 2.7 and 3.x and only uses modules provided by the standard library.
 
-The Go version can be compiled to run natively on Linux, macOS and Windows. 
+The Go version can be compiled to run natively on Linux, macOS and Windows.
 
 To install it, run:
 

bazelisk.py

@@ -46,15 +46,10 @@
 
 LATEST_PATTERN = re.compile(r"latest(-(?P<offset>\d+))?$")
 
-LAST_GREEN_COMMIT_BASE_PATH = (
-    "https://storage.googleapis.com/bazel-untrusted-builds/last_green_commit/"
+LAST_GREEN_COMMIT_PATH = (
+    "https://storage.googleapis.com/bazel-builds/last_green_commit/github.com/bazelbuild/bazel.git/publish-bazel-binaries"
 )
 
-LAST_GREEN_COMMIT_PATH_SUFFIXES = {
-    "last_green": "github.com/bazelbuild/bazel.git/bazel-bazel",
-    "last_downstream_green": "downstream_pipeline",
-}
-
 BAZEL_GCS_PATH_PATTERN = (
     "https://storage.googleapis.com/bazel-builds/artifacts/{platform}/{commit}/bazel"
 )
@@ -119,9 +114,8 @@ def resolve_version_label_to_number_or_commit(bazelisk_directory, version):
             of an unreleased Bazel binary,
         2. An indicator for whether the returned version refers to a commit.
     """
-    suffix = LAST_GREEN_COMMIT_PATH_SUFFIXES.get(version)
-    if suffix:
-        return get_last_green_commit(suffix), True
+    if version == "last_green":
+        return get_last_green_commit(), True
 
     if "latest" in version:
         match = LATEST_PATTERN.match(version)
@@ -140,8 +134,11 @@ def resolve_version_label_to_number_or_commit(bazelisk_directory, version):
     return version, False
 
 
-def get_last_green_commit(path_suffix):
-    return read_remote_text_file(LAST_GREEN_COMMIT_BASE_PATH + path_suffix).strip()
+def get_last_green_commit():
+    commit = read_remote_text_file(LAST_GREEN_COMMIT_PATH).strip()
+    if not re.match(r"^[0-9a-f]{40}$", commit):
+        raise Exception("Invalid commit hash: {}".format(commit))
+    return commit
 
 
 def get_releases_json(bazelisk_directory):

bazelisk_test.sh

@@ -257,17 +257,6 @@ function test_bazel_last_green() {
       (echo "FAIL: 'bazelisk version' of an unreleased binary must not print a build label."; exit 1)
 }
 
-function test_bazel_last_downstream_green() {
-  setup
-
-  USE_BAZEL_VERSION="last_downstream_green" \
-      BAZELISK_HOME="$BAZELISK_HOME" \
-      bazelisk version 2>&1 | tee log
-
-  ! grep "Build label:" log || \
-      (echo "FAIL: 'bazelisk version' of an unreleased binary must not print a build label."; exit 1)
-}
-
 function test_BAZELISK_NOJDK() {
   setup
 
@@ -516,10 +505,6 @@ echo "# test_bazel_last_green"
 test_bazel_last_green
 echo
 
-echo "# test_bazel_last_downstream_green"
-test_bazel_last_downstream_green
-echo
-
 echo "# test_BAZELISK_NOJDK"
 test_BAZELISK_NOJDK
 echo

core/repositories.go

@@ -70,10 +70,8 @@ type ForkRepo interface {
 // CommitRepo represents a repository that stores Bazel binaries built at specific commits.
 // It can also return the hashes of the most recent commits that passed Bazel CI pipelines successfully.
 type CommitRepo interface {
-	// GetLastGreenCommit returns the most recent commit at which a Bazel binary passed a specific Bazel CI pipeline.
-	// If downstreamGreen is true, the pipeline is https://buildkite.com/bazel/bazel-at-head-plus-downstream, otherwise
-	// it's https://buildkite.com/bazel/bazel-bazel
-	GetLastGreenCommit(bazeliskHome string, downstreamGreen bool) (string, error)
+	// GetLastGreenCommit returns the most recent commit at which a Bazel binary is successfully built.
+	GetLastGreenCommit(bazeliskHome string) (string, error)
 
 	// DownloadAtCommit downloads a Bazel binary built at the given commit into the specified location and returns the absolute path.
 	DownloadAtCommit(commit, destDir, destFile string, config config.Config) (string, error)
@@ -122,7 +120,7 @@ func (r *Repositories) ResolveVersion(bazeliskHome, fork, version string, config
 
 func (r *Repositories) resolveFork(bazeliskHome string, vi *versions.Info, config config.Config) (string, DownloadFunc, error) {
 	if vi.IsRelative && (vi.IsCandidate || vi.IsCommit) {
-		return "", nil, errors.New("forks do not support last_rc, last_green and last_downstream_green")
+		return "", nil, errors.New("forks do not support last_rc and last_green")
 	}
 	lister := func(bazeliskHome string) ([]string, error) {
 		return r.Fork.GetVersions(bazeliskHome, vi.Fork)
@@ -174,7 +172,7 @@ func (r *Repositories) resolveCommit(bazeliskHome string, vi *versions.Info, con
 	version := vi.Value
 	if vi.IsRelative {
 		var err error
-		version, err = r.Commits.GetLastGreenCommit(bazeliskHome, vi.IsDownstream)
+		version, err = r.Commits.GetLastGreenCommit(bazeliskHome)
 		if err != nil {
 			return "", nil, fmt.Errorf("cannot resolve last green commit: %v", err)
 		}
@@ -376,7 +374,7 @@ type noCommitRepo struct {
 	err error
 }
 
-func (nlgr *noCommitRepo) GetLastGreenCommit(bazeliskHome string, downstreamGreen bool) (string, error) {
+func (nlgr *noCommitRepo) GetLastGreenCommit(bazeliskHome string) (string, error) {
 	return "", nlgr.err
 }
 

repositories/gcs.go

@@ -21,15 +21,7 @@ import (
 const (
 	candidateBaseURL    = "https://releases.bazel.build"
 	nonCandidateBaseURL = "https://storage.googleapis.com/bazel-builds/artifacts"
-	lastGreenBaseURL    = "https://storage.googleapis.com/bazel-untrusted-builds/last_green_commit/"
-)
-
-var (
-	// key == includeDownstream
-	lastGreenCommitPathSuffixes = map[bool]string{
-		false: "github.com/bazelbuild/bazel.git/bazel-bazel",
-		true:  "downstream_pipeline",
-	}
+	lastGreenCommitURL  = "https://storage.googleapis.com/bazel-builds/last_green_commit/github.com/bazelbuild/bazel.git/publish-bazel-binaries"
 )
 
 // GCSRepo represents a Bazel repository on Google Cloud Storage that contains Bazel releases, release candidates and Bazel binaries built at arbitrary commits.
@@ -249,16 +241,20 @@ func (gcs *GCSRepo) DownloadCandidate(version, destDir, destFile string, config
 
 // CommitRepo
 
-// GetLastGreenCommit returns the most recent commit at which a Bazel binary passed a specific Bazel CI pipeline.
-// If downstreamGreen is true, the pipeline is https://buildkite.com/bazel/bazel-at-head-plus-downstream, otherwise
-// it's https://buildkite.com/bazel/bazel-bazel
-func (gcs *GCSRepo) GetLastGreenCommit(bazeliskHome string, downstreamGreen bool) (string, error) {
-	pathSuffix := lastGreenCommitPathSuffixes[downstreamGreen]
-	content, _, err := httputil.ReadRemoteFile(lastGreenBaseURL+pathSuffix, "")
+// GetLastGreenCommit returns the most recent commit at which a Bazel binary is successfully built.
+func (gcs *GCSRepo) GetLastGreenCommit(bazeliskHome string) (string, error) {
+	content, _, err := httputil.ReadRemoteFile(lastGreenCommitURL, "")
 	if err != nil {
 		return "", fmt.Errorf("could not determine last green commit: %v", err)
 	}
-	return strings.TrimSpace(string(content)), nil
+
+	// Validate the content does look like a commit hash
+	commit := strings.TrimSpace(string(content))
+	if !versions.MatchCommitPattern(commit) {
+		return "", fmt.Errorf("invalid commit hash: %s", commit)
+	}
+
+	return commit, nil
 }
 
 // DownloadAtCommit downloads a Bazel binary built at the given commit into the specified location and returns the absolute path.

test.sh

@@ -9,7 +9,6 @@ env -u USE_BAZEL_VERSION ./bin/bazelisk-darwin-"$arch" version
 USE_BAZEL_VERSION="latest" ./bin/bazelisk-darwin-"$arch" version
 USE_BAZEL_VERSION="0.28.0" ./bin/bazelisk-darwin-amd64 version
 USE_BAZEL_VERSION="last_green" ./bin/bazelisk-darwin-"$arch" version
-USE_BAZEL_VERSION="last_downstream_green" ./bin/bazelisk-darwin-"$arch" version
 USE_BAZEL_VERSION="last_rc" ./bin/bazelisk-darwin-"$arch" version
 USE_BAZEL_VERSION="bazelbuild/latest" ./bin/bazelisk-darwin-"$arch" version
 USE_BAZEL_VERSION="bazelbuild/0.27.0" ./bin/bazelisk-darwin-amd64 version

versions/versions.go

@@ -27,7 +27,7 @@ var (
 
 // Info represents a structured Bazel version identifier.
 type Info struct {
-	IsRelease, IsCandidate, IsCommit, IsFork, IsRolling, IsRelative, IsDownstream bool
+	IsRelease, IsCandidate, IsCommit, IsFork, IsRolling, IsRelative bool
 	Fork, Value                                                                   string
 	LatestOffset, TrackRestriction                                                int
 }
@@ -68,10 +68,6 @@ func Parse(fork, version string) (*Info, error) {
 	} else if version == "last_green" {
 		vi.IsCommit = true
 		vi.IsRelative = true
-	} else if version == "last_downstream_green" {
-		vi.IsCommit = true
-		vi.IsRelative = true
-		vi.IsDownstream = true
 	} else if rollingPattern.MatchString(version) {
 		vi.IsRolling = true
 	} else if version == "rolling" {
@@ -106,7 +102,11 @@ func GetInAscendingOrder(versions []string) []string {
 	return sorted
 }
 
+func MatchCommitPattern(version string) bool {
+	return commitPattern.MatchString(version)
+}
+
 // IsCommit returns whether the given version refers to a commit.
 func IsCommit(version string) bool {
-	return version == "last_green" || version == "last_downstream_green" || commitPattern.MatchString(version)
+	return version == "last_green" || commitPattern.MatchString(version)
 }