Don't write execution results to SSBC cache and remove unnecessary additional call to workspace.Diff (#777)

This is the src-cli portion of sourcegraph/sourcegraph#36758. It also optimizes the execution generally, because we run one less workspace.Diff.
After this PR, nothing requires us to still write or read execution results from the cache. Since this is a bigger code cleanup/refactor, I filed a separate ticket to address this and put it in sustaining. This PR mostly focuses on getting the Diff call out of the execution and stops to write the unnecessary cache result to the JSON logs so we send less log data.

Author: eseliger

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/sourcegraph/go-diff v0.6.1
 	github.com/sourcegraph/jsonx v0.0.0-20200629203448-1a936bd500cf
 	github.com/sourcegraph/scip v0.1.0
-	github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614231716-f73c9fa26c46
+	github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614233452-81dc06f6e96e
 	github.com/stretchr/testify v1.7.2
 	golang.org/x/net v0.0.0-20220526153639-5463443f8c37
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c

go.sum

@@ -351,8 +351,8 @@ github.com/sourcegraph/log v0.0.0-20220613150728-bb50c87ba841 h1:kiYxuyQ1zSNA4YP
 github.com/sourcegraph/log v0.0.0-20220613150728-bb50c87ba841/go.mod h1:A+9F6IicYvBbl2aT0R81lMraKcXjVfdfw352yPe2yJI=
 github.com/sourcegraph/scip v0.1.0 h1:kTs0CJaLQvcRZjg+HpGrcJPNX2Tx31+d6szWio3ZOkQ=
 github.com/sourcegraph/scip v0.1.0/go.mod h1:/AZ8RvsnRfeCZy232PJuVZqcl9f82fJnYwdWZeU2JCo=
-github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614231716-f73c9fa26c46 h1:ehDPNEMxvaCKWlV6oGh2jvkcs4bJzL020aSHjxNLSZs=
-github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614231716-f73c9fa26c46/go.mod h1:Orrt+5wdseAvxsVxgdswYPzJgVkTk0rLlmFHZW61epo=
+github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614233452-81dc06f6e96e h1:O+XG50CIAdzjuELX2gHhkc9Br1AaAXaOkZWg1YSRFpQ=
+github.com/sourcegraph/sourcegraph/lib v0.0.0-20220614233452-81dc06f6e96e/go.mod h1:Orrt+5wdseAvxsVxgdswYPzJgVkTk0rLlmFHZW61epo=
 github.com/sourcegraph/yaml v1.0.1-0.20200714132230-56936252f152 h1:z/MpntplPaW6QW95pzcAR/72Z5TWDyDnSo0EOcyij9o=
 github.com/sourcegraph/yaml v1.0.1-0.20200714132230-56936252f152/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I=
 github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ=

internal/batches/executor/run_steps.go

@@ -80,6 +80,7 @@ func runSteps(ctx context.Context, opts *executionOpts) (result execution.Result
 		// If we have cached results and don't need to execute any more steps,
 		// we can quit
 		if lastStep == len(opts.task.Steps)-1 {
+			// Build the execution result from the step result.
 			changes, err := git.ChangesInDiff([]byte(opts.task.CachedResult.Diff))
 			if err != nil {
 				return execResult, nil, errors.Wrap(err, "parsing cached step diff")
@@ -100,6 +101,8 @@ func runSteps(ctx context.Context, opts *executionOpts) (result execution.Result
 		)
 	}
 
+	var lastDiff string
+
 	for i := startStep; i < len(opts.task.Steps); i++ {
 		step := opts.task.Steps[i]
 
@@ -201,17 +204,11 @@ func runSteps(ctx context.Context, opts *executionOpts) (result execution.Result
 		}
 
 		opts.ui.StepFinished(i+1, stepResult.Diff, result.Files, stepResult.Outputs)
-	}
 
-	opts.ui.CalculatingDiffStarted()
-	diffOut, err := ws.Diff(ctx)
-	if err != nil {
-		return execResult, nil, errors.Wrap(err, "git diff failed")
+		lastDiff = stepResult.Diff
 	}
 
-	opts.ui.CalculatingDiffFinished()
-
-	execResult.Diff = string(diffOut)
+	execResult.Diff = lastDiff
 	execResult.ChangedFiles = previousStepResult.Files
 
 	return execResult, stepResults, err

internal/batches/executor/ui.go

@@ -45,9 +45,6 @@ type StepsExecutionUI interface {
 
 	StepOutputWriter(context.Context, *Task, int) StepOutputWriter
 
-	CalculatingDiffStarted()
-	CalculatingDiffFinished()
-
 	StepFinished(idx int, diff string, changes *git.Changes, outputs map[string]interface{})
 	StepFailed(idx int, err error, exitCode int)
 }
@@ -68,8 +65,6 @@ func (noop NoopStepsExecUI) StepStarted(step int, runScript string, env map[stri
 func (noop NoopStepsExecUI) StepOutputWriter(ctx context.Context, task *Task, step int) StepOutputWriter {
 	return NoopStepOutputWriter{}
 }
-func (noop NoopStepsExecUI) CalculatingDiffStarted()  {}
-func (noop NoopStepsExecUI) CalculatingDiffFinished() {}
 func (noop NoopStepsExecUI) StepFinished(idx int, diff string, changes *git.Changes, outputs map[string]interface{}) {
 }
 func (noop NoopStepsExecUI) StepFailed(idx int, err error, exitCode int) {

internal/batches/ui/json_lines.go

@@ -175,10 +175,7 @@ func (ui *JSONLines) ExecutionError(err error) {
 var _ executor.JSONCacheWriter = &JSONLines{}
 
 func (ui *JSONLines) WriteExecutionResult(key string, value execution.Result) {
-	logOperationSuccess(batcheslib.LogEventOperationCacheResult, &batcheslib.CacheResultMetadata{
-		Key:   key,
-		Value: value,
-	})
+	// Noop, we don't need execution results.
 }
 
 func (ui *JSONLines) WriteAfterStepResult(key string, value execution.AfterStepResult) {
@@ -363,13 +360,6 @@ func (ui *stepsExecutionJSONLines) StepFailed(step int, err error, exitCode int)
 	)
 }
 
-func (ui *stepsExecutionJSONLines) CalculatingDiffStarted() {
-	logOperationStart(batcheslib.LogEventOperationTaskCalculatingDiff, &batcheslib.TaskCalculatingDiffMetadata{TaskID: ui.linesTask.ID})
-}
-func (ui *stepsExecutionJSONLines) CalculatingDiffFinished() {
-	logOperationSuccess(batcheslib.LogEventOperationTaskCalculatingDiff, &batcheslib.TaskCalculatingDiffMetadata{TaskID: ui.linesTask.ID})
-}
-
 func logOperationStart(op batcheslib.LogEventOperation, metadata interface{}) {
 	logEvent(batcheslib.LogEvent{Operation: op, Status: batcheslib.LogEventStatusStarted, Metadata: metadata})
 }

internal/batches/ui/task_exec_tui.go

@@ -471,12 +471,6 @@ func (ui stepsExecTUI) StepOutputWriter(ctx context.Context, task *executor.Task
 	return executor.NoopStepOutputWriter{}
 }
 
-func (ui stepsExecTUI) CalculatingDiffStarted() {
-	ui.updateStatusBar("Calculating diff")
-}
-func (ui stepsExecTUI) CalculatingDiffFinished() {
-	// noop right now
-}
 func (ui stepsExecTUI) StepFinished(idx int, diff string, changes *git.Changes, outputs map[string]interface{}) {
 	// noop right now
 }