Support newer versions of REAPI

.golangci.yml

@@ -110,6 +110,12 @@ linters:
           - staticcheck
         path: (.+)\.go$
         text: ST1005
+      # REAPI marks fields deprecated as it evolves, but we still need them to stay compatible with
+      # servers speaking older versions.
+      - linters:
+          - staticcheck
+        path: (.+)\.go$
+        text: 'SA1019: .*github\.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2\.'
     paths:
       - test_data
       - third_party$

docs/remote_builds.html

@@ -51,14 +51,18 @@ <h1 id="remote-build-execution" class="title-1">Remote build execution</h1>
 <ul class="bulleted-list">
   <li>
     <span>
-      Please requires version 2.1 of the Remote Execution API (for the
+      Please supports versions 2.1 to 2.12 of the Remote Execution API. It needs at
+      least 2.1 for the
       <a
         class="copy-link"
         href="https://github.com/bazelbuild/remote-apis/pull/96"
         >non-specific output path</a
       >
-      field). At the time of writing relatively few servers identify as this, but we
-      hope they'll upgrade soon.</span
+      field. If the server only supports one of these versions as deprecated, Please
+      will still use it but warn about it. If your server doesn't support any of them,
+      you can set <code class="code">suppressversioncheck</code> in the
+      <a class="copy-link" href="/config.html#remote">[remote]</a> section of your
+      config to try it anyway.</span
     >
   </li>
   <li>

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/ProtonMail/go-crypto v1.4.1
 	github.com/alessio/shellescape v1.4.2
 	github.com/bazelbuild/remote-apis v0.0.0-20260331222004-becdd8f9ff81
-	github.com/bazelbuild/remote-apis-sdks v0.0.0-20260610142741-7ffd493e6686
+	github.com/bazelbuild/remote-apis-sdks v0.0.0-20260903234436-d5824b1a2286
 	github.com/cespare/xxhash/v2 v2.3.0
 	github.com/coreos/go-semver v0.3.1
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc

go.sum

@@ -22,8 +22,8 @@ github.com/alessio/shellescape v1.4.2 h1:MHPfaU+ddJ0/bYWpgIeUnQUqKrlJ1S7BfEYPM4u
 github.com/alessio/shellescape v1.4.2/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
 github.com/bazelbuild/remote-apis v0.0.0-20260331222004-becdd8f9ff81 h1:vAHLeMHi+CywqDw5V/s5mHj1ahkhYMRtRFqWe18F0kc=
 github.com/bazelbuild/remote-apis v0.0.0-20260331222004-becdd8f9ff81/go.mod h1:7Tyi5f5+hG+6LwC0X/G/EjCQS4ZYJUcpY0geSsU2NAw=
-github.com/bazelbuild/remote-apis-sdks v0.0.0-20260610142741-7ffd493e6686 h1:u0Z7+bxKIg71P2lDGhuCqIFqEjv73KwCbpPUyujHK1Y=
-github.com/bazelbuild/remote-apis-sdks v0.0.0-20260610142741-7ffd493e6686/go.mod h1:TJlbnIaiJQr3pgZ+EBscj/bpu9wBawCHcqd+YAgumOs=
+github.com/bazelbuild/remote-apis-sdks v0.0.0-20260903234436-d5824b1a2286 h1:nhGbYP7vcyj+S0cH7C0RbRg9KRWWn6mVeQvVCq9+SZk=
+github.com/bazelbuild/remote-apis-sdks v0.0.0-20260903234436-d5824b1a2286/go.mod h1:TJlbnIaiJQr3pgZ+EBscj/bpu9wBawCHcqd+YAgumOs=
 github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=

src/remote/action.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"slices"
 	"sort"
 	"strings"
