Use runner.Task in GCC preprocessor

It slightly simplifies code, but also provide the basis for the next
commits.

Author: cmaglie

internal/arduino/builder/internal/detector/detector.go

@@ -312,6 +312,19 @@ func (l *SketchLibrariesDetector) findIncludes(
 	return nil
 }
 
+func (l *SketchLibrariesDetector) gccPreprocessTask(sourceFile *sourceFile, buildProperties *properties.Map) *runner.Task {
+	// Libraries may require the "utility" directory to be added to the include
+	// search path, but only for the source code of the library, so we temporary
+	// copy the current search path list and add the library' utility directory
+	// if needed.
+	includeFolders := l.includeFolders
+	if extraInclude := sourceFile.ExtraIncludePath; extraInclude != nil {
+		includeFolders = append(includeFolders, extraInclude)
+	}
+
+	return preprocessor.GCC(sourceFile.SourcePath, paths.NullPath(), includeFolders, buildProperties)
+}
+
 func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit(
 	ctx context.Context,
 	sourceFileQueue *uniqueSourceFileQueue,
@@ -343,15 +356,7 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit(
 	for {
 		l.cache.Expect(&detectorCacheEntry{Compile: sourceFile})
 
-		// Libraries may require the "utility" directory to be added to the include
-		// search path, but only for the source code of the library, so we temporary
-		// copy the current search path list and add the library' utility directory
-		// if needed.
-		includeFolders := l.includeFolders
-		if extraInclude := sourceFile.ExtraIncludePath; extraInclude != nil {
-			includeFolders = append(includeFolders, extraInclude)
-		}
-
+		preprocTask := l.gccPreprocessTask(sourceFile, buildProperties)
 		var preprocErr error
 		var preprocResult *runner.Result
 
@@ -363,7 +368,8 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit(
 			}
 			first = false
 		} else {
-			preprocResult, preprocErr = preprocessor.GCC(ctx, sourcePath, paths.NullPath(), includeFolders, buildProperties)
+			preprocResult = preprocTask.Run(ctx)
+			preprocErr = preprocResult.Error
 			if l.logger.VerbosityLevel() == logger.VerbosityVerbose {
 				l.logger.WriteStdout(preprocResult.Stdout)
 			}
@@ -396,7 +402,8 @@ func (l *SketchLibrariesDetector) findMissingIncludesInCompilationUnit(
 
 			// If preprocess result came from cache, run the preprocessor to obtain the actual error to show
 			if preprocErr == nil || len(preprocResult.Stderr) == 0 {
-				preprocResult, preprocErr = preprocessor.GCC(ctx, sourcePath, paths.NullPath(), includeFolders, buildProperties)
+				preprocResult = preprocTask.Run(ctx)
+				preprocErr = preprocResult.Error
 				if l.logger.VerbosityLevel() == logger.VerbosityVerbose {
 					l.logger.WriteStdout(preprocResult.Stdout)
 				}

internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go

@@ -45,11 +45,11 @@ func PreprocessSketchWithArduinoPreprocessor(
 
 	sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
 	targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
-	gccResult, err := GCC(ctx, sourceFile, targetFile, includeFolders, buildProperties)
+	gccResult := GCC(sourceFile, targetFile, includeFolders, buildProperties).Run(ctx)
 	verboseOut.Write(gccResult.Stdout)
 	verboseOut.Write(gccResult.Stderr)
-	if err != nil {
-		return nil, err
+	if gccResult.Error != nil {
+		return nil, gccResult.Error
 	}
 
 	arduinoPreprocessorProperties := properties.NewMap()

internal/arduino/builder/internal/preprocessor/ctags.go

@@ -57,10 +57,10 @@ func PreprocessSketchWithCtags(
 
 	// Run GCC preprocessor
 	sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp")
-	result, err := GCC(ctx, sourceFile, ctagsTarget, includes, buildProperties)
+	result := GCC(sourceFile, ctagsTarget, includes, buildProperties).Run(ctx)
 	stdout.Write(result.Stdout)
 	stderr.Write(result.Stderr)
-	if err != nil {
+	if err := result.Error; err != nil {
 		if !onlyUpdateCompilationDatabase {
 			return &runner.Result{Args: result.Args, Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, err
 		}

internal/arduino/builder/internal/preprocessor/gcc.go

@@ -16,15 +16,10 @@
 package preprocessor
 
 import (
-	"bytes"
-	"context"
-	"errors"
-	"fmt"
 	"strings"
 
 	"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/runner"
-	"github.com/arduino/arduino-cli/internal/i18n"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 	"go.bug.st/f"
@@ -33,10 +28,9 @@ import (
 // GCC performs a run of the gcc preprocess (macro/includes expansion). The function outputs the result
 // to targetFilePath. Returns the stdout/stderr of gcc if any.
 func GCC(
-	ctx context.Context,
 	sourceFilePath, targetFilePath *paths.Path,
 	includes paths.PathList, buildProperties *properties.Map,
-) (*runner.Result, error) {
+) *runner.Task {
 	gccBuildProperties := properties.NewMap()
 	gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC")
 	gccBuildProperties.Merge(buildProperties)
@@ -60,10 +54,6 @@ func GCC(
 	}
 
 	pattern := gccBuildProperties.Get(gccPreprocRecipeProperty)
-	if pattern == "" {
-		return nil, errors.New(i18n.Tr("%s pattern is missing", gccPreprocRecipeProperty))
-	}
-
 	commandLine := gccBuildProperties.ExpandPropsInString(pattern)
 	commandLine = properties.DeleteUnexpandedPropsFromString(commandLine)
 	args, _ := properties.SplitQuotedString(commandLine, `"'`, false)
@@ -72,46 +62,7 @@ func GCC(
 	// to create a /dev/null.d dependency file, which won't work.
 	args = f.Filter(args, f.NotEquals("-MMD"))
 
-	proc, err := paths.NewProcess(nil, args...)
-	if err != nil {
-		return nil, err
-	}
-
-	stdout := bytes.NewBuffer(nil)
-	stderr := bytes.NewBuffer(nil)
-
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-	count := 0
-	stderrLimited := writerFunc(func(p []byte) (int, error) {
-		// Limit the size of the stderr buffer to 100KB
-		n, err := stderr.Write(p)
-		count += n
-		if count > 100*1024 {
-			fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated."))
-			cancel()
-		}
-		return n, err
-	})
-
-	proc.RedirectStdoutTo(stdout)
-	proc.RedirectStderrTo(stderrLimited)
-
-	// Append gcc arguments to stdout before running the command
-	fmt.Fprintln(stdout, strings.Join(args, " "))
-
-	if err := proc.Start(); err != nil {
-		return &runner.Result{}, err
-	}
-
-	// Wait for the process to finish
-	err = proc.WaitWithinContext(ctx)
-
-	return &runner.Result{Args: proc.GetArgs(), Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, err
-}
-
-type writerFunc func(p []byte) (n int, err error)
-
-func (f writerFunc) Write(p []byte) (n int, err error) {
-	return f(p)
+	// Limit the stderr output to 100 KiB
+	// https://github.com/arduino/arduino-cli/pull/2883
+	return runner.NewTaskWithLimitedStderr(100*1024, args...)
 }

internal/arduino/builder/internal/runner/task.go

@@ -16,23 +16,31 @@
 package runner
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"strings"
 
+	"github.com/arduino/arduino-cli/internal/i18n"
 	"github.com/arduino/go-paths-helper"
 )
 
 // Task is a command to be executed
 type Task struct {
-	Args []string `json:"args"`
+	Args        []string `json:"args"`
+	LimitStderr int      `json:"-"`
 }
 
 // NewTask creates a new Task
 func NewTask(args ...string) *Task {
 	return &Task{Args: args}
 }
 
+// NewTaskWithLimitedStderr creates a new Task with a hard-limit on the stderr output
+func NewTaskWithLimitedStderr(limit int, args ...string) *Task {
+	return &Task{Args: args, LimitStderr: limit}
+}
+
 func (t *Task) String() string {
 	return strings.Join(t.Args, " ")
 }
@@ -51,10 +59,45 @@ func (t *Task) Run(ctx context.Context) *Result {
 	if err != nil {
 		return &Result{Args: t.Args, Error: err}
 	}
-	stdout, stderr, err := proc.RunAndCaptureOutput(ctx)
+
+	stdout := bytes.NewBuffer(nil)
+	stderr := bytes.NewBuffer(nil)
+	proc.RedirectStdoutTo(stdout)
+
+	if t.LimitStderr > 0 {
+		innerCtx, cancel := context.WithCancel(ctx)
+		defer cancel()
+
+		count := 0
+		stderrLimited := writerFunc(func(p []byte) (int, error) {
+			n, err := stderr.Write(p)
+			count += n
+			if count > t.LimitStderr {
+				fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated."))
+				cancel()
+			}
+			return n, err
+		})
+
+		ctx = innerCtx
+		proc.RedirectStderrTo(stderrLimited)
+	} else {
+		proc.RedirectStderrTo(stderr)
+	}
 
 	// Append arguments to stdout
-	stdout = append([]byte(fmt.Sprintln(t)), stdout...)
+	fmt.Fprintln(stdout, t.String())
+
+	// Execute command and wait for the process to finish
+	if err := proc.Start(); err != nil {
+		return &Result{Error: err}
+	}
+	err = proc.WaitWithinContext(ctx)
+	return &Result{Args: proc.GetArgs(), Stdout: stdout.Bytes(), Stderr: stderr.Bytes(), Error: err}
+}
+
+type writerFunc func(p []byte) (n int, err error)
 
-	return &Result{Args: proc.GetArgs(), Stdout: stdout, Stderr: stderr, Error: err}
+func (f writerFunc) Write(p []byte) (n int, err error) {
+	return f(p)
 }