Library update/install with --no-overwrite will perform the update if it's possible to keep already installed dependencies at their current version (#2431)

cmaglie · web-flow · commit 0381aa57bda9 · 2023-11-27T15:34:31.000+01:00
* Updated semver library

* Improved behaviour of 'lib install --no-overwrite' flag

Previously the --no-overwrite flag would fail to install if a library
dependency was already installed but not at the latest version.
After this change the already present library may be accepted as a
dependency if it match the version constraints of the installed library.

* Fixed integration test

* Added integration test

* Always use 'installed' version if available

* Allow a bit more time for slow CI
diff --git a/.licenses/go/go.bug.st/relaxed-semver.dep.yml b/.licenses/go/go.bug.st/relaxed-semver.dep.yml
@@ -1,6 +1,6 @@
 ---
 name: go.bug.st/relaxed-semver
-version: v0.11.0
+version: v0.12.0
 type: go
 summary: 
 homepage: https://pkg.go.dev/go.bug.st/relaxed-semver
diff --git a/arduino/libraries/librariesindex/index.go b/arduino/libraries/librariesindex/index.go
@@ -44,7 +44,7 @@ type Library struct {
 type Release struct {
 	Author           string
 	Version          *semver.Version
-	Dependencies     []semver.Dependency
+	Dependencies     []*Dependency
 	Maintainer       string
 	Sentence         string
 	Paragraph        string
@@ -85,7 +85,7 @@ func (r *Release) GetVersion() *semver.Version {
 }
 
 // GetDependencies returns the dependencies of this library.
-func (r *Release) GetDependencies() []semver.Dependency {
+func (r *Release) GetDependencies() []*Dependency {
 	return r.Dependencies
 }
 
@@ -144,31 +144,31 @@ func (idx *Index) FindLibraryUpdate(lib *libraries.Library) *Release {
 	return nil
 }
 
-// ResolveDependencies returns the dependencies of a library release.
-func (idx *Index) ResolveDependencies(lib *Release) []*Release {
-	// Box lib index *Release to be digested by dep-resolver
-	// (TODO: There is a better use of golang interfaces to avoid this?)
-	allReleases := map[string]semver.Releases{}
-	for _, indexLib := range idx.Libraries {
-		releases := semver.Releases{}
+// ResolveDependencies resolve the dependencies of a library release and returns a
+// possible solution (the set of library releases to install together with the library).
+// An optional "override" releases may be passed if we want to exclude the same
+// libraries from the index (for example if we want to keep an installed library).
+func (idx *Index) ResolveDependencies(lib *Release, overrides []*Release) []*Release {
+	resolver := semver.NewResolver[*Release, *Dependency]()
+
+	overridden := map[string]bool{}
+	for _, override := range overrides {
+		resolver.AddRelease(override)
+		overridden[override.GetName()] = true
+	}
+
+	// Create and populate the library resolver
+	for libName, indexLib := range idx.Libraries {
+		if _, ok := overridden[libName]; ok {
+			continue
+		}
 		for _, indexLibRelease := range indexLib.Releases {
-			releases = append(releases, indexLibRelease)
+			resolver.AddRelease(indexLibRelease)
 		}
-		allReleases[indexLib.Name] = releases
 	}
 
 	// Perform lib resolution
-	archive := &semver.Archive{
-		Releases: allReleases,
-	}
-	deps := archive.Resolve(lib)
-
-	// Unbox resolved deps back into *Release
-	res := []*Release{}
-	for _, dep := range deps {
-		res = append(res, dep.(*Release))
-	}
-	return res
+	return resolver.Resolve(lib)
 }
 
 // Versions returns an array of all versions available of the library
diff --git a/arduino/libraries/librariesindex/index_test.go b/arduino/libraries/librariesindex/index_test.go
@@ -90,7 +90,7 @@ func TestIndexer(t *testing.T) {
 	rtcInexistent2 := index.FindLibraryUpdate(&libraries.Library{Name: "RTCZero-blah", Version: semver.MustParse("1.0.0")})
 	require.Nil(t, rtcInexistent2)
 
-	resolve1 := index.ResolveDependencies(alp.Releases["1.2.1"])
+	resolve1 := index.ResolveDependencies(alp.Releases["1.2.1"], nil)
 	require.Len(t, resolve1, 2)
 	require.Contains(t, resolve1, alp.Releases["1.2.1"])
 	require.Contains(t, resolve1, rtc.Releases["1.6.0"])
@@ -108,7 +108,7 @@ func TestIndexer(t *testing.T) {
 	require.NotNil(t, http040)
 	require.Equal(t, "ArduinoHttpClient@0.4.0", http040.String())
 
-	resolve2 := index.ResolveDependencies(oauth010)
+	resolve2 := index.ResolveDependencies(oauth010, nil)
 	require.Len(t, resolve2, 4)
 	require.Contains(t, resolve2, oauth010)
 	require.Contains(t, resolve2, eccx135)
diff --git a/arduino/libraries/librariesindex/json.go b/arduino/libraries/librariesindex/json.go
@@ -126,8 +126,8 @@ func (indexLib *indexRelease) extractReleaseIn(library *Library) {
 	}
 }
 
-func (indexLib *indexRelease) extractDependencies() []semver.Dependency {
-	res := []semver.Dependency{}
+func (indexLib *indexRelease) extractDependencies() []*Dependency {
+	res := []*Dependency{}
 	if indexLib.Dependencies == nil || len(indexLib.Dependencies) == 0 {
 		return res
 	}
diff --git a/arduino/libraries/librariesmanager/librariesmanager.go b/arduino/libraries/librariesmanager/librariesmanager.go
@@ -225,6 +225,20 @@ func (lm *LibrariesManager) FindByReference(libRef *librariesindex.Reference, in
 	return alternatives.FilterByVersionAndInstallLocation(libRef.Version, installLocation)
 }
 
+// FindAllInstalled returns all the installed libraries
+func (lm *LibrariesManager) FindAllInstalled() libraries.List {
+	var res libraries.List
+	for _, libAlternatives := range lm.Libraries {
+		for _, libRelease := range libAlternatives {
+			if libRelease.InstallDir == nil {
+				continue
+			}
+			res.Add(libRelease)
+		}
+	}
+	return res
+}
+
 func (lm *LibrariesManager) clearLibraries() {
 	for k := range lm.Libraries {
 		delete(lm.Libraries, k)
diff --git a/commands/lib/install.go b/commands/lib/install.go
@@ -47,9 +47,10 @@ func LibraryInstall(ctx context.Context, req *rpc.LibraryInstallRequest, downloa
 		}
 	} else {
 		res, err := LibraryResolveDependencies(ctx, &rpc.LibraryResolveDependenciesRequest{
-			Instance: req.GetInstance(),
-			Name:     req.GetName(),
-			Version:  req.GetVersion(),
+			Instance:                      req.GetInstance(),
+			Name:                          req.GetName(),
+			Version:                       req.GetVersion(),
+			DoNotUpdateInstalledLibraries: req.GetNoOverwrite(),
 		})
 		if err != nil {
 			return err
diff --git a/commands/lib/resolve_deps.go b/commands/lib/resolve_deps.go
@@ -22,8 +22,10 @@ import (
 
 	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/libraries"
+	"github.com/arduino/arduino-cli/arduino/libraries/librariesindex"
 	"github.com/arduino/arduino-cli/commands/internal/instances"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	semver "go.bug.st/relaxed-semver"
 )
 
 // LibraryResolveDependencies FIXMEDOC
@@ -46,7 +48,21 @@ func LibraryResolveDependencies(ctx context.Context, req *rpc.LibraryResolveDepe
 	}
 
 	// Resolve all dependencies...
-	deps := lm.Index.ResolveDependencies(reqLibRelease)
+	var overrides []*librariesindex.Release
+	if req.GetDoNotUpdateInstalledLibraries() {
+		libs := lm.FindAllInstalled()
+		libs = libs.FilterByVersionAndInstallLocation(nil, libraries.User)
+		for _, lib := range libs {
+			release := lm.Index.FindRelease(&librariesindex.Reference{
+				Name:    lib.Name,
+				Version: lib.Version,
+			})
+			if release != nil {
+				overrides = append(overrides, release)
+			}
+		}
+	}
+	deps := lm.Index.ResolveDependencies(reqLibRelease, overrides)
 
 	// If no solution has been found
 	if len(deps) == 0 {
@@ -65,14 +81,19 @@ func LibraryResolveDependencies(ctx context.Context, req *rpc.LibraryResolveDepe
 	res := []*rpc.LibraryDependencyStatus{}
 	for _, dep := range deps {
 		// ...and add information on currently installed versions of the libraries
-		installed := ""
+		var installed *semver.Version
+		required := dep.GetVersion()
 		if installedLib, has := installedLibs[dep.GetName()]; has {
-			installed = installedLib.Version.String()
+			installed = installedLib.Version
+			if installed != nil && required != nil && installed.Equal(required) {
+				// avoid situations like installed=0.53 and required=0.53.0
+				required = installed
+			}
 		}
 		res = append(res, &rpc.LibraryDependencyStatus{
 			Name:             dep.GetName(),
-			VersionRequired:  dep.GetVersion().String(),
-			VersionInstalled: installed,
+			VersionRequired:  required.String(),
+			VersionInstalled: installed.String(),
 		})
 	}
 	sort.Slice(res, func(i, j int) bool {
diff --git a/commands/lib/search.go b/commands/lib/search.go
@@ -118,7 +118,7 @@ func getLibraryParameters(rel *librariesindex.Release) *rpc.LibraryRelease {
 	}
 }
 
-func getLibraryDependenciesParameter(deps []semver.Dependency) []*rpc.LibraryDependency {
+func getLibraryDependenciesParameter(deps []*librariesindex.Dependency) []*rpc.LibraryDependency {
 	res := []*rpc.LibraryDependency{}
 	for _, dep := range deps {
 		res = append(res, &rpc.LibraryDependency{
diff --git a/go.mod b/go.mod
@@ -36,7 +36,7 @@ require (
 	github.com/xeipuuv/gojsonschema v1.2.0
 	go.bug.st/cleanup v1.0.0
 	go.bug.st/downloader/v2 v2.1.1
-	go.bug.st/relaxed-semver v0.11.0
+	go.bug.st/relaxed-semver v0.12.0
 	go.bug.st/serial v1.6.1
 	go.bug.st/testifyjson v1.1.1
 	golang.org/x/term v0.14.0
diff --git a/go.sum b/go.sum
@@ -455,8 +455,8 @@ go.bug.st/cleanup v1.0.0/go.mod h1:EqVmTg2IBk4znLbPD28xne3abjsJftMdqqJEjhn70bk=
 go.bug.st/downloader/v2 v2.1.1 h1:nyqbUizo3E2IxCCm4YFac4FtSqqFpqWP+Aae5GCMuw4=
 go.bug.st/downloader/v2 v2.1.1/go.mod h1:VZW2V1iGKV8rJL2ZEGIDzzBeKowYv34AedJz13RzVII=
 go.bug.st/relaxed-semver v0.9.0/go.mod h1:ug0/W/RPYUjliE70Ghxg77RDHmPxqpo7SHV16ijss7Q=
-go.bug.st/relaxed-semver v0.11.0 h1:ngzpUlBEZ5F9hJnMZP55LIFbgX3bCztBBufMhJViAsY=
-go.bug.st/relaxed-semver v0.11.0/go.mod h1:rqPEm+790OTQlAdfSJSHWwpKOg3A8UyvAWMZxYkQivc=
+go.bug.st/relaxed-semver v0.12.0 h1:se8v3lTdAAFp68+/RS/0Y/nFdnpdzkP5ICY04SPau4E=
+go.bug.st/relaxed-semver v0.12.0/go.mod h1:Cpcbiig6Omwlq6bS7i3MQWiqS7W7HDd8CAnZFC40Cl0=
 go.bug.st/serial v1.3.2/go.mod h1:jDkjqASf/qSjmaOxHSHljwUQ6eHo/ZX/bxJLQqSlvZg=
 go.bug.st/serial v1.6.1 h1:VSSWmUxlj1T/YlRo2J104Zv3wJFrjHIl/T3NeruWAHY=
 go.bug.st/serial v1.6.1/go.mod h1:UABfsluHAiaNI+La2iESysd9Vetq7VRdpxvjx7CmmOE=
diff --git a/internal/cli/lib/check_deps.go b/internal/cli/lib/check_deps.go
@@ -33,6 +33,7 @@ import (
 )
 
 func initDepsCommand() *cobra.Command {
+	var noOverwrite bool
 	depsCommand := &cobra.Command{
 		Use:   fmt.Sprintf("deps %s[@%s]...", tr("LIBRARY"), tr("VERSION_NUMBER")),
 		Short: tr("Check dependencies status for the specified library."),
@@ -41,15 +42,18 @@ func initDepsCommand() *cobra.Command {
 			"  " + os.Args[0] + " lib deps AudioZero       # " + tr("for the latest version.") + "\n" +
 			"  " + os.Args[0] + " lib deps AudioZero@1.0.0 # " + tr("for the specific version."),
 		Args: cobra.ExactArgs(1),
-		Run:  runDepsCommand,
+		Run: func(cmd *cobra.Command, args []string) {
+			runDepsCommand(args, noOverwrite)
+		},
 		ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 			return arguments.GetInstalledLibraries(), cobra.ShellCompDirectiveDefault
 		},
 	}
+	depsCommand.Flags().BoolVar(&noOverwrite, "no-overwrite", false, tr("Do not try to update library dependencies if already installed."))
 	return depsCommand
 }
 
-func runDepsCommand(cmd *cobra.Command, args []string) {
+func runDepsCommand(args []string, noOverwrite bool) {
 	instance := instance.CreateAndInit()
 	logrus.Info("Executing `arduino-cli lib deps`")
 	libRef, err := ParseLibraryReferenceArgAndAdjustCase(instance, args[0])
@@ -58,9 +62,10 @@ func runDepsCommand(cmd *cobra.Command, args []string) {
 	}
 
 	deps, err := lib.LibraryResolveDependencies(context.Background(), &rpc.LibraryResolveDependenciesRequest{
-		Instance: instance,
-		Name:     libRef.Name,
-		Version:  libRef.Version,
+		Instance:                      instance,
+		Name:                          libRef.Name,
+		Version:                       libRef.Version,
+		DoNotUpdateInstalledLibraries: noOverwrite,
 	})
 	if err != nil {
 		feedback.Fatal(tr("Error resolving dependencies for %[1]s: %[2]s", libRef, err), feedback.ErrGeneric)
diff --git a/internal/integrationtest/lib/lib_test.go b/internal/integrationtest/lib/lib_test.go
@@ -589,8 +589,10 @@ func TestInstallLibraryWithDependencies(t *testing.T) {
 	require.NoError(t, err)
 	_, _, err = cli.Run("lib", "install", "SD@1.2.3")
 	require.NoError(t, err)
-	_, _, err = cli.Run("lib", "install", "Arduino_Builtin", "--no-overwrite")
-	require.Error(t, err)
+	// This time it should accept the installation with the currently installed SD 1.2.3
+	out, _, err := cli.Run("lib", "install", "Arduino_Builtin", "--no-overwrite")
+	require.NoError(t, err)
+	require.Contains(t, string(out), "Already installed SD@1.2.3")
 }
 
 func TestInstallNoDeps(t *testing.T) {
@@ -1653,9 +1655,6 @@ func TestDependencyResolver(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()
 
-	_, _, err := cli.Run("lib", "update-index")
-	require.NoError(t, err)
-
 	done := make(chan bool)
 	go func() {
 		_, _, err := cli.Run("lib", "install", "NTPClient_Generic")
@@ -1665,7 +1664,54 @@ func TestDependencyResolver(t *testing.T) {
 
 	select {
 	case <-done:
-	case <-time.After(time.Second * 2):
+	case <-time.After(time.Second * 10):
 		require.FailNow(t, "The install command didn't complete in the allocated time")
 	}
 }
+
+func TestDependencyResolverNoOverwrite(t *testing.T) {
+	// https://github.com/arduino/arduino-cli/issues/1799
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	_, _, err := cli.Run("lib", "install", "Bounce2@2.53.0")
+	require.NoError(t, err)
+
+	out, _, err := cli.Run("lib", "deps", "EncoderTool@2.2.0", "--format", "json")
+	require.NoError(t, err)
+	outjson := requirejson.Parse(t, out)
+	outjson.MustContain(`{
+		"dependencies": [
+			{
+				"name": "Bounce2",
+				"version_installed": "2.53"
+			},
+			{
+				"name": "EncoderTool",
+				"version_required": "2.2.0"
+			}
+		]
+	}`)
+	require.NotEqual(t, outjson.Query("dependencies[0].version_required").String(), `"2.53.0"`)
+	require.NotEqual(t, outjson.Query("dependencies[0].version_required").String(), `"2.53"`)
+
+	out, _, err = cli.Run("lib", "deps", "EncoderTool@2.2.0", "--no-overwrite", "--format", "json")
+	require.NoError(t, err)
+	outjson = requirejson.Parse(t, out)
+	outjson.MustContain(`{
+		"dependencies": [
+			{
+				"name": "Bounce2",
+				"version_required": "2.53",
+				"version_installed": "2.53"
+			},
+			{
+				"name": "EncoderTool",
+				"version_required": "2.2.0"
+			}
+		]
+	}`)
+
+	_, _, err = cli.Run("lib", "install", "EncoderTool@2.2.0", "--no-overwrite")
+	require.NoError(t, err)
+}
diff --git a/rpc/cc/arduino/cli/commands/v1/lib.pb.go b/rpc/cc/arduino/cli/commands/v1/lib.pb.go
diff --git a/rpc/cc/arduino/cli/commands/v1/lib.proto b/rpc/cc/arduino/cli/commands/v1/lib.proto

Original file line number	Diff line number	Diff line change
`@@ -126,8 +126,8 @@ func (indexLib indexRelease) extractReleaseIn(library Library) {`
`126`	`126`	`}`
`127`	`127`	`}`
`128`	`128`
`129`		`-func (indexLib *indexRelease) extractDependencies() []semver.Dependency {`
`130`		`- res := []semver.Dependency{}`
	`129`	`+func (indexLib indexRelease) extractDependencies() []Dependency {`
	`130`	`+ res := []*Dependency{}`
`131`	`131`	`if indexLib.Dependencies == nil \|\| len(indexLib.Dependencies) == 0 {`
`132`	`132`	`return res`
`133`	`133`	`}`
Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,7 @@ func getLibraryParameters(rel librariesindex.Release) rpc.LibraryRelease {`
`118`	`118`	`}`
`119`	`119`	`}`
`120`	`120`
`121`		`-func getLibraryDependenciesParameter(deps []semver.Dependency) []*rpc.LibraryDependency {`
	`121`	`+func getLibraryDependenciesParameter(deps []librariesindex.Dependency) []rpc.LibraryDependency {`
`122`	`122`	`res := []*rpc.LibraryDependency{}`
`123`	`123`	`for _, dep := range deps {`
`124`	`124`	`res = append(res, &rpc.LibraryDependency{`