@@ -128,7 +127,7 @@ func (c *Client) buildCommand(target *core.BuildTarget, inputRoot *pb.Directory,
 	}
 	cmd, err := core.ReplaceSequences(state, target, cmd)
 	return &pb.Command{
-		Platform:             c.targetPlatformProperties(target), //nolint:staticcheck
+		Platform:             c.targetPlatformProperties(target),
 		Arguments:            process.BashCommand(c.shellPath, commandPrefixBuilder.String()+cmd, state.Config.Build.ExitOnError),
 		EnvironmentVariables: c.buildEnv(target, c.stampedBuildEnvironment(state, target, inputRoot, stamp, isTest || isRun), target.Sandbox),
 		OutputPaths:          outs,
@@ -161,14 +160,7 @@ func (c *Client) buildTestCommand(state *core.BuildState, target *core.BuildTarg
 	}
 	cmd, err := core.TestCommand(state, target)
 	return &pb.Command{
-		Platform: &pb.Platform{ //nolint:staticcheck
-			Properties: []*pb.Platform_Property{
-				{
-					Name:  "OSFamily",
-					Value: translateOS(target.Subrepo),
-				},
-			},
-		},
+		Platform:             c.targetPlatformProperties(target),
 		Arguments:            process.BashCommand(c.shellPath, commandPrefix+cmd, state.Config.Build.ExitOnError),
 		EnvironmentVariables: c.buildEnv(nil, core.TestEnvironment(state, target, ".", run), target.Test.Sandbox),
 		OutputPaths:          paths,
@@ -182,7 +174,7 @@ func (c *Client) buildRunCommand(state *core.BuildState, target *core.BuildTarge
 		return nil, fmt.Errorf("Target %s has no outputs, it can't be run with `plz run`", target)
 	}
 	return &pb.Command{
-		Platform:             c.platform, //nolint:staticcheck
+		Platform:             c.targetPlatformProperties(target),
 		Arguments:            outs,
 		EnvironmentVariables: c.buildEnv(target, core.GeneralBuildEnvironment(state), false),
 	}, nil
@@ -437,15 +429,7 @@ func outputsForActionResult(ar *pb.ActionResult) map[string]bool {
 	for _, o := range ar.OutputDirectories {
 		ret[o.Path] = true
 	}
-	for _, o := range ar.OutputSymlinks {
-		ret[o.Path] = true
-	}
-
-	// TODO(jpoole): remove these two after REAPI 2.1
-	for _, o := range ar.OutputFileSymlinks { //nolint:staticcheck
-		ret[o.Path] = true
-	}
-	for _, o := range ar.OutputDirectorySymlinks { //nolint:staticcheck
+	for _, o := range outputSymlinks(ar) {
 		ret[o.Path] = true
 	}
 	return ret
@@ -479,7 +463,7 @@ func (c *Client) verifyActionResult(target *core.BuildTarget, command *pb.Comman
 		}
 
 		if len(target.EntryPoints) > 0 {
-			flatOuts, err := c.client.FlattenActionOutputs(context.Background(), ar)
+			flatOuts, err := c.client.FlattenActionOutputs(context.Background(), sdkActionResult(ar))
 			if err != nil {
 				return fmt.Errorf("error checking for entry point in outputs: %w", err)
 			}
@@ -516,15 +500,15 @@ func (c *Client) verifyActionResult(target *core.BuildTarget, command *pb.Comman
 	}
 	start := time.Now()
 	// Do more in-depth validation that blobs exist remotely.
-	outputs, err := c.client.FlattenActionOutputs(context.Background(), ar)
+	outputs, err := c.client.FlattenActionOutputs(context.Background(), sdkActionResult(ar))
 	if err != nil {
 		return fmt.Errorf("Failed to verify action result: %s", err)
 	}
 	// At this point it's verified all the directories, but not the files themselves.
 	digests := make([]digest.Digest, 0, len(outputs))
 	for _, output := range outputs {
-		// FlattenTree doesn't populate the digest in for empty dirs... we don't need to check them anyway
-		if !output.IsEmptyDirectory {
+		// FlattenTree doesn't populate the digest in for empty dirs or symlinks... we don't need to check them anyway
+		if !output.IsEmptyDirectory && output.SymlinkTarget == "" {
 			digests = append(digests, output.Digest)
 		}
 	}
@@ -557,23 +541,6 @@ func (c *Client) uploadLocalTarget(target *core.BuildTarget) error {
 	return c.setOutputs(target, outs)
 }
 
-// translateOS converts the OS name of a subrepo into a Bazel-style OS name.
-func translateOS(subrepo *core.Subrepo) string {
-	if subrepo == nil {
-		return reallyTranslateOS(runtime.GOOS)
-	}
-	return reallyTranslateOS(subrepo.Arch.OS)
-}
-
-func reallyTranslateOS(os string) string {
-	switch os {
-	case "darwin":
-		return "macos"
-	default:
-		return os
-	}
-}
-
 // buildEnv translates the set of environment variables for this target to a proto.
 func (c *Client) buildEnv(target *core.BuildTarget, env core.BuildEnv, sandbox bool) []*pb.Command_EnvironmentVariable {
 	if sandbox {

src/remote/remote.go

@@ -44,8 +44,14 @@ import (
 
 var log = logging.Log
 
-// The API version we support.
-var apiVersion = semver.SemVer{Major: 2, Minor: 1}
+// The range of API versions we support (inclusive).
+// 2.1 is the minimum since we use Command.output_paths. Nothing after that up to 2.12 imposes
+// any new requirements on us as a client, since we only use SHA-1 / SHA-256 and don't opt into
+// any of the newer optional features.
+var (
+	lowAPIVersion  = semver.SemVer{Major: 2, Minor: 1}
+	highAPIVersion = semver.SemVer{Major: 2, Minor: 12}
+)
 
 var remoteCacheReadDuration = metrics.NewHistogramVec(
 	"remote",
@@ -240,12 +246,15 @@ func (c *Client) initExec() error {
 	if err != nil {
 		return err
 	}
-	if lessThan(&apiVersion, resp.LowApiVersion) || lessThan(resp.HighApiVersion, &apiVersion) {
-		err := fmt.Errorf("Unsupported API version; we require %s but server only supports %s - %s", printVer(&apiVersion), printVer(resp.LowApiVersion), printVer(resp.HighApiVersion))
+	if version, deprecated, err := negotiateAPIVersion(resp); err != nil {
 		if !c.state.Config.Remote.SuppressVersionCheck {
 			return err
 		}
 		log.Warning("%s", err)
+	} else if deprecated {
+		log.Warning("Using API version %s, which is deprecated by the server. It recommends %s - %s, but we only support %s - %s", printVer(version), printVer(resp.LowApiVersion), printVer(resp.HighApiVersion), printVer(&lowAPIVersion), printVer(&highAPIVersion))
+	} else {
+		log.Debug("Using API version %s", printVer(version))
 	}
 	caps := resp.CacheCapabilities
 	if caps == nil {
@@ -486,9 +495,10 @@ func (c *Client) downloadActionOutputs(ctx context.Context, ar *pb.ActionResult,
 	for _, d := range ar.OutputDirectories {
 		d.Path = target.GetRealOutput(d.Path)
 	}
-	for _, s := range ar.OutputSymlinks {
+	for _, s := range outputSymlinks(ar) {
 		s.Path = target.GetRealOutput(s.Path)
 	}
+	ar = sdkActionResult(ar)
 	// We can download straight into the out dir if there are no outdirs to worry about
 	if len(target.OutputDirectories) == 0 {
 		_, err := c.client.DownloadActionOutputs(ctx, ar, target.OutDir(), c.fileMetadataCache)
@@ -586,7 +596,7 @@ func (c *Client) Test(target *core.BuildTarget, run int) (metadata *core.BuildMe
 	metadata, ar, err := c.execute(target, command, digest, true, false, run)
 
 	if ar != nil {
-		_, dlErr := c.client.DownloadActionOutputs(context.Background(), ar, target.TestDir(run), c.fileMetadataCache)
+		_, dlErr := c.client.DownloadActionOutputs(context.Background(), sdkActionResult(ar), target.TestDir(run), c.fileMetadataCache)
 		if dlErr != nil {
 			log.Warningf("%v: failed to download test outputs: %v", target.Label, dlErr)
 		}

src/remote/remote_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/bazelbuild/remote-apis-sdks/go/pkg/digest"
 	pb "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2"
+	"github.com/bazelbuild/remote-apis/build/bazel/semver"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -370,7 +371,7 @@ func TestTargetPlatform(t *testing.T) {
 				Value: "linux",
 			},
 		},
-	}, cmd.Platform) //nolint:staticcheck
+	}, cmd.Platform)
 
 	target.Labels = []string{"remote-platform-property:size=chomky"}
 	cmd, err = c.buildCommand(target, &pb.Directory{}, false, false, false, 0)
@@ -386,7 +387,7 @@ func TestTargetPlatform(t *testing.T) {
 				Value: "linux",
 			},
 		},
-	}, cmd.Platform) //nolint:staticcheck
+	}, cmd.Platform)
 }
 
 // Store is a small hack that stores a target's outputs for testing only.
@@ -419,3 +420,144 @@ func TestBuildTestCommand(t *testing.T) {
 		`expected suffix "$TEST --foo --bar 2>&1" on %q`, cmd.Arguments[len(cmd.Arguments)-1],
 	)
 }
+
+func TestNegotiateAPIVersion(t *testing.T) {
+	v := func(major, minor int32) *semver.SemVer {
+		return &semver.SemVer{Major: major, Minor: minor}
+	}
+	tests := []struct {
+		name                    string
+		deprecated, low, high   *semver.SemVer
+		expected                *semver.SemVer
+		expectDeprecated, fails bool
+	}{
+		{name: "please-servers mettle", low: v(2, 0), high: v(2, 1), expected: v(2, 1)},
+		{name: "please-servers flair", low: v(2, 0), high: v(2, 3), expected: v(2, 3)},
+		{name: "Buildbarn", deprecated: v(2, 2), low: v(2, 3), high: v(2, 12), expected: v(2, 12)},
+		{name: "newer than us", low: v(2, 3), high: v(2, 20), expected: v(2, 12)},
+		{name: "only deprecated overlap", deprecated: v(2, 0), low: v(2, 13), high: v(2, 14), expected: v(2, 12), expectDeprecated: true},
+		{name: "too old", low: v(2, 0), high: v(2, 0), fails: true},
+		{name: "too new", deprecated: v(2, 13), low: v(2, 14), high: v(3, 0), fails: true},
+		{name: "unset", fails: true},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			version, deprecated, err := negotiateAPIVersion(&pb.ServerCapabilities{
+				DeprecatedApiVersion: test.deprecated,
+				LowApiVersion:        test.low,
+				HighApiVersion:       test.high,
+			})
+			if test.fails {
+				assert.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, printVer(test.expected), printVer(version))
+			assert.Equal(t, test.expectDeprecated, deprecated)
+		})
+	}
+}
+
+func TestOutputSymlinks(t *testing.T) {
+	link := &pb.OutputSymlink{Path: "link", Target: "file"}
+	dirLink := &pb.OutputSymlink{Path: "dirlink", Target: "dir"}
+	// A 2.1+ server only populates output_symlinks
+	assert.Equal(t, []*pb.OutputSymlink{link, dirLink}, outputSymlinks(&pb.ActionResult{
+		OutputSymlinks: []*pb.OutputSymlink{link, dirLink},
+	}))
+	// One compatible with 2.0 populates both, which shouldn't be duplicated
+	assert.Equal(t, []*pb.OutputSymlink{link, dirLink}, outputSymlinks(&pb.ActionResult{
+		OutputSymlinks:          []*pb.OutputSymlink{link, dirLink},
+		OutputFileSymlinks:      []*pb.OutputSymlink{link},
+		OutputDirectorySymlinks: []*pb.OutputSymlink{dirLink},
+	}))
+	// A 2.0 server only populates the deprecated fields
+	assert.Equal(t, []*pb.OutputSymlink{link, dirLink}, outputSymlinks(&pb.ActionResult{
+		OutputFileSymlinks:      []*pb.OutputSymlink{link},
+		OutputDirectorySymlinks: []*pb.OutputSymlink{dirLink},
+	}))
+}
+
+func TestSDKActionResult(t *testing.T) {
+	link := &pb.OutputSymlink{Path: "link", Target: "file"}
+	ar := &pb.ActionResult{OutputSymlinks: []*pb.OutputSymlink{link}}
+	sdkAR := sdkActionResult(ar)
+	assert.Equal(t, []*pb.OutputSymlink{link}, sdkAR.OutputFileSymlinks)
+	assert.Empty(t, ar.OutputFileSymlinks, "original action result should not be modified")
+	// If the deprecated fields are already populated, it should be left alone.
+	ar = &pb.ActionResult{OutputSymlinks: []*pb.OutputSymlink{link}, OutputFileSymlinks: []*pb.OutputSymlink{link}}
+	assert.Same(t, ar, sdkActionResult(ar))
+}
+
+// TestOutputSymlinksDownloaded builds a target against a server that only populates output_symlinks
+// (as Buildbarn does) and checks that we record and download the symlink.
+func TestOutputSymlinksDownloaded(t *testing.T) {
+	defer server.Reset()
+	c := newClientInstance("mock")
+
+	content := []byte("this is the content of the real file")
+	contentDigest := digest.NewFromBlob(content)
+	server.blobs[contentDigest.Hash] = content
+	server.mockActionResult = &pb.ActionResult{
+		OutputFiles:    []*pb.OutputFile{{Path: "real.txt", Digest: contentDigest.ToProto()}},
+		OutputSymlinks: []*pb.OutputSymlink{{Path: "link.txt", Target: "real.txt"}},
+		ExecutionMetadata: &pb.ExecutedActionMetadata{
+			Worker:                      "kev",
+			QueuedTimestamp:             timestamppb.Now(),
+			ExecutionStartTimestamp:     timestamppb.Now(),
+			ExecutionCompletedTimestamp: timestamppb.Now(),
+		},
+	}
+
+	target := core.NewBuildTarget(core.BuildLabel{PackageName: "package", Name: "symlink_target"})
+	target.AddOutput("real.txt")
+	target.AddOutput("link.txt")
+	target.Command = "echo hello > real.txt && ln -s real.txt link.txt"
+	c.state.AddOriginalTarget(target.Label)
+	c.state.OutputDownload = core.OriginalOutputDownload
+	c.state.Graph.AddTarget(target)
+	_, err := c.Build(target)
+	require.NoError(t, err)
+	require.NoError(t, c.Download(target))
+
+	outs := c.targetOutputs(target.Label)
+	require.NotNil(t, outs)
+	require.Len(t, outs.Symlinks, 1)
+	assert.Equal(t, "link.txt", outs.Symlinks[0].Name)
+	assert.Equal(t, "real.txt", outs.Symlinks[0].Target)
+
+	dest, err := os.Readlink(filepath.Join(target.OutDir(), "link.txt"))
+	require.NoError(t, err)
+	assert.Equal(t, "real.txt", dest)
+}
+
+// TestCommandPlatformMatchesAction checks that every kind of Command we build requests the same
+// platform as its Action (which is what servers on REAPI 2.2+ actually use).
+func TestCommandPlatformMatchesAction(t *testing.T) {
+	c := newClientInstance("platform_match_test")
+	require.NoError(t, c.CheckInitialised())
+	target := core.NewBuildTarget(core.BuildLabel{PackageName: "package", Name: "platform_target"})
+	target.AddOutput("out")
+	target.Labels = []string{"remote-platform-property:size=chomky"}
+	target.Test = &core.TestFields{Timeout: time.Minute, Command: "$TEST"}
+	target.IsBinary = true
+	expected := &pb.Platform{
+		Properties: []*pb.Platform_Property{
+			{Name: "size", Value: "chomky"},
+			{Name: "OSFamily", Value: "linux"},
+		},
+	}
+	require.Equal(t, expected, c.targetPlatformProperties(target), "Action platform should include target and config properties")
+
+	buildCmd, err := c.buildCommand(target, &pb.Directory{}, false, false, false, 0)
+	require.NoError(t, err)
+	assert.Equal(t, expected, buildCmd.Platform)
+
+	testCmd, err := c.buildCommand(target, &pb.Directory{}, true, false, false, 1)
+	require.NoError(t, err)
+	assert.Equal(t, expected, testCmd.Platform)
+
+	runCmd, err := c.buildCommand(target, &pb.Directory{}, false, true, false, 0)
+	require.NoError(t, err)
+	assert.Equal(t, expected, runCmd.Platform)
+}