refactoring the cmd.Exec in favour of executils

arduino · Sep 4, 2023 · a64582c · a64582c
1 parent d97cac7
commit a64582c
Show file tree

Hide file tree

Showing 15 changed files with 70 additions and 61 deletions.
diff --git a/arduino/builder/compilation_database.go b/arduino/builder/compilation_database.go
@@ -19,8 +19,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"os/exec"
 
+	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/go-paths-helper"
 )
 
@@ -68,8 +68,8 @@ func (db *CompilationDatabase) SaveToFile() {
 }
 
 // Add adds a new CompilationDatabase entry
-func (db *CompilationDatabase) Add(target *paths.Path, command *exec.Cmd) {
-	commandDir := command.Dir
+func (db *CompilationDatabase) Add(target *paths.Path, command *executils.Process) {
+	commandDir := command.GetDir()
 	if commandDir == "" {
 		// This mimics what Cmd.Run also does: Use Dir if specified,
 		// current directory otherwise
@@ -82,7 +82,7 @@ func (db *CompilationDatabase) Add(target *paths.Path, command *exec.Cmd) {
 
 	entry := CompilationCommand{
 		Directory: commandDir,
-		Arguments: command.Args,
+		Arguments: command.GetArgs(),
 		File:      target.String(),
 	}
 

diff --git a/arduino/builder/compilation_database_test.go b/arduino/builder/compilation_database_test.go
@@ -16,9 +16,9 @@
 package builder
 
 import (
-	"os/exec"
 	"testing"
 
+	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/require"
 )
@@ -28,7 +28,8 @@ func TestCompilationDatabase(t *testing.T) {
 	require.NoError(t, err)
 	defer tmpfile.Remove()
 
-	cmd := exec.Command("gcc", "arg1", "arg2")
+	cmd, err := executils.NewProcess(nil, "gcc", "arg1", "arg2")
+	require.NoError(t, err)
 	db := NewCompilationDatabase(tmpfile)
 	db.Add(paths.New("test"), cmd)
 	db.SaveToFile()

diff --git a/arduino/builder/cpp/cpp.go b/arduino/builder/cpp/cpp.go
@@ -106,3 +106,7 @@ func ParseString(line string) (string, string, bool) {
 		i += width
 	}
 }
+
+func WrapWithHyphenI(value string) string {
+	return "\"-I" + value + "\""
+}
diff --git a/arduino/builder/preprocessor/gcc.go b/arduino/builder/preprocessor/gcc.go
@@ -20,9 +20,9 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino/builder/cpp"
 	"github.com/arduino/arduino-cli/executils"
 	f "github.com/arduino/arduino-cli/internal/algorithms"
-	"github.com/arduino/arduino-cli/arduino/builder/utils"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/pkg/errors"
@@ -38,7 +38,7 @@ func GCC(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.
 	gccBuildProperties.SetPath("source_file", sourceFilePath)
 	gccBuildProperties.SetPath("preprocessed_file_path", targetFilePath)
 
-	includesStrings := f.Map(includes.AsStrings(), utils.WrapWithHyphenI)
+	includesStrings := f.Map(includes.AsStrings(), cpp.WrapWithHyphenI)
 	gccBuildProperties.Set("includes", strings.Join(includesStrings, " "))
 
 	const gccPreprocRecipeProperty = "recipe.preproc.macros"

diff --git a/arduino/builder/utils/utils.go b/arduino/builder/utils/utils.go
@@ -180,6 +180,3 @@ func FindFilesInFolder(dir *paths.Path, recurse bool, extensions ...string) (pat
 	return dir.ReadDir(fileFilter)
 }
 
-func WrapWithHyphenI(value string) string {
-	return "\"-I" + value + "\""
-}
diff --git a/executils/process.go b/executils/process.go
@@ -137,6 +137,11 @@ func (p *Process) SetDir(dir string) {
 	p.cmd.Dir = dir
 }
 
+// GetDir gets the working directory of the command.
+func (p *Process) GetDir() string {
+	return p.cmd.Dir
+}
+
 // SetDirFromPath sets the working directory of the command. If path is nil, Run
 // runs the command in the calling process's current directory.
 func (p *Process) SetDirFromPath(path *paths.Path) {
@@ -187,3 +192,8 @@ func (p *Process) RunAndCaptureOutput(ctx context.Context) ([]byte, []byte, erro
 	err := p.RunWithinContext(ctx)
 	return stdout.Bytes(), stderr.Bytes(), err
 }
+
+// GetArgs returns the command arguments
+func (p *Process) GetArgs() []string {
+	return p.cmd.Args
+}
diff --git a/legacy/builder/builder_utils/utils.go b/legacy/builder/builder_utils/utils.go
@@ -18,14 +18,14 @@ package builder_utils
 import (
 	"fmt"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
 
 	bUtils "github.com/arduino/arduino-cli/arduino/builder/utils"
 	"github.com/arduino/arduino-cli/arduino/globals"
+	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/arduino-cli/i18n"
 	"github.com/arduino/arduino-cli/legacy/builder/constants"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -173,7 +173,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
 		return nil, errors.WithStack(err)
 	}
 
-	command, err := PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, err := PrepareCommandForRecipe(properties, recipe, false)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
@@ -244,7 +244,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile
 		properties.SetPath(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, archiveFilePath)
 		properties.SetPath(constants.BUILD_PROPERTIES_OBJECT_FILE, objectFile)
 
-		command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+		command, err := PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false)
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
@@ -260,7 +260,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath *paths.Path, archiveFile
 
 const COMMANDLINE_LIMIT = 30000
 
-func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool, toolEnv []string) (*exec.Cmd, error) {
+func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, removeUnsetProperties bool) (*executils.Process, error) {
 	pattern := buildProperties.Get(recipe)
 	if pattern == "" {
 		return nil, errors.Errorf(tr("%[1]s pattern is missing"), recipe)
@@ -275,24 +275,30 @@ func PrepareCommandForRecipe(buildProperties *properties.Map, recipe string, rem
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
-	command := exec.Command(parts[0], parts[1:]...)
-	command.Env = append(os.Environ(), toolEnv...)
 
 	// if the overall commandline is too long for the platform
 	// try reducing the length by making the filenames relative
 	// and changing working directory to build.path
+	var relativePath string
 	if len(commandLine) > COMMANDLINE_LIMIT {
-		relativePath := buildProperties.Get("build.path")
-		for i, arg := range command.Args {
+		relativePath = buildProperties.Get("build.path")
+		for i, arg := range parts {
 			if _, err := os.Stat(arg); os.IsNotExist(err) {
 				continue
 			}
 			rel, err := filepath.Rel(relativePath, arg)
 			if err == nil && !strings.Contains(rel, "..") && len(rel) < len(arg) {
-				command.Args[i] = rel
+				parts[i] = rel
 			}
 		}
-		command.Dir = relativePath
+	}
+
+	command, err := executils.NewProcess(nil, parts...)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+	if relativePath != "" {
+		command.SetDir(relativePath)
 	}
 
 	return command, nil

diff --git a/legacy/builder/create_cmake_rule.go b/legacy/builder/create_cmake_rule.go
@@ -367,9 +367,9 @@ func extractCompileFlags(ctx *types.Context, recipe string, defines, dynamicLibs
 		return target
 	}
 
-	command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, recipe, true)
 
-	for _, arg := range command.Args {
+	for _, arg := range command.GetArgs() {
 		if strings.HasPrefix(arg, "-D") {
 			*defines = appendIfNotPresent(*defines, arg)
 			continue

diff --git a/legacy/builder/phases/core_builder.go b/legacy/builder/phases/core_builder.go
@@ -22,13 +22,13 @@ import (
 	"os"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino/builder/cpp"
 	"github.com/arduino/arduino-cli/buildcache"
 	"github.com/arduino/arduino-cli/i18n"
 	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/constants"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
-	"github.com/arduino/arduino-cli/arduino/builder/utils"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 	"github.com/pkg/errors"
@@ -80,7 +80,7 @@ func compileCore(ctx *types.Context, buildPath *paths.Path, buildCachePath *path
 	if variantFolder != nil && variantFolder.IsDir() {
 		includes = append(includes, variantFolder.String())
 	}
-	includes = f.Map(includes, utils.WrapWithHyphenI)
+	includes = f.Map(includes, cpp.WrapWithHyphenI)
 
 	var err error
 

diff --git a/legacy/builder/phases/libraries_builder.go b/legacy/builder/phases/libraries_builder.go
@@ -18,7 +18,7 @@ package phases
 import (
 	"strings"
 
-	"github.com/arduino/arduino-cli/arduino/builder/utils"
+	"github.com/arduino/arduino-cli/arduino/builder/cpp"
 	"github.com/arduino/arduino-cli/arduino/libraries"
 	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
@@ -38,7 +38,7 @@ func (s *LibrariesBuilder) Run(ctx *types.Context) error {
 	librariesBuildPath := ctx.LibrariesBuildPath
 	buildProperties := ctx.BuildProperties
 	includesFolders := ctx.SketchLibrariesDetector.IncludeFolders()
-	includes := f.Map(includesFolders.AsStrings(), utils.WrapWithHyphenI)
+	includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
 	libs := ctx.SketchLibrariesDetector.ImportedLibraries()
 
 	if err := librariesBuildPath.MkdirAll(); err != nil {
@@ -67,9 +67,9 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
 	// Add fpu specifications if they exist
 	// To do so, resolve recipe.cpp.o.pattern,
 	// search for -mfpu=xxx -mfloat-abi=yyy and add to a subfolder
-	command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, "recipe.cpp.o.pattern", true, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, _ := builder_utils.PrepareCommandForRecipe(ctx.BuildProperties, "recipe.cpp.o.pattern", true)
 	fpuSpecs := ""
-	for _, el := range strings.Split(command.String(), " ") {
+	for _, el := range command.GetArgs() {
 		if strings.Contains(el, FPU_CFLAG) {
 			toAdd := strings.Split(el, "=")
 			if len(toAdd) > 1 {
@@ -78,7 +78,7 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
 			}
 		}
 	}
-	for _, el := range strings.Split(command.String(), " ") {
+	for _, el := range command.GetArgs() {
 		if strings.Contains(el, FLOAT_ABI_CFLAG) {
 			toAdd := strings.Split(el, "=")
 			if len(toAdd) > 1 {
@@ -201,7 +201,7 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
 		}
 	} else {
 		if library.UtilityDir != nil {
-			includes = append(includes, utils.WrapWithHyphenI(library.UtilityDir.String()))
+			includes = append(includes, cpp.WrapWithHyphenI(library.UtilityDir.String()))
 		}
 		libObjectFiles, err := builder_utils.CompileFiles(ctx, library.SourceDir, libraryBuildPath, buildProperties, includes)
 		if err != nil {

diff --git a/legacy/builder/phases/linker.go b/legacy/builder/phases/linker.go
@@ -93,7 +93,7 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
 			properties.SetPath("archive_file_path", archive)
 			properties.SetPath("object_file", object)
 
-			command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+			command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_AR_PATTERN, false)
 			if err != nil {
 				return errors.WithStack(err)
 			}
@@ -114,10 +114,10 @@ func link(ctx *types.Context, objectFiles paths.PathList, coreDotARelPath *paths
 	properties.Set(constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH, coreArchiveFilePath.String())
 	properties.Set("object_files", objectFileList)
 
-	command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, err := builder_utils.PrepareCommandForRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false)
 	if err != nil {
 		return err
-	}
+}
 
 	_, _, err = utils.ExecCommand(ctx, command, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
 	return err

diff --git a/legacy/builder/phases/sizer.go b/legacy/builder/phases/sizer.go
@@ -50,7 +50,7 @@ func (s *Sizer) Run(ctx *types.Context) error {
 }
 
 func checkSizeAdvanced(ctx *types.Context, properties *properties.Map) error {
-	command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.advanced_size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.advanced_size.pattern", false)
 	if err != nil {
 		return errors.New(tr("Error while determining sketch size: %s", err))
 	}
@@ -179,7 +179,7 @@ func checkSize(ctx *types.Context, buildProperties *properties.Map) error {
 }
 
 func execSizeRecipe(ctx *types.Context, properties *properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) {
-	command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.size.pattern", false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+	command, err := builder_utils.PrepareCommandForRecipe(properties, "recipe.size.pattern", false)
 	if err != nil {
 		resErr = fmt.Errorf(tr("Error while determining sketch size: %s"), err)
 		return

diff --git a/legacy/builder/phases/sketch_builder.go b/legacy/builder/phases/sketch_builder.go
@@ -16,7 +16,7 @@
 package phases
 
 import (
-	"github.com/arduino/arduino-cli/arduino/builder/utils"
+	"github.com/arduino/arduino-cli/arduino/builder/cpp"
 	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
@@ -29,7 +29,7 @@ func (s *SketchBuilder) Run(ctx *types.Context) error {
 	sketchBuildPath := ctx.SketchBuildPath
 	buildProperties := ctx.BuildProperties
 	includesFolders := ctx.SketchLibrariesDetector.IncludeFolders()
-	includes := f.Map(includesFolders.AsStrings(), utils.WrapWithHyphenI)
+	includes := f.Map(includesFolders.AsStrings(), cpp.WrapWithHyphenI)
 
 	if err := sketchBuildPath.MkdirAll(); err != nil {
 		return errors.WithStack(err)

diff --git a/legacy/builder/recipe_runner.go b/legacy/builder/recipe_runner.go
@@ -44,14 +44,14 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error {
 	for _, recipe := range recipes {
 		logrus.Debugf(fmt.Sprintf("Running recipe: %s", recipe))
 
-		command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false, ctx.PackageManager.GetEnvVarsForSpawnedProcess())
+		command, err := builder_utils.PrepareCommandForRecipe(properties, recipe, false)
 		if err != nil {
 			return errors.WithStack(err)
 		}
 
 		if ctx.OnlyUpdateCompilationDatabase && s.SkipIfOnlyUpdatingCompilationDatabase {
 			if ctx.Verbose {
-				ctx.Info(tr("Skipping: %[1]s", strings.Join(command.Args, " ")))
+				ctx.Info(tr("Skipping: %[1]s", strings.Join(command.GetArgs(), " ")))
 			}
 			return nil
 		}