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