diff --git a/go/pkg/fakes/server.go b/go/pkg/fakes/server.go index b4c3f800..178938fd 100644 --- a/go/pkg/fakes/server.go +++ b/go/pkg/fakes/server.go @@ -288,7 +288,7 @@ func (f *InputFile) apply(ac *repb.ActionResult, s *Server, execRoot string) err if err := os.MkdirAll(filepath.Join(execRoot, filepath.Dir(f.Path)), os.ModePerm); err != nil { return fmt.Errorf("failed to create input dir %v: %v", filepath.Dir(f.Path), err) } - err := os.WriteFile(filepath.Join(execRoot, f.Path), nil, 0755) + err := os.WriteFile(filepath.Join(execRoot, f.Path), bytes, 0755) if err != nil { return fmt.Errorf("failed to setup file %v under temp exec root %v: %v", f.Path, execRoot, err) } diff --git a/go/pkg/tool/BUILD.bazel b/go/pkg/tool/BUILD.bazel index b1d850d3..887a25a3 100644 --- a/go/pkg/tool/BUILD.bazel +++ b/go/pkg/tool/BUILD.bazel @@ -33,5 +33,6 @@ go_test( "//go/pkg/fakes", "//go/pkg/outerr", "@com_github_google_go_cmp//cmp:go_default_library", + "@org_golang_google_protobuf//encoding/prototext:go_default_library", ], ) diff --git a/go/pkg/tool/tool.go b/go/pkg/tool/tool.go index d44f00bb..939d2b20 100644 --- a/go/pkg/tool/tool.go +++ b/go/pkg/tool/tool.go @@ -371,15 +371,14 @@ func (c *Client) prepProtos(ctx context.Context, actionRoot string) (string, err if err != nil { return "", err } - cmdProto := &repb.Command{} - if err := prototext.Unmarshal(cmdTxt, cmdProto); err != nil { + cmdPb := &repb.Command{} + if err := prototext.Unmarshal(cmdTxt, cmdPb); err != nil { return "", err } - cmdPb, err := proto.Marshal(cmdProto) + ue, err := uploadinfo.EntryFromProto(cmdPb) if err != nil { return "", err } - ue := uploadinfo.EntryFromBlob(cmdPb) if _, _, err := c.GrpcClient.UploadIfMissing(ctx, ue); err != nil { return "", err } @@ -387,20 +386,27 @@ func (c *Client) prepProtos(ctx context.Context, actionRoot string) (string, err if err != nil { return "", err } - actionProto := &repb.Action{} - if err := prototext.Unmarshal(ac, actionProto); err != nil { + acPb := &repb.Action{} + if err := prototext.Unmarshal(ac, acPb); err != nil { + return "", err + } + dg, err := digest.NewFromMessage(cmdPb) + if err != nil { return "", err } - actionProto.CommandDigest = digest.NewFromBlob(cmdPb).ToProto() - acPb, err := proto.Marshal(actionProto) + acPb.CommandDigest = dg.ToProto() + ue, err = uploadinfo.EntryFromProto(acPb) if err != nil { return "", err } - ue = uploadinfo.EntryFromBlob(acPb) if _, _, err := c.GrpcClient.UploadIfMissing(ctx, ue); err != nil { return "", err } - return digest.NewFromBlob(acPb).String(), nil + dg, err = digest.NewFromMessage(acPb) + if err != nil { + return "", err + } + return dg.String(), nil } // ExecuteAction executes an action in a canonical structure remotely. diff --git a/go/pkg/tool/tool_test.go b/go/pkg/tool/tool_test.go index eae98bf2..4f8ce9b3 100644 --- a/go/pkg/tool/tool_test.go +++ b/go/pkg/tool/tool_test.go @@ -12,6 +12,9 @@ import ( "github.com/bazelbuild/remote-apis-sdks/go/pkg/fakes" "github.com/bazelbuild/remote-apis-sdks/go/pkg/outerr" "github.com/google/go-cmp/cmp" + "google.golang.org/protobuf/encoding/prototext" + + repb "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" ) func TestTool_DownloadActionResult(t *testing.T) { @@ -83,8 +86,8 @@ Platform Inputs ====== -[Root directory digest: e23e10be0d14b5b2b1b7af32de78dea554a74df5bb22b31ae6c49583c1a8aa0e/75] -a/b/input.txt: [File digest: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855/0] +[Root directory digest: 1b1d5b9e3eb97866805deca901299fcdde98b5747e147f35187e12c16e120186/75] +a/b/input.txt: [File digest: c96c6d5be8d08a12e7b5cdc1b207fa6b2430974c86803d8891675e76fd992c20/5] ------------------------------------------------------------------------ Action Result @@ -109,29 +112,20 @@ func TestTool_CheckDeterminism(t *testing.T) { e, cleanup := fakes.NewTestEnv(t) defer cleanup() cmd := &command.Command{ - Args: []string{"foo bar baz"}, + Args: []string{"foo", "bar", "baz"}, ExecRoot: e.ExecRoot, InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, OutputFiles: []string{"a/b/out"}, } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "i1"), []byte("i1"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) - } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) - } - out := "output" - opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: true, DownloadOutErr: true} - _, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}) + _, acDg, _, _ := e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}) client := &Client{GrpcClient: e.Client.GrpcClient} if err := client.CheckDeterminism(context.Background(), acDg.String(), "", 2); err != nil { t.Errorf("CheckDeterminism returned an error: %v", err) } - // Now execute again with changed inputs. + // Now execute again and return a different output. testOnlyStartDeterminismExec = func() { - out = "output2" - e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}) + e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, &fakes.OutputFile{Path: "a/b/out", Contents: "out2"}) } defer func() { testOnlyStartDeterminismExec = func() {} }() if err := client.CheckDeterminism(context.Background(), acDg.String(), "", 2); err == nil { @@ -139,25 +133,77 @@ func TestTool_CheckDeterminism(t *testing.T) { } } -func TestTool_ExecuteAction(t *testing.T) { +func TestTool_DownloadAction(t *testing.T) { e, cleanup := fakes.NewTestEnv(t) defer cleanup() cmd := &command.Command{ - Args: []string{"foo bar baz"}, + Args: []string{"foo", "bar", "baz"}, ExecRoot: e.ExecRoot, - InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, + InputSpec: &command.InputSpec{Inputs: []string{"i1", "a/b/i2"}}, OutputFiles: []string{"a/b/out"}, } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "i1"), []byte("i1"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) + _, acDg, _, _ := e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "a/b/i2", Contents: "i2"}) + + client := &Client{GrpcClient: e.Client.GrpcClient} + tmpDir := filepath.Join(t.TempDir(), "action_root") + os.MkdirAll(tmpDir, os.ModePerm) + if err := client.DownloadAction(context.Background(), acDg.String(), tmpDir); err != nil { + t.Errorf("error DownloadAction: %v", err) } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) + + reCmdPb := &repb.Command{ + Arguments: []string{"foo", "bar", "baz"}, + OutputFiles: []string{"a/b/out"}, + } + acPb := &repb.Action{ + CommandDigest: digest.TestNewFromMessage(reCmdPb).ToProto(), + InputRootDigest: &repb.Digest{Hash: "2f33e7247d11728cfc1515b378040ecf3bd8be4eb4330ca7b6023b23c43b348b", SizeBytes: 153}, + } + expectedContents := []struct { + path string + contents string + }{ + { + path: "ac.textproto", + contents: prototext.Format(acPb), + }, + { + path: "cmd.textproto", + contents: prototext.Format(reCmdPb), + }, + { + path: "input/i1", + contents: "i1", + }, + { + path: "input/a/b/i2", + contents: "i2", + }, + } + for _, ec := range expectedContents { + fp := filepath.Join(tmpDir, ec.path) + got, err := os.ReadFile(fp) + if err != nil { + t.Fatalf("Unable to read downloaded action file %v: %v", fp, err) + } + if diff := cmp.Diff(ec.contents, string(got)); diff != "" { + t.Errorf("Incorrect content in downloaded file %v: diff (-want +got): %v\n\ngot: %s\n\nwant: %v\n", fp, diff, got, ec.contents) + } + } +} + +func TestTool_ExecuteAction(t *testing.T) { + e, cleanup := fakes.NewTestEnv(t) + defer cleanup() + cmd := &command.Command{ + Args: []string{"foo", "bar", "baz"}, + ExecRoot: e.ExecRoot, + InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, + OutputFiles: []string{"a/b/out"}, } - out := "output" opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: true, DownloadOutErr: true} - _, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, - fakes.StdOut("stdout"), fakes.StdErr("stderr")) + _, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}, + &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, fakes.StdOut("stdout"), fakes.StdErr("stderr")) client := &Client{GrpcClient: e.Client.GrpcClient} oe := outerr.NewRecordingOutErr() @@ -171,16 +217,24 @@ func TestTool_ExecuteAction(t *testing.T) { t.Errorf("Incorrect stdout %v, expected \"stdout\"", oe.Stdout()) } // Now execute again with changed inputs. - tmpDir := t.TempDir() - if err := os.WriteFile(filepath.Join(tmpDir, "i1"), []byte("i11"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) + tmpDir := filepath.Join(t.TempDir(), "action_root") + os.MkdirAll(tmpDir, os.ModePerm) + inputRoot := filepath.Join(tmpDir, "input") + if err := client.DownloadAction(context.Background(), acDg.String(), tmpDir); err != nil { + t.Errorf("error DownloadAction: %v", err) } - if err := os.WriteFile(filepath.Join(tmpDir, "i2"), []byte("i22"), 0644); err != nil { - t.Fatalf("failed creating input file: %v", err) + if err := os.WriteFile(filepath.Join(inputRoot, "i1"), []byte("i11"), 0644); err != nil { + t.Fatalf("failed overriding input file: %v", err) + } + if err := os.WriteFile(filepath.Join(inputRoot, "i2"), []byte("i22"), 0644); err != nil { + t.Fatalf("failed overriding input file: %v", err) } - cmd.ExecRoot = tmpDir - _, acDg2, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, + cmd.ExecRoot = inputRoot + _, acDg2, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out2"}, fakes.StdOut("stdout2"), fakes.StdErr("stderr2")) + if diff := cmp.Diff(acDg, acDg2); diff == "" { + t.Fatalf("expected action digest to change after input change, got %v\n", acDg) + } oe = outerr.NewRecordingOutErr() if _, err := client.ExecuteAction(context.Background(), acDg2.String(), "", tmpDir, oe); err != nil { t.Errorf("error executeAction: %v", err) @@ -191,14 +245,14 @@ func TestTool_ExecuteAction(t *testing.T) { if err != nil { t.Fatalf("Unable to read downloaded output %v: %v", fp, err) } - if string(c) != out { - t.Fatalf("Incorrect content in downloaded file %v, want %s, got %s", fp, out, c) + if string(c) != "out2" { + t.Fatalf("Incorrect content in downloaded file %v, want \"out2\", got %s", fp, c) } if string(oe.Stderr()) != "stderr2" { - t.Errorf("Incorrect stderr %v, expected \"stderr\"", oe.Stderr()) + t.Errorf("Incorrect stderr %v, expected \"stderr2\"", oe.Stderr()) } if string(oe.Stdout()) != "stdout2" { - t.Errorf("Incorrect stdout %v, expected \"stdout\"", oe.Stdout()) + t.Errorf("Incorrect stdout %v, expected \"stdout2\"", oe.Stdout()) } } @@ -206,7 +260,7 @@ func TestTool_ExecuteActionFromRoot(t *testing.T) { e, cleanup := fakes.NewTestEnv(t) defer cleanup() cmd := &command.Command{ - Args: []string{"foo bar baz"}, + Args: []string{"foo", "bar", "baz"}, ExecRoot: e.ExecRoot, InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, OutputFiles: []string{"a/b/out"}, @@ -218,9 +272,8 @@ func TestTool_ExecuteActionFromRoot(t *testing.T) { if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { t.Fatalf("failed creating input file: %v", err) } - out := "output" opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: false, DownloadOutErr: true} - e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, + e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}, fakes.StdOut("stdout"), fakes.StdErr("stderr")) client := &Client{GrpcClient: e.Client.GrpcClient} @@ -233,13 +286,17 @@ func TestTool_ExecuteActionFromRoot(t *testing.T) { if err := os.WriteFile(filepath.Join(e.ExecRoot, "input", "i2"), []byte("i2"), 0644); err != nil { t.Fatalf("failed creating input file: %v", err) } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "cmd.textproto"), []byte(`arguments: "foo bar baz" - output_files: "a/b/out"`), 0644); err != nil { - t.Fatalf("failed creating command file: %v", err) + reCmdPb := &repb.Command{ + Arguments: []string{"foo", "bar", "baz"}, + OutputFiles: []string{"a/b/out"}, } - if err := os.WriteFile(filepath.Join(e.ExecRoot, "ac.textproto"), []byte(""), 0644); err != nil { + if err := os.WriteFile(filepath.Join(e.ExecRoot, "cmd.textproto"), []byte(prototext.Format(reCmdPb)), 0644); err != nil { t.Fatalf("failed creating command file: %v", err) } + // The tool will embed the Command proto digest into the Action proto, so the `command_digest` field is effectively ignored: + if err := os.WriteFile(filepath.Join(e.ExecRoot, "ac.textproto"), []byte(`command_digest: {hash: "whatever"}`), 0644); err != nil { + t.Fatalf("failed creating action file: %v", err) + } if _, err := client.ExecuteAction(context.Background(), "", e.ExecRoot, "", oe); err != nil { t.Errorf("error executeAction: %v", err) }