Skip to content

Commit

Permalink
introduce diagnosticmanager instead of using a callback approach
Browse files Browse the repository at this point in the history
  • Loading branch information
alessio-perugini committed Jan 31, 2024
1 parent 7b4f713 commit 6584638
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 41 deletions.
43 changes: 12 additions & 31 deletions internal/arduino/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/arduino/arduino-cli/internal/arduino/builder/internal/compilation"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/detector"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/progress"
Expand All @@ -36,7 +37,6 @@ import (
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
"github.com/sirupsen/logrus"
)

// ErrSketchCannotBeLocatedInBuildPath fixdoc
Expand Down Expand Up @@ -94,11 +94,7 @@ type Builder struct {

toolEnv []string

// This is a function used to parse the output of the compiler
// It is used to extract errors and warnings
compilerOutputParser diagnostics.CompilerOutputParserCB
// and here are the diagnostics parsed from the compiler
compilerDiagnostics diagnostics.Diagnostics
diagnosticsManager *diagnosticmanager.Manager
}

// buildArtifacts contains the result of various build
Expand Down Expand Up @@ -199,6 +195,7 @@ func NewBuilder(
logger.Warn(string(verboseOut))
}

diagnosticmanager := diagnosticmanager.New()
b := &Builder{
sketch: sk,
buildProperties: buildProperties,
Expand All @@ -220,12 +217,6 @@ func NewBuilder(
targetPlatform: targetPlatform,
actualPlatform: actualPlatform,
toolEnv: toolEnv,
libsDetector: detector.NewSketchLibrariesDetector(
libsManager, libsResolver,
useCachedLibrariesResolution,
onlyUpdateCompilationDatabase,
logger,
),
buildOptions: newBuildOptions(
hardwareDirs, otherLibrariesDirs,
builtInLibrariesDirs, buildPath,
Expand All @@ -237,25 +228,15 @@ func NewBuilder(
buildProperties.GetPath("runtime.platform.path"),
buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
),
diagnosticsManager: diagnosticmanager,
libsDetector: detector.NewSketchLibrariesDetector(
libsManager, libsResolver,
useCachedLibrariesResolution,
onlyUpdateCompilationDatabase,
logger,
diagnosticmanager,
),
}

b.compilerOutputParser = func(cmdline []string, out []byte) {
compiler := diagnostics.DetectCompilerFromCommandLine(
cmdline,
false, // at the moment compiler-probing is not required
)
if compiler == nil {
logrus.Warnf("Could not detect compiler from: %s", cmdline)
return
}
diags, err := diagnostics.ParseCompilerOutput(compiler, out)
if err != nil {
logrus.Warnf("Error parsing compiler output: %s", err)
return
}
b.compilerDiagnostics = append(b.compilerDiagnostics, diags...)
}

return b, nil
}

Expand All @@ -281,7 +262,7 @@ func (b *Builder) ImportedLibraries() libraries.List {

// CompilerDiagnostics returns the parsed compiler diagnostics
func (b *Builder) CompilerDiagnostics() diagnostics.Diagnostics {
return b.compilerDiagnostics
return b.diagnosticsManager.CompilerDiagnostics()
}

// Preprocess fixdoc
Expand Down
6 changes: 3 additions & 3 deletions internal/arduino/builder/compilation.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ func (b *Builder) compileFileWithRecipe(
b.logger.WriteStderr(commandStderr.Bytes())

// Parse the output of the compiler to gather errors and warnings...
if b.compilerOutputParser != nil {
b.compilerOutputParser(command.GetArgs(), commandStdout.Bytes())
b.compilerOutputParser(command.GetArgs(), commandStderr.Bytes())
if b.diagnosticsManager != nil {
b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStdout.Bytes())
b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStderr.Bytes())
}

// ...and then return the error
Expand Down
8 changes: 6 additions & 2 deletions internal/arduino/builder/internal/detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strings"
"time"

"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
Expand Down Expand Up @@ -57,6 +58,7 @@ type SketchLibrariesDetector struct {
librariesResolutionResults map[string]libraryResolutionResult
includeFolders paths.PathList
logger *logger.BuilderLogger
diagnosticManager *diagnosticmanager.Manager
}

// NewSketchLibrariesDetector todo
Expand All @@ -66,6 +68,7 @@ func NewSketchLibrariesDetector(
useCachedLibrariesResolution bool,
onlyUpdateCompilationDatabase bool,
logger *logger.BuilderLogger,
diagnosticManager *diagnosticmanager.Manager,
) *SketchLibrariesDetector {
return &SketchLibrariesDetector{
librariesManager: lm,
Expand All @@ -76,6 +79,7 @@ func NewSketchLibrariesDetector(
includeFolders: paths.PathList{},
onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
logger: logger,
diagnosticManager: diagnosticManager,
}
}

Expand Down Expand Up @@ -347,7 +351,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
}
} else {
var preprocStdout []byte
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
if l.logger.Verbose() {
l.logger.WriteStdout(preprocStdout)
}
Expand Down Expand Up @@ -379,7 +383,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
if preprocErr == nil || preprocStderr == nil {
// Filename came from cache, so run preprocessor to obtain error to show
var preprocStdout []byte
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
if l.logger.Verbose() {
l.logger.WriteStdout(preprocStdout)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package diagnosticmanager

import (
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
"github.com/sirupsen/logrus"
)

type Manager struct {
results diagnostics.Diagnostics
lastInsertIndex int
}

func New() *Manager {
return &Manager{lastInsertIndex: -1}
}

func (m *Manager) Parse(cmdline []string, out []byte) {
compiler := diagnostics.DetectCompilerFromCommandLine(
cmdline,
false, // at the moment compiler-probing is not required
)
if compiler == nil {
logrus.Warnf("Could not detect compiler from: %s", cmdline)
return
}
diags, err := diagnostics.ParseCompilerOutput(compiler, out)
if err != nil {
logrus.Warnf("Error parsing compiler output: %s", err)
return
}
m.lastInsertIndex += len(diags)
m.results = append(m.results, diags...)
}

func (m *Manager) CompilerDiagnostics() diagnostics.Diagnostics {
return m.results
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"path/filepath"
"runtime"

"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
"github.com/arduino/arduino-cli/internal/arduino/sketch"
"github.com/arduino/go-paths-helper"
Expand All @@ -30,7 +31,11 @@ import (

// PreprocessSketchWithArduinoPreprocessor performs preprocessing of the arduino sketch
// using arduino-preprocessor (https://github.com/arduino/arduino-preprocessor).
func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList, lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool) ([]byte, []byte, error) {
func PreprocessSketchWithArduinoPreprocessor(
sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList,
lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
diagnosticmanager *diagnosticmanager.Manager,
) ([]byte, []byte, error) {
verboseOut := &bytes.Buffer{}
normalOut := &bytes.Buffer{}
if err := buildPath.Join("preproc").MkdirAll(); err != nil {
Expand All @@ -39,7 +44,7 @@ func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths

sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties)
gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties, diagnosticmanager)
verboseOut.Write(gccStdout)
verboseOut.Write(gccStderr)
if err != nil {
Expand Down
9 changes: 7 additions & 2 deletions internal/arduino/builder/internal/preprocessor/ctags.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strings"

"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor/internal/ctags"
"github.com/arduino/arduino-cli/internal/arduino/sketch"
"github.com/arduino/arduino-cli/internal/i18n"
Expand All @@ -40,7 +41,11 @@ var tr = i18n.Tr
var DebugPreprocessor bool

// PreprocessSketchWithCtags performs preprocessing of the arduino sketch using CTags.
func PreprocessSketchWithCtags(sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList, lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool) ([]byte, []byte, error) {
func PreprocessSketchWithCtags(
sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList,
lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
diagnosticManager *diagnosticmanager.Manager,
) ([]byte, []byte, error) {
// Create a temporary working directory
tmpDir, err := paths.MkTempDir("", "")
if err != nil {
Expand All @@ -54,7 +59,7 @@ func PreprocessSketchWithCtags(sketch *sketch.Sketch, buildPath *paths.Path, inc

// Run GCC preprocessor
sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp")
gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties)
gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties, diagnosticManager)
verboseOutput.Write(gccStdout)
verboseOutput.Write(gccStderr)
normalOutput.Write(gccStderr)
Expand Down
11 changes: 10 additions & 1 deletion internal/arduino/builder/internal/preprocessor/gcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@ import (

f "github.com/arduino/arduino-cli/internal/algorithms"
"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
)

// 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(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.PathList, buildProperties *properties.Map) ([]byte, []byte, error) {
func GCC(
sourceFilePath, targetFilePath *paths.Path,
includes paths.PathList, buildProperties *properties.Map,
diagnosticManager *diagnosticmanager.Manager,
) ([]byte, []byte, error) {
gccBuildProperties := properties.NewMap()
gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC")
gccBuildProperties.Merge(buildProperties)
Expand Down Expand Up @@ -74,6 +79,10 @@ func GCC(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.
}
stdout, stderr, err := proc.RunAndCaptureOutput(context.Background())

if diagnosticManager != nil {
diagnosticManager.ParseOutput(proc.GetArgs(), stderr)
}

// Append gcc arguments to stdout
stdout = append([]byte(fmt.Sprintln(strings.Join(args, " "))), stdout...)

Expand Down
1 change: 1 addition & 0 deletions internal/arduino/builder/preprocess_sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (b *Builder) preprocessSketch(includes paths.PathList) error {
normalOutput, verboseOutput, err := preprocessor.PreprocessSketchWithCtags(
b.sketch, b.buildPath, includes, b.lineOffset,
b.buildProperties, b.onlyUpdateCompilationDatabase,
b.diagnosticsManager,
)
if b.logger.Verbose() {
b.logger.WriteStdout(verboseOutput)
Expand Down

0 comments on commit 6584638

Please sign in to comment.