Skip to content

Commit 58df5f8

Browse files
matthijskooijmanmatthijskooijman
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 d81cca8 commit 58df5f8

File tree

6 files changed

+58
-46
lines changed

6 files changed

+58
-46
lines changed

Diff for: ‎src/arduino.cc/builder/builder_utils/utils.go

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

3232
import (
33-
"bytes"
3433
"fmt"
3534
"io"
3635
"os"
@@ -181,7 +180,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string,
181180
}
182181

183182
if !objIsUpToDate {
184-
_, err = ExecRecipe(properties, recipe, false, ctx.Verbose, ctx.Verbose, logger)
183+
_, _, err = ExecRecipe(ctx, properties, recipe, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show)
185184
if err != nil {
186185
return "", i18n.WrapError(err)
187186
}
@@ -348,7 +347,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri
348347
properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = archiveFilePath
349348
properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = objectFile
350349

351-
_, err := ExecRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.Verbose, ctx.Verbose, logger)
350+
_, _, err := ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show)
352351
if err != nil {
353352
return "", i18n.WrapError(err)
354353
}
@@ -357,28 +356,17 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri
357356
return archiveFilePath, nil
358357
}
359358

360-
func ExecRecipe(properties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) {
361-
command, err := PrepareCommandForRecipe(properties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger)
359+
// See util.ExecCommand for stdout/stderr arguments
360+
func ExecRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool, stdout int, stderr int) ([]byte, []byte, error) {
361+
command, err := PrepareCommandForRecipe(ctx, buildProperties, recipe, removeUnsetProperties)
362362
if err != nil {
363-
return nil, i18n.WrapError(err)
364-
}
365-
366-
if echoOutput {
367-
command.Stdout = os.Stdout
368-
}
369-
370-
command.Stderr = os.Stderr
371-
372-
if echoOutput {
373-
err := command.Run()
374-
return nil, i18n.WrapError(err)
363+
return nil, nil, i18n.WrapError(err)
375364
}
376-
377-
bytes, err := command.Output()
378-
return bytes, i18n.WrapError(err)
365+
return utils.ExecCommand(ctx, command, stdout, stderr)
379366
}
380367

381-
func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) (*exec.Cmd, error) {
368+
func PrepareCommandForRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) {
369+
logger := ctx.GetLogger()
382370
pattern := buildProperties[recipe]
383371
if pattern == constants.EMPTY_STRING {
384372
return nil, i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, recipe)
@@ -398,25 +386,13 @@ func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, remo
398386
return nil, i18n.WrapError(err)
399387
}
400388

401-
if echoCommandLine {
389+
if ctx.Verbose {
402390
fmt.Println(commandLine)
403391
}
404392

405393
return command, nil
406394
}
407395

408-
func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) {
409-
command, err := PrepareCommandForRecipe(buildProperties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger)
410-
if err != nil {
411-
return nil, i18n.WrapError(err)
412-
}
413-
414-
buffer := &bytes.Buffer{}
415-
command.Stderr = buffer
416-
err = command.Run()
417-
return buffer.Bytes(), err
418-
}
419-
420396
func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) {
421397
buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS] = strings.Replace(buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS], "-MMD", "", -1)
422398
}

Diff for: ‎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
}

Diff for: ‎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

Diff for: ‎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

Diff for: ‎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
}

Diff for: ‎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
"io/ioutil"
@@ -254,6 +255,46 @@ func PrepareCommand(pattern string, logger i18n.Logger) (*exec.Cmd, error) {
254255
return PrepareCommandFilteredArgs(pattern, filterEmptyArg, logger)
255256
}
256257

258+
const (
259+
Ignore = 0 // Redirect to null
260+
Show = 1 // Show on stdout/stderr as normal
261+
ShowIfVerbose = 2 // Show if verbose is set, Ignore otherwise
262+
Capture = 3 // Capture into buffer
263+
)
264+
265+
func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
266+
if stdout == Capture {
267+
buffer := &bytes.Buffer{}
268+
command.Stdout = buffer
269+
} else if stdout == Show || stdout == ShowIfVerbose && ctx.Verbose {
270+
command.Stdout = os.Stdout
271+
}
272+
273+
if stderr == Capture {
274+
buffer := &bytes.Buffer{}
275+
command.Stderr = buffer
276+
} else if stderr == Show || stderr == ShowIfVerbose && ctx.Verbose {
277+
command.Stderr = os.Stderr
278+
}
279+
280+
err := command.Start()
281+
if err != nil {
282+
return nil, nil, i18n.WrapError(err)
283+
}
284+
285+
err = command.Wait()
286+
287+
var outbytes, errbytes []byte
288+
if buf, ok := command.Stdout.(*bytes.Buffer); ok {
289+
outbytes = buf.Bytes()
290+
}
291+
if buf, ok := command.Stderr.(*bytes.Buffer); ok {
292+
errbytes = buf.Bytes()
293+
}
294+
295+
return outbytes, errbytes, i18n.WrapError(err)
296+
}
297+
257298
func MapHas(aMap map[string]interface{}, key string) bool {
258299
_, ok := aMap[key]
259300
return ok

0 commit comments

Comments
 (0)