Skip to content

Commit 2a2d040

Browse files
author
Fabian Holler
committed
exec: refactor command execution
- When a command is executed, all it's stdout and stderr output is stored in a buffer in memory. The buffer is used when evaluating the command output and to print it if the command execution should fail. This can be very memory intensive when executed tasks generate a lot of output. To reduce the memory usage: - only store up to 16KB of the beginning and up to 16KB of the end of the output per stdout and stderr for the eventual error message when the command execution fails. The truncated part is marked in the error message. This should be sufficient context to understand failures. This is implemented with a modified copy of the prefisuffixsaver from the os/exec stdlib package. - The Run() function does not store and return the whole output of a command on success anymore. Run() is used to execute the commands of baur tasks. This will reduce the memory usage. - The new RunCombinedOut() stores the stdout output in a buffer. This function is used to run baur internal commands that only generate few output and the output is required for evaluation. - running commands can now be canceled via a passed context. This is not used by anything yet. It will be used to specify timeouts for "baur run". - Cmd now supports to specify an additional io.Writer to stream the stdout and stderr output of a command. This enables efficient processing of larger outputs and will be used in a following Pull-Request, to run "git ls-files" to obtain input file digests. - when the command output is streamed to the terminal (baur run --show-task-output), the stderr output is now colored red, that makes it distinguishable from stdout output - when --show-task-output is not passed to baur run and the execution of a command fails, the stderr and stdout output will now be shown after each other instead of intermixed. This ensures that output of both file descriptors can not become interlaced and result in bogus output.
1 parent 38adf96 commit 2a2d040

File tree

16 files changed

+457
-180
lines changed

16 files changed

+457
-180
lines changed

internal/command/command_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package command
55

66
import (
7+
"context"
78
"encoding/csv"
89
"os"
910
"path/filepath"
@@ -167,7 +168,7 @@ func TestRunningPendingTasksChangesStatus(t *testing.T) {
167168

168169
gittest.CommitFilesToGit(t, ".")
169170

170-
res, err := exec.Command("git", "rev-parse", "HEAD").ExpectSuccess().Run()
171+
res, err := exec.Command("git", "rev-parse", "HEAD").ExpectSuccess().RunCombinedOut(context.Background())
171172
require.NoError(t, err)
172173

173174
commit = strings.TrimSpace(res.StrOutput())
@@ -199,7 +200,7 @@ func TestRunningPendingTasksWithInputStringChangesStatus(t *testing.T) {
199200
gittest.CreateRepository(t, ".")
200201
gittest.CommitFilesToGit(t, ".")
201202

202-
res, err := exec.Command("git", "rev-parse", "HEAD").ExpectSuccess().Run()
203+
res, err := exec.Command("git", "rev-parse", "HEAD").ExpectSuccess().RunCombinedOut(context.Background())
203204
require.NoError(t, err)
204205

205206
commit := strings.TrimSpace(res.StrOutput())

internal/command/root.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ var exitFunc = func(code int) { os.Exit(code) }
3838
func initSb(_ *cobra.Command, _ []string) {
3939
if verboseFlag {
4040
log.StdLogger.EnableDebug(verboseFlag)
41-
exec.DefaultDebugfFn = log.StdLogger.Debugf
41+
exec.DefaultLogFn = log.StdLogger.Debugf
4242
}
4343

4444
if noColorFlag {

internal/command/run.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/spf13/cobra"
1313

1414
"github.com/simplesurance/baur/v3/internal/command/term"
15-
"github.com/simplesurance/baur/v3/internal/exec"
1615
"github.com/simplesurance/baur/v3/internal/log"
1716
"github.com/simplesurance/baur/v3/internal/routines"
1817
"github.com/simplesurance/baur/v3/internal/upload/docker"
@@ -155,6 +154,9 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
155154

156155
c.taskRunnerRoutinePool = routines.NewPool(c.taskRunnerGoRoutines)
157156
c.taskRunner = baur.NewTaskRunner()
157+
if c.showOutput && !verboseFlag {
158+
c.taskRunner.LogFn = stdout.Printf
159+
}
158160

159161
c.dockerClient, err = docker.NewClient(log.StdLogger.Debugf)
160162
exitOnErr(err)
@@ -190,10 +192,6 @@ func (c *runCmd) run(_ *cobra.Command, args []string) {
190192
len(pendingTasks), len(tasks), term.ColoredTaskStatus(baur.TaskStatusExecutionPending))
191193
}
192194

193-
if c.showOutput {
194-
exec.DefaultDebugfFn = stdout.Printf
195-
}
196-
197195
for _, pt := range pendingTasks {
198196
// copy the iteration variable, to prevent that its value
199197
// changes in the closure to 't' of the next iteration before
@@ -287,7 +285,7 @@ func (c *runCmd) runTask(task *baur.Task) (*baur.RunResult, error) {
287285
}
288286

289287
if result.Result.ExitCode != 0 {
290-
if c.showOutput {
288+
if c.showOutput || verboseFlag {
291289
stderr.Printf("%s: execution %s (%s), command exited with code %d\n",
292290
term.Highlight(task),
293291
statusStrFailed,
@@ -297,14 +295,15 @@ func (c *runCmd) runTask(task *baur.Task) (*baur.RunResult, error) {
297295
result.ExitCode,
298296
)
299297
} else {
300-
stderr.Printf("%s: execution %s (%s), command exited with code %d, output:\n%s\n",
298+
err := result.Result.ExpectSuccess()
299+
stderr.Printf("%s: execution %s (%s): %s\n",
301300
term.Highlight(task),
302301
statusStrFailed,
303302
term.FormatDuration(
304303
result.StopTime.Sub(result.StartTime),
305304
),
306-
result.ExitCode,
307-
result.StrOutput())
305+
err.Error(),
306+
)
308307
}
309308

310309
return nil, fmt.Errorf("execution failed with exit code %d", result.ExitCode)

internal/command/util_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,16 @@ func redirectOutputToLogger(t *testing.T) {
6060
// parallel running tests
6161
log.RedirectToTestingLog(t)
6262

63-
oldExecDebugFfN := exec.DefaultDebugfFn
64-
exec.DefaultDebugfFn = t.Logf
63+
oldExecDebugFfN := exec.DefaultLogFn
64+
exec.DefaultLogFn = t.Logf
6565

6666
oldStdout := stdout
6767
stdout = term.NewStream(logwriter.New(t, io.Discard))
6868
oldStderr := stderr
6969
stderr = term.NewStream(logwriter.New(t, io.Discard))
7070

7171
t.Cleanup(func() {
72-
exec.DefaultDebugfFn = oldExecDebugFfN
72+
exec.DefaultLogFn = oldExecDebugFfN
7373
stdout = oldStdout
7474
stderr = oldStderr
7575
})

0 commit comments

Comments
 (0)