From 1214f57c5259be199630058c82ed34f4142df118 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Jan 2025 12:28:11 +0100 Subject: [PATCH 01/10] Made builder logger verbosity an explicit type --- commands/service_compile.go | 7 +++++- .../arduino/builder/archive_compiled_files.go | 5 ++-- internal/arduino/builder/builder.go | 18 +++++++-------- internal/arduino/builder/compilation.go | 7 +++--- internal/arduino/builder/core.go | 7 +++--- .../builder/internal/detector/detector.go | 18 +++++++-------- internal/arduino/builder/libraries.go | 5 ++-- internal/arduino/builder/linker.go | 3 ++- .../builder/{internal => }/logger/logger.go | 23 +++++++++++++------ internal/arduino/builder/preprocess_sketch.go | 6 +++-- internal/arduino/builder/recipe.go | 3 ++- internal/arduino/builder/sizer.go | 5 ++-- internal/arduino/builder/sketch.go | 5 ++-- 13 files changed, 68 insertions(+), 44 deletions(-) rename internal/arduino/builder/{internal => }/logger/logger.go (78%) diff --git a/commands/service_compile.go b/commands/service_compile.go index a71195382cf..2ca418df7cf 100644 --- a/commands/service_compile.go +++ b/commands/service_compile.go @@ -28,6 +28,7 @@ import ( "github.com/arduino/arduino-cli/commands/cmderrors" "github.com/arduino/arduino-cli/commands/internal/instances" "github.com/arduino/arduino-cli/internal/arduino/builder" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager" "github.com/arduino/arduino-cli/internal/arduino/sketch" "github.com/arduino/arduino-cli/internal/arduino/utils" @@ -244,6 +245,10 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu Message: &rpc.CompileResponse_Progress{Progress: p}, }) } + var verbosity logger.Verbosity = logger.VerbosityNormal + if req.GetVerbose() { + verbosity = logger.VerbosityVerbose + } sketchBuilder, err := builder.NewBuilder( ctx, sk, @@ -265,7 +270,7 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu req.GetSkipLibrariesDiscovery(), libsManager, paths.NewPathList(req.GetLibrary()...), - outStream, errStream, req.GetVerbose(), req.GetWarnings(), + outStream, errStream, verbosity, req.GetWarnings(), progressCB, pme.GetEnvVarsForSpawnedProcess(), ) diff --git a/internal/arduino/builder/archive_compiled_files.go b/internal/arduino/builder/archive_compiled_files.go index ed87225e2ff..32208a59092 100644 --- a/internal/arduino/builder/archive_compiled_files.go +++ b/internal/arduino/builder/archive_compiled_files.go @@ -16,6 +16,7 @@ package builder import ( + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" ) @@ -23,7 +24,7 @@ import ( // ArchiveCompiledFiles fixdoc func (b *Builder) archiveCompiledFiles(archiveFilePath *paths.Path, objectFilesToArchive paths.PathList) (*paths.Path, error) { if b.onlyUpdateCompilationDatabase { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Skipping archive creation of: %[1]s", archiveFilePath)) } return archiveFilePath, nil @@ -46,7 +47,7 @@ func (b *Builder) archiveCompiledFiles(archiveFilePath *paths.Path, objectFilesT return nil, err } } else { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Using previously compiled file: %[1]s", archiveFilePath)) } return archiveFilePath, nil diff --git a/internal/arduino/builder/builder.go b/internal/arduino/builder/builder.go index 58d607c827a..0c711273a5d 100644 --- a/internal/arduino/builder/builder.go +++ b/internal/arduino/builder/builder.go @@ -27,9 +27,9 @@ 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/diagnostics" - "github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/progress" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/arduino/cores" "github.com/arduino/arduino-cli/internal/arduino/libraries" "github.com/arduino/arduino-cli/internal/arduino/libraries/librariesmanager" @@ -136,7 +136,7 @@ func NewBuilder( useCachedLibrariesResolution bool, librariesManager *librariesmanager.LibrariesManager, libraryDirs paths.PathList, - stdout, stderr io.Writer, verbose bool, warningsLevel string, + stdout, stderr io.Writer, verbosity logger.Verbosity, warningsLevel string, progresCB rpc.TaskProgressCB, toolEnv []string, ) (*Builder, error) { @@ -189,7 +189,7 @@ func NewBuilder( return nil, ErrSketchCannotBeLocatedInBuildPath } - logger := logger.New(stdout, stderr, verbose, warningsLevel) + log := logger.New(stdout, stderr, verbosity, warningsLevel) libsManager, libsResolver, verboseOut, err := detector.LibrariesLoader( useCachedLibrariesResolution, librariesManager, builtInLibrariesDirs, libraryDirs, otherLibrariesDirs, @@ -198,8 +198,8 @@ func NewBuilder( if err != nil { return nil, err } - if logger.Verbose() { - logger.Warn(string(verboseOut)) + if log.VerbosityLevel() == logger.VerbosityVerbose { + log.Warn(string(verboseOut)) } diagnosticStore := diagnostics.NewStore() @@ -215,7 +215,7 @@ func NewBuilder( customBuildProperties: customBuildPropertiesArgs, coreBuildCachePath: coreBuildCachePath, extraCoreBuildCachePaths: extraCoreBuildCachePaths, - logger: logger, + logger: log, clean: clean, sourceOverrides: sourceOverrides, onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase, @@ -242,7 +242,7 @@ func NewBuilder( libsManager, libsResolver, useCachedLibrariesResolution, onlyUpdateCompilationDatabase, - logger, + log, diagnosticStore, ), } @@ -341,7 +341,7 @@ func (b *Builder) preprocess() error { } func (b *Builder) logIfVerbose(warn bool, msg string) { - if !b.logger.Verbose() { + if b.logger.VerbosityLevel() != logger.VerbosityVerbose { return } if warn { @@ -526,7 +526,7 @@ func (b *Builder) prepareCommandForRecipe(buildProperties *properties.Map, recip } func (b *Builder) execCommand(command *paths.Process) error { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(utils.PrintableCommand(command.GetArgs())) command.RedirectStdoutTo(b.logger.Stdout()) } diff --git a/internal/arduino/builder/compilation.go b/internal/arduino/builder/compilation.go index d3a1459da25..c739a2e37de 100644 --- a/internal/arduino/builder/compilation.go +++ b/internal/arduino/builder/compilation.go @@ -23,6 +23,7 @@ import ( "sync" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/arduino/globals" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" @@ -152,7 +153,7 @@ func (b *Builder) compileFileWithRecipe( command.RedirectStdoutTo(commandStdout) command.RedirectStderrTo(commandStderr) - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(utils.PrintableCommand(command.GetArgs())) } // Since this compile could be multithreaded, we first capture the command output @@ -161,7 +162,7 @@ func (b *Builder) compileFileWithRecipe( } err := command.Wait() // and transfer all at once at the end... - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.WriteStdout(commandStdout.Bytes()) } b.logger.WriteStderr(commandStderr.Bytes()) @@ -176,7 +177,7 @@ func (b *Builder) compileFileWithRecipe( if err != nil { return nil, err } - } else if b.logger.Verbose() { + } else if b.logger.VerbosityLevel() == logger.VerbosityVerbose { if objIsUpToDate { b.logger.Info(i18n.Tr("Using previously compiled file: %[1]s", objectFile)) } else { diff --git a/internal/arduino/builder/core.go b/internal/arduino/builder/core.go index f541eaeda41..c6d87ec7d66 100644 --- a/internal/arduino/builder/core.go +++ b/internal/arduino/builder/core.go @@ -24,6 +24,7 @@ import ( "github.com/arduino/arduino-cli/internal/arduino/builder/cpp" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/buildcache" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" @@ -116,7 +117,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) { return nil, nil, errors.New(i18n.Tr("creating core cache folder: %s", err)) } // use archived core - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Using precompiled core: %[1]s", targetArchivedCore)) } return targetArchivedCore, variantObjectFiles, nil @@ -128,7 +129,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) { extraTargetArchivedCore := extraCoreBuildCachePath.Join(archivedCoreName, "core.a") if canUseArchivedCore(extraTargetArchivedCore) { // use archived core - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Using precompiled core: %[1]s", extraTargetArchivedCore)) } return extraTargetArchivedCore, variantObjectFiles, nil @@ -158,7 +159,7 @@ func (b *Builder) compileCore() (*paths.Path, paths.PathList, error) { // archive core.a if targetArchivedCore != nil && !b.onlyUpdateCompilationDatabase { err := archiveFile.CopyTo(targetArchivedCore) - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { if err == nil { b.logger.Info(i18n.Tr("Archiving built core (caching) in: %[1]s", targetArchivedCore)) } else if os.IsNotExist(err) { diff --git a/internal/arduino/builder/internal/detector/detector.go b/internal/arduino/builder/internal/detector/detector.go index eb3d887d3fe..a983370b075 100644 --- a/internal/arduino/builder/internal/detector/detector.go +++ b/internal/arduino/builder/internal/detector/detector.go @@ -28,9 +28,9 @@ import ( "time" "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/preprocessor" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/arduino/cores" "github.com/arduino/arduino-cli/internal/arduino/globals" "github.com/arduino/arduino-cli/internal/arduino/libraries" @@ -87,7 +87,7 @@ func (l *SketchLibrariesDetector) resolveLibrary(header, platformArch string) *l importedLibraries := l.importedLibraries candidates := l.librariesResolver.AlternativesFor(header) - if l.logger.Verbose() { + if l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info(i18n.Tr("Alternatives for %[1]s: %[2]s", header, candidates)) l.logger.Info(fmt.Sprintf("ResolveLibrary(%s)", header)) l.logger.Info(fmt.Sprintf(" -> %s: %s", i18n.Tr("candidates"), candidates)) @@ -144,7 +144,7 @@ func (l *SketchLibrariesDetector) PrintUsedAndNotUsedLibraries(sketchError bool) // - as warning, when the sketch didn't compile // - as info, when verbose is on // - otherwise, output nothing - if !sketchError && !l.logger.Verbose() { + if !sketchError && l.logger.VerbosityLevel() != logger.VerbosityVerbose { return } @@ -239,7 +239,7 @@ func (l *SketchLibrariesDetector) findIncludes( if err := json.Unmarshal(d, &l.includeFolders); err != nil { return err } - if l.logger.Verbose() { + if l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info("Using cached library discovery: " + librariesResolutionCache.String()) } return nil @@ -347,12 +347,12 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone( var missingIncludeH string if unchanged && cache.valid { missingIncludeH = cache.Next().Include - if first && l.logger.Verbose() { + if first && l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info(i18n.Tr("Using cached library dependencies for file: %[1]s", sourcePath)) } } else { preprocFirstResult, preprocErr = preprocessor.GCC(ctx, sourcePath, targetFilePath, includeFolders, buildProperties) - if l.logger.Verbose() { + if l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.WriteStdout(preprocFirstResult.Stdout()) } // Unwrap error and see if it is an ExitError. @@ -365,7 +365,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone( return preprocErr } else { missingIncludeH = IncludesFinderWithRegExp(string(preprocFirstResult.Stderr())) - if missingIncludeH == "" && l.logger.Verbose() { + if missingIncludeH == "" && l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info(i18n.Tr("Error while detecting libraries included by %[1]s", sourcePath)) } } @@ -383,7 +383,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone( if preprocErr == nil || preprocFirstResult.Stderr() == nil { // Filename came from cache, so run preprocessor to obtain error to show result, err := preprocessor.GCC(ctx, sourcePath, targetFilePath, includeFolders, buildProperties) - if l.logger.Verbose() { + if l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.WriteStdout(result.Stdout()) } if err == nil { @@ -410,7 +410,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone( if library.Precompiled && library.PrecompiledWithSources { // Fully precompiled libraries should have no dependencies to avoid ABI breakage - if l.logger.Verbose() { + if l.logger.VerbosityLevel() == logger.VerbosityVerbose { l.logger.Info(i18n.Tr("Skipping dependencies detection for precompiled library %[1]s", library.Name)) } } else { diff --git a/internal/arduino/builder/libraries.go b/internal/arduino/builder/libraries.go index 6bb28f96ed2..d2558e4954f 100644 --- a/internal/arduino/builder/libraries.go +++ b/internal/arduino/builder/libraries.go @@ -21,6 +21,7 @@ import ( "time" "github.com/arduino/arduino-cli/internal/arduino/builder/cpp" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/arduino/libraries" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" @@ -129,7 +130,7 @@ func (b *Builder) compileLibraries(libraries libraries.List, includes []string) } func (b *Builder) compileLibrary(library *libraries.Library, includes []string) (paths.PathList, error) { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr(`Compiling library "%[1]s"`, library.Name)) } libraryBuildPath := b.librariesBuildPath.Join(library.DirName) @@ -292,7 +293,7 @@ func (b *Builder) warnAboutArchIncompatibleLibraries(importedLibraries libraries // TODO here we can completly remove this part as it's duplicated in what we can // read in the gRPC response func (b *Builder) printUsedLibraries(importedLibraries libraries.List) { - if !b.logger.Verbose() || len(importedLibraries) == 0 { + if b.logger.VerbosityLevel() != logger.VerbosityVerbose || len(importedLibraries) == 0 { return } diff --git a/internal/arduino/builder/linker.go b/internal/arduino/builder/linker.go index 20032608db5..77f450a9182 100644 --- a/internal/arduino/builder/linker.go +++ b/internal/arduino/builder/linker.go @@ -18,6 +18,7 @@ package builder import ( "strings" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" "go.bug.st/f" @@ -26,7 +27,7 @@ import ( // link fixdoc func (b *Builder) link() error { if b.onlyUpdateCompilationDatabase { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Skip linking of final executable.")) } return nil diff --git a/internal/arduino/builder/internal/logger/logger.go b/internal/arduino/builder/logger/logger.go similarity index 78% rename from internal/arduino/builder/internal/logger/logger.go rename to internal/arduino/builder/logger/logger.go index 992c15a25b2..d4a730d04e6 100644 --- a/internal/arduino/builder/internal/logger/logger.go +++ b/internal/arduino/builder/logger/logger.go @@ -22,18 +22,27 @@ import ( "sync" ) +// Verbosity is the verbosity level of the Logger +type Verbosity int + +const ( + VerbosityQuiet Verbosity = -1 + VerbosityNormal Verbosity = 0 + VerbosityVerbose Verbosity = 1 +) + // BuilderLogger fixdoc type BuilderLogger struct { stdLock sync.Mutex stdout io.Writer stderr io.Writer - verbose bool + verbosity Verbosity warningsLevel string } -// New fixdoc -func New(stdout, stderr io.Writer, verbose bool, warningsLevel string) *BuilderLogger { +// New creates a new Logger to the given output streams with the specified verbosity and warnings level +func New(stdout, stderr io.Writer, verbosity Verbosity, warningsLevel string) *BuilderLogger { if stdout == nil { stdout = os.Stdout } @@ -46,7 +55,7 @@ func New(stdout, stderr io.Writer, verbose bool, warningsLevel string) *BuilderL return &BuilderLogger{ stdout: stdout, stderr: stderr, - verbose: verbose, + verbosity: verbosity, warningsLevel: warningsLevel, } } @@ -79,9 +88,9 @@ func (l *BuilderLogger) WriteStderr(data []byte) (int, error) { return l.stderr.Write(data) } -// Verbose fixdoc -func (l *BuilderLogger) Verbose() bool { - return l.verbose +// VerbosityLevel returns the verbosity level of the logger +func (l *BuilderLogger) VerbosityLevel() Verbosity { + return l.verbosity } // WarningsLevel fixdoc diff --git a/internal/arduino/builder/preprocess_sketch.go b/internal/arduino/builder/preprocess_sketch.go index b7fe178db02..290b2963664 100644 --- a/internal/arduino/builder/preprocess_sketch.go +++ b/internal/arduino/builder/preprocess_sketch.go @@ -17,6 +17,7 @@ package builder import ( "github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/go-paths-helper" ) @@ -26,10 +27,11 @@ func (b *Builder) preprocessSketch(includes paths.PathList) error { result, err := preprocessor.PreprocessSketchWithCtags( b.ctx, b.sketch, b.buildPath, includes, b.lineOffset, - b.buildProperties, b.onlyUpdateCompilationDatabase, b.logger.Verbose(), + b.buildProperties, b.onlyUpdateCompilationDatabase, + b.logger.VerbosityLevel() == logger.VerbosityVerbose, ) if result != nil { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.WriteStdout(result.Stdout()) } b.logger.WriteStderr(result.Stderr()) diff --git a/internal/arduino/builder/recipe.go b/internal/arduino/builder/recipe.go index c8fa8962850..b4c456e360f 100644 --- a/internal/arduino/builder/recipe.go +++ b/internal/arduino/builder/recipe.go @@ -19,6 +19,7 @@ import ( "sort" "strings" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/i18n" properties "github.com/arduino/go-properties-orderedmap" "github.com/sirupsen/logrus" @@ -43,7 +44,7 @@ func (b *Builder) RunRecipe(prefix, suffix string, skipIfOnlyUpdatingCompilation } if b.onlyUpdateCompilationDatabase && skipIfOnlyUpdatingCompilationDatabase { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(i18n.Tr("Skipping: %[1]s", strings.Join(command.GetArgs(), " "))) } return nil diff --git a/internal/arduino/builder/sizer.go b/internal/arduino/builder/sizer.go index 84cf8012a32..a9ac3de2c8c 100644 --- a/internal/arduino/builder/sizer.go +++ b/internal/arduino/builder/sizer.go @@ -24,6 +24,7 @@ import ( "strconv" "github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/i18n" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/arduino/go-properties-orderedmap" @@ -78,7 +79,7 @@ func (b *Builder) checkSizeAdvanced() (ExecutablesFileSections, error) { if err != nil { return nil, errors.New(i18n.Tr("Error while determining sketch size: %s", err)) } - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(utils.PrintableCommand(command.GetArgs())) } out := &bytes.Buffer{} @@ -215,7 +216,7 @@ func (b *Builder) execSizeRecipe(properties *properties.Map) (textSize int, data resErr = errors.New(i18n.Tr("Error while determining sketch size: %s", err)) return } - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(utils.PrintableCommand(command.GetArgs())) } commandStdout := &bytes.Buffer{} diff --git a/internal/arduino/builder/sketch.go b/internal/arduino/builder/sketch.go index 1f64d207c81..76e77f4c8e8 100644 --- a/internal/arduino/builder/sketch.go +++ b/internal/arduino/builder/sketch.go @@ -26,6 +26,7 @@ import ( "fortio.org/safecast" "github.com/arduino/arduino-cli/internal/arduino/builder/cpp" + "github.com/arduino/arduino-cli/internal/arduino/builder/logger" "github.com/arduino/arduino-cli/internal/i18n" "github.com/arduino/go-paths-helper" "github.com/marcinbor85/gohex" @@ -240,7 +241,7 @@ func (b *Builder) mergeSketchWithBootloader() error { bootloaderPath := b.buildProperties.GetPath("runtime.platform.path").Join("bootloaders", bootloader) if bootloaderPath.NotExist() { - if b.logger.Verbose() { + if b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Warn(i18n.Tr("Bootloader file specified but missing: %[1]s", bootloaderPath)) } return nil @@ -255,7 +256,7 @@ func (b *Builder) mergeSketchWithBootloader() error { maximumBinSize *= 2 } err := merge(builtSketchPath, bootloaderPath, mergedSketchPath, maximumBinSize) - if err != nil && b.logger.Verbose() { + if err != nil && b.logger.VerbosityLevel() == logger.VerbosityVerbose { b.logger.Info(err.Error()) } From 95080cfd9ec36c455675897dc5ba94e35c0c29ff Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Jan 2025 12:28:59 +0100 Subject: [PATCH 02/10] Compile with --quiet flag set will not output final stats --- internal/cli/compile/compile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index 5cccf62ad46..f269d0a3dc3 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -372,7 +372,7 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer ProfileOut: profileOut, Success: compileError == nil, showPropertiesMode: showProperties, - hideStats: preprocess, + hideStats: preprocess || quiet, } if compileError != nil { From d9cfdf7942b9ba3ad17838511ba0c4eedbdb9b1b Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Jan 2025 12:33:32 +0100 Subject: [PATCH 03/10] Suppress size output if --quiet is specified --- commands/service_compile.go | 3 +++ internal/arduino/builder/sizer.go | 28 +++++++++++++++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/commands/service_compile.go b/commands/service_compile.go index 2ca418df7cf..24cbfd52b2c 100644 --- a/commands/service_compile.go +++ b/commands/service_compile.go @@ -246,6 +246,9 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu }) } var verbosity logger.Verbosity = logger.VerbosityNormal + if req.GetQuiet() { + verbosity = logger.VerbosityQuiet + } if req.GetVerbose() { verbosity = logger.VerbosityVerbose } diff --git a/internal/arduino/builder/sizer.go b/internal/arduino/builder/sizer.go index a9ac3de2c8c..ae188b48f2b 100644 --- a/internal/arduino/builder/sizer.go +++ b/internal/arduino/builder/sizer.go @@ -156,19 +156,21 @@ func (b *Builder) checkSize() (ExecutablesFileSections, error) { return nil, nil } - b.logger.Info(i18n.Tr("Sketch uses %[1]s bytes (%[3]s%%) of program storage space. Maximum is %[2]s bytes.", - strconv.Itoa(textSize), - strconv.Itoa(maxTextSize), - strconv.Itoa(textSize*100/maxTextSize))) - if dataSize >= 0 { - if maxDataSize > 0 { - b.logger.Info(i18n.Tr("Global variables use %[1]s bytes (%[3]s%%) of dynamic memory, leaving %[4]s bytes for local variables. Maximum is %[2]s bytes.", - strconv.Itoa(dataSize), - strconv.Itoa(maxDataSize), - strconv.Itoa(dataSize*100/maxDataSize), - strconv.Itoa(maxDataSize-dataSize))) - } else { - b.logger.Info(i18n.Tr("Global variables use %[1]s bytes of dynamic memory.", strconv.Itoa(dataSize))) + if b.logger.VerbosityLevel() > logger.VerbosityQuiet { + b.logger.Info(i18n.Tr("Sketch uses %[1]s bytes (%[3]s%%) of program storage space. Maximum is %[2]s bytes.", + strconv.Itoa(textSize), + strconv.Itoa(maxTextSize), + strconv.Itoa(textSize*100/maxTextSize))) + if dataSize >= 0 { + if maxDataSize > 0 { + b.logger.Info(i18n.Tr("Global variables use %[1]s bytes (%[3]s%%) of dynamic memory, leaving %[4]s bytes for local variables. Maximum is %[2]s bytes.", + strconv.Itoa(dataSize), + strconv.Itoa(maxDataSize), + strconv.Itoa(dataSize*100/maxDataSize), + strconv.Itoa(maxDataSize-dataSize))) + } else { + b.logger.Info(i18n.Tr("Global variables use %[1]s bytes of dynamic memory.", strconv.Itoa(dataSize))) + } } } From 2e4d48fcdb7754b1ddaa606b4f7ec41cddfc6b95 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 21 Jan 2025 12:54:12 +0100 Subject: [PATCH 04/10] Added -q shortcut to --quiet flag in compile command --- internal/cli/compile/compile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index f269d0a3dc3..85575a79b4a 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -116,7 +116,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) * compileCommand.Flags().StringVar(&warnings, "warnings", "none", i18n.Tr(`Optional, can be: %s. Used to tell gcc which warning level to use (-W flag).`, "none, default, more, all")) compileCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, i18n.Tr("Optional, turns on verbose mode.")) - compileCommand.Flags().BoolVar(&quiet, "quiet", false, i18n.Tr("Optional, suppresses almost every output.")) + compileCommand.Flags().BoolVarP(&quiet, "quiet", "q", false, i18n.Tr("Optional, suppresses almost every output.")) compileCommand.Flags().BoolVarP(&uploadAfterCompile, "upload", "u", false, i18n.Tr("Upload the binary after the compilation.")) portArgs.AddToCommand(compileCommand, srv) compileCommand.Flags().BoolVarP(&verify, "verify", "t", false, i18n.Tr("Verify uploaded binary after the upload.")) From 48a6ad3abef06d40cf51dbaf0f1e33ead76a6dc6 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 22 Jan 2025 11:11:30 +0100 Subject: [PATCH 05/10] Compile recap is hidden also on a successful compile with default verbosity --- internal/cli/compile/compile.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index 85575a79b4a..31331f77653 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -362,6 +362,7 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer } stdIO := stdIORes() + successful := (compileError == nil) res := &compileResult{ CompilerOut: stdIO.Stdout, CompilerErr: stdIO.Stderr, @@ -370,9 +371,9 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer UpdatedUploadPort: result.NewPort(uploadRes.GetUpdatedUploadPort()), }, ProfileOut: profileOut, - Success: compileError == nil, + Success: successful, showPropertiesMode: showProperties, - hideStats: preprocess || quiet, + hideStats: preprocess || quiet || (!verbose && successful), } if compileError != nil { From f967107eda90e43c7e6a521099e72a4584bc7205 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 23 Jan 2025 12:58:14 +0100 Subject: [PATCH 06/10] Made --quiet and --verbose mutually exclusive --- internal/cli/compile/compile.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index 31331f77653..4b0e6767dc6 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -117,6 +117,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) * i18n.Tr(`Optional, can be: %s. Used to tell gcc which warning level to use (-W flag).`, "none, default, more, all")) compileCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, i18n.Tr("Optional, turns on verbose mode.")) compileCommand.Flags().BoolVarP(&quiet, "quiet", "q", false, i18n.Tr("Optional, suppresses almost every output.")) + compileCommand.MarkFlagsMutuallyExclusive("quiet", "verbose") compileCommand.Flags().BoolVarP(&uploadAfterCompile, "upload", "u", false, i18n.Tr("Upload the binary after the compilation.")) portArgs.AddToCommand(compileCommand, srv) compileCommand.Flags().BoolVarP(&verify, "verify", "t", false, i18n.Tr("Verify uploaded binary after the upload.")) From a596028dd3b0affafc26e2494ffd650a63716035 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 23 Jan 2025 13:40:35 +0100 Subject: [PATCH 07/10] Output 'ctags' command line to stdout instead of stderr --- .../builder/internal/preprocessor/ctags.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/arduino/builder/internal/preprocessor/ctags.go b/internal/arduino/builder/internal/preprocessor/ctags.go index fe36cfc89e5..c77d22e783b 100644 --- a/internal/arduino/builder/internal/preprocessor/ctags.go +++ b/internal/arduino/builder/internal/preprocessor/ctags.go @@ -83,8 +83,9 @@ func PreprocessSketchWithCtags( } // Run CTags on gcc-preprocessed source - ctagsOutput, ctagsStdErr, err := RunCTags(ctx, ctagsTarget, buildProperties) + ctagsOutput, ctagsStdErr, ctagsCmdLine, err := RunCTags(ctx, ctagsTarget, buildProperties) if verbose { + stdout.Write([]byte(ctagsCmdLine + "\n")) stderr.Write(ctagsStdErr) } if err != nil { @@ -178,7 +179,7 @@ func isFirstFunctionOutsideOfSource(firstFunctionLine int, sourceRows []string) } // RunCTags performs a run of ctags on the given source file. Returns the ctags output and the stderr contents. -func RunCTags(ctx context.Context, sourceFile *paths.Path, buildProperties *properties.Map) ([]byte, []byte, error) { +func RunCTags(ctx context.Context, sourceFile *paths.Path, buildProperties *properties.Map) ([]byte, []byte, string, error) { ctagsBuildProperties := properties.NewMap() ctagsBuildProperties.Set("tools.ctags.path", "{runtime.tools.ctags.path}") ctagsBuildProperties.Set("tools.ctags.cmd.path", "{path}/ctags") @@ -189,24 +190,22 @@ func RunCTags(ctx context.Context, sourceFile *paths.Path, buildProperties *prop pattern := ctagsBuildProperties.Get("pattern") if pattern == "" { - return nil, nil, errors.New(i18n.Tr("%s pattern is missing", "ctags")) + return nil, nil, "", errors.New(i18n.Tr("%s pattern is missing", "ctags")) } commandLine := ctagsBuildProperties.ExpandPropsInString(pattern) parts, err := properties.SplitQuotedString(commandLine, `"'`, false) if err != nil { - return nil, nil, err + return nil, nil, "", err } proc, err := paths.NewProcess(nil, parts...) if err != nil { - return nil, nil, err + return nil, nil, "", err } stdout, stderr, err := proc.RunAndCaptureOutput(ctx) - // Append ctags arguments to stderr args := fmt.Sprintln(strings.Join(parts, " ")) - stderr = append([]byte(args), stderr...) - return stdout, stderr, err + return stdout, stderr, args, err } func filterSketchSource(sketch *sketch.Sketch, source io.Reader, removeLineMarkers bool) string { From 6268f3978106872728b73ebc24f8c056f07b5db5 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 23 Jan 2025 13:50:52 +0100 Subject: [PATCH 08/10] Do not output empty lines if the message is empty --- internal/arduino/builder/logger/logger.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/arduino/builder/logger/logger.go b/internal/arduino/builder/logger/logger.go index d4a730d04e6..347c98ce9f8 100644 --- a/internal/arduino/builder/logger/logger.go +++ b/internal/arduino/builder/logger/logger.go @@ -62,6 +62,9 @@ func New(stdout, stderr io.Writer, verbosity Verbosity, warningsLevel string) *B // Info fixdoc func (l *BuilderLogger) Info(msg string) { + if msg == "" { + return + } l.stdLock.Lock() defer l.stdLock.Unlock() fmt.Fprintln(l.stdout, msg) @@ -69,6 +72,9 @@ func (l *BuilderLogger) Info(msg string) { // Warn fixdoc func (l *BuilderLogger) Warn(msg string) { + if msg == "" { + return + } l.stdLock.Lock() defer l.stdLock.Unlock() fmt.Fprintln(l.stderr, msg) From c9122ebd371f88dd4f923982092652e54cdc3e9d Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 23 Jan 2025 13:52:07 +0100 Subject: [PATCH 09/10] Added integration tests --- .../compile_3/compile_verbosity_test.go | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 internal/integrationtest/compile_3/compile_verbosity_test.go diff --git a/internal/integrationtest/compile_3/compile_verbosity_test.go b/internal/integrationtest/compile_3/compile_verbosity_test.go new file mode 100644 index 00000000000..db87afa7562 --- /dev/null +++ b/internal/integrationtest/compile_3/compile_verbosity_test.go @@ -0,0 +1,103 @@ +// This file is part of arduino-cli. +// +// Copyright 2022 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package compile_test + +import ( + "testing" + + "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" +) + +func TestCompileVerbosity(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + _, _, err := cli.Run("core", "update-index") + require.NoError(t, err) + _, _, err = cli.Run("core", "install", "arduino:avr") + require.NoError(t, err) + + goodSketch, err := paths.New("testdata", "bare_minimum").Abs() + require.NoError(t, err) + badSketch, err := paths.New("testdata", "blink_with_error_directive").Abs() + require.NoError(t, err) + + hasSketchSize := func(t *testing.T, out []byte) { + require.Contains(t, string(out), "Sketch uses") + } + noSketchSize := func(t *testing.T, out []byte) { + require.NotContains(t, string(out), "Sketch uses") + } + hasRecapTable := func(t *testing.T, out []byte) { + require.Contains(t, string(out), "Used platform") + } + noRecapTable := func(t *testing.T, out []byte) { + require.NotContains(t, string(out), "Used platform") + } + + t.Run("DefaultVerbosity/SuccessfulBuild", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", goodSketch.String()) + require.NoError(t, err) + hasSketchSize(t, stdout) + noRecapTable(t, stdout) + require.Empty(t, stderr) + }) + + t.Run("DefaultVerbosity/BuildWithErrors", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", badSketch.String()) + require.Error(t, err) + hasRecapTable(t, stdout) + require.NotEmpty(t, stderr) + }) + + t.Run("VerboseVerbosity/SuccessfulBuild", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-v", goodSketch.String()) + require.NoError(t, err) + hasSketchSize(t, stdout) + hasRecapTable(t, stdout) + require.Empty(t, stderr) + }) + + t.Run("VerboseVerbosity/BuildWithErrors", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-v", badSketch.String()) + require.Error(t, err) + hasRecapTable(t, stdout) + require.NotEmpty(t, stderr) + }) + + t.Run("QuietVerbosity/SuccessfulBuild", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-q", goodSketch.String()) + require.NoError(t, err) + noSketchSize(t, stdout) + noRecapTable(t, stdout) + require.Empty(t, stdout) // Empty output + require.Empty(t, stderr) + }) + + t.Run("QuietVerbosity/BuildWithErrors", func(t *testing.T) { + stdout, stderr, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-q", badSketch.String()) + require.Error(t, err) + noRecapTable(t, stdout) + require.NotEmpty(t, stderr) + }) + + t.Run("ConflictingVerbosityOptions", func(t *testing.T) { + _, _, err := cli.Run("compile", "--fqbn", "arduino:avr:uno", "-v", "-q", goodSketch.String()) + require.Error(t, err) + }) +} From 17ced8fc4fcf529cb32df003607d120a3c064aa0 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 23 Jan 2025 13:58:43 +0100 Subject: [PATCH 10/10] Use internal function to check for arguments exclusivity Because it has a better error message than the cobra-provided one: Can't use the following flags together: --quiet, --verbose Instead of: Error: if any flags in the group [quiet verbose] are set none of the others can be; [quiet verbose] were all set --- internal/cli/compile/compile.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index 4b0e6767dc6..69ec3d52966 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -85,6 +85,9 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) * " " + os.Args[0] + ` compile -b arduino:avr:uno --build-property "build.extra_flags=-DPIN=2 \"-DMY_DEFINE=\"hello world\"\"" /home/user/Arduino/MySketch` + "\n" + " " + os.Args[0] + ` compile -b arduino:avr:uno --build-property build.extra_flags=-DPIN=2 --build-property "compiler.cpp.extra_flags=\"-DSSID=\"hello world\"\"" /home/user/Arduino/MySketch` + "\n", Args: cobra.MaximumNArgs(1), + PreRun: func(cmd *cobra.Command, args []string) { + arguments.CheckFlagsConflicts(cmd, "quiet", "verbose") + }, Run: func(cmd *cobra.Command, args []string) { if cmd.Flag("build-cache-path").Changed { feedback.Warning(i18n.Tr("The flag --build-cache-path has been deprecated. Please use just --build-path alone or configure the build cache path in the Arduino CLI settings.")) @@ -117,7 +120,6 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) * i18n.Tr(`Optional, can be: %s. Used to tell gcc which warning level to use (-W flag).`, "none, default, more, all")) compileCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, i18n.Tr("Optional, turns on verbose mode.")) compileCommand.Flags().BoolVarP(&quiet, "quiet", "q", false, i18n.Tr("Optional, suppresses almost every output.")) - compileCommand.MarkFlagsMutuallyExclusive("quiet", "verbose") compileCommand.Flags().BoolVarP(&uploadAfterCompile, "upload", "u", false, i18n.Tr("Upload the binary after the compilation.")) portArgs.AddToCommand(compileCommand, srv) compileCommand.Flags().BoolVarP(&verify, "verify", "t", false, i18n.Tr("Verify uploaded binary after the upload."))