Skip to content

Commit

Permalink
On compile command: implemented --quiet flag / removed libraries re…
Browse files Browse the repository at this point in the history
…cap 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
  • Loading branch information
cmaglie authored Jan 24, 2025
1 parent c5cfe4a commit 92fae9c
Show file tree
Hide file tree
Showing 16 changed files with 209 additions and 68 deletions.
10 changes: 9 additions & 1 deletion commands/service_compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
)
Expand Down
5 changes: 3 additions & 2 deletions internal/arduino/builder/archive_compiled_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 9 additions & 9 deletions internal/arduino/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -215,7 +215,7 @@ func NewBuilder(
customBuildProperties: customBuildPropertiesArgs,
coreBuildCachePath: coreBuildCachePath,
extraCoreBuildCachePaths: extraCoreBuildCachePaths,
logger: logger,
logger: log,
clean: clean,
sourceOverrides: sourceOverrides,
onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
Expand All @@ -242,7 +242,7 @@ func NewBuilder(
libsManager, libsResolver,
useCachedLibrariesResolution,
onlyUpdateCompilationDatabase,
logger,
log,
diagnosticStore,
),
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
}
Expand Down
7 changes: 4 additions & 3 deletions internal/arduino/builder/compilation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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())
Expand All @@ -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 {
Expand Down
7 changes: 4 additions & 3 deletions internal/arduino/builder/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions internal/arduino/builder/internal/detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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))
}
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
15 changes: 7 additions & 8 deletions internal/arduino/builder/internal/preprocessor/ctags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions internal/arduino/builder/libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion internal/arduino/builder/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
Loading

0 comments on commit 92fae9c

Please sign in to comment.