From 6f443f0075707b444937b3068549585ff74bf3a7 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 30 May 2023 01:05:24 +0200 Subject: [PATCH 1/3] resolver: Ensure that requested release is in the archive --- resolver.go | 25 ++++++++++++++++++++++--- resolver_test.go | 12 ++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/resolver.go b/resolver.go index ca469fa..7dee81c 100644 --- a/resolver.go +++ b/resolver.go @@ -55,9 +55,28 @@ type Archive struct { // Resolve will try to depp-resolve dependencies from the Release passed as // arguent using a backtracking algorithm. func (ar *Archive) Resolve(release Release) []Release { - solution := map[string]Release{release.GetName(): release} - depsToProcess := release.GetDependencies() - return ar.resolve(solution, depsToProcess) + mainDep := &bareDependency{ + name: release.GetName(), + version: release.GetVersion(), + } + return ar.resolve(map[string]Release{}, []Dependency{mainDep}) +} + +type bareDependency struct { + name string + version *Version +} + +func (b *bareDependency) GetName() string { + return b.name +} + +func (b *bareDependency) GetConstraint() Constraint { + return &Equals{Version: b.version} +} + +func (b *bareDependency) String() string { + return b.GetName() + b.GetConstraint().String() } func (ar *Archive) resolve(solution map[string]Release, depsToProcess []Dependency) []Release { diff --git a/resolver_test.go b/resolver_test.go index 55b6a5c..3e2c79f 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -74,6 +74,10 @@ func rel(name, ver string, deps []Dependency) Release { } func TestResolver(t *testing.T) { + a100 := rel("A", "1.0.0", deps("B>=1.2.0", "C>=2.0.0")) + a110 := rel("A", "1.1.0", deps("B=1.2.0", "C>=2.0.0")) + a111 := rel("A", "1.1.1", deps("B", "C=1.1.1")) + a120 := rel("A", "1.2.0", deps("B=1.2.0", "C>2.0.0")) b131 := rel("B", "1.3.1", deps("C<2.0.0")) b130 := rel("B", "1.3.0", deps()) b121 := rel("B", "1.2.1", deps()) @@ -97,6 +101,7 @@ func TestResolver(t *testing.T) { e101 := rel("E", "1.0.1", deps("F")) // INVALID arch := &Archive{ Releases: map[string]Releases{ + "A": {a100, a110, a111, a120}, "B": {b131, b130, b121, b120, b111, b110, b100}, "C": {c200, c120, c111, c110, c102, c101, c100, c021, c020, c010}, "D": {d100, d120}, @@ -104,10 +109,9 @@ func TestResolver(t *testing.T) { }, } - a100 := rel("A", "1.0.0", deps("B>=1.2.0", "C>=2.0.0")) - a110 := rel("A", "1.1.0", deps("B=1.2.0", "C>=2.0.0")) - a111 := rel("A", "1.1.1", deps("B", "C=1.1.1")) - a120 := rel("A", "1.2.0", deps("B=1.2.0", "C>2.0.0")) + a130 := rel("A", "1.3.0", deps()) + r0 := arch.Resolve(a130) // Non-existent in archive + require.Nil(t, r0) r1 := arch.Resolve(a100) require.Len(t, r1, 3) From 9a0ab1909fe25718f70f575f63385f8b1f616c98 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 30 May 2023 01:08:09 +0200 Subject: [PATCH 2/3] Added test for worst-case scenario --- resolver_test.go | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/resolver_test.go b/resolver_test.go index 3e2c79f..70242c1 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -9,6 +9,7 @@ package semver import ( "fmt" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -78,6 +79,7 @@ func TestResolver(t *testing.T) { a110 := rel("A", "1.1.0", deps("B=1.2.0", "C>=2.0.0")) a111 := rel("A", "1.1.1", deps("B", "C=1.1.1")) a120 := rel("A", "1.2.0", deps("B=1.2.0", "C>2.0.0")) + a121 := rel("A", "1.2.1", deps("B", "C", "G", "H", "I", "E=1.0.1")) b131 := rel("B", "1.3.1", deps("C<2.0.0")) b130 := rel("B", "1.3.0", deps()) b121 := rel("B", "1.2.1", deps()) @@ -99,13 +101,34 @@ func TestResolver(t *testing.T) { d120 := rel("D", "1.2.0", deps("E")) e100 := rel("E", "1.0.0", deps()) e101 := rel("E", "1.0.1", deps("F")) // INVALID + g130 := rel("G", "1.3.0", deps()) + g140 := rel("G", "1.4.0", deps()) + g150 := rel("G", "1.5.0", deps()) + g160 := rel("G", "1.6.0", deps()) + g170 := rel("G", "1.7.0", deps()) + g180 := rel("G", "1.8.0", deps()) + h130 := rel("H", "1.3.0", deps()) + h140 := rel("H", "1.4.0", deps()) + h150 := rel("H", "1.5.0", deps()) + h160 := rel("H", "1.6.0", deps()) + h170 := rel("H", "1.7.0", deps()) + h180 := rel("H", "1.8.0", deps()) + i130 := rel("I", "1.3.0", deps()) + i140 := rel("I", "1.4.0", deps()) + i150 := rel("I", "1.5.0", deps()) + i160 := rel("I", "1.6.0", deps()) + i170 := rel("I", "1.7.0", deps()) + i180 := rel("I", "1.8.0", deps()) arch := &Archive{ Releases: map[string]Releases{ - "A": {a100, a110, a111, a120}, + "A": {a100, a110, a111, a120, a121}, "B": {b131, b130, b121, b120, b111, b110, b100}, "C": {c200, c120, c111, c110, c102, c101, c100, c021, c020, c010}, "D": {d100, d120}, "E": {e100, e101}, + "G": {g130, g140, g150, g160, g170, g180}, + "H": {h130, h140, h150, h160, h170, h180}, + "I": {i130, i140, i150, i160, i170, i180}, }, } @@ -143,4 +166,17 @@ func TestResolver(t *testing.T) { require.Contains(t, r5, d120) require.Contains(t, r5, e100) fmt.Println(r5) + + done := make(chan bool) + go func() { + r6 := arch.Resolve(a121) + require.Nil(t, r6) + fmt.Println(r6) + close(done) + }() + select { + case <-done: + case <-time.After(time.Second): + require.FailNow(t, "test didn't complete in the allocated time") + } } From 7cce8cf51e85a35b0cae7ad9992c03665df4f4b3 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 30 May 2023 01:09:49 +0200 Subject: [PATCH 3/3] Speedup in worst-case scenario --- resolver.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/resolver.go b/resolver.go index 7dee81c..32675f1 100644 --- a/resolver.go +++ b/resolver.go @@ -59,7 +59,7 @@ func (ar *Archive) Resolve(release Release) []Release { name: release.GetName(), version: release.GetVersion(), } - return ar.resolve(map[string]Release{}, []Dependency{mainDep}) + return ar.resolve(map[string]Release{}, []Dependency{mainDep}, map[Dependency]int{}) } type bareDependency struct { @@ -79,7 +79,7 @@ func (b *bareDependency) String() string { return b.GetName() + b.GetConstraint().String() } -func (ar *Archive) resolve(solution map[string]Release, depsToProcess []Dependency) []Release { +func (ar *Archive) resolve(solution map[string]Release, depsToProcess []Dependency, problematicDeps map[Dependency]int) []Release { debug("deps to process: %s", depsToProcess) if len(depsToProcess) == 0 { debug("All dependencies have been resolved.") @@ -99,7 +99,7 @@ func (ar *Archive) resolve(solution map[string]Release, depsToProcess []Dependen if existingRelease, has := solution[depName]; has { if match(existingRelease, dep) { debug("%s already in solution and matching", existingRelease) - return ar.resolve(solution, depsToProcess[1:]) + return ar.resolve(solution, depsToProcess[1:], problematicDeps) } debug("%s already in solution do not match... rollingback", existingRelease) return nil @@ -131,12 +131,18 @@ func (ar *Archive) resolve(solution map[string]Release, depsToProcess []Dependen } solution[depName] = release - res := ar.resolve(solution, append(depsToProcess[1:], deps...)) - if res != nil { + newDepsToProcess := append(depsToProcess[1:], deps...) + // bubble up problematics deps so they are processed first + sort.Slice(newDepsToProcess, func(i, j int) bool { + return problematicDeps[newDepsToProcess[i]] > problematicDeps[newDepsToProcess[j]] + }) + if res := ar.resolve(solution, newDepsToProcess, problematicDeps); res != nil { return res } debug("%s did not work...", release) delete(solution, depName) } + + problematicDeps[dep]++ return nil }