From 64d019d22429b492535d852405b78697e2712bcf Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 27 Jun 2023 15:16:51 +0200 Subject: [PATCH] [breaking] Do not prevent CLI startup if `inventory.yaml` is corrupted (#2232) * Made `inventory` package private. * If an error occurs reading the inventory.xml just log it and replace it. * Added integration test --- commands/board/list.go | 2 +- commands/compile/compile.go | 2 +- docs/UPGRADING.md | 4 ++++ internal/cli/cli.go | 2 +- internal/cli/updater/updater.go | 2 +- internal/integrationtest/board/board_test.go | 18 ++++++++++++++++ .../inventory}/inventory.go | 21 +++++++++---------- 7 files changed, 36 insertions(+), 15 deletions(-) rename {inventory => internal/inventory}/inventory.go (87%) diff --git a/commands/board/list.go b/commands/board/list.go index 2b3ff3f3893..5098ced4cfd 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -32,7 +32,7 @@ import ( "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/arduino/httpclient" "github.com/arduino/arduino-cli/commands" - "github.com/arduino/arduino-cli/inventory" + "github.com/arduino/arduino-cli/internal/inventory" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" diff --git a/commands/compile/compile.go b/commands/compile/compile.go index 57e72a6dc60..ba904fddad0 100644 --- a/commands/compile/compile.go +++ b/commands/compile/compile.go @@ -31,7 +31,7 @@ import ( "github.com/arduino/arduino-cli/commands" "github.com/arduino/arduino-cli/configuration" "github.com/arduino/arduino-cli/i18n" - "github.com/arduino/arduino-cli/inventory" + "github.com/arduino/arduino-cli/internal/inventory" "github.com/arduino/arduino-cli/legacy/builder" "github.com/arduino/arduino-cli/legacy/builder/types" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 90e2ed3fa1c..31490513af2 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -4,6 +4,10 @@ Here you can find a list of migration guides to handle breaking changes between ## 0.34.0 +### golang package `github.com/arduino/arduino-cli/inventory` removed from public API + +The package `inventory` is no more a public golang API. + ### `board list --watch` command JSON output has changed `board list --watch` command JSON output changed from: diff --git a/internal/cli/cli.go b/internal/cli/cli.go index c7f6753182f..4f0e98c28bd 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -43,7 +43,7 @@ import ( "github.com/arduino/arduino-cli/internal/cli/upgrade" "github.com/arduino/arduino-cli/internal/cli/upload" "github.com/arduino/arduino-cli/internal/cli/version" - "github.com/arduino/arduino-cli/inventory" + "github.com/arduino/arduino-cli/internal/inventory" versioninfo "github.com/arduino/arduino-cli/version" "github.com/fatih/color" "github.com/mattn/go-colorable" diff --git a/internal/cli/updater/updater.go b/internal/cli/updater/updater.go index 09b9ab18125..bd779b16af1 100644 --- a/internal/cli/updater/updater.go +++ b/internal/cli/updater/updater.go @@ -25,7 +25,7 @@ import ( "github.com/arduino/arduino-cli/configuration" "github.com/arduino/arduino-cli/i18n" "github.com/arduino/arduino-cli/internal/cli/feedback" - "github.com/arduino/arduino-cli/inventory" + "github.com/arduino/arduino-cli/internal/inventory" "github.com/arduino/arduino-cli/version" "github.com/fatih/color" semver "go.bug.st/relaxed-semver" diff --git a/internal/integrationtest/board/board_test.go b/internal/integrationtest/board/board_test.go index cf86506853e..daf4e948bbd 100644 --- a/internal/integrationtest/board/board_test.go +++ b/internal/integrationtest/board/board_test.go @@ -589,3 +589,21 @@ func TestBoardListWithFailedBuiltinInstallation(t *testing.T) { require.Empty(t, stderr) require.Contains(t, string(stdout), "Downloading missing tool builtin:serial-discovery") } + +func TestCLIStartupWithCorruptedInventory(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + _, _, err := cli.Run("core", "update-index") + require.NoError(t, err) + + f, err := cli.DataDir().Join("inventory.yaml").Append() + require.NoError(t, err) + _, err = f.WriteString(`data: '[{"name":"WCH;32?'","fqbn":"esp32:esp32:esp32s3camlcd"}]'`) + require.NoError(t, err) + + // the CLI should not be able to load inventory and report it to the logs + _, stderr, err := cli.Run("core", "update-index", "-v") + require.NoError(t, err) + require.Contains(t, string(stderr), "Error loading inventory store") +} diff --git a/inventory/inventory.go b/internal/inventory/inventory.go similarity index 87% rename from inventory/inventory.go rename to internal/inventory/inventory.go index 8411a58fc2c..ff7aab2c549 100644 --- a/inventory/inventory.go +++ b/internal/inventory/inventory.go @@ -23,6 +23,7 @@ import ( "github.com/arduino/arduino-cli/i18n" "github.com/gofrs/uuid" + "github.com/sirupsen/logrus" "github.com/spf13/viper" ) @@ -46,17 +47,15 @@ func Init(configPath string) error { Store.AddConfigPath(configPath) // Attempt to read config file if err := Store.ReadInConfig(); err != nil { - // ConfigFileNotFoundError is acceptable, anything else - // should be reported to the user - if _, ok := err.(viper.ConfigFileNotFoundError); ok { - if err := generateInstallationData(); err != nil { - return err - } - if err := WriteStore(); err != nil { - return err - } - } else { - return fmt.Errorf(tr("reading inventory file: %w"), err) + if _, ok := err.(viper.ConfigFileNotFoundError); !ok { + // If an error occurs during initalization of the store, just log it and recreate it from scratch. + logrus.WithError(err).Error("Error loading inventory store") + } + if err := generateInstallationData(); err != nil { + return err + } + if err := WriteStore(); err != nil { + return err } }