From ef27958b56c705306d2de777a0b90924125b3016 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 11 Dec 2020 14:55:11 -0800 Subject: [PATCH 1/3] Disable logging immediately By default, logging is disabled, but this was previously done in configuration.Initialize(), meaning that any log output above the default level made before that part of the configuration ran was still displayed. --- configuration/configuration.go | 15 +++++++++++---- configuration/defaults.go | 4 ++-- main.go | 5 +++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index a9554d23..71eb001a 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -18,6 +18,7 @@ package configuration import ( "fmt" + "io/ioutil" "os" "strconv" "strings" @@ -56,15 +57,13 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { } } - logrus.SetOutput(defaultLogOutput) - if logFormatString, ok := os.LookupEnv("ARDUINO_CHECK_LOG_FORMAT"); ok { logFormat, err := logFormatFromString(logFormatString) if err != nil { return fmt.Errorf("--log-format flag value %s not valid", logFormatString) } logrus.SetFormatter(logFormat) - logrus.SetOutput(os.Stderr) // Enable log output. + EnableLogging(true) } if logLevelString, ok := os.LookupEnv("ARDUINO_CHECK_LOG_LEVEL"); ok { @@ -73,7 +72,7 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { return fmt.Errorf("--log-level flag value %s not valid", logLevelString) } logrus.SetLevel(logLevel) - logrus.SetOutput(os.Stderr) // Enable log output. + EnableLogging(true) } superprojectTypeFilterString, _ := flags.GetString("project-type") @@ -232,3 +231,11 @@ func SchemasPath() *paths.Path { } return paths.New(executablePath).Parent().Join("etc", "schemas") } + +func EnableLogging(enable bool) { + if enable { + logrus.SetOutput(defaultLogOutput) // Enable log output. + } else { + logrus.SetOutput(ioutil.Discard) + } +} diff --git a/configuration/defaults.go b/configuration/defaults.go index 7f2f3d5d..38fde8af 100644 --- a/configuration/defaults.go +++ b/configuration/defaults.go @@ -18,7 +18,7 @@ package configuration // The default configuration settings. import ( - "io/ioutil" + "os" "github.com/arduino/arduino-check/configuration/checkmode" "github.com/arduino/arduino-check/project/projecttype" @@ -61,4 +61,4 @@ var defaultCheckModes = map[projecttype.Type]map[checkmode.Type]bool{ }, } -var defaultLogOutput = ioutil.Discard // Default to no log output. +var defaultLogOutput = os.Stderr diff --git a/main.go b/main.go index 0adae901..e920bbb3 100644 --- a/main.go +++ b/main.go @@ -19,9 +19,14 @@ import ( "os" "github.com/arduino/arduino-check/cli" + "github.com/arduino/arduino-check/configuration" "github.com/arduino/arduino-check/result/feedback" ) +func init() { + configuration.EnableLogging(false) +} + func main() { rootCommand := cli.Root() if err := rootCommand.Execute(); err != nil { From c09c82d7cd686387161efc02cf6679d6493c3679 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 11 Dec 2020 14:59:57 -0800 Subject: [PATCH 2/3] Use clear and consistent error output Previously, it was not always clear that the error messages were such. Adding a prefix to the feedback.Error*() output ensures this will be clear and that the error messages will follow a consistent format. --- check/checkdata/library.go | 2 +- command/command.go | 2 +- configuration/configuration.go | 2 +- project/project.go | 4 ++-- result/feedback/feedback.go | 3 ++- result/result.go | 4 ++-- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/check/checkdata/library.go b/check/checkdata/library.go index 9abf9b6c..addb554e 100644 --- a/check/checkdata/library.go +++ b/check/checkdata/library.go @@ -62,7 +62,7 @@ func InitializeForLibrary(project project.Type, schemasPath *paths.Path) { url := "http://downloads.arduino.cc/libraries/library_index.json" httpResponse, err := http.Get(url) if err != nil { - feedback.Errorf("%s Unable to download Library Manager index from %s", err, url) + feedback.Errorf("Unable to download Library Manager index from %s: %s", err, url) os.Exit(1) } defer httpResponse.Body.Close() diff --git a/command/command.go b/command/command.go index 13e453b1..6999eb08 100644 --- a/command/command.go +++ b/command/command.go @@ -33,7 +33,7 @@ import ( // ArduinoCheck is the root command function. func ArduinoCheck(rootCommand *cobra.Command, cliArguments []string) { if err := configuration.Initialize(rootCommand.Flags(), cliArguments); err != nil { - feedback.Errorf("Configuration error: %v", err) + feedback.Errorf("Invalid configuration: %v", err) os.Exit(1) } diff --git a/configuration/configuration.go b/configuration/configuration.go index 71eb001a..131e1317 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -107,7 +107,7 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { targetPath := paths.New(projectPath) targetPathExists, err := targetPath.ExistCheck() if err != nil { - return fmt.Errorf("Problem processing PROJECT_PATH argument value %v: %v", targetPath, err) + return fmt.Errorf("Unable to process PROJECT_PATH argument value %v: %v", targetPath, err) } if !targetPathExists { return fmt.Errorf("PROJECT_PATH argument %v does not exist", targetPath) diff --git a/project/project.go b/project/project.go index b93f9a4f..fd85be4f 100644 --- a/project/project.go +++ b/project/project.go @@ -88,7 +88,7 @@ func findProjects(targetPath *paths.Path) ([]Type, error) { return foundProjects, nil } - return nil, fmt.Errorf("specified path %s is not an Arduino project", targetPath) + return nil, fmt.Errorf("Specified path %s is not an Arduino project", targetPath) } if configuration.SuperprojectTypeFilter() == projecttype.All || configuration.Recursive() { @@ -110,7 +110,7 @@ func findProjects(targetPath *paths.Path) ([]Type, error) { } if foundProjects == nil { - return nil, fmt.Errorf("no projects found under %s", targetPath) + return nil, fmt.Errorf("No projects found under %s", targetPath) } return foundProjects, nil diff --git a/result/feedback/feedback.go b/result/feedback/feedback.go index da79d815..969f99b3 100644 --- a/result/feedback/feedback.go +++ b/result/feedback/feedback.go @@ -66,8 +66,9 @@ func Errorf(format string, v ...interface{}) { Error(fmt.Sprintf(format, v...)) } -// Error behaves like fmt.Print but adds a newline and also logs the error. +// Error behaves like fmt.Print but adds a prefix, newline and also logs the error. func Error(v ...interface{}) { + fmt.Fprint(os.Stderr, "error: ") fmt.Fprintln(os.Stderr, v...) logrus.Error(fmt.Sprint(v...)) } diff --git a/result/result.go b/result/result.go index c8b72b4f..df6776ca 100644 --- a/result/result.go +++ b/result/result.go @@ -236,13 +236,13 @@ func (results Type) WriteReport() error { if !reportFilePathParentExists { err = reportFilePath.Parent().MkdirAll() if err != nil { - return fmt.Errorf("Error while creating report file path (%v): %v", reportFilePath.Parent(), err) + return fmt.Errorf("Unable to create report file path (%v): %v", reportFilePath.Parent(), err) } } err = reportFilePath.WriteFile(results.jsonReportRaw()) if err != nil { - return fmt.Errorf("Error while writing report: %v", err) + return fmt.Errorf("While writing report: %v", err) } return nil From ab81928580d47ce3170f96cdcdbda2c35f36c1f4 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 11 Dec 2020 15:24:10 -0800 Subject: [PATCH 3/3] Panic on inability to resolve working directory This is an unexpected error with no clear cause, so the panic is more appropriate. --- configuration/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configuration/configuration.go b/configuration/configuration.go index 131e1317..843c08e7 100644 --- a/configuration/configuration.go +++ b/configuration/configuration.go @@ -99,7 +99,7 @@ func Initialize(flags *pflag.FlagSet, projectPaths []string) error { // Default to using current working directory. workingDirectoryPath, err := os.Getwd() if err != nil { - return fmt.Errorf("Error when setting default PROJECT_PATH argument: %s", err) + panic(err) } targetPaths.Add(paths.New(workingDirectoryPath)) } else {