Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Viper to manage the configuration file #487

Merged
merged 21 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 5 additions & 10 deletions arduino/cores/packagemanager/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,20 @@ import (
"strings"

"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/configs"
"github.com/arduino/arduino-cli/configuration"
"github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
semver "go.bug.st/relaxed-semver"
)

// LoadHardware read all plaforms from the configured paths
func (pm *PackageManager) LoadHardware(config *configs.Configuration) error {
dirs, err := config.HardwareDirectories()
if err != nil {
return fmt.Errorf("getting hardware directory: %s", err)
}
func (pm *PackageManager) LoadHardware() error {
dirs := configuration.HardwareDirectories()
if err := pm.LoadHardwareFromDirectories(dirs); err != nil {
return err
}
dirs, err = config.BundleToolsDirectories()
if err != nil {
return fmt.Errorf("getting hardware directory: %s", err)
}

dirs = configuration.BundleToolsDirectories()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the error was removed because these functions always returned nil

return pm.LoadToolsFromBundleDirectories(dirs)
}

Expand Down
20 changes: 12 additions & 8 deletions arduino/cores/packagemanager/package_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ package packagemanager_test
import (
"fmt"
"net/url"
"os"
"testing"

"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
"github.com/arduino/arduino-cli/configs"
"github.com/arduino/arduino-cli/configuration"
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
semver "go.bug.st/relaxed-semver"
)
Expand Down Expand Up @@ -212,14 +214,16 @@ func TestBoardOptionsFunctions(t *testing.T) {
}

func TestFindToolsRequiredForBoard(t *testing.T) {
os.Setenv("ARDUINO_DATA_DIR", dataDir1.String())
configuration.Init("")
fmt.Println(viper.AllSettings())
pm := packagemanager.NewPackageManager(
dataDir1,
dataDir1.Join("packages"),
dataDir1.Join("staging"),
dataDir1)
conf := &configs.Configuration{
DataDir: dataDir1,
}
paths.New(viper.GetString("directories.Packages")),
paths.New(viper.GetString("directories.Downloads")),
dataDir1,
)

loadIndex := func(addr string) {
res, err := url.Parse(addr)
require.NoError(t, err)
Expand All @@ -228,7 +232,7 @@ func TestFindToolsRequiredForBoard(t *testing.T) {
loadIndex("https://dl.espressif.com/dl/package_esp32_index.json")
loadIndex("http://arduino.esp8266.com/stable/package_esp8266com_index.json")
loadIndex("https://adafruit.github.io/arduino-board-index/package_adafruit_index.json")
require.NoError(t, pm.LoadHardware(conf))
require.NoError(t, pm.LoadHardware())
esp32, err := pm.FindBoardWithFQBN("esp32:esp32:esp32")
require.NoError(t, err)
esptool231 := pm.FindToolDependency(&cores.ToolDependency{
Expand Down
134 changes: 97 additions & 37 deletions cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"strings"

"github.com/arduino/arduino-cli/cli/board"
Expand All @@ -37,10 +39,12 @@ import (
"github.com/arduino/arduino-cli/cli/sketch"
"github.com/arduino/arduino-cli/cli/upload"
"github.com/arduino/arduino-cli/cli/version"
"github.com/arduino/arduino-cli/configuration"
"github.com/mattn/go-colorable"
"github.com/rifflock/lfshook"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/viper"
)

var (
Expand All @@ -54,13 +58,8 @@ var (
}

verbose bool
logFile string
logFormat string
outputFormat string
)

const (
defaultLogLevel = "info"
configFile string
)

// Init the cobra root command
Expand All @@ -82,12 +81,16 @@ func createCliCommandTree(cmd *cobra.Command) {
cmd.AddCommand(version.NewCommand())

cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Print the logs on the standard output.")
cmd.PersistentFlags().StringVar(&globals.LogLevel, "log-level", defaultLogLevel, "Messages with this level and above will be logged.")
cmd.PersistentFlags().StringVar(&logFile, "log-file", "", "Path to the file where logs will be written.")
cmd.PersistentFlags().StringVar(&logFormat, "log-format", "text", "The output format for the logs, can be [text|json].")
cmd.PersistentFlags().String("log-level", "", "Messages with this level and above will be logged.")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need a var anymore since the flag value is stored in the settings objects directly (this applies to following similar cases)

viper.BindPFlag("logging.level", cmd.PersistentFlags().Lookup("log-level"))
cmd.PersistentFlags().String("log-file", "", "Path to the file where logs will be written.")
viper.BindPFlag("logging.file", cmd.PersistentFlags().Lookup("log-file"))
cmd.PersistentFlags().String("log-format", "", "The output format for the logs, can be [text|json].")
viper.BindPFlag("logging.format", cmd.PersistentFlags().Lookup("log-format"))
cmd.PersistentFlags().StringVar(&outputFormat, "format", "text", "The output format, can be [text|json].")
cmd.PersistentFlags().StringVar(&globals.YAMLConfigFile, "config-file", "", "The custom config file (if not specified the default will be used).")
cmd.PersistentFlags().StringSliceVar(&globals.AdditionalUrls, "additional-urls", []string{}, "Additional URLs for the board manager.")
cmd.PersistentFlags().StringVar(&configFile, "config-file", "", "The custom config file (if not specified the default will be used).")
cmd.PersistentFlags().StringSlice("additional-urls", []string{}, "Additional URLs for the board manager.")
viper.BindPFlag("board_manager.additional_urls", cmd.PersistentFlags().Lookup("additional-urls"))
}

// convert the string passed to the `--log-level` option to the corresponding
Expand Down Expand Up @@ -115,14 +118,73 @@ func parseFormatString(arg string) (feedback.OutputFormat, bool) {
return f, found
}

// This function is here to replicate the old logic looking for a config
// file in the parent tree of the CWD, aka "project config".
// Please
func searchConfigTree(cwd string) string {
// go back up to root and search for the config file
for {
if _, err := os.Stat(path.Join(cwd, "arduino-cli.yaml")); os.IsNotExist(err) {
// no config file found
next := path.Join(cwd, "..")
if filepath.Clean(next) == filepath.Clean(cwd) {
return ""
}
cwd = next
} else {
return cwd
}
}
}

func preRun(cmd *cobra.Command, args []string) {
// normalize the format strings
outputFormat = strings.ToLower(outputFormat)
// configure the output package
output.OutputFormat = outputFormat
logFormat = strings.ToLower(logFormat)
//
// Prepare the configuration system
//
configPath := ""

// get cwd, if something is wrong don't do anything and let
// configuration init proceed
if cwd, err := os.Getwd(); err == nil {
configPath = searchConfigTree(cwd)
}

// override the config path if --config-file was passed
if fi, err := os.Stat(configFile); err == nil {
if fi.IsDir() {
configPath = configFile
} else {
configPath = filepath.Dir(configFile)
}
}

// initialize the config system
configuration.Init(configPath)
configFile := viper.ConfigFileUsed()

//
// Prepare logging
//

// decide whether we should log to stdout
if verbose {
// if we print on stdout, do it in full colors
logrus.SetOutput(colorable.NewColorableStdout())
logrus.SetFormatter(&logrus.TextFormatter{
ForceColors: true,
})
} else {
logrus.SetOutput(ioutil.Discard)
}

// set the Logger format
logFormat := strings.ToLower(viper.GetString("logging.format"))
if logFormat == "json" {
logrus.SetFormatter(&logrus.JSONFormatter{})
}

// should we log to file?
logFile := viper.GetString("logging.file")
if logFile != "" {
file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
if err != nil {
Expand All @@ -138,30 +200,22 @@ func preRun(cmd *cobra.Command, args []string) {
}
}

// should we log to stdout?
if verbose {
logrus.SetOutput(colorable.NewColorableStdout())
logrus.SetFormatter(&logrus.TextFormatter{
ForceColors: true,
})
} else {
// Discard logrus output if no writer was set
logrus.SetOutput(ioutil.Discard)
}

// configure logging filter
if lvl, found := toLogLevel(globals.LogLevel); !found {
fmt.Printf("Invalid option for --log-level: %s", globals.LogLevel)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated but we should use feedback instead of fmt across the codebase

if lvl, found := toLogLevel(viper.GetString("logging.level")); !found {
feedback.Errorf("Invalid option for --log-level: %s", viper.GetString("logging.level"))
os.Exit(errorcodes.ErrBadArgument)
} else {
logrus.SetLevel(lvl)
}

// set the Logger format
if logFormat == "json" {
logrus.SetFormatter(&logrus.JSONFormatter{})
}
//
// Prepare the Feedback system
//

// normalize the format strings
outputFormat = strings.ToLower(outputFormat)
// configure the output package
output.OutputFormat = outputFormat
// check the right output format was passed
format, found := parseFormatString(outputFormat)
if !found {
Expand All @@ -172,12 +226,18 @@ func preRun(cmd *cobra.Command, args []string) {
// use the output format to configure the Feedback
feedback.SetFormat(format)

globals.InitConfigs()
//
// Print some status info and check command is consistent
//

if configFile != "" {
logrus.Infof("Using config file: %s", configFile)
} else {
logrus.Info("Config file not found, using default values")
}

logrus.Info(globals.VersionInfo.Application + "-" + globals.VersionInfo.VersionString)
logrus.Info("Starting root command preparation (`arduino`)")
logrus.Info(globals.VersionInfo.Application + " version " + globals.VersionInfo.VersionString)

logrus.Info("Formatter set")
if outputFormat != "text" {
cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) {
logrus.Warn("Calling help on JSON format")
Expand Down