Skip to content

Commit e5816b1

Browse files
matthijskooijmancmaglie
matthijskooijman
authored and
cmaglie
committed
Merge ExecRecipeCollectStdErr into ExecRecipe
This unifies these similar methods into a single method. The interface is additionally changed to: - Accepts a `Context` argument. - Allow for defaults and named arguments, using an `ExecOptions` struct that is passed as an argument. - Allow configuring command output handling in a flexible way. Instead of passing bools for some specific configurations, you can now pass either `Ignore`, `Show` or `Capture` for both stdout and stderr independently. By default, stdout is is shown when verbose is true, or ignored when verbose is false. Stderr is shown by default. - Actually redirect stdout to `/dev/null` when it is not needed (by leaving `command.Stdout` at nil). Previously, `ExecRecipe` would either show or capture stdout, and the captured output was usually just thrown away. To allow for even more reuse, the biggest part of `ExecRecipe` is extracted into a new `utils.ExecCommand()` function which executes an arbitrary `exec.Cmd` object, with configurable output redirection. Signed-off-by: Matthijs Kooijman <[email protected]>
1 parent 0ab3532 commit e5816b1

File tree

6 files changed

+62
-58
lines changed

6 files changed

+62
-58
lines changed

Diff for: ‎builder_utils/utils.go

+10-43
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
package builder_utils
3131

3232
import (
33-
"bytes"
3433
"io"
3534
"os"
3635
"os/exec"
@@ -213,7 +212,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string,
213212
}
214213

215214
if !objIsUpToDate {
216-
_, err = ExecRecipe(properties, recipe, false, ctx.Verbose, ctx.Verbose, logger)
215+
_, _, err = ExecRecipe(ctx, properties, recipe, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show)
217216
if err != nil {
218217
return "", i18n.WrapError(err)
219218
}
@@ -380,7 +379,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri
380379
properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = archiveFilePath
381380
properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = objectFile
382381

383-
_, err := ExecRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.Verbose, ctx.Verbose, logger)
382+
_, _, err := ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show)
384383
if err != nil {
385384
return "", i18n.WrapError(err)
386385
}
@@ -389,40 +388,20 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri
389388
return archiveFilePath, nil
390389
}
391390

392-
func ExecRecipe(properties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) {
393-
command, err := PrepareCommandForRecipe(properties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger)
391+
// See util.ExecCommand for stdout/stderr arguments
392+
func ExecRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool, stdout int, stderr int) ([]byte, []byte, error) {
393+
command, err := PrepareCommandForRecipe(ctx, buildProperties, recipe, removeUnsetProperties)
394394
if err != nil {
395-
return nil, i18n.WrapError(err)
396-
}
397-
398-
if echoOutput {
399-
printToStdOut := func(data []byte) {
400-
logger.UnformattedWrite(os.Stdout, data)
401-
}
402-
stdout := &types.BufferedUntilNewLineWriter{PrintFunc: printToStdOut, Buffer: bytes.Buffer{}}
403-
defer stdout.Flush()
404-
command.Stdout = stdout
405-
}
406-
407-
printToStdErr := func(data []byte) {
408-
logger.UnformattedWrite(os.Stderr, data)
409-
}
410-
stderr := &types.BufferedUntilNewLineWriter{PrintFunc: printToStdErr, Buffer: bytes.Buffer{}}
411-
defer stderr.Flush()
412-
command.Stderr = stderr
413-
414-
if echoOutput {
415-
err := command.Run()
416-
return nil, i18n.WrapError(err)
395+
return nil, nil, i18n.WrapError(err)
417396
}
418397

419-
bytes, err := command.Output()
420-
return bytes, i18n.WrapError(err)
398+
return utils.ExecCommand(ctx, command, stdout, stderr)
421399
}
422400

423401
const COMMANDLINE_LIMIT = 30000
424402

425-
func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) (*exec.Cmd, error) {
403+
func PrepareCommandForRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) {
404+
logger := ctx.GetLogger()
426405
pattern := buildProperties[recipe]
427406
if pattern == constants.EMPTY_STRING {
428407
return nil, i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, recipe)
@@ -445,25 +424,13 @@ func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, remo
445424
return nil, i18n.WrapError(err)
446425
}
447426

448-
if echoCommandLine {
427+
if ctx.Verbose {
449428
logger.UnformattedFprintln(os.Stdout, commandLine)
450429
}
451430

452431
return command, nil
453432
}
454433

455-
func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) {
456-
command, err := PrepareCommandForRecipe(buildProperties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger)
457-
if err != nil {
458-
return nil, i18n.WrapError(err)
459-
}
460-
461-
buffer := &bytes.Buffer{}
462-
command.Stderr = buffer
463-
err = command.Run()
464-
return buffer.Bytes(), err
465-
}
466-
467434
func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) {
468435
buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS] = strings.Replace(buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS], "-MMD", "", -1)
469436
}

Diff for: ‎gcc_preproc_runner.go

+2-7
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,7 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath
5151
properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN])
5252
}
5353

54-
verbose := ctx.Verbose
55-
logger := ctx.GetLogger()
56-
_, err = builder_utils.ExecRecipe(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger)
54+
_, _, err = builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show)
5755
if err != nil {
5856
return i18n.WrapError(err)
5957
}
@@ -67,15 +65,12 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s
6765
return nil, i18n.WrapError(err)
6866
}
6967

70-
verbose := ctx.Verbose
71-
logger := ctx.GetLogger()
72-
7368
if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING {
7469
//generate PREPROC_MACROS from RECIPE_CPP_PATTERN
7570
properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN])
7671
}
7772

78-
stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger)
73+
_, stderr, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Capture)
7974
if err != nil {
8075
return stderr, i18n.WrapError(err)
8176
}

Diff for: ‎phases/linker.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func link(ctx *types.Context, objectFiles []string, coreDotARelPath string, core
8383
properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = coreArchiveFilePath
8484
properties[constants.BUILD_PROPERTIES_OBJECT_FILES] = objectFileList
8585

86-
_, err := builder_utils.ExecRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.Verbose, ctx.Verbose, ctx.GetLogger())
86+
_, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show)
8787
return err
8888
}
8989

Diff for: ‎phases/sizer.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"github.com/arduino/arduino-builder/constants"
3939
"github.com/arduino/arduino-builder/i18n"
4040
"github.com/arduino/arduino-builder/types"
41+
"github.com/arduino/arduino-builder/utils"
4142
"github.com/arduino/go-properties-map"
4243
)
4344

@@ -126,7 +127,7 @@ func checkSize(ctx *types.Context, buildProperties properties.Map) error {
126127
}
127128

128129
func execSizeRecipe(ctx *types.Context, properties properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) {
129-
out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, ctx.Verbose, false, ctx.GetLogger())
130+
out, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_SIZE_PATTERN, false /* stdout */, utils.Capture /* stderr */, utils.Show)
130131
if err != nil {
131132
resErr = errors.New("Error while determining sketch size: " + err.Error())
132133
return

Diff for: ‎recipe_runner.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030
package builder
3131

3232
import (
33+
"os"
34+
"sort"
35+
"strings"
36+
3337
"github.com/arduino/arduino-builder/builder_utils"
3438
"github.com/arduino/arduino-builder/constants"
3539
"github.com/arduino/arduino-builder/i18n"
3640
"github.com/arduino/arduino-builder/types"
37-
"os"
38-
"sort"
39-
"strings"
41+
"github.com/arduino/arduino-builder/utils"
4042
)
4143

4244
type RecipeByPrefixSuffixRunner struct {
@@ -51,16 +53,14 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error {
5153
}
5254

5355
buildProperties := ctx.BuildProperties.Clone()
54-
verbose := ctx.Verbose
55-
5656
recipes := findRecipes(buildProperties, s.Prefix, s.Suffix)
5757

5858
properties := buildProperties.Clone()
5959
for _, recipe := range recipes {
6060
if ctx.DebugLevel >= 10 {
6161
logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, constants.MSG_RUNNING_RECIPE, recipe)
6262
}
63-
_, err := builder_utils.ExecRecipe(properties, recipe, false, verbose, verbose, logger)
63+
_, _, err := builder_utils.ExecRecipe(ctx, properties, recipe, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show)
6464
if err != nil {
6565
return i18n.WrapError(err)
6666
}

Diff for: ‎utils/utils.go

+41
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
package utils
3131

3232
import (
33+
"bytes"
3334
"crypto/md5"
3435
"encoding/hex"
3536
"fmt"
@@ -276,6 +277,46 @@ func PrepareCommand(pattern string, logger i18n.Logger, relativePath string) (*e
276277
return PrepareCommandFilteredArgs(pattern, filterEmptyArg, logger, relativePath)
277278
}
278279

280+
const (
281+
Ignore = 0 // Redirect to null
282+
Show = 1 // Show on stdout/stderr as normal
283+
ShowIfVerbose = 2 // Show if verbose is set, Ignore otherwise
284+
Capture = 3 // Capture into buffer
285+
)
286+
287+
func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
288+
if stdout == Capture {
289+
buffer := &bytes.Buffer{}
290+
command.Stdout = buffer
291+
} else if stdout == Show || stdout == ShowIfVerbose && ctx.Verbose {
292+
command.Stdout = os.Stdout
293+
}
294+
295+
if stderr == Capture {
296+
buffer := &bytes.Buffer{}
297+
command.Stderr = buffer
298+
} else if stderr == Show || stderr == ShowIfVerbose && ctx.Verbose {
299+
command.Stderr = os.Stderr
300+
}
301+
302+
err := command.Start()
303+
if err != nil {
304+
return nil, nil, i18n.WrapError(err)
305+
}
306+
307+
err = command.Wait()
308+
309+
var outbytes, errbytes []byte
310+
if buf, ok := command.Stdout.(*bytes.Buffer); ok {
311+
outbytes = buf.Bytes()
312+
}
313+
if buf, ok := command.Stderr.(*bytes.Buffer); ok {
314+
errbytes = buf.Bytes()
315+
}
316+
317+
return outbytes, errbytes, i18n.WrapError(err)
318+
}
319+
279320
func MapHas(aMap map[string]interface{}, key string) bool {
280321
_, ok := aMap[key]
281322
return ok

0 commit comments

Comments
 (0)