Skip to content

Commit 5e021dc

Browse files
matthijskooijmanfacchinm
authored andcommitted
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 a2c8e5d commit 5e021dc

File tree

6 files changed

+58
-55
lines changed

6 files changed

+58
-55
lines changed

src/arduino.cc/builder/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)
@@ -448,25 +427,13 @@ func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, remo
448427
return nil, i18n.WrapError(err)
449428
}
450429

451-
if echoCommandLine {
430+
if ctx.Verbose {
452431
logger.UnformattedFprintln(os.Stdout, commandLine)
453432
}
454433

455434
return command, nil
456435
}
457436

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

src/arduino.cc/builder/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
}

src/arduino.cc/builder/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

src/arduino.cc/builder/phases/sizer.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"arduino.cc/builder/constants"
3939
"arduino.cc/builder/i18n"
4040
"arduino.cc/builder/types"
41+
"arduino.cc/builder/utils"
4142
"arduino.cc/properties"
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

src/arduino.cc/builder/recipe_runner.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"arduino.cc/builder/constants"
3535
"arduino.cc/builder/i18n"
3636
"arduino.cc/builder/types"
37+
"arduino.cc/builder/utils"
3738
"os"
3839
"sort"
3940
"strings"
@@ -51,16 +52,14 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error {
5152
}
5253

5354
buildProperties := ctx.BuildProperties.Clone()
54-
verbose := ctx.Verbose
55-
5655
recipes := findRecipes(buildProperties, s.Prefix, s.Suffix)
5756

5857
properties := buildProperties.Clone()
5958
for _, recipe := range recipes {
6059
if ctx.DebugLevel >= 10 {
6160
logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, constants.MSG_RUNNING_RECIPE, recipe)
6261
}
63-
_, err := builder_utils.ExecRecipe(properties, recipe, false, verbose, verbose, logger)
62+
_, _, err := builder_utils.ExecRecipe(ctx, properties, recipe, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show)
6463
if err != nil {
6564
return i18n.WrapError(err)
6665
}

src/arduino.cc/builder/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"
@@ -277,6 +278,46 @@ func PrepareCommand(pattern string, logger i18n.Logger, relativePath string) (*e
277278
return PrepareCommandFilteredArgs(pattern, filterEmptyArg, logger, relativePath)
278279
}
279280

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

0 commit comments

Comments
 (0)