From 4cc81287db19cb1b3a315b533610873d0901c77a Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Sun, 18 Feb 2024 23:48:01 +0530 Subject: [PATCH 1/4] fix: report in case of cyclic dependency Signed-off-by: Akash Kumar --- pkg/client/client.go | 88 +++++++++++++++++---------------------- pkg/client/client_test.go | 6 ++- pkg/cmd/cmd_graph.go | 2 +- pkg/graph/graph.go | 83 ------------------------------------ pkg/reporter/reporter.go | 1 + 5 files changed, 45 insertions(+), 135 deletions(-) delete mode 100644 pkg/graph/graph.go diff --git a/pkg/client/client.go b/pkg/client/client.go index fa92363a..d7f57e57 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -17,7 +17,6 @@ import ( "kcl-lang.io/kpm/pkg/env" "kcl-lang.io/kpm/pkg/errors" "kcl-lang.io/kpm/pkg/git" - pkgGraph "kcl-lang.io/kpm/pkg/graph" "kcl-lang.io/kpm/pkg/oci" "kcl-lang.io/kpm/pkg/opt" pkg "kcl-lang.io/kpm/pkg/package" @@ -596,7 +595,7 @@ func (c *KpmClient) AddDepToPkg(kclPkg *pkg.KclPkg, d *pkg.Dependency) error { } // download all the dependencies. - changedDeps, _, err := c.downloadDeps(kclPkg.ModFile.Dependencies, kclPkg.Dependencies) + changedDeps, _, err := c.InitGraphAndDownloadDeps(kclPkg) if err != nil { return err @@ -1097,34 +1096,24 @@ func (c *KpmClient) ParseOciOptionFromString(oci string, tag string) (*opt.OciOp return ociOpt, nil } -// GetDependencyGraph will get the dependency graph of kcl package dependencies -func (c *KpmClient) GetDependencyGraph(kclPkg *pkg.KclPkg) (graph.Graph[string, string], error) { - _, depGraph, err := c.downloadDeps(kclPkg.ModFile.Dependencies, kclPkg.Dependencies) - if err != nil { - return nil, err - } +// InitGraphAndDownloadDeps initializes a dependency graph and call downloadDeps function. +func (c *KpmClient) InitGraphAndDownloadDeps(kclPkg *pkg.KclPkg) (*pkg.Dependencies, graph.Graph[string, string], error) { - sources, err := pkgGraph.FindSources(depGraph) - if err != nil { - return nil, err - } + depGraph := graph.New(graph.StringHash, graph.Directed(), graph.PreventCycles()) // add the root vertex(package name) to the dependency graph. root := fmt.Sprintf("%s@%s", kclPkg.GetPkgName(), kclPkg.GetPkgVersion()) - err = depGraph.AddVertex(root) + err := depGraph.AddVertex(root) if err != nil { - return nil, err + return nil, nil, err } - // make an edge between the root vertex and all the sources of the dependency graph. - for _, source := range sources { - err = depGraph.AddEdge(root, source) - if err != nil { - return nil, err - } + changedDeps, err := c.downloadDeps(kclPkg.ModFile.Dependencies, kclPkg.Dependencies, depGraph, root) + if err != nil { + return nil, nil, err } - return depGraph, nil + return changedDeps, depGraph, nil } // dependencyExists will check whether the dependency exists in the local filesystem. @@ -1151,7 +1140,7 @@ func (c *KpmClient) dependencyExists(dep *pkg.Dependency, lockDeps *pkg.Dependen } // downloadDeps will download all the dependencies of the current kcl package. -func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencies) (*pkg.Dependencies, graph.Graph[string, string], error) { +func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencies, depGraph graph.Graph[string, string], parent string) (*pkg.Dependencies, error) { newDeps := pkg.Dependencies{ Deps: make(map[string]pkg.Dependency), } @@ -1159,7 +1148,7 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie // Traverse all dependencies in kcl.mod for _, d := range deps.Deps { if len(d.Name) == 0 { - return nil, nil, errors.InvalidDependency + return nil, errors.InvalidDependency } existDep := c.dependencyExists(&d, &lockDeps) @@ -1171,16 +1160,15 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie expectedSum := lockDeps.Deps[d.Name].Sum // Clean the cache if len(c.homePath) == 0 || len(d.FullName) == 0 { - return nil, nil, errors.InternalBug + return nil, errors.InternalBug } dir := filepath.Join(c.homePath, d.FullName) os.RemoveAll(dir) // download dependencies - lockedDep, err := c.Download(&d, dir) if err != nil { - return nil, nil, err + return nil, err } if !lockedDep.IsFromLocal() { @@ -1188,7 +1176,7 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie lockedDep.Sum != expectedSum && existDep != nil && existDep.FullName == d.FullName { - return nil, nil, reporter.NewErrorEvent( + return nil, reporter.NewErrorEvent( reporter.CheckSumMismatch, errors.CheckSumMismatchError, fmt.Sprintf("checksum for '%s' changed in lock file", lockedDep.Name), @@ -1201,10 +1189,16 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie lockDeps.Deps[d.Name] = *lockedDep } - depGraph := graph.New(graph.StringHash, graph.Directed()) + // necessary to make a copy as when we are updating kcl.mod in below for loop + // then newDeps.Deps gets updated and range gets an extra value to iterate through + // this messes up the dependency graph + newDepsCopy := make(map[string]pkg.Dependency) + for k, v := range newDeps.Deps { + newDepsCopy[k] = v + } // Recursively download the dependencies of the new dependencies. - for _, d := range newDeps.Deps { + for _, d := range newDepsCopy { // Load kcl.mod file of the new downloaded dependencies. deppkg, err := pkg.LoadKclPkg(filepath.Join(c.homePath, d.FullName)) if len(d.LocalFullPath) != 0 { @@ -1215,37 +1209,31 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie if os.IsNotExist(err) { continue } - return nil, nil, err - } - - // Download the dependencies. - nested, nestedDepGraph, err := c.downloadDeps(deppkg.ModFile.Dependencies, lockDeps) - if err != nil { - return nil, nil, err + return nil, err } source := fmt.Sprintf("%s@%s", d.Name, d.Version) err = depGraph.AddVertex(source) if err != nil && err != graph.ErrVertexAlreadyExists { - return nil, nil, err + return nil, err } - sourcesOfNestedDepGraph, err := pkgGraph.FindSources(nestedDepGraph) + err = depGraph.AddEdge(parent, source) if err != nil { - return nil, nil, err + if err == graph.ErrEdgeCreatesCycle { + return nil, reporter.NewErrorEvent( + reporter.CircularDependencyExist, + nil, + "adding dependencies results in a cycle", + ) + } + return nil, err } - depGraph, err = pkgGraph.Union(depGraph, nestedDepGraph) + // Download the dependencies. + nested, err := c.downloadDeps(deppkg.ModFile.Dependencies, lockDeps, depGraph, source) if err != nil { - return nil, nil, err - } - - // make an edge between the source of all nested dep graph and main dep graph - for _, sourceOfNestedDepGraph := range sourcesOfNestedDepGraph { - err = depGraph.AddEdge(source, sourceOfNestedDepGraph) - if err != nil && err != graph.ErrEdgeAlreadyExists { - return nil, nil, err - } + return nil, err } // Update kcl.mod. @@ -1256,7 +1244,7 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie } } - return &newDeps, depGraph, nil + return &newDeps, nil } // pullTarFromOci will pull a kcl package tar file from oci registry. diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index b9706ea3..f0de55fc 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -126,7 +126,7 @@ func TestDependencyGraph(t *testing.T) { kclPkg, err := kpmcli.LoadPkgFromPath(testDir) assert.Equal(t, err, nil) - depGraph, err := kpmcli.GetDependencyGraph(kclPkg) + _, depGraph, err := kpmcli.InitGraphAndDownloadDeps(kclPkg) assert.Equal(t, err, nil) adjMap, err := depGraph.AdjacencyMap() assert.Equal(t, err, nil) @@ -157,6 +157,10 @@ func TestDependencyGraph(t *testing.T) { ) } +func TestCyclicDependency(t *testing.T) { + +} + func TestInitEmptyPkg(t *testing.T) { testDir := initTestDir("test_init_empty_mod") kclPkg := pkg.NewKclPkg(&opt.InitOptions{Name: "test_name", InitPath: testDir}) diff --git a/pkg/cmd/cmd_graph.go b/pkg/cmd/cmd_graph.go index fae1ac1e..3d1ec0f1 100644 --- a/pkg/cmd/cmd_graph.go +++ b/pkg/cmd/cmd_graph.go @@ -62,7 +62,7 @@ func KpmGraph(c *cli.Context, kpmcli *client.KpmClient) error { return err } - depGraph, err := kpmcli.GetDependencyGraph(kclPkg) + _, depGraph, err := kpmcli.InitGraphAndDownloadDeps(kclPkg) if err != nil { return err } diff --git a/pkg/graph/graph.go b/pkg/graph/graph.go deleted file mode 100644 index 82dc7586..00000000 --- a/pkg/graph/graph.go +++ /dev/null @@ -1,83 +0,0 @@ -package graph - -import ( - "fmt" - "github.com/dominikbraun/graph" -) - -// Union combines two given graphs into a new graph. The vertex hashes in both -// graphs are expected to be unique. The two input graphs will remain unchanged. -// -// Both graphs should be either directed or undirected. All traits for the new -// graph will be derived from g. -// -// If the same vertex/edge happens to be in both g and h, then an error will not be -// thrown as happens in original Union function and successful operation takes place. -func Union[K comparable, T any](g, h graph.Graph[K, T]) (graph.Graph[K, T], error) { - union, err := g.Clone() - if err != nil { - return union, fmt.Errorf("failed to clone g: %w", err) - } - - adjacencyMap, err := h.AdjacencyMap() - if err != nil { - return union, fmt.Errorf("failed to get adjacency map: %w", err) - } - - addedEdges := make(map[K]map[K]struct{}) - - for currentHash := range adjacencyMap { - vertex, err := h.Vertex(currentHash) - if err != nil { - return union, fmt.Errorf("failed to get vertex %v: %w", currentHash, err) - } - - err = union.AddVertex(vertex) - if err != nil && err != graph.ErrVertexAlreadyExists { - return union, fmt.Errorf("failed to add vertex %v: %w", currentHash, err) - } - } - - for _, adjacencies := range adjacencyMap { - for _, edge := range adjacencies { - if _, sourceOK := addedEdges[edge.Source]; sourceOK { - if _, targetOK := addedEdges[edge.Source][edge.Target]; targetOK { - // If the edge addedEdges[source][target] exists, the edge - // has already been created and thus can be skipped here. - continue - } - } - - err = union.AddEdge(edge.Source, edge.Target) - if err != nil && err != graph.ErrEdgeAlreadyExists { - return union, fmt.Errorf("failed to add edge (%v, %v): %w", edge.Source, edge.Target, err) - } - - if _, ok := addedEdges[edge.Source]; !ok { - addedEdges[edge.Source] = make(map[K]struct{}) - } - addedEdges[edge.Source][edge.Target] = struct{}{} - } - } - - return union, nil -} - -func FindSources[K comparable, T any](g graph.Graph[K, T]) ([]K, error) { - if !g.Traits().IsDirected { - return nil, fmt.Errorf("cannot find source of a non-DAG graph ") - } - - predecessorMap, err := g.PredecessorMap() - if err != nil { - return nil, fmt.Errorf("failed to get predecessor map: %w", err) - } - - var sources []K - for vertex, predecessors := range predecessorMap { - if len(predecessors) == 0 { - sources = append(sources, vertex) - } - } - return sources, nil -} \ No newline at end of file diff --git a/pkg/reporter/reporter.go b/pkg/reporter/reporter.go index 71a5b76e..29e0ccf9 100644 --- a/pkg/reporter/reporter.go +++ b/pkg/reporter/reporter.go @@ -103,6 +103,7 @@ const ( AddItselfAsDep PkgTagExists DependencyNotFound + CircularDependencyExist RemoveDep AddDep KclModNotFound From 8c4937861f1e85e4e335341059a61a69bd2e1dfb Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Mon, 19 Feb 2024 11:31:08 +0530 Subject: [PATCH 2/4] add unit test Signed-off-by: Akash Kumar --- pkg/client/client_test.go | 27 ++++++++++++++++++- .../test_cyclic_dependency/aaa/kcl.mod | 7 +++++ .../test_cyclic_dependency/aaa/kcl.mod.lock | 6 +++++ .../test_cyclic_dependency/bbb/kcl.mod | 7 +++++ .../test_cyclic_dependency/bbb/kcl.mod.lock | 8 ++++++ 5 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod create mode 100644 pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod.lock create mode 100644 pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod create mode 100644 pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod.lock diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index f0de55fc..2775390d 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -19,6 +19,7 @@ import ( "kcl-lang.io/kpm/pkg/git" "kcl-lang.io/kpm/pkg/opt" pkg "kcl-lang.io/kpm/pkg/package" + "kcl-lang.io/kpm/pkg/reporter" "kcl-lang.io/kpm/pkg/runner" "kcl-lang.io/kpm/pkg/utils" ) @@ -158,7 +159,31 @@ func TestDependencyGraph(t *testing.T) { } func TestCyclicDependency(t *testing.T) { - + testDir := getTestDir("test_cyclic_dependency") + assert.Equal(t, utils.DirExists(filepath.Join(testDir, "aaa")), true) + assert.Equal(t, utils.DirExists(filepath.Join(testDir, "aaa/kcl.mod")), true) + assert.Equal(t, utils.DirExists(filepath.Join(testDir, "bbb")), true) + assert.Equal(t, utils.DirExists(filepath.Join(testDir, "bbb/kcl.mod")), true) + + pkg_path := filepath.Join(testDir, "aaa") + + kpmcli, err := NewKpmClient() + assert.Equal(t, err, nil) + kclPkg, err := kpmcli.LoadPkgFromPath(pkg_path) + assert.Equal(t, err, nil) + + currentDir, err := os.Getwd() + assert.Equal(t, err, nil) + err = os.Chdir(pkg_path) + assert.Equal(t, err, nil) + + _, _, err = kpmcli.InitGraphAndDownloadDeps(kclPkg) + assert.Equal(t, err, reporter.NewErrorEvent( + reporter.CircularDependencyExist, nil, "adding dependencies results in a cycle", + )) + + err = os.Chdir(currentDir) + assert.Equal(t, err, nil) } func TestInitEmptyPkg(t *testing.T) { diff --git a/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod b/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod new file mode 100644 index 00000000..79bceba5 --- /dev/null +++ b/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod @@ -0,0 +1,7 @@ +[package] +name = "aaa" +edition = "0.0.1" +version = "0.0.1" + +[dependencies] +bbb = { path = "../bbb" } diff --git a/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod.lock b/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod.lock new file mode 100644 index 00000000..4eb93c26 --- /dev/null +++ b/pkg/client/test_data/test_cyclic_dependency/aaa/kcl.mod.lock @@ -0,0 +1,6 @@ +[dependencies] + [dependencies.bbb] + name = "bbb" + full_name = "bbb_" + sum = "Cgx5yepaNrS3/p4fK04RQWf1dEpG3j49ntFqzrJJsxU=" + path = "../bbb" diff --git a/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod b/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod new file mode 100644 index 00000000..a70b8351 --- /dev/null +++ b/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod @@ -0,0 +1,7 @@ +[package] +name = "bbb" +edition = "0.0.1" +version = "0.0.1" + +[dependencies] +aaa = { path = "../aaa" } diff --git a/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod.lock b/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod.lock new file mode 100644 index 00000000..e3672021 --- /dev/null +++ b/pkg/client/test_data/test_cyclic_dependency/bbb/kcl.mod.lock @@ -0,0 +1,8 @@ +[dependencies] + [dependencies.aaa] + name = "aaa" + full_name = "aaa_0.0.1" + version = "0.0.1" + sum = "hgAM5wYFzXo83S+OiT0LaBDnHU7Q9vqq3BPXlTDr8TY=" + path = "../aaa" + From fe8a344e772922affeb4864fd263fa0ae28a9856 Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Mon, 19 Feb 2024 12:45:08 +0530 Subject: [PATCH 3/4] fix e2e tests Signed-off-by: Akash Kumar --- .../kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout | 2 +- .../kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout index 5a9b9790..c9d63c8d 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout @@ -2,4 +2,4 @@ adding dependency 'kcl1' the lastest version '0.0.1' will be added downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' -add dependency 'kcl1' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout index d777dc5e..f3e64906 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout @@ -3,4 +3,4 @@ the lastest version '0.0.1' will be added downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'kcl2' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout index d777dc5e..f3e64906 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout @@ -3,4 +3,4 @@ the lastest version '0.0.1' will be added downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'kcl2' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout index e09b3055..37ffe0b0 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout @@ -1,4 +1,4 @@ adding dependency 'a_kcl_pkg_dep_pkg_name' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'a_kcl_pkg_dep_pkg_name:0.0.1' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout index 121a3934..34999107 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout @@ -2,4 +2,4 @@ downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'a_kcl_pkg_dep_pkg_name:0.0.1' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout index 121a3934..34999107 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout @@ -2,4 +2,4 @@ downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'a_kcl_pkg_dep_pkg_name:0.0.1' successfully +adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout index 121a3934..34999107 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout @@ -2,4 +2,4 @@ downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -add dependency 'a_kcl_pkg_dep_pkg_name:0.0.1' successfully +adding dependencies results in a cycle From 6071084c25703ee04857e2af28f70e1fa062fe8b Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Mon, 19 Feb 2024 19:33:10 +0530 Subject: [PATCH 4/4] address review comments Signed-off-by: Akash Kumar --- pkg/client/client.go | 4 ++-- pkg/client/client_test.go | 2 +- .../kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout | 4 +--- .../kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout | 3 +-- .../kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout | 1 - .../kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout | 3 +-- .../kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout | 3 +-- .../kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout | 1 - .../kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout | 3 +-- 9 files changed, 8 insertions(+), 16 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index d7f57e57..fddf5166 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -1211,8 +1211,8 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie } return nil, err } - source := fmt.Sprintf("%s@%s", d.Name, d.Version) + source = strings.TrimRight(source, "@") err = depGraph.AddVertex(source) if err != nil && err != graph.ErrVertexAlreadyExists { return nil, err @@ -1224,7 +1224,7 @@ func (c *KpmClient) downloadDeps(deps pkg.Dependencies, lockDeps pkg.Dependencie return nil, reporter.NewErrorEvent( reporter.CircularDependencyExist, nil, - "adding dependencies results in a cycle", + fmt.Sprintf("adding %s as a dependency results in a cycle", source), ) } return nil, err diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 2775390d..273ed5a8 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -179,7 +179,7 @@ func TestCyclicDependency(t *testing.T) { _, _, err = kpmcli.InitGraphAndDownloadDeps(kclPkg) assert.Equal(t, err, reporter.NewErrorEvent( - reporter.CircularDependencyExist, nil, "adding dependencies results in a cycle", + reporter.CircularDependencyExist, nil, "adding bbb as a dependency results in a cycle", )) err = os.Chdir(currentDir) diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout index c9d63c8d..664b1334 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_1/test_suite.stdout @@ -1,5 +1,3 @@ adding dependency 'kcl1' the lastest version '0.0.1' will be added -downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' -adding dependencies results in a cycle +downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout index f3e64906..ee2e9c3e 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_2/test_suite.stdout @@ -2,5 +2,4 @@ the lastest version '0.0.1' will be added downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' -downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle +downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout index f3e64906..38f8193c 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_name_3/test_suite.stdout @@ -3,4 +3,3 @@ the lastest version '0.0.1' will be added downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout index 37ffe0b0..079b3648 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_2/test_suite.stdout @@ -1,4 +1,3 @@ adding dependency 'a_kcl_pkg_dep_pkg_name' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' -downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle +downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout index 34999107..f4f1cdda 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_3/test_suite.stdout @@ -1,5 +1,4 @@ adding dependency 'a_kcl_pkg_dep_pkg_name' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' -downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle +downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout index 34999107..dd9bfb4a 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_4/test_suite.stdout @@ -2,4 +2,3 @@ downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle diff --git a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout index 34999107..f4f1cdda 100644 --- a/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout +++ b/test/e2e/test_suites/kpm/exec_inside_pkg/add_with_path_5/test_suite.stdout @@ -1,5 +1,4 @@ adding dependency 'a_kcl_pkg_dep_pkg_name' downloading 'test/k8s:1.27' from 'localhost:5001/test/k8s:1.27' downloading 'test/kcl2:0.0.1' from 'localhost:5001/test/kcl2:0.0.1' -downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' -adding dependencies results in a cycle +downloading 'test/kcl1:0.0.1' from 'localhost:5001/test/kcl1:0.0.1' \ No newline at end of file