diff --git a/arduino/errors.go b/arduino/errors.go index f5c9356760d..379e69e19b9 100644 --- a/arduino/errors.go +++ b/arduino/errors.go @@ -17,6 +17,7 @@ package arduino import ( "fmt" + "strings" "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/i18n" @@ -715,3 +716,24 @@ func (e *SignatureVerificationFailedError) Unwrap() error { func (e *SignatureVerificationFailedError) ToRPCStatus() *status.Status { return status.New(codes.Unavailable, e.Error()) } + +// MultiplePlatformsError is returned when trying to detect +// the Platform the user is trying to interact with and +// and multiple results are found. +type MultiplePlatformsError struct { + Platforms []string + UserPlatform string +} + +func (e *MultiplePlatformsError) Error() string { + return tr("Found %d platform for reference \"%s\":\n%s", + len(e.Platforms), + e.UserPlatform, + strings.Join(e.Platforms, "\n"), + ) +} + +// ToRPCStatus converts the error into a *status.Status +func (e *MultiplePlatformsError) ToRPCStatus() *status.Status { + return status.New(codes.InvalidArgument, e.Error()) +} diff --git a/cli/arguments/reference.go b/cli/arguments/reference.go index 96371b4726b..b06a3ef52c5 100644 --- a/cli/arguments/reference.go +++ b/cli/arguments/reference.go @@ -18,6 +18,12 @@ package arguments import ( "fmt" "strings" + + "github.com/arduino/arduino-cli/arduino" + "github.com/arduino/arduino-cli/cli/instance" + "github.com/arduino/arduino-cli/commands/core" + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" + "github.com/sirupsen/logrus" ) // Reference represents a reference item (core or library) passed to the CLI @@ -37,10 +43,10 @@ func (r *Reference) String() string { // ParseReferences is a convenient wrapper that operates on a slice of strings and // calls ParseReference for each of them. It returns at the first invalid argument. -func ParseReferences(args []string, parseArch bool) ([]*Reference, error) { +func ParseReferences(args []string) ([]*Reference, error) { ret := []*Reference{} for _, arg := range args { - reference, err := ParseReference(arg, parseArch) + reference, err := ParseReference(arg) if err != nil { return nil, err } @@ -49,10 +55,13 @@ func ParseReferences(args []string, parseArch bool) ([]*Reference, error) { return ret, nil } -// ParseReference parses a string and returns a Reference object. If `parseArch` is passed, -// the method also tries to parse the architecture bit, i.e. string must be in the form -// "packager:arch@version", useful to represent a platform (or core) name. -func ParseReference(arg string, parseArch bool) (*Reference, error) { +// ParseReference parses a string and returns a Reference object. +// It tries to infer the platform the user is asking for. +// To achieve that, it tries to use github.com/arduino/arduino-cli/commands/core.GetPlatform +// Note that the Reference is returned rightaway if the arg inserted by the user matches perfectly one in the response of core.GetPlatform +// A MultiplePlatformsError is returned if the platform searched by the user matches multiple platforms +func ParseReference(arg string) (*Reference, error) { + logrus.Infof("Parsing reference %s", arg) ret := &Reference{} if arg == "" { return nil, fmt.Errorf(tr("invalid empty core argument")) @@ -69,20 +78,49 @@ func ParseReference(arg string, parseArch bool) (*Reference, error) { ret.Version = toks[1] } - if parseArch { - toks = strings.Split(ret.PackageName, ":") - if len(toks) != 2 { - return nil, fmt.Errorf(tr("invalid item %s"), arg) + toks = strings.Split(ret.PackageName, ":") + if len(toks) != 2 { + return nil, fmt.Errorf(tr("invalid item %s"), arg) + } + if toks[0] == "" { + return nil, fmt.Errorf(tr("invalid empty core name '%s'"), arg) + } + ret.PackageName = toks[0] + if toks[1] == "" { + return nil, fmt.Errorf(tr("invalid empty core architecture '%s'"), arg) + } + ret.Architecture = toks[1] + + // Now that we have the required informations in `ret` we can + // try to use core.GetPlatforms to optimize what the user typed + // (by replacing the PackageName and Architecture in ret with the content of core.GetPlatform()) + platforms, _ := core.GetPlatforms(&rpc.PlatformListRequest{ + Instance: instance.CreateAndInit(), + UpdatableOnly: false, + All: true, // this is true because we want also the installable platforms + }) + foundPlatforms := []string{} + for _, platform := range platforms { + platformID := platform.GetId() + platformUser := ret.PackageName + ":" + ret.Architecture + // At first we check if the platform the user is searching for matches an available one, + // this way we do not need to adapt the casing and we can return it directly + if platformUser == platformID { + return ret, nil } - if toks[0] == "" { - return nil, fmt.Errorf(tr("invalid empty core name '%s'"), arg) + if strings.EqualFold(platformUser, platformID) { + logrus.Infof("Found possible match for reference %s -> %s", platformUser, platformID) + toks = strings.Split(platformID, ":") + foundPlatforms = append(foundPlatforms, platformID) } + } + // replace the returned Reference only if only one occurrence is found, + // otherwise return an error to the user because we don't know on which platform operate + if len(foundPlatforms) == 1 { ret.PackageName = toks[0] - if toks[1] == "" { - return nil, fmt.Errorf(tr("invalid empty core architecture '%s'"), arg) - } ret.Architecture = toks[1] + } else { + return nil, &arduino.MultiplePlatformsError{Platforms: foundPlatforms, UserPlatform: arg} } - return ret, nil } diff --git a/cli/arguments/reference_test.go b/cli/arguments/reference_test.go index 58d9a6d1167..9895ae89e90 100644 --- a/cli/arguments/reference_test.go +++ b/cli/arguments/reference_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/arduino/arduino-cli/cli/arguments" + "github.com/arduino/arduino-cli/configuration" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -45,6 +46,10 @@ var badCores = []struct { {"", nil}, } +func init() { + configuration.Settings = configuration.Init("") +} + func TestArgsStringify(t *testing.T) { for _, core := range goodCores { require.Equal(t, core.in, core.expected.String()) @@ -53,13 +58,13 @@ func TestArgsStringify(t *testing.T) { func TestParseReferenceCores(t *testing.T) { for _, tt := range goodCores { - actual, err := arguments.ParseReference(tt.in, true) + actual, err := arguments.ParseReference(tt.in) assert.Nil(t, err) assert.Equal(t, tt.expected, actual) } for _, tt := range badCores { - actual, err := arguments.ParseReference(tt.in, true) + actual, err := arguments.ParseReference(tt.in) require.NotNil(t, err, "Testing bad core '%s'", tt.in) require.Equal(t, tt.expected, actual, "Testing bad core '%s'", tt.in) } @@ -71,7 +76,7 @@ func TestParseArgs(t *testing.T) { input = append(input, tt.in) } - refs, err := arguments.ParseReferences(input, true) + refs, err := arguments.ParseReferences(input) assert.Nil(t, err) assert.Equal(t, len(goodCores), len(refs)) diff --git a/cli/core/download.go b/cli/core/download.go index d1a396d0dac..7b45a502313 100644 --- a/cli/core/download.go +++ b/cli/core/download.go @@ -53,7 +53,7 @@ func runDownloadCommand(cmd *cobra.Command, args []string) { logrus.Info("Executing `arduino-cli core download`") - platformsRefs, err := arguments.ParseReferences(args, true) + platformsRefs, err := arguments.ParseReferences(args) if err != nil { feedback.Errorf(tr("Invalid argument passed: %v"), err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/core/install.go b/cli/core/install.go index 6b69275aa86..ce107ccdb41 100644 --- a/cli/core/install.go +++ b/cli/core/install.go @@ -61,7 +61,7 @@ func runInstallCommand(cmd *cobra.Command, args []string) { inst := instance.CreateAndInit() logrus.Info("Executing `arduino-cli core install`") - platformsRefs, err := arguments.ParseReferences(args, true) + platformsRefs, err := arguments.ParseReferences(args) if err != nil { feedback.Errorf(tr("Invalid argument passed: %v"), err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/core/uninstall.go b/cli/core/uninstall.go index 9c80536298a..c7ce20835f0 100644 --- a/cli/core/uninstall.go +++ b/cli/core/uninstall.go @@ -50,7 +50,7 @@ func runUninstallCommand(cmd *cobra.Command, args []string) { inst := instance.CreateAndInit() logrus.Info("Executing `arduino-cli core uninstall`") - platformsRefs, err := arguments.ParseReferences(args, true) + platformsRefs, err := arguments.ParseReferences(args) if err != nil { feedback.Errorf(tr("Invalid argument passed: %v"), err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/core/upgrade.go b/cli/core/upgrade.go index 0401849fa9f..905dee4e040 100644 --- a/cli/core/upgrade.go +++ b/cli/core/upgrade.go @@ -76,7 +76,7 @@ func runUpgradeCommand(cmd *cobra.Command, args []string) { // proceed upgrading, if anything is upgradable exitErr := false - platformsRefs, err := arguments.ParseReferences(args, true) + platformsRefs, err := arguments.ParseReferences(args) if err != nil { feedback.Errorf(tr("Invalid argument passed: %v"), err) os.Exit(errorcodes.ErrBadArgument) diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index fd8747bd1bc..17a720a5f53 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -2,6 +2,19 @@ Here you can find a list of migration guides to handle breaking changes between releases of the CLI. +## Unreleased + +### `github.com/arduino/arduino-cli/cli/arguments.ParseReferences` function change + +The `parseArch` parameter was removed since it was unused and was always true. This means that the architecture gets +always parsed by the function. + +### `github.com/arduino/arduino-cli/cli/arguments.ParseReference` function change + +The `parseArch` parameter was removed since it was unused and was always true. This means that the architecture gets +always parsed by the function. Furthermore the function now should also correctly interpret `packager:arch` spelled with +the wrong casing. + ## 0.20.0 ### `board details` arguments change diff --git a/test/test_core.py b/test/test_core.py index 5e5ece7075a..6cb9e01f10a 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -240,6 +240,10 @@ def test_core_download(run_command, downloads_dir): result = run_command(["core", "download", "bananas:avr"]) assert result.failed + # Wrong casing + result = run_command(["core", "download", "Arduino:Samd@1.8.12"]) + assert os.path.exists(os.path.join(downloads_dir, "packages", "core-ArduinoCore-samd-1.8.12.tar.bz2")) + def _in(jsondata, name, version=None): installed_cores = json.loads(jsondata) @@ -685,6 +689,46 @@ def test_core_list_platform_without_platform_txt(run_command, data_dir): assert core["name"] == "some-packager-some-arch" +@pytest.mark.skipif( + platform.system() in ["Darwin", "Windows"], + reason="macOS by default is case insensitive https://github.com/actions/virtual-environments/issues/865 " + + "Windows too is case insensitive" + + "https://stackoverflow.com/questions/7199039/file-paths-in-windows-environment-not-case-sensitive", +) +def test_core_download_multiple_platforms(run_command, data_dir): + assert run_command(["update"]) + + # Verifies no core is installed + res = run_command(["core", "list", "--format", "json"]) + assert res.ok + cores = json.loads(res.stdout) + assert len(cores) == 0 + + # Simulates creation of two new cores in the sketchbook hardware folder + test_boards_txt = Path(__file__).parent / "testdata" / "boards.local.txt" + boards_txt = Path(data_dir, "packages", "PACKAGER", "hardware", "ARCH", "1.0.0", "boards.txt") + boards_txt.parent.mkdir(parents=True, exist_ok=True) + boards_txt.touch() + assert boards_txt.write_bytes(test_boards_txt.read_bytes()) + + boards_txt1 = Path(data_dir, "packages", "packager", "hardware", "arch", "1.0.0", "boards.txt") + boards_txt1.parent.mkdir(parents=True, exist_ok=True) + boards_txt1.touch() + assert boards_txt1.write_bytes(test_boards_txt.read_bytes()) + + # Verifies the two cores are detected + res = run_command(["core", "list", "--format", "json"]) + assert res.ok + cores = json.loads(res.stdout) + assert len(cores) == 2 + + # Try to do an operation on the fake cores. + # The cli should not allow it since optimizing the casing results in finding two cores + res = run_command(["core", "upgrade", "Packager:Arch"]) + assert res.failed + assert "Invalid argument passed: Found 2 platform for reference" in res.stderr + + def test_core_with_wrong_custom_board_options_is_loaded(run_command, data_dir): test_platform_name = "platform_with_wrong_custom_board_options" platform_install_dir = Path(data_dir, "hardware", "arduino-beta-dev", test_platform_name)