sweet: apply flags to cockroachdb 'go build' based on 'go version'

Currently, the cockroachdb benchmark is built by passing a build tag
(untested_go_version) unconditionally, and we also try to build with and
without '-checklinkname=0' passed to the linker. These are required
because some CockroachDB packages reach into the runtime internals.

The current approach doesn't work well across Go versions because
setting "untested_go_version" for older toolchains causes a symbol
conflict (it ends up enabling multiple files with the same definition).
We can fix this upstream, but we also need a fix in the short term.
Plus, it'll help to clean up this logic and make it more explicit.

So, this CL parses 'go version' in a dead-simple way to determine what
to do. If we're using an older version of Go and the version is
well-defined, then we never use 'untested_go_version'. But if the
version of Go is explicitly a development version or newer than Go
1.24, then we do.

Simultaneously, we only pass -checklinkname=0 to the linker if the
toolchain version is >= 1.23. We always pass it for development Go
versions.

Fixes golang/go#68555.

Change-Id: Ieaad89cb6a92bdeee2636dc62b5b3a26ac317b5f
Cq-Include-Trybots: luci.golang.try:x_benchmarks-gotip-linux-amd64-longtest,x_benchmarks-go1.22-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/600475
Reviewed-by: Austin Clements <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>

Author: mknyszek

sweet/common/gotool.go

@@ -72,6 +72,17 @@ func (g *Go) BuildPackage(pkg, out string) error {
 	return g.Do("", "build", "-o", out, pkg)
 }
 
+func (g *Go) Version() (string, error) {
+	cmd := exec.Command(g.Tool, "version")
+	cmd.Env = g.Env.Collapse()
+	log.TraceCommand(cmd, false)
+	out, err := cmd.Output()
+	if err != nil {
+		return "", fmt.Errorf("error running 'go version': %w", err)
+	}
+	return string(out), nil
+}
+
 func (g *Go) BuildPath(path, out string, args ...string) error {
 	if path[0] != '/' && path[0] != '.' {
 		path = "./" + path

sweet/harnesses/cockroachdb.go

@@ -5,12 +5,12 @@
 package harnesses
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"time"
 
 	"golang.org/x/benchmarks/sweet/common"
@@ -98,22 +98,32 @@ func (h CockroachDB) Build(cfg *common.Config, bcfg *common.BuildConfig) error {
 		return err
 	}
 
-	// Finally build the cockroach binary with `go build`. Build the
-	// cockroach-short binary as it is functionally the same, but
-	// without the UI, making it much quicker to build.
+	// Get the Go version. Then, finall build the cockroach binary with `go build`.
 	//
-	// As of go1.23, we need to pass the `-ldflags=-checklinkname=0` flag
-	// to build cockroach. However, benchmark release branches are on older
-	// versions that don't recognize the flag. Try first with the flag and
-	// again without if there is an error.
+	// Build the cockroach-short binary as it is functionally the same, but without
+	// the UI, making it much quicker to build.
 	//
-	// Cockroachdb reaches into runtime internals, so it has a negative build
-	// tag to exclude unreleased Go versions. Set untested_go_version so we can
-	// run at tip (and cross our fingers).
-	if buildWithFlagErr := cfg.GoTool().BuildPath(filepath.Join(bcfg.SrcDir, "pkg/cmd/cockroach-short"), bcfg.BinDir, "-tags=untested_go_version", "-ldflags=-checklinkname=0"); buildWithFlagErr != nil {
-		if buildWithoutFlagErr := cfg.GoTool().BuildPath(filepath.Join(bcfg.SrcDir, "pkg/cmd/cockroach-short"), bcfg.BinDir, "-tags=untested_go_version"); buildWithoutFlagErr != nil {
-			return errors.Join(buildWithFlagErr, buildWithoutFlagErr)
-		}
+	// CockroachDB reaches into runtime internals, so we need to pass -checklinkname=0
+	// to the linker to get it to build with Go 1.23+. Also, CockroachDB has a negative
+	// build tag to exclude unreleased Go versions, so we need to explicitly set it for
+	// new-enough Go versions.
+	//
+	// Note that since the version of CockroachDB is pinned, we can generally rely on
+	// the checking below. However as CockroachDB and Go evolve, the logic below may need
+	// to change.
+	ver, err := cfg.GoTool().Version()
+	if err != nil {
+		return fmt.Errorf("getting go version for toolchain: %v", err)
+	}
+	var goBuildArgs []string
+	if v := strings.TrimPrefix(ver, "go version "); strings.HasPrefix(v, "devel ") || v >= "go1.23" {
+		goBuildArgs = append(goBuildArgs, "-ldflags=-checklinkname=0")
+	}
+	if v := strings.TrimPrefix(ver, "go version "); strings.HasPrefix(v, "devel ") || v >= "go1.24" {
+		goBuildArgs = append(goBuildArgs, "-tags=untested_go_version")
+	}
+	if err := cfg.GoTool().BuildPath(filepath.Join(bcfg.SrcDir, "pkg/cmd/cockroach-short"), bcfg.BinDir, goBuildArgs...); err != nil {
+		return err
 	}
 
 	// Rename the binary from cockroach-short to cockroach for
@@ -126,7 +136,6 @@ func (h CockroachDB) Build(cfg *common.Config, bcfg *common.BuildConfig) error {
 	if err := cfg.GoTool().BuildPath(bcfg.BenchDir, filepath.Join(bcfg.BinDir, "cockroachdb-bench")); err != nil {
 		return err
 	}
-
 	return nil
 }