From 1b7041c060a3aa4c323cbc7f54e21102c839f4d0 Mon Sep 17 00:00:00 2001 From: Akash Kumar Date: Fri, 17 May 2024 15:46:24 +0530 Subject: [PATCH] refactor: address review comments Signed-off-by: Akash Kumar --- pkg/client/client.go | 17 +++++ pkg/client/client_test.go | 40 +++++++++- pkg/cmd/cmd_add.go | 4 +- pkg/cmd/cmd_update.go | 156 +++++++++++++++++--------------------- pkg/mvs/mvs.go | 44 +++++++---- 5 files changed, 154 insertions(+), 107 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index f3ab6022..331c2863 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -313,6 +313,23 @@ func (c *KpmClient) ResolvePkgDepsMetadata(kclPkg *pkg.KclPkg, update bool) erro return nil } +func GetReleasesFromSource(sourceType, uri string) ([]string, error) { + var releases []string + var err error + + switch sourceType { + case pkg.GIT: + releases, err = git.GetAllGithubReleases(uri) + case pkg.OCI: + releases, err = oci.GetAllImageTags(uri) + } + if err != nil { + return nil, err + } + + return releases, nil +} + // UpdateDeps will update the dependencies. func (c *KpmClient) UpdateDeps(kclPkg *pkg.KclPkg) error { _, err := c.ResolveDepsMetadataInJsonStr(kclPkg, true) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 79ee1665..6fb53403 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -10,10 +10,12 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "testing" "github.com/dominikbraun/graph" + "github.com/hashicorp/go-version" "github.com/otiai10/copy" "github.com/stretchr/testify/assert" "golang.org/x/mod/module" @@ -815,6 +817,43 @@ func TestParseOciOptionFromString(t *testing.T) { assert.Equal(t, ociOption.Tag, "test_tag") } +func TestGetReleasesFromSource(t *testing.T) { + sortVersions := func(versions []string) ([]string, error) { + var vers []*version.Version + for _, raw := range versions { + v, err := version.NewVersion(raw) + if err != nil { + return nil, err + } + vers = append(vers, v) + } + sort.Slice(vers, func(i, j int) bool { + return vers[i].LessThan(vers[j]) + }) + var res []string + for _, v := range vers { + res = append(res, v.Original()) + } + return res, nil + } + + releases, err := GetReleasesFromSource(pkg.GIT, "https://github.com/kcl-lang/kpm") + assert.Equal(t, err, nil) + length := len(releases) + assert.True(t, length >= 5) + releasesVersions, err := sortVersions(releases) + assert.Equal(t, err, nil) + assert.Equal(t, releasesVersions[:5], []string{"v0.1.0", "v0.2.0", "v0.2.1", "v0.2.2", "v0.2.3"}) + + releases, err = GetReleasesFromSource(pkg.OCI, "oci://ghcr.io/kcl-lang/k8s") + assert.Equal(t, err, nil) + length = len(releases) + assert.True(t, length >= 5) + releasesVersions, err = sortVersions(releases) + assert.Equal(t, err, nil) + assert.Equal(t, releasesVersions[:5], []string{"1.14", "1.15", "1.16", "1.17", "1.18"}) +} + func TestUpdateWithKclMod(t *testing.T) { kpmcli, err := NewKpmClient() assert.Equal(t, err, nil) @@ -1469,4 +1508,3 @@ func testRunWithOciDownloader(t *testing.T) { assert.Equal(t, buf.String(), "downloading 'zong-zhe/helloworld:0.0.3' from 'ghcr.io/zong-zhe/helloworld:0.0.3'\n") assert.Equal(t, res.GetRawYamlResult(), "The_first_kcl_program: Hello World!") } - diff --git a/pkg/cmd/cmd_add.go b/pkg/cmd/cmd_add.go index ec311e7d..95338de1 100644 --- a/pkg/cmd/cmd_add.go +++ b/pkg/cmd/cmd_add.go @@ -208,7 +208,7 @@ func parseGitRegistryOptions(c *cli.Context) (*opt.RegistryOptions, *reporter.Kp // parseOciRegistryOptions will parse the oci registry information from user cli inputs. func parseOciRegistryOptions(c *cli.Context, kpmcli *client.KpmClient) (*opt.RegistryOptions, error) { ociPkgRef := c.Args().First() - name, version, err := parseOciPkgNameAndVersion(ociPkgRef) + name, version, err := ParseOciPkgNameAndVersion(ociPkgRef) if err != nil { return nil, err } @@ -243,7 +243,7 @@ func parseLocalPathOptions(c *cli.Context) (*opt.RegistryOptions, *reporter.KpmE // parseOciPkgNameAndVersion will parse package name and version // from string ":". -func parseOciPkgNameAndVersion(s string) (string, string, error) { +func ParseOciPkgNameAndVersion(s string) (string, string, error) { parts := strings.Split(s, ":") if len(parts) == 1 { return parts[0], "", nil diff --git a/pkg/cmd/cmd_update.go b/pkg/cmd/cmd_update.go index 5a99c8b0..32a2acc0 100644 --- a/pkg/cmd/cmd_update.go +++ b/pkg/cmd/cmd_update.go @@ -13,7 +13,6 @@ import ( "github.com/dominikbraun/graph" "github.com/urfave/cli/v2" "golang.org/x/mod/module" - mvsExt "kcl-lang.io/kpm/pkg/3rdparty/mvs" "kcl-lang.io/kpm/pkg/client" "kcl-lang.io/kpm/pkg/env" "kcl-lang.io/kpm/pkg/mvs" @@ -64,14 +63,6 @@ func KpmUpdate(c *cli.Context, kpmcli *client.KpmClient) error { return reporter.NewErrorEvent(reporter.Bug, err, "internal bugs, please contact us to fix it.") } - fetchNameAndVersion := func(pkgInfo string) (string, string) { - // the version is specified after : - if strings.Contains(pkgInfo, ":") { - return strings.Split(pkgInfo, ":")[0], strings.Split(pkgInfo, ":")[1] - } - return pkgInfo, "" - } - kclPkg, err := kpmcli.LoadPkgFromPath(pwd) if err != nil { return err @@ -92,47 +83,10 @@ func KpmUpdate(c *cli.Context, kpmcli *client.KpmClient) error { modulesToDowngrade []module.Version ) - // validate if the packages is present in kcl.mod file and - // find the latest version if version is not specified for _, pkgInfo := range pkgInfos { - pkgInfo = strings.TrimSpace(pkgInfo) - pkgName, pkgVersion := fetchNameAndVersion(pkgInfo) - - var dep pkg.Dependency - var ok bool - if dep, ok = kclPkg.Deps[pkgName]; !ok { - reporter.Report(fmt.Sprintf("package %s not found in kcl.mod file", pkgName)) - continue - } - - if pkgVersion == "" { - var releases []string - if dep.Git != nil { - releases, err = mvs.GetReleasesFromSource(dep.GetSourceType(), dep.Git.Url) - } else if dep.Oci != nil { - releases, err = mvs.GetReleasesFromSource(dep.GetSourceType(), "fjdkasd") - } - if err != nil { - reporter.ReportEventToStdout(reporter.NewEvent( - reporter.FailedGetReleases, - fmt.Sprintf("failed to get releases for %s", pkgName), - )) - continue - } - pkgVersion, err = semver.LatestCompatibleVersion(releases, dep.Version) - if err != nil { - reporter.NewErrorEvent( - reporter.FailedSelectLatestCompatibleVersion, - err, - fmt.Sprintf("failed to find the latest version for %s", pkgName), - ) - continue - } - } - if pkgVersion < dep.Version { - modulesToDowngrade = append(modulesToDowngrade, module.Version{Path: pkgName, Version: pkgVersion}) - } else if pkgVersion > dep.Version { - modulesToUpgrade = append(modulesToUpgrade, module.Version{Path: pkgName, Version: pkgVersion}) + err = GetModulesToUpdate(kclPkg, modulesToUpgrade, modulesToDowngrade, pkgInfo) + if err != nil { + reporter.Report(err) } } @@ -148,11 +102,12 @@ func KpmUpdate(c *cli.Context, kpmcli *client.KpmClient) error { } target := module.Version{Path: kclPkg.GetPkgName(), Version: kclPkg.GetPkgVersion()} - buildList, err := UpdateBuildList(target, modulesToUpgrade, modulesToDowngrade, &reqs) + buildList, err := mvs.UpdateBuildList(target, modulesToUpgrade, modulesToDowngrade, &reqs) if err != nil { return reporter.NewErrorEvent(reporter.FailedUpdatingBuildList, err, "failed to update build list") } + // get all the vertices in the graph modules, err := graph.TopologicalSort(depGraph) if err != nil { return reporter.NewErrorEvent(reporter.FailedTopologicalSort, err, "failed to sort the dependencies") @@ -161,29 +116,10 @@ func KpmUpdate(c *cli.Context, kpmcli *client.KpmClient) error { kclPkg.ModFile.Dependencies.Deps = make(map[string]pkg.Dependency) for _, module := range modules { - if module.Path == target.Path || !slices.Contains(buildList, module) { - continue - } - d := pkg.Dependency{ - Name: module.Path, - Version: module.Version, - } - d.FullName = d.GenDepFullName() - _, properties, err := depGraph.VertexWithProperties(module) + err = InsertModuleToDeps(kclPkg, module, target, buildList, reqs) if err != nil { - return reporter.NewErrorEvent(reporter.FailedGetVertexProperties, err, "failed to get vertex with properties") - } - // there must be one property depending on the download source type - for sourceType, uri := range properties.Attributes { - d.Source, err = pkg.GenSource(sourceType, uri, module.Version) - if err != nil { - return reporter.NewErrorEvent(reporter.FailedGenerateSource, err, "failed to generate source") - } - // if d.Oci != nil && d.Oci.Reg == "ghcr.io" { - // d.Oci = nil - // } + return err } - kclPkg.ModFile.Dependencies.Deps[module.Path] = d } err = kpmcli.UpdateDeps(kclPkg) @@ -193,27 +129,71 @@ func KpmUpdate(c *cli.Context, kpmcli *client.KpmClient) error { return nil } -func UpdateBuildList(target module.Version, modulesToUpgrade []module.Version, modulesToDowngrade []module.Version, reqs *mvs.ReqsGraph) ([]module.Version, error) { - var ( - UpdBuildLists []module.Version - err error - ) +// GetModulesToUpdate validates if the packages is present in kcl.mod file and +// find the latest version if version is not specified. Depending on the value of pkgVersion, +// modulesToUpgrade or modulesToDowngrade will be updated. +func GetModulesToUpdate(kclPkg *pkg.KclPkg, modulesToUpgrade []module.Version, modulesToDowngrade []module.Version, pkgInfo string) error { + pkgInfo = strings.TrimSpace(pkgInfo) + pkgName, pkgVersion, err := ParseOciPkgNameAndVersion(pkgInfo) + if err != nil { + return err + } + + var dep pkg.Dependency + var ok bool + if dep, ok = kclPkg.Deps[pkgName]; !ok { + return err + } - if len(modulesToUpgrade) == 0 { - UpdBuildLists, err = mvsExt.UpgradeAll(target, reqs) - } else { - UpdBuildLists, err = mvsExt.Upgrade(target, reqs, modulesToUpgrade...) + if pkgVersion == "" { + var releases []string + releases, err = client.GetReleasesFromSource(dep.GetSourceType(), dep.GetDownloadPath()) + if err != nil { + return reporter.NewErrorEvent( + reporter.FailedGetReleases, + err, + fmt.Sprintf("failed to get releases for %s", pkgName), + ) + } + pkgVersion, err = semver.LatestCompatibleVersion(releases, dep.Version) + if err != nil { + return reporter.NewErrorEvent( + reporter.FailedSelectLatestCompatibleVersion, + err, + fmt.Sprintf("failed to find the latest version for %s", pkgName), + ) + } } - if err != nil { - return []module.Version{}, err + if pkgVersion < dep.Version { + modulesToDowngrade = append(modulesToDowngrade, module.Version{Path: pkgName, Version: pkgVersion}) + } else if pkgVersion > dep.Version { + modulesToUpgrade = append(modulesToUpgrade, module.Version{Path: pkgName, Version: pkgVersion}) } + return nil +} - if len(modulesToDowngrade) != 0 { - UpdBuildLists, err = mvsExt.Downgrade(target, reqs, modulesToDowngrade...) +// InsertModuleToDeps checks whether module is present in the buildList and it is not the same as the target module, +// and inserts it to the dependencies of kclPkg +func InsertModuleToDeps(kclPkg *pkg.KclPkg, module module.Version, target module.Version, buildList []module.Version, reqs mvs.ReqsGraph) (error) { + if module.Path == target.Path || !slices.Contains(buildList, module) { + return nil + } + d := pkg.Dependency{ + Name: module.Path, + Version: module.Version, } + d.FullName = d.GenDepFullName() + _, properties, err := reqs.VertexWithProperties(module) if err != nil { - return []module.Version{}, err + return reporter.NewErrorEvent(reporter.FailedGetVertexProperties, err, "failed to get vertex with properties") } - - return UpdBuildLists, nil -} + // there must be one property depending on the download source type + for sourceType, uri := range properties.Attributes { + d.Source, err = pkg.GenSource(sourceType, uri, module.Version) + if err != nil { + return reporter.NewErrorEvent(reporter.FailedGenerateSource, err, "failed to generate source") + } + } + kclPkg.ModFile.Dependencies.Deps[module.Path] = d + return nil +} \ No newline at end of file diff --git a/pkg/mvs/mvs.go b/pkg/mvs/mvs.go index 521e8e57..52f414cf 100644 --- a/pkg/mvs/mvs.go +++ b/pkg/mvs/mvs.go @@ -6,10 +6,9 @@ import ( "github.com/dominikbraun/graph" "github.com/hashicorp/go-version" "golang.org/x/mod/module" + "kcl-lang.io/kpm/pkg/3rdparty/mvs" "kcl-lang.io/kpm/pkg/client" errInt "kcl-lang.io/kpm/pkg/errors" - "kcl-lang.io/kpm/pkg/git" - "kcl-lang.io/kpm/pkg/oci" pkg "kcl-lang.io/kpm/pkg/package" "kcl-lang.io/kpm/pkg/reporter" "kcl-lang.io/kpm/pkg/semver" @@ -55,9 +54,9 @@ func (r ReqsGraph) Upgrade(m module.Version) (module.Version, error) { return module.Version{}, errInt.MultipleSources } - var releases []string + var releases []string for sourceType, uri := range properties.Attributes { - releases, err = GetReleasesFromSource(sourceType, uri) + releases, err = client.GetReleasesFromSource(sourceType, uri) if err != nil { return module.Version{}, err } @@ -113,7 +112,7 @@ func (r ReqsGraph) Previous(m module.Version) (module.Version, error) { var releases []string for sourceType, uri := range properties.Attributes { - releases, err = GetReleasesFromSource(sourceType, uri) + releases, err = client.GetReleasesFromSource(sourceType, uri) if err != nil { return module.Version{}, err } @@ -176,19 +175,32 @@ func (r ReqsGraph) Required(m module.Version) ([]module.Version, error) { return reqs, nil } -func GetReleasesFromSource(sourceType, uri string) ([]string, error) { - var releases []string - var err error +// UpdateBuildList decides whether to upgrade or downgrade based on modulesToUpgrade and modulesToDowngrade. +// if modulesToUpgrade is empty, upgrade all dependencies. if modulesToUpgrade is not empty, upgrade the dependencies. +// if modulesToDowngrade is not empty, downgrade the dependencies. +// if modulesToUpgrade and modulesToDowngrade are both empty, first apply upgrade operation and +// then downgrade the build list returned from previous operation. +func UpdateBuildList(target module.Version, modulesToUpgrade []module.Version, modulesToDowngrade []module.Version, reqs *ReqsGraph) ([]module.Version, error) { + var ( + UpdBuildLists []module.Version + err error + ) + + if len(modulesToUpgrade) == 0 { + UpdBuildLists, err = mvs.UpgradeAll(target, reqs) + } else { + UpdBuildLists, err = mvs.Upgrade(target, reqs, modulesToUpgrade...) + } + if err != nil { + return []module.Version{}, err + } - switch sourceType { - case pkg.GIT: - releases, err = git.GetAllGithubReleases(uri) - case pkg.OCI: - releases, err = oci.GetAllImageTags(uri) + if len(modulesToDowngrade) != 0 { + UpdBuildLists, err = mvs.Downgrade(target, reqs, modulesToDowngrade...) } if err != nil { - return nil, err + return []module.Version{}, err } - return releases, nil -} \ No newline at end of file + return UpdBuildLists, nil +}