Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7be50aa

Browse files
alessio-peruginicmaglie
alessio-perugini
authored and
cmaglie
committedSep 5, 2023
refactoring the cmd.Exec in favour of executils
1 parent 7f0e580 commit 7be50aa

File tree

15 files changed

+70
-61
lines changed

15 files changed

+70
-61
lines changed
 

‎arduino/builder/compilation_database.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import (
1919
"encoding/json"
2020
"fmt"
2121
"os"
22-
"os/exec"
2322

23+
"github.com/arduino/arduino-cli/executils"
2424
"github.com/arduino/go-paths-helper"
2525
)
2626

@@ -68,8 +68,8 @@ func (db *CompilationDatabase) SaveToFile() {
6868
}
6969

7070
// Add adds a new CompilationDatabase entry
71-
func (db *CompilationDatabase) Add(target *paths.Path, command *exec.Cmd) {
72-
commandDir := command.Dir
71+
func (db *CompilationDatabase) Add(target *paths.Path, command *executils.Process) {
72+
commandDir := command.GetDir()
7373
if commandDir == "" {
7474
// This mimics what Cmd.Run also does: Use Dir if specified,
7575
// current directory otherwise
@@ -82,7 +82,7 @@ func (db *CompilationDatabase) Add(target *paths.Path, command *exec.Cmd) {
8282

8383
entry := CompilationCommand{
8484
Directory: commandDir,
85-
Arguments: command.Args,
85+
Arguments: command.GetArgs(),
8686
File: target.String(),
8787
}
8888

‎arduino/builder/compilation_database_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
package builder
1717

1818
import (
19-
"os/exec"
2019
"testing"
2120

21+
"github.com/arduino/arduino-cli/executils"
2222
"github.com/arduino/go-paths-helper"
2323
"github.com/stretchr/testify/require"
2424
)
@@ -28,7 +28,8 @@ func TestCompilationDatabase(t *testing.T) {
2828
require.NoError(t, err)
2929
defer tmpfile.Remove()
3030

31-
cmd := exec.Command("gcc", "arg1", "arg2")
31+
cmd, err := executils.NewProcess(nil, "gcc", "arg1", "arg2")
32+
require.NoError(t, err)
3233
db := NewCompilationDatabase(tmpfile)
3334
db.Add(paths.New("test"), cmd)
3435
db.SaveToFile()

‎arduino/builder/cpp/cpp.go

+4
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,7 @@ func ParseString(line string) (string, string, bool) {
106106
i += width
107107
}
108108
}
109+
110+
func WrapWithHyphenI(value string) string {
111+
return "\"-I" + value + "\""
112+
}

‎arduino/builder/preprocessor/gcc.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import (
2020
"fmt"
2121
"strings"
2222

23+
"github.com/arduino/arduino-cli/arduino/builder/cpp"
2324
"github.com/arduino/arduino-cli/executils"
2425
f "github.com/arduino/arduino-cli/internal/algorithms"
25-
"github.com/arduino/arduino-cli/arduino/builder/utils"
2626
"github.com/arduino/go-paths-helper"
2727
"github.com/arduino/go-properties-orderedmap"
2828
"github.com/pkg/errors"
@@ -38,7 +38,7 @@ func GCC(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.
3838
gccBuildProperties.SetPath("source_file", sourceFilePath)
3939
gccBuildProperties.SetPath("preprocessed_file_path", targetFilePath)
4040

41-
includesStrings := f.Map(includes.AsStrings(), utils.WrapWithHyphenI)
41+
includesStrings := f.Map(includes.AsStrings(), cpp.WrapWithHyphenI)
4242
gccBuildProperties.Set("includes", strings.Join(includesStrings, " "))
4343

4444
const gccPreprocRecipeProperty = "recipe.preproc.macros"

‎arduino/builder/utils/utils.go

-3
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,3 @@ func FindFilesInFolder(dir *paths.Path, recurse bool, extensions ...string) (pat
180180
return dir.ReadDir(fileFilter)
181181
}
182182

183-
func WrapWithHyphenI(value string) string {
184-
return "\"-I" + value + "\""
185-
}

‎executils/process.go

+10
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ func (p *Process) SetDir(dir string) {
137137
p.cmd.Dir = dir
138138
}
139139

140+
// GetDir gets the working directory of the command.
141+
func (p *Process) GetDir() string {
142+
return p.cmd.Dir
143+
}
144+
140145
// SetDirFromPath sets the working directory of the command. If path is nil, Run
141146
// runs the command in the calling process's current directory.
142147
func (p *Process) SetDirFromPath(path *paths.Path) {
@@ -187,3 +192,8 @@ func (p *Process) RunAndCaptureOutput(ctx context.Context) ([]byte, []byte, erro
187192
err := p.RunWithinContext(ctx)
188193
return stdout.Bytes(), stderr.Bytes(), err
189194
}
195+
196+
// GetArgs returns the command arguments
197+
func (p *Process) GetArgs() []string {
198+
return p.cmd.Args
199+
}

‎legacy/builder/builder_utils/utils.go

+16-10
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ package builder_utils
1818
import (
1919
"fmt"
2020
"os"
21-
"os/exec"
2221
"path/filepath"
2322
"runtime"
2423
"strings"
2524
"sync"
2625

2726
bUtils "github.com/arduino/arduino-cli/arduino/builder/utils"
2827
"github.com/arduino/arduino-cli/arduino/globals"
28+
"github.com/arduino/arduino-cli/executils"
2929
"github.com/arduino/arduino-cli/i18n"
3030
"github.com/arduino/arduino-cli/legacy/builder/constants"
3131
"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -173,7 +173,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
173173
return nil, errors.WithStack(err)
174174
}
175175

176-
command, err := PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
176+
command, err := PrepareCommandForRecipe(properties, recipe, false)
177177
if err != nil {
178178
return nil, errors.WithStack(err)
179179
}
@@ -244,7 +244,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile
244244
properties.SetPath(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, archiveFilePath)
245245
properties.SetPath(constants.BUILD_PROPERTIES_OBJECT_FILE, objectFile)
246246

247-
command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
247+
command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false)
248248
if err != nil {
249249
return nil, errors.WithStack(err)
250250
}
@@ -260,7 +260,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile
260260

261261
const COMMANDLINE_LIMIT = 30000
262262

263-
func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool, toolEnv []string) (*exec.Cmd, error) {
263+
func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool) (*executils.Process, error) {
264264
pattern := buildProperties.Get(recipe)
265265
if pattern == "" {
266266
return nil, errors.Errorf(tr("%[1]s pattern is missing"), recipe)
@@ -275,24 +275,30 @@ func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, rem
275275
if err != nil {
276276
return nil, errors.WithStack(err)
277277
}
278-
command := exec.Command(parts[0], parts[1:]...)
279-
command.Env = append(os.Environ(), toolEnv...)
280278

281279
// if the overall commandline is too long for the platform
282280
// try reducing the length by making the filenames relative
283281
// and changing working directory to build.path
282+
var relativePath string
284283
if len(commandLine) > COMMANDLINE_LIMIT {
285-
relativePath := buildProperties.Get("build.path")
286-
for i, arg := range command.Args {
284+
relativePath = buildProperties.Get("build.path")
285+
for i, arg := range parts {
287286
if _, err := os.Stat(arg); os.IsNotExist(err) {
288287
continue
289288
}
290289
rel, err := filepath.Rel(relativePath, arg)
291290
if err == nil && !strings.Contains(rel, "..") && len(rel) < len(arg) {
292-
command.Args[i] = rel
291+
parts[i] = rel
293292
}
294293
}
295-
command.Dir = relativePath
294+
}
295+
296+
command, err := executils.NewProcess(nil, parts...)
297+
if err != nil {
298+
return nil, errors.WithStack(err)
299+
}
300+
if relativePath != "" {
301+
command.SetDir(relativePath)
296302
}
297303

298304
return command, nil

‎legacy/builder/create_cmake_rule.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ func extractCompileFlags(ctx *types.Context, recipe string, defines, dynamicLibs
367367
return target
368368
}
369369

370-
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
370+
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true)
371371

372-
for _, arg := range command.Args {
372+
for _, arg := range command.GetArgs() {
373373
if strings.HasPrefix(arg, "-D") {
374374
*defines = appendIfNotPresent(*defines, arg)
375375
continue

‎legacy/builder/phases/core_builder.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ import (
2222
"os"
2323
"strings"
2424

25+
"github.com/arduino/arduino-cli/arduino/builder/cpp"
2526
"github.com/arduino/arduino-cli/buildcache"
2627
"github.com/arduino/arduino-cli/i18n"
2728
f "github.com/arduino/arduino-cli/internal/algorithms"
2829
"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
2930
"github.com/arduino/arduino-cli/legacy/builder/constants"
3031
"github.com/arduino/arduino-cli/legacy/builder/types"
31-
"github.com/arduino/arduino-cli/arduino/builder/utils"
3232
"github.com/arduino/go-paths-helper"
3333
"github.com/arduino/go-properties-orderedmap"
3434
"github.com/pkg/errors"
@@ -80,7 +80,7 @@ func compileCore(ctx *types.Context, buildPath *paths.Path, buildCachePath *path
8080
if variantFolder != nil && variantFolder.IsDir() {
8181
includes = append(includes, variantFolder.String())
8282
}
83-
includes = f.Map(includes, utils.WrapWithHyphenI)
83+
includes = f.Map(includes, cpp.WrapWithHyphenI)
8484

8585
var err error
8686

‎legacy/builder/phases/libraries_builder.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package phases
1818
import (
1919
"strings"
2020

21-
"github.com/arduino/arduino-cli/arduino/builder/utils"
21+
"github.com/arduino/arduino-cli/arduino/builder/cpp"
2222
"github.com/arduino/arduino-cli/arduino/libraries"
2323
f "github.com/arduino/arduino-cli/internal/algorithms"
2424
"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
@@ -38,7 +38,7 @@ func (s *LibrariesBuilder) Run(ctx *types.Context) error {
3838
librariesBuildPath := ctx.LibrariesBuildPath
3939
buildProperties := ctx.BuildProperties
4040
includesFolders := ctx.SketchLibrariesDetector.IncludeFolders()
41-
includes := f.Map(includesFolders.AsStrings(), utils.WrapWithHyphenI)
41+
includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
4242
libs := ctx.SketchLibrariesDetector.ImportedLibraries()
4343

4444
if err := librariesBuildPath.MkdirAll(); err != nil {
@@ -67,9 +67,9 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
6767
// Add fpu specifications if they exist
6868
// To do so, resolve recipe.cpp.o.pattern,
6969
// search for -mfpu=xxx -mfloat-abi=yyy and add to a subfolder
70-
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, "recipe.cpp.o.pattern", true, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
70+
command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, "recipe.cpp.o.pattern", true)
7171
fpuSpecs := ""
72-
for _, el := range strings.Split(command.String(), " ") {
72+
for _, el := range command.GetArgs() {
7373
if strings.Contains(el, FPU_CFLAG) {
7474
toAdd := strings.Split(el, "=")
7575
if len(toAdd) > 1 {
@@ -78,7 +78,7 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
7878
}
7979
}
8080
}
81-
for _, el := range strings.Split(command.String(), " ") {
81+
for _, el := range command.GetArgs() {
8282
if strings.Contains(el, FLOAT_ABI_CFLAG) {
8383
toAdd := strings.Split(el, "=")
8484
if len(toAdd) > 1 {
@@ -201,7 +201,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
201201
}
202202
} else {
203203
if library.UtilityDir != nil {
204-
includes = append(includes, utils.WrapWithHyphenI(library.UtilityDir.String()))
204+
includes = append(includes, cpp.WrapWithHyphenI(library.UtilityDir.String()))
205205
}
206206
libObjectFiles, err := builder_utils.CompileFiles(ctx, library.SourceDir, libraryBuildPath, buildProperties, includes)
207207
if err != nil {

‎legacy/builder/phases/linker.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
9393
properties.SetPath("archive_file_path", archive)
9494
properties.SetPath("object_file", object)
9595

96-
command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
96+
command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false)
9797
if err != nil {
9898
return errors.WithStack(err)
9999
}
@@ -114,10 +114,10 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
114114
properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String())
115115
properties.Set("object_files", objectFileList)
116116

117-
command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
117+
command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false)
118118
if err != nil {
119119
return err
120-
}
120+
}
121121

122122
_, _, err = utils.ExecCommand(ctx, command, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
123123
return err

‎legacy/builder/phases/sizer.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (s *Sizer) Run(ctx *types.Context) error {
5050
}
5151

5252
func checkSizeAdvanced(ctx *types.Context, properties *properties.Map) error {
53-
command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.advanced_size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
53+
command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.advanced_size.pattern", false)
5454
if err != nil {
5555
return errors.New(tr("Error while determining sketch size: %s", err))
5656
}
@@ -179,7 +179,7 @@ func checkSize(ctx *types.Context, buildProperties *properties.Map) error {
179179
}
180180

181181
func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) {
182-
command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
182+
command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.size.pattern", false)
183183
if err != nil {
184184
resErr = fmt.Errorf(tr("Error while determining sketch size: %s"), err)
185185
return

‎legacy/builder/phases/sketch_builder.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
package phases
1717

1818
import (
19-
"github.com/arduino/arduino-cli/arduino/builder/utils"
19+
"github.com/arduino/arduino-cli/arduino/builder/cpp"
2020
f "github.com/arduino/arduino-cli/internal/algorithms"
2121
"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
2222
"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -29,7 +29,7 @@ func (s *SketchBuilder) Run(ctx *types.Context) error {
2929
sketchBuildPath := ctx.SketchBuildPath
3030
buildProperties := ctx.BuildProperties
3131
includesFolders := ctx.SketchLibrariesDetector.IncludeFolders()
32-
includes := f.Map(includesFolders.AsStrings(), utils.WrapWithHyphenI)
32+
includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
3333

3434
if err := sketchBuildPath.MkdirAll(); err != nil {
3535
return errors.WithStack(err)

‎legacy/builder/recipe_runner.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error {
4444
for _, recipe := range recipes {
4545
logrus.Debugf(fmt.Sprintf("Running recipe: %s", recipe))
4646

47-
command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
47+
command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false)
4848
if err != nil {
4949
return errors.WithStack(err)
5050
}
5151

5252
if ctx.OnlyUpdateCompilationDatabase && s.SkipIfOnlyUpdatingCompilationDatabase {
5353
if ctx.Verbose {
54-
ctx.Info(tr("Skipping: %[1]s", strings.Join(command.Args, " ")))
54+
ctx.Info(tr("Skipping: %[1]s", strings.Join(command.GetArgs(), " ")))
5555
}
5656
return nil
5757
}

‎legacy/builder/utils/utils.go

+12-21
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ package utils
1818
import (
1919
"bytes"
2020
"os"
21-
"os/exec"
2221
"strings"
2322

23+
"github.com/arduino/arduino-cli/executils"
2424
f "github.com/arduino/arduino-cli/internal/algorithms"
2525
"github.com/arduino/arduino-cli/legacy/builder/types"
2626
"github.com/pkg/errors"
@@ -51,30 +51,30 @@ const (
5151
Capture = 3 // Capture into buffer
5252
)
5353

54-
func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
54+
func ExecCommand(ctx *types.Context, command *executils.Process, stdout int, stderr int) ([]byte, []byte, error) {
5555
if ctx.Verbose {
56-
ctx.Info(PrintableCommand(command.Args))
56+
ctx.Info(PrintableCommand(command.GetArgs()))
5757
}
5858

59+
stdoutBuffer := &bytes.Buffer{}
5960
if stdout == Capture {
60-
buffer := &bytes.Buffer{}
61-
command.Stdout = buffer
61+
command.RedirectStdoutTo(stdoutBuffer)
6262
} else if stdout == Show || (stdout == ShowIfVerbose && ctx.Verbose) {
6363
if ctx.Stdout != nil {
64-
command.Stdout = ctx.Stdout
64+
command.RedirectStdoutTo(ctx.Stdout)
6565
} else {
66-
command.Stdout = os.Stdout
66+
command.RedirectStdoutTo(os.Stdout)
6767
}
6868
}
6969

70+
stderrBuffer := &bytes.Buffer{}
7071
if stderr == Capture {
71-
buffer := &bytes.Buffer{}
72-
command.Stderr = buffer
72+
command.RedirectStderrTo(stderrBuffer)
7373
} else if stderr == Show || (stderr == ShowIfVerbose && ctx.Verbose) {
7474
if ctx.Stderr != nil {
75-
command.Stderr = ctx.Stderr
75+
command.RedirectStderrTo(ctx.Stderr)
7676
} else {
77-
command.Stderr = os.Stderr
77+
command.RedirectStderrTo(os.Stderr)
7878
}
7979
}
8080

@@ -84,16 +84,7 @@ func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int)
8484
}
8585

8686
err = command.Wait()
87-
88-
var outbytes, errbytes []byte
89-
if buf, ok := command.Stdout.(*bytes.Buffer); ok {
90-
outbytes = buf.Bytes()
91-
}
92-
if buf, ok := command.Stderr.(*bytes.Buffer); ok {
93-
errbytes = buf.Bytes()
94-
}
95-
96-
return outbytes, errbytes, errors.WithStack(err)
87+
return stdoutBuffer.Bytes(), stderrBuffer.Bytes(), errors.WithStack(err)
9788
}
9889

9990
type loggerAction struct {

0 commit comments

Comments
 (0)
Please sign in to comment.