Implementation of Viper replacement library

- Using configmap allows better handling of config types - The CLI "instances" now caches all the configuration at Create-time - Setting are now only accessible via gRPC calls - Helper methods have been implemented to access all key/value pairs
arduino · cmaglie · May 15, 2024 · Mar 13, 2024 · Mar 13, 2024 · Mar 13, 2024
commit 4c50868a2ad1733a280479acb3b5f1a3f3bbccbc
diff --git a/commands/instances.go b/commands/instances.go
@@ -36,7 +36,6 @@ import (
 	"github.com/arduino/arduino-cli/internal/arduino/resources"
 	"github.com/arduino/arduino-cli/internal/arduino/sketch"
 	"github.com/arduino/arduino-cli/internal/arduino/utils"
-	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	"github.com/arduino/arduino-cli/internal/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
@@ -72,7 +71,7 @@ func (s *arduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateReque
 	}
 
 	// Setup downloads directory
-	downloadsDir := configuration.DownloadsDir(s.settings)
+	downloadsDir := s.settings.DownloadsDir()
 	if downloadsDir.NotExist() {
 		err := downloadsDir.MkdirAll()
 		if err != nil {
@@ -81,8 +80,9 @@ func (s *arduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateReque
 	}
 
 	// Setup data directory
-	dataDir := configuration.DataDir(s.settings)
-	packagesDir := configuration.PackagesDir(s.settings)
+	dataDir := s.settings.DataDir()
+	userPackagesDir := s.settings.UserDir().Join("hardware")
+	packagesDir := s.settings.PackagesDir()
 	if packagesDir.NotExist() {
 		err := packagesDir.MkdirAll()
 		if err != nil {
@@ -94,7 +94,7 @@ func (s *arduinoCoreServerImpl) Create(ctx context.Context, req *rpc.CreateReque
 	if err != nil {
 		return nil, err
 	}
-	inst, err := instances.Create(dataDir, packagesDir, downloadsDir, userAgent, config)
+	inst, err := instances.Create(dataDir, packagesDir, userPackagesDir, downloadsDir, userAgent, config)
 	if err != nil {
 		return nil, err
 	}
@@ -177,7 +177,7 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 	defaultIndexURL, _ := utils.URLParse(globals.DefaultIndexURL)
 	allPackageIndexUrls := []*url.URL{defaultIndexURL}
 	if profile == nil {
-		for _, u := range s.settings.GetStringSlice("board_manager.additional_urls") {
+		for _, u := range s.settings.BoardManagerAdditionalUrls() {
 			URL, err := utils.URLParse(u)
 			if err != nil {
 				e := &cmderrors.InitFailedError{
@@ -192,7 +192,7 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 		}
 	}
 
-	if err := firstUpdate(ctx, s, req.GetInstance(), configuration.DataDir(s.settings), downloadCallback, allPackageIndexUrls); err != nil {
+	if err := firstUpdate(ctx, s, req.GetInstance(), s.settings.DataDir(), downloadCallback, allPackageIndexUrls); err != nil {
 		e := &cmderrors.InitFailedError{
 			Code:   codes.InvalidArgument,
 			Cause:  err,
@@ -245,7 +245,7 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 
 		// Load Platforms
 		if profile == nil {
-			for _, err := range pmb.LoadHardware(s.settings) {
+			for _, err := range pmb.LoadHardware() {
 				s := &cmderrors.PlatformLoadingError{Cause: err}
 				responseError(s.GRPCStatus())
 			}
@@ -349,7 +349,7 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 
 	if profile == nil {
 		// Add directories of libraries bundled with IDE
-		if bundledLibsDir := configuration.IDEBuiltinLibrariesDir(s.settings); bundledLibsDir != nil {
+		if bundledLibsDir := s.settings.IDEBuiltinLibrariesDir(); bundledLibsDir != nil {
 			lmb.AddLibrariesDir(librariesmanager.LibrariesDir{
 				Path:     bundledLibsDir,
 				Location: libraries.IDEBuiltIn,
@@ -358,14 +358,14 @@ func (s *arduinoCoreServerImpl) Init(req *rpc.InitRequest, stream rpc.ArduinoCor
 
 		// Add libraries directory from config file
 		lmb.AddLibrariesDir(librariesmanager.LibrariesDir{
-			Path:     configuration.LibrariesDir(s.settings),
+			Path:     s.settings.LibrariesDir(),
 			Location: libraries.User,
 		})
 	} else {
 		// Load libraries required for profile
 		for _, libraryRef := range profile.Libraries {
 			uid := libraryRef.InternalUniqueIdentifier()
-			libRoot := configuration.ProfilesCacheDir(s.settings).Join(uid)
+			libRoot := s.settings.ProfilesCacheDir().Join(uid)
 
10000
			libDir := libRoot.Join(libraryRef.Library)
 
 			if !libDir.IsDir() {
@@ -548,7 +548,7 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream
 			Message: &rpc.UpdateIndexResponse_DownloadProgress{DownloadProgress: p},
 		})
 	}
-	indexpath := configuration.DataDir(s.settings)
+	indexpath := s.settings.DataDir()
 
 	urls := []string{globals.DefaultIndexURL}
 	if !req.GetIgnoreCustomPackageIndexes() {
@@ -614,6 +614,7 @@ func (s *arduinoCoreServerImpl) UpdateIndex(req *rpc.UpdateIndexRequest, stream
 			downloadCB.Start(u, tr("Downloading index: %s", filepath.Base(URL.Path)))
 			downloadCB.End(false, tr("Invalid network configuration: %s", err))
 			failed = true
+			continue
 		}
 
 		if strings.HasSuffix(URL.Host, "arduino.cc") && strings.HasSuffix(URL.Path, ".json") {

diff --git a/commands/internal/instances/instances.go b/commands/internal/instances/instances.go
@@ -134,15 +134,15 @@ func SetLibraryManager(inst *rpc.Instance, lm *librariesmanager.LibrariesManager
 }
 
 // Create a new *rpc.Instance ready to be initialized
-func Create(dataDir, packagesDir, downloadsDir *paths.Path, extraUserAgent string, downloaderConfig downloader.Config) (*rpc.Instance, error) {
+func Create(dataDir, packagesDir, userPackagesDir, downloadsDir *paths.Path, extraUserAgent string, downloaderConfig downloader.Config) (*rpc.Instance, error) {
 	// Create package manager
 	userAgent := "arduino-cli/" + version.VersionInfo.VersionString
 	if extraUserAgent != "" {
 		userAgent += " " + extraUserAgent
 	}
 	tempDir := dataDir.Join("tmp")
 
-	pm := packagemanager.NewBuilder(dataDir, packagesDir, downloadsDir, tempDir, userAgent, downloaderConfig).Build()
+	pm := packagemanager.NewBuilder(dataDir, packagesDir, userPackagesDir, downloadsDir, tempDir, userAgent, downloaderConfig).Build()
 	lm, _ := librariesmanager.NewBuilder().Build()
 
 	instance := &coreInstance{

diff --git a/commands/service.go b/commands/service.go
@@ -19,28 +19,30 @@ import (
 	"context"
 
 	"github.com/arduino/arduino-cli/internal/cli/configuration"
+	"github.com/arduino/arduino-cli/internal/i18n"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/arduino/arduino-cli/version"
 )
 
 // NewArduinoCoreServer returns an implementation of the ArduinoCoreService gRPC service
 // that uses the provided version string.
-func NewArduinoCoreServer(version string, settings *configuration.Settings) rpc.ArduinoCoreServiceServer {
-	return &arduinoCoreServerImpl{
-		versionString: version,
-		settings:      settings,
-	}
+func NewArduinoCoreServer() rpc.ArduinoCoreServiceServer {
+	settings := configuration.NewSettings()
+
+	// Setup i18n
+	i18n.Init(settings.Locale())
+
+	return &arduinoCoreServerImpl{settings: settings}
 }
 
 type arduinoCoreServerImpl struct {
 	rpc.UnsafeArduinoCoreServiceServer // Force compile error for unimplemented methods
 
-	versionString string
-
 	// Settings holds configurations of the CLI and the gRPC consumers
 	settings *configuration.Settings
 }
 
 // Version returns the version of the Arduino CLI
 func (s *arduinoCoreServerImpl) Version(ctx context.Context, req *rpc.VersionRequest) (*rpc.VersionResponse, error) {
-	return &rpc.VersionResponse{Version: s.versionString}, nil
+	return &rpc.VersionResponse{Version: version.VersionInfo.VersionString}, nil
 }
diff --git a/commands/service_board_list_test.go b/commands/service_board_list_test.go
@@ -48,59 +48,67 @@ func TestGetByVidPid(t *testing.T) {
 	defer ts.Close()
 
 	vidPidURL = ts.URL
-	res, err := apiByVidPid("0xf420", "0XF069", configuration.Init(""))
+	settings := configuration.NewSettings()
+	res, err := apiByVidPid("0xf420", "0XF069", settings)
 	require.Nil(t, err)
 	require.Len(t, res, 1)
 	require.Equal(t, "Arduino/Genuino MKR1000", res[0].GetName())
 	require.Equal(t, "arduino:samd:mkr1000", res[0].GetFqbn())
 
 	// wrong vid (too long), wrong pid (not an hex value)
 
-	_, err = apiByVidPid("0xfffff", "0xDEFG", configuration.Init(""))
+	_, err = apiByVidPid("0xfffff", "0xDEFG", settings)
 	require.NotNil(t, err)
 }
 
 func TestGetByVidPidNotFound(t *testing.T) {
+	settings := configuration.NewSettings()
+
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer ts.Close()
 
 	vidPidURL = ts.URL
-	res, err := apiByVidPid("0x0420", "0x0069", configuration.Init(""))
+	res, err := apiByVidPid("0x0420", "0x0069", settings)
 	require.NoError(t, err)
 	require.Empty(t, res)
 }
 
 func TestGetByVidPid5xx(t *testing.T) {
+	settings := configuration.NewSettings()
+
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte("500 - Ooooops!"))
 	}))
 	defer ts.Close()
 
	vidPidURL = ts.URL
-	res, err := apiByVidPid("0x0420", "0x0069", configuration.Init(""))
+	res, err := apiByVidPid("0x0420", "0x0069", settings)
 	require.NotNil(t, err)
 	require.Equal(t, "the server responded with status 500 Internal Server Error", err.Error())
 	require.Len(t, res, 0)
 }
 
 func TestGetByVidPidMalformedResponse(t *testing.T) {
+	settings := configuration.NewSettings()
+
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		fmt.Fprintln(w, "{}")
 	}))
 	defer ts.Close()
 
 	vidPidURL = ts.URL
-	res, err := apiByVidPid("0x0420", "0x0069", configuration.Init(""))
+	res, err := apiByVidPid("0x0420", "0x0069", settings)
 	require.NotNil(t, err)
 	require.Equal(t, "wrong format in server response", err.Error())
 	require.Len(t, res, 0)
 }
 
 func TestBoardDetectionViaAPIWithNonUSBPort(t *testing.T) {
-	items, err := identifyViaCloudAPI(properties.NewMap(), configuration.Init(""))
+	settings := configuration.NewSettings()
+	items, err := identifyViaCloudAPI(properties.NewMap(), settings)
 	require.NoError(t, err)
 	require.Empty(t, items)
 }
@@ -112,7 +120,7 @@ func TestBoardIdentifySorting(t *testing.T) {
 	defer paths.TempDir().Join("test").RemoveAll()
 
 	// We don't really care about the paths in this case
-	pmb := packagemanager.NewBuilder(dataDir, dataDir, dataDir, dataDir, "test", downloader.GetDefaultConfig())
+	pmb := packagemanager.NewBuilder(dataDir, dataDir, nil, dataDir, dataDir, "test", downloader.GetDefaultConfig())
 
 	// Create some boards with identical VID:PID combination
 	pack := pmb.GetOrCreatePackage("packager")
@@ -148,7 +156,7 @@ func TestBoardIdentifySorting(t *testing.T) {
 	pme, release := pm.NewExplorer()
 	defer release()
 
-	settings := configuration.Init("")
+	settings := configuration.NewSettings()
 	res, err := identify(pme, &discovery.Port{Properties: idPrefs}, settings)
 	require.NoError(t, err)
 	require.NotNil(t, res)

diff --git a/commands/service_cache_clean.go b/commands/service_cache_clean.go
@@ -18,13 +18,12 @@ package commands
 import (
 	"context"
 
-	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 )
 
 // CleanDownloadCacheDirectory clean the download cache directory (where archives are downloaded).
 func (s *arduinoCoreServerImpl) CleanDownloadCacheDirectory(ctx context.Context, req *rpc.CleanDownloadCacheDirectoryRequest) (*rpc.CleanDownloadCacheDirectoryResponse, error) {
-	cachePath := configuration.DownloadsDir(s.settings)
+	cachePath := s.settings.DownloadsDir()
 	err := cachePath.RemoveAll()
 	if err != nil {
 		return nil, err

diff --git a/commands/service_compile.go b/commands/service_compile.go
@@ -32,7 +32,6 @@ import (
 	"github.com/arduino/arduino-cli/internal/arduino/sketch"
 	"github.com/arduino/arduino-cli/internal/arduino/utils"
 	"github.com/arduino/arduino-cli/internal/buildcache"
-	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	"github.com/arduino/arduino-cli/internal/inventory"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
@@ -67,7 +66,7 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 	ctx := stream.Context()
 	syncSend := NewSynchronizedSend(stream.Send)
 
-	exportBinaries := s.settings.GetBool("sketch.always_export_binaries")
+	exportBinaries := s.settings.SketchAlwaysExportBinaries()
 	if e := req.ExportBinaries; e != nil {
 		exportBinaries = *e
 	}
@@ -175,8 +174,8 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 	// cache is purged after compilation to not remove entries that might be required
 
 	defer maybePurgeBuildCache(
-		s.settings.GetUint("build_cache.compilations_before_purge"),
-		s.settings.GetDuration("build_cache.ttl").Abs())
+		s.settings.GetCompilationsBeforeBuildCachePurge(),
+		s.settings.GetBuildCacheTTL().Abs())
 
 	var coreBuildCachePath *paths.Path
 	if req.GetBuildCachePath() == "" {
@@ -198,7 +197,7 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 
 	actualPlatform := buildPlatform
 	otherLibrariesDirs := paths.NewPathList(req.GetLibraries()...)
-	otherLibrariesDirs.Add(configuration.LibrariesDir(s.settings))
+	otherLibrariesDirs.Add(s.settings.LibrariesDir())
 
 	var libsManager *librariesmanager.LibrariesManager
 	if pme.GetProfile() != nil {
@@ -231,9 +230,9 @@ func (s *arduinoCoreServerImpl) Compile(req *rpc.CompileRequest, stream rpc.Ardu
 		coreBuildCachePath,
 		int(req.GetJobs()),
 		req.GetBuildProperties(),
-		configuration.HardwareDirectories(s.settings),
+		s.settings.HardwareDirectories(),
 		otherLibrariesDirs,
-		configuration.IDEBuiltinLibrariesDir(s.settings),
+		s.settings.IDEBuiltinLibrariesDir(),
 		fqbn,
 		req.GetClean(),
 		req.GetSourceOverride(),

diff --git a/commands/service_debug_test.go b/commands/service_debug_test.go
@@ -37,7 +37,7 @@ func TestGetCommandLine(t *testing.T) {
 	sketchPath := paths.New("testdata", "debug", sketch)
 	require.NoError(t, sketchPath.ToAbs())
 
-	pmb := packagemanager.NewBuilder(nil, nil, nil, nil, "test", downloader.GetDefaultConfig())
+	pmb := packagemanager.NewBuilder(nil, nil, nil, nil, nil, "test", downloader.GetDefaultConfig())
 	pmb.LoadHardwareFromDirectory(customHardware)
 	pmb.LoadHardwareFromDirectory(dataDir)
 

diff --git a/commands/service_platform_search_test.go b/commands/service_platform_search_test.go
@@ -19,7 +19,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/arduino/arduino-cli/internal/cli/configuration"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/require"
@@ -36,9 +35,17 @@ func TestPlatformSearch(t *testing.T) {
 	err := paths.New("testdata", "platform", "package_index.json").CopyTo(dataDir.Join("package_index.json"))
 	require.Nil(t, err)
 
-	settings := configuration.Init(paths.TempDir().Join("test", "arduino-cli.yaml").String())
-	srv := NewArduinoCoreServer("", settings)
 	ctx := context.Background()
+	srv := NewArduinoCoreServer()
+
+	conf, err := paths.TempDir().Join("test", "arduino-cli.yaml").ReadFile()
+	require.NoError(t, err)
+	_, err = srv.ConfigurationOpen(ctx, &rpc.ConfigurationOpenRequest{
+		SettingsFormat:  "yaml",
+		EncodedSettings: string(conf),
+	})
+	require.NoError(t, err)
+
 	createResp, err := srv.Create(ctx, &rpc.CreateRequest{})
 	require.NoError(t, err)
 
@@ -337,9 +344,16 @@ func TestPlatformSearchSorting(t *testing.T) {
 	err := paths.New("testdata", "platform", "package_index.json").CopyTo(dataDir.Join("package_index.json"))
 	require.Nil(t, err)
 
-	settings := configuration.Init(paths.TempDir().Join("test", "arduino-cli.yaml").String())
-	srv := NewArduinoCoreServer("", settings)
 	ctx := context.Background()
+	srv := NewArduinoCoreServer()
+
+	conf, err := paths.TempDir().Join("test", "arduino-cli.yaml").ReadFile()
+	require.NoError(t, err)
+	_, err = srv.ConfigurationOpen(ctx, &rpc.ConfigurationOpenRequest{
+		SettingsFormat:  "yaml",
+		EncodedSettings: string(conf),
+	})
+	require.NoError(t, err)
 
 	createResp, err := srv.Create(ctx, &rpc.CreateRequest{})
 	require.NoError(t, err)