On compile command: implemented --quiet flag / removed libraries recap on default verbosity level. (#2820)

* Made builder logger verbosity an explicit type

* Compile with --quiet flag set will not output final stats

* Suppress size output if --quiet is specified

* Added -q shortcut to --quiet flag in compile command

* Compile recap is hidden also on a successful compile with default verbosity

* Made --quiet and --verbose mutually exclusive

* Output 'ctags' command line to stdout instead of stderr

* Do not output empty lines if the message is empty

* Added integration tests

* 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

Author: cmaglie

Diff for: 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,13 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 			Message: &rpc.CompileResponse_Progress{Progress: p},
 		})
 	}
+	var verbosity logger.Verbosity = logger.VerbosityNormal
+	if req.GetQuiet() {
+		verbosity = logger.VerbosityQuiet
+	}
+	if req.GetVerbose() {
+		verbosity = logger.VerbosityVerbose
+	}
 	sketchBuilder, err := builder.NewBuilder(
 		ctx,
 		sk,
@@ -265,7 +273,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 for: internal/arduino/builder/archive_compiled_files.go

@@ -16,14 +16,15 @@
 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"
 )
 
 // 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 for: 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 for: 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 for: 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 for: 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 for: 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 {

Diff for: 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 for: 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