From f4d8b57cfe888e9d0adf7a5756b782b21099acf5 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 20 Mar 2020 12:26:58 +0100 Subject: [PATCH 1/5] Library install arguments are no more case sensitive This should make easier to install libraries from command line. --- cli/lib/install.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cli/lib/install.go b/cli/lib/install.go index 5ceacc8bfa5..d708df0670d 100644 --- a/cli/lib/install.go +++ b/cli/lib/install.go @@ -18,6 +18,7 @@ package lib import ( "context" "os" + "strings" "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" @@ -48,6 +49,26 @@ var installFlags struct { noDeps bool } +func adjustLibraryReferenceArgCaseSensitivty(instance *rpc.Instance, libRef *globals.LibraryReferenceArg) { + res, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchReq{ + Instance: instance, + Query: libRef.Name, + }) + if err != nil { + return + } + candidates := []*rpc.SearchedLibrary{} + for _, foundLib := range res.GetLibraries() { + if strings.ToLower(foundLib.GetName()) == strings.ToLower(libRef.Name) { + candidates = append(candidates, foundLib) + } + } + if len(candidates) != 1 { + return + } + libRef.Name = candidates[0].GetName() +} + func runInstallCommand(cmd *cobra.Command, args []string) { instance := instance.CreateInstanceIgnorePlatformIndexErrors() libRefs, err := globals.ParseLibraryReferenceArgs(args) @@ -56,6 +77,10 @@ func runInstallCommand(cmd *cobra.Command, args []string) { os.Exit(errorcodes.ErrBadArgument) } + for _, libRef := range libRefs { + adjustLibraryReferenceArgCaseSensitivty(instance, libRef) + } + toInstall := map[string]*rpc.LibraryDependencyStatus{} if installFlags.noDeps { for _, libRef := range libRefs { From 7f1bf8893a7ddad00adafb6ed8d57d9b084031d4 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 20 Mar 2020 12:33:49 +0100 Subject: [PATCH 2/5] Moved lib args parsing functions in cli/lib --- cli/globals/args.go | 48 --------------------------- cli/globals/args_test.go | 45 ------------------------- cli/lib/args.go | 69 ++++++++++++++++++++++++++++++++++++++ cli/lib/args_test.go | 71 ++++++++++++++++++++++++++++++++++++++++ cli/lib/check_deps.go | 3 +- cli/lib/download.go | 2 +- cli/lib/install.go | 4 +-- cli/lib/uninstall.go | 3 +- 8 files changed, 145 insertions(+), 100 deletions(-) create mode 100644 cli/lib/args.go create mode 100644 cli/lib/args_test.go diff --git a/cli/globals/args.go b/cli/globals/args.go index 7f48142b024..cdfda0f5a77 100644 --- a/cli/globals/args.go +++ b/cli/globals/args.go @@ -86,51 +86,3 @@ func ParseReferenceArg(arg string, parseArch bool) (*ReferenceArg, error) { return ret, nil } - -// LibraryReferenceArg is a command line argument that reference a library. -type LibraryReferenceArg struct { - Name string - Version string -} - -func (r *LibraryReferenceArg) String() string { - if r.Version != "" { - return r.Name + "@" + r.Version - } - return r.Name -} - -// ParseLibraryReferenceArg parse a command line argument that reference a -// library in the form "LibName@Version" or just "LibName". -func ParseLibraryReferenceArg(arg string) (*LibraryReferenceArg, error) { - tokens := strings.SplitN(arg, "@", 2) - - ret := &LibraryReferenceArg{} - // TODO: check library Name constraints - // TODO: check library Version constraints - if tokens[0] == "" { - return nil, fmt.Errorf("invalid empty library name") - } - ret.Name = tokens[0] - if len(tokens) > 1 { - if tokens[1] == "" { - return nil, fmt.Errorf("invalid empty library version: %s", arg) - } - ret.Version = tokens[1] - } - return ret, nil -} - -// ParseLibraryReferenceArgs is a convenient wrapper that operates on a slice of strings and -// calls ParseLibraryReferenceArg for each of them. It returns at the first invalid argument. -func ParseLibraryReferenceArgs(args []string) ([]*LibraryReferenceArg, error) { - ret := []*LibraryReferenceArg{} - for _, arg := range args { - if reference, err := ParseLibraryReferenceArg(arg); err == nil { - ret = append(ret, reference) - } else { - return nil, err - } - } - return ret, nil -} diff --git a/cli/globals/args_test.go b/cli/globals/args_test.go index 89e23866b62..92036a76114 100644 --- a/cli/globals/args_test.go +++ b/cli/globals/args_test.go @@ -31,14 +31,6 @@ var goodCores = []struct { {"arduino:avr@1.6.20", &globals.ReferenceArg{"arduino", "avr", "1.6.20"}}, } -var goodLibs = []struct { - in string - expected *globals.LibraryReferenceArg -}{ - {"mylib", &globals.LibraryReferenceArg{"mylib", ""}}, - {"mylib@1.0", &globals.LibraryReferenceArg{"mylib", "1.0"}}, -} - var badCores = []struct { in string expected *globals.ReferenceArg @@ -53,18 +45,7 @@ var badCores = []struct { {"", nil}, } -var badLibs = []struct { - in string - expected *globals.LibraryReferenceArg -}{ - {"", nil}, - {"mylib@", nil}, -} - func TestArgsStringify(t *testing.T) { - for _, lib := range goodLibs { - require.Equal(t, lib.in, lib.expected.String()) - } for _, core := range goodCores { require.Equal(t, core.in, core.expected.String()) } @@ -84,32 +65,6 @@ func TestParseReferenceArgCores(t *testing.T) { } } -func TestParseReferenceArgLibs(t *testing.T) { - for _, tt := range goodLibs { - actual, err := globals.ParseLibraryReferenceArg(tt.in) - assert.Nil(t, err, "Testing good arg '%s'", tt.in) - assert.Equal(t, tt.expected, actual, "Testing good arg '%s'", tt.in) - } - for _, tt := range badLibs { - res, err := globals.ParseLibraryReferenceArg(tt.in) - require.Nil(t, res, "Testing bad arg '%s'", tt.in) - require.NotNil(t, err, "Testing bad arg '%s'", tt.in) - } -} - -func TestParseLibraryReferenceArgs(t *testing.T) { - args := []string{} - for _, tt := range goodLibs { - args = append(args, tt.in) - } - refs, err := globals.ParseLibraryReferenceArgs(args) - require.Nil(t, err) - require.Len(t, refs, len(goodLibs)) - for i, tt := range goodLibs { - assert.Equal(t, tt.expected, refs[i]) - } -} - func TestParseArgs(t *testing.T) { input := []string{} for _, tt := range goodCores { diff --git a/cli/lib/args.go b/cli/lib/args.go new file mode 100644 index 00000000000..9bba9c154dd --- /dev/null +++ b/cli/lib/args.go @@ -0,0 +1,69 @@ +// This file is part of arduino-cli. +// +// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package lib + +import ( + "fmt" + "strings" +) + +// LibraryReferenceArg is a command line argument that reference a library. +type LibraryReferenceArg struct { + Name string + Version string +} + +func (r *LibraryReferenceArg) String() string { + if r.Version != "" { + return r.Name + "@" + r.Version + } + return r.Name +} + +// ParseLibraryReferenceArg parse a command line argument that reference a +// library in the form "LibName@Version" or just "LibName". +func ParseLibraryReferenceArg(arg string) (*LibraryReferenceArg, error) { + tokens := strings.SplitN(arg, "@", 2) + + ret := &LibraryReferenceArg{} + // TODO: check library Name constraints + // TODO: check library Version constraints + if tokens[0] == "" { + return nil, fmt.Errorf("invalid empty library name") + } + ret.Name = tokens[0] + if len(tokens) > 1 { + if tokens[1] == "" { + return nil, fmt.Errorf("invalid empty library version: %s", arg) + } + ret.Version = tokens[1] + } + return ret, nil +} + +// ParseLibraryReferenceArgs is a convenient wrapper that operates on a slice of strings and +// calls ParseLibraryReferenceArg for each of them. It returns at the first invalid argument. +func ParseLibraryReferenceArgs(args []string) ([]*LibraryReferenceArg, error) { + ret := []*LibraryReferenceArg{} + for _, arg := range args { + if reference, err := ParseLibraryReferenceArg(arg); err == nil { + ret = append(ret, reference) + } else { + return nil, err + } + } + return ret, nil +} diff --git a/cli/lib/args_test.go b/cli/lib/args_test.go new file mode 100644 index 00000000000..68bfbe2813f --- /dev/null +++ b/cli/lib/args_test.go @@ -0,0 +1,71 @@ +// This file is part of arduino-cli. +// +// Copyright 2020 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package lib + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var goodLibs = []struct { + in string + expected *LibraryReferenceArg +}{ + {"mylib", &LibraryReferenceArg{"mylib", ""}}, + {"mylib@1.0", &LibraryReferenceArg{"mylib", "1.0"}}, +} + +var badLibs = []struct { + in string + expected *LibraryReferenceArg +}{ + {"", nil}, + {"mylib@", nil}, +} + +func TestArgsStringify(t *testing.T) { + for _, lib := range goodLibs { + require.Equal(t, lib.in, lib.expected.String()) + } +} + +func TestParseReferenceArgLibs(t *testing.T) { + for _, tt := range goodLibs { + actual, err := ParseLibraryReferenceArg(tt.in) + assert.Nil(t, err, "Testing good arg '%s'", tt.in) + assert.Equal(t, tt.expected, actual, "Testing good arg '%s'", tt.in) + } + for _, tt := range badLibs { + res, err := ParseLibraryReferenceArg(tt.in) + require.Nil(t, res, "Testing bad arg '%s'", tt.in) + require.NotNil(t, err, "Testing bad arg '%s'", tt.in) + } +} + +func TestParseLibraryReferenceArgs(t *testing.T) { + args := []string{} + for _, tt := range goodLibs { + args = append(args, tt.in) + } + refs, err := ParseLibraryReferenceArgs(args) + require.Nil(t, err) + require.Len(t, refs, len(goodLibs)) + for i, tt := range goodLibs { + assert.Equal(t, tt.expected, refs[i]) + } +} diff --git a/cli/lib/check_deps.go b/cli/lib/check_deps.go index 6e028f8101c..74e6f1510a1 100644 --- a/cli/lib/check_deps.go +++ b/cli/lib/check_deps.go @@ -22,7 +22,6 @@ import ( "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" - "github.com/arduino/arduino-cli/cli/globals" "github.com/arduino/arduino-cli/cli/instance" "github.com/arduino/arduino-cli/commands/lib" rpc "github.com/arduino/arduino-cli/rpc/commands" @@ -45,7 +44,7 @@ func initDepsCommand() *cobra.Command { } func runDepsCommand(cmd *cobra.Command, args []string) { - libRef, err := globals.ParseLibraryReferenceArg(args[0]) + libRef, err := ParseLibraryReferenceArg(args[0]) if err != nil { feedback.Errorf("Arguments error: %v", err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/lib/download.go b/cli/lib/download.go index fed820e86a1..60cd3299efe 100644 --- a/cli/lib/download.go +++ b/cli/lib/download.go @@ -45,7 +45,7 @@ func initDownloadCommand() *cobra.Command { func runDownloadCommand(cmd *cobra.Command, args []string) { instance := instance.CreateInstanceIgnorePlatformIndexErrors() - refs, err := globals.ParseLibraryReferenceArgs(args) + refs, err := ParseLibraryReferenceArgs(args) if err != nil { feedback.Errorf("Invalid argument passed: %v", err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/lib/install.go b/cli/lib/install.go index d708df0670d..22b141d8370 100644 --- a/cli/lib/install.go +++ b/cli/lib/install.go @@ -49,7 +49,7 @@ var installFlags struct { noDeps bool } -func adjustLibraryReferenceArgCaseSensitivty(instance *rpc.Instance, libRef *globals.LibraryReferenceArg) { +func adjustLibraryReferenceArgCaseSensitivty(instance *rpc.Instance, libRef *LibraryReferenceArg) { res, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchReq{ Instance: instance, Query: libRef.Name, @@ -71,7 +71,7 @@ func adjustLibraryReferenceArgCaseSensitivty(instance *rpc.Instance, libRef *glo func runInstallCommand(cmd *cobra.Command, args []string) { instance := instance.CreateInstanceIgnorePlatformIndexErrors() - libRefs, err := globals.ParseLibraryReferenceArgs(args) + libRefs, err := ParseLibraryReferenceArgs(args) if err != nil { feedback.Errorf("Arguments error: %v", err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/lib/uninstall.go b/cli/lib/uninstall.go index baf840c7fdf..608310c2205 100644 --- a/cli/lib/uninstall.go +++ b/cli/lib/uninstall.go @@ -21,7 +21,6 @@ import ( "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" - "github.com/arduino/arduino-cli/cli/globals" "github.com/arduino/arduino-cli/cli/instance" "github.com/arduino/arduino-cli/cli/output" "github.com/arduino/arduino-cli/commands/lib" @@ -46,7 +45,7 @@ func runUninstallCommand(cmd *cobra.Command, args []string) { logrus.Info("Executing `arduino lib uninstall`") instance := instance.CreateInstanceIgnorePlatformIndexErrors() - refs, err := globals.ParseLibraryReferenceArgs(args) + refs, err := ParseLibraryReferenceArgs(args) if err != nil { feedback.Errorf("Invalid argument passed: %v", err) os.Exit(errorcodes.ErrBadArgument) From 0b1257a298f0a5802cccbb40a2f678fd4ff3724c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 20 Mar 2020 13:49:08 +0100 Subject: [PATCH 3/5] Factored ParseLibraryReferenceArgAndAdjustCase function Now the cli/lib module uses ParseLibraryReferenceArgAndAdjustCase so the cose-insensitive argument is allowed in all lib commands. --- cli/lib/args.go | 42 ++++++++++++++++++++++++++++++++++++++++++ cli/lib/check_deps.go | 4 ++-- cli/lib/download.go | 2 +- cli/lib/install.go | 27 +-------------------------- cli/lib/uninstall.go | 2 +- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/cli/lib/args.go b/cli/lib/args.go index 9bba9c154dd..113f440a705 100644 --- a/cli/lib/args.go +++ b/cli/lib/args.go @@ -16,8 +16,12 @@ package lib import ( + "context" "fmt" "strings" + + "github.com/arduino/arduino-cli/commands/lib" + rpc "github.com/arduino/arduino-cli/rpc/commands" ) // LibraryReferenceArg is a command line argument that reference a library. @@ -67,3 +71,41 @@ func ParseLibraryReferenceArgs(args []string) ([]*LibraryReferenceArg, error) { } return ret, nil } + +// ParseLibraryReferenceArgAndAdjustCase parse a command line argument that reference a +// library and possibly adjust the case of the name to match a library in the index +func ParseLibraryReferenceArgAndAdjustCase(instance *rpc.Instance, arg string) (*LibraryReferenceArg, error) { + libRef, err := ParseLibraryReferenceArg(arg) + res, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchReq{ + Instance: instance, + Query: libRef.Name, + }) + if err != nil { + return nil, err + } + + candidates := []*rpc.SearchedLibrary{} + for _, foundLib := range res.GetLibraries() { + if strings.ToLower(foundLib.GetName()) == strings.ToLower(libRef.Name) { + candidates = append(candidates, foundLib) + } + } + if len(candidates) == 1 { + libRef.Name = candidates[0].GetName() + } + return libRef, nil +} + +// ParseLibraryReferenceArgsAndAdjustCase is a convenient wrapper that operates on a slice of +// strings and calls ParseLibraryReferenceArgAndAdjustCase for each of them. It returns at the first invalid argument. +func ParseLibraryReferenceArgsAndAdjustCase(instance *rpc.Instance, args []string) ([]*LibraryReferenceArg, error) { + ret := []*LibraryReferenceArg{} + for _, arg := range args { + if reference, err := ParseLibraryReferenceArgAndAdjustCase(instance, arg); err == nil { + ret = append(ret, reference) + } else { + return nil, err + } + } + return ret, nil +} diff --git a/cli/lib/check_deps.go b/cli/lib/check_deps.go index 74e6f1510a1..de797162255 100644 --- a/cli/lib/check_deps.go +++ b/cli/lib/check_deps.go @@ -44,13 +44,13 @@ func initDepsCommand() *cobra.Command { } func runDepsCommand(cmd *cobra.Command, args []string) { - libRef, err := ParseLibraryReferenceArg(args[0]) + instance := instance.CreateInstanceIgnorePlatformIndexErrors() + libRef, err := ParseLibraryReferenceArgAndAdjustCase(instance, args[0]) if err != nil { feedback.Errorf("Arguments error: %v", err) os.Exit(errorcodes.ErrBadArgument) } - instance := instance.CreateInstanceIgnorePlatformIndexErrors() deps, err := lib.LibraryResolveDependencies(context.Background(), &rpc.LibraryResolveDependenciesReq{ Instance: instance, Name: libRef.Name, diff --git a/cli/lib/download.go b/cli/lib/download.go index 60cd3299efe..b1d1e012f5b 100644 --- a/cli/lib/download.go +++ b/cli/lib/download.go @@ -45,7 +45,7 @@ func initDownloadCommand() *cobra.Command { func runDownloadCommand(cmd *cobra.Command, args []string) { instance := instance.CreateInstanceIgnorePlatformIndexErrors() - refs, err := ParseLibraryReferenceArgs(args) + refs, err := ParseLibraryReferenceArgsAndAdjustCase(instance, args) if err != nil { feedback.Errorf("Invalid argument passed: %v", err) os.Exit(errorcodes.ErrBadArgument) diff --git a/cli/lib/install.go b/cli/lib/install.go index 22b141d8370..906099983e9 100644 --- a/cli/lib/install.go +++ b/cli/lib/install.go @@ -18,7 +18,6 @@ package lib import ( "context" "os" - "strings" "github.com/arduino/arduino-cli/cli/errorcodes" "github.com/arduino/arduino-cli/cli/feedback" @@ -49,38 +48,14 @@ var installFlags struct { noDeps bool } -func adjustLibraryReferenceArgCaseSensitivty(instance *rpc.Instance, libRef *LibraryReferenceArg) { - res, err := lib.LibrarySearch(context.Background(), &rpc.LibrarySearchReq{ - Instance: instance, - Query: libRef.Name, - }) - if err != nil { - return - } - candidates := []*rpc.SearchedLibrary{} - for _, foundLib := range res.GetLibraries() { - if strings.ToLower(foundLib.GetName()) == strings.ToLower(libRef.Name) { - candidates = append(candidates, foundLib) - } - } - if len(candidates) != 1 { - return - } - libRef.Name = candidates[0].GetName() -} - func runInstallCommand(cmd *cobra.Command, args []string) { instance := instance.CreateInstanceIgnorePlatformIndexErrors() - libRefs, err := ParseLibraryReferenceArgs(args) + libRefs, err := ParseLibraryReferenceArgsAndAdjustCase(instance, args) if err != nil { feedback.Errorf("Arguments error: %v", err) os.Exit(errorcodes.ErrBadArgument) } - for _, libRef := range libRefs { - adjustLibraryReferenceArgCaseSensitivty(instance, libRef) - } - toInstall := map[string]*rpc.LibraryDependencyStatus{} if installFlags.noDeps { for _, libRef := range libRefs { diff --git a/cli/lib/uninstall.go b/cli/lib/uninstall.go index 608310c2205..31437e02189 100644 --- a/cli/lib/uninstall.go +++ b/cli/lib/uninstall.go @@ -45,7 +45,7 @@ func runUninstallCommand(cmd *cobra.Command, args []string) { logrus.Info("Executing `arduino lib uninstall`") instance := instance.CreateInstanceIgnorePlatformIndexErrors() - refs, err := ParseLibraryReferenceArgs(args) + refs, err := ParseLibraryReferenceArgsAndAdjustCase(instance, args) if err != nil { feedback.Errorf("Invalid argument passed: %v", err) os.Exit(errorcodes.ErrBadArgument) From ea29bf04802167fbd8ab079ff09df4a19844f50e Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 20 Mar 2020 14:05:23 +0100 Subject: [PATCH 4/5] Added test for case sensitiveness in cli/lib params --- test/test_lib.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/test_lib.py b/test/test_lib.py index 39d4b15571e..a03f5dc13d9 100644 --- a/test/test_lib.py +++ b/test/test_lib.py @@ -92,6 +92,13 @@ def test_uninstall_spaces(run_command): assert result.ok assert len(json.loads(result.stdout)) == 0 +def test_lib_ops_caseinsensitive(run_command): + key = 'pcm' + assert run_command("lib install {}".format(key)) + assert run_command("lib uninstall {}".format(key)) + result = run_command("lib list --format json") + assert result.ok + assert len(json.loads(result.stdout)) == 0 def test_search(run_command): assert run_command("lib update-index") From a71fdff7c2f9b9a5b8c8c79479363886fc5d001c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 8 Apr 2020 17:02:15 +0200 Subject: [PATCH 5/5] Update test/test_lib.py Co-Authored-By: Roberto Sora --- test/test_lib.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_lib.py b/test/test_lib.py index a03f5dc13d9..4dcc6f5eb29 100644 --- a/test/test_lib.py +++ b/test/test_lib.py @@ -93,6 +93,21 @@ def test_uninstall_spaces(run_command): assert len(json.loads(result.stdout)) == 0 def test_lib_ops_caseinsensitive(run_command): + """ + This test is supposed to (un)install the following library, + As you can see the name is all caps: + + Name: "PCM" + Author: David Mellis , Michael Smith + Maintainer: David Mellis + Sentence: Playback of short audio samples. + Paragraph: These samples are encoded directly in the Arduino sketch as an array of numbers. + Website: http://highlowtech.org/?p=1963 + Category: Signal Input/Output + Architecture: avr + Types: Contributed + Versions: [1.0.0] + """ key = 'pcm' assert run_command("lib install {}".format(key)) assert run_command("lib uninstall {}".format(key))