From 3d306c709e6e451b5b9a909a99fdad7c92946198 Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 20 Jul 2023 14:31:03 +0800 Subject: [PATCH 1/3] fix: fix the null pointer when 'kpm pull'. --- pkg/cmd/cmd_pull.go | 2 +- pkg/utils/utils.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/cmd_pull.go b/pkg/cmd/cmd_pull.go index 8717d3b1..0106ff7b 100644 --- a/pkg/cmd/cmd_pull.go +++ b/pkg/cmd/cmd_pull.go @@ -66,7 +66,7 @@ func KpmPull(c *cli.Context) error { ociOpt, event := opt.ParseOciOptionFromOciUrl(ociUrlOrPkgName, tag) - if event.Type() == reporter.IsNotUrl || event.Type() == reporter.UrlSchemeNotOci { + if event != nil && (event.Type() == reporter.IsNotUrl || event.Type() == reporter.UrlSchemeNotOci) { settings := settings.GetSettings() if settings.ErrorEvent != nil { return settings.ErrorEvent diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index f0d84d52..cbb3480d 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -364,5 +364,7 @@ func RmNewline(s string) string { // JoinPath will join the 'elem' to the 'base' with '/'. func JoinPath(base, elem string) string { - return fmt.Sprintf("%s/%s", base, elem) + base = strings.TrimSuffix(base, "/") + elem = strings.TrimPrefix(elem, "/") + return base + "/" + elem } From 5a4aad701ea64a431fc3e92b0fbecd3ee8e20087 Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 20 Jul 2023 14:32:06 +0800 Subject: [PATCH 2/3] fix: add more test cases. --- .../kpm/no_args/kpm_pull_with_oci_url/test_suite.env | 2 ++ .../kpm/no_args/kpm_pull_with_oci_url/test_suite.input | 1 + .../kpm/no_args/kpm_pull_with_oci_url/test_suite.stderr | 0 .../kpm/no_args/kpm_pull_with_oci_url/test_suite.stdout | 4 ++++ .../kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.env | 2 ++ .../kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.input | 1 + .../kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stderr | 0 .../kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stdout | 3 +++ 8 files changed, 13 insertions(+) create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.env create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.input create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stderr create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stdout create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.env create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.input create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stderr create mode 100644 test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stdout diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.env b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.env new file mode 100644 index 00000000..4c789529 --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.env @@ -0,0 +1,2 @@ +KPM_HOME="" +KCLVM_VENDOR_HOME="" \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.input b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.input new file mode 100644 index 00000000..6350380f --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.input @@ -0,0 +1 @@ +kpm pull oci://ghcr.io/kcl-lang/k8s \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stderr b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stderr new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stdout b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stdout new file mode 100644 index 00000000..8b30ef87 --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url/test_suite.stdout @@ -0,0 +1,4 @@ +kpm: start to pull 'oci://ghcr.io/kcl-lang/k8s'. +kpm: the lastest version '1.27' will be pulled. +kpm: pulling '/kcl-lang/k8s:1.27' from 'ghcr.io/kcl-lang/k8s'. +kpm: pulled 'oci://ghcr.io/kcl-lang/k8s' in '/ghcr.io/kcl-lang/k8s' successfully. diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.env b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.env new file mode 100644 index 00000000..4c789529 --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.env @@ -0,0 +1,2 @@ +KPM_HOME="" +KCLVM_VENDOR_HOME="" \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.input b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.input new file mode 100644 index 00000000..2b0b3868 --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.input @@ -0,0 +1 @@ +kpm pull --tag 1.14 oci://ghcr.io/kcl-lang/k8s \ No newline at end of file diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stderr b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stderr new file mode 100644 index 00000000..e69de29b diff --git a/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stdout b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stdout new file mode 100644 index 00000000..cc20e13f --- /dev/null +++ b/test/e2e/test_suites/kpm/no_args/kpm_pull_with_oci_url_tag/test_suite.stdout @@ -0,0 +1,3 @@ +kpm: start to pull 'oci://ghcr.io/kcl-lang/k8s' with tag '1.14'. +kpm: pulling '/kcl-lang/k8s:1.14' from 'ghcr.io/kcl-lang/k8s'. +kpm: pulled 'oci://ghcr.io/kcl-lang/k8s' in '/ghcr.io/kcl-lang/k8s/1.14' successfully. From 046b19186f0f80f00c1c3bf1cadf1e41a356ad7c Mon Sep 17 00:00:00 2001 From: zongz Date: Thu, 20 Jul 2023 14:58:35 +0800 Subject: [PATCH 3/3] fix: add more test cases. --- pkg/cmd/cmd_pull.go | 8 ++------ pkg/cmd/cmd_push.go | 5 +---- pkg/mod/modfile.go | 12 ++---------- pkg/mod/modfile_test.go | 7 ++----- pkg/package/package_test.go | 21 +++++++++++++++++++-- pkg/utils/utils_test.go | 10 ++++++++++ 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/cmd_pull.go b/pkg/cmd/cmd_pull.go index 0106ff7b..94ab3c93 100644 --- a/pkg/cmd/cmd_pull.go +++ b/pkg/cmd/cmd_pull.go @@ -4,7 +4,6 @@ package cmd import ( "fmt" - "net/url" "os" "path/filepath" @@ -65,17 +64,14 @@ func KpmPull(c *cli.Context) error { } ociOpt, event := opt.ParseOciOptionFromOciUrl(ociUrlOrPkgName, tag) - + var err error if event != nil && (event.Type() == reporter.IsNotUrl || event.Type() == reporter.UrlSchemeNotOci) { settings := settings.GetSettings() if settings.ErrorEvent != nil { return settings.ErrorEvent } - urlpath, err := url.JoinPath(settings.DefaultOciRepo(), ociUrlOrPkgName) - if err != nil { - return reporter.NewErrorEvent(reporter.Bug, err) - } + urlpath := utils.JoinPath(settings.DefaultOciRepo(), ociUrlOrPkgName) ociOpt, err = opt.ParseOciRef(urlpath) if err != nil { diff --git a/pkg/cmd/cmd_push.go b/pkg/cmd/cmd_push.go index 4826aff3..e94b12e6 100644 --- a/pkg/cmd/cmd_push.go +++ b/pkg/cmd/cmd_push.go @@ -60,10 +60,7 @@ func genDefaultOciUrlForKclPkg(pkg *pkg.KclPkg) (string, error) { return "", settings.ErrorEvent } - urlPath, err := url.JoinPath(settings.DefaultOciRepo(), pkg.GetPkgName()) - if err != nil { - return "", err - } + urlPath := utils.JoinPath(settings.DefaultOciRepo(), pkg.GetPkgName()) u := &url.URL{ Scheme: oci.OCI_SCHEME, diff --git a/pkg/mod/modfile.go b/pkg/mod/modfile.go index 898e5d60..12293c1c 100644 --- a/pkg/mod/modfile.go +++ b/pkg/mod/modfile.go @@ -3,7 +3,6 @@ package modfile import ( "fmt" - "net/url" "os" "path/filepath" @@ -110,10 +109,7 @@ func (dep *Dependency) FillDepInfo() error { return settings.ErrorEvent } dep.Source.Oci.Reg = settings.DefaultOciRegistry() - urlpath, err := url.JoinPath(settings.DefaultOciRepo(), dep.Name) - if err != nil { - return err - } + urlpath := utils.JoinPath(settings.DefaultOciRepo(), dep.Name) dep.Source.Oci.Repo = urlpath } return nil @@ -377,11 +373,7 @@ func ParseOpt(opt *opt.RegistryOptions) *Dependency { } } if opt.Oci != nil { - repoPath, err := url.JoinPath(opt.Oci.Repo, opt.Oci.PkgName) - if err != nil { - reporter.Report("kpm: failed to parse oci rul") - return nil - } + repoPath := utils.JoinPath(opt.Oci.Repo, opt.Oci.PkgName) ociSource := Oci{ Reg: opt.Oci.Reg, Repo: repoPath, diff --git a/pkg/mod/modfile_test.go b/pkg/mod/modfile_test.go index c7812105..c89760e1 100644 --- a/pkg/mod/modfile_test.go +++ b/pkg/mod/modfile_test.go @@ -1,7 +1,6 @@ package modfile import ( - "net/url" "os" "path/filepath" "testing" @@ -181,8 +180,7 @@ func TestDownloadOci(t *testing.T) { err := os.MkdirAll(testPath, 0755) assert.Equal(t, err, nil) - urlpath, err := url.JoinPath(settings.DEFAULT_REPO, "k8s") - assert.Equal(t, err, nil) + urlpath := utils.JoinPath(settings.DEFAULT_REPO, "k8s") depFromOci := Dependency{ Name: "k8s", @@ -221,8 +219,7 @@ func TestDownloadLatestOci(t *testing.T) { err := os.MkdirAll(testPath, 0755) assert.Equal(t, err, nil) - urlpath, err := url.JoinPath(settings.DEFAULT_REPO, "k8s") - assert.Equal(t, err, nil) + urlpath := utils.JoinPath(settings.DEFAULT_REPO, "k8s") depFromOci := Dependency{ Name: "k8s", diff --git a/pkg/package/package_test.go b/pkg/package/package_test.go index 43b3bacf..6c1e9b97 100644 --- a/pkg/package/package_test.go +++ b/pkg/package/package_test.go @@ -2,6 +2,7 @@ package pkg import ( "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -137,8 +138,16 @@ func TestUpdateKclModAndLock(t *testing.T) { assert.Equal(t, len(kclPkg.modFile.Deps), 2) expectKclMod, _ := os.ReadFile(filepath.Join(expectDir, "kcl.mod")) expectKclModReverse, _ := os.ReadFile(filepath.Join(expectDir, "kcl.reverse.mod")) + + gotKclModStr := utils.RmNewline(string(gotKclMod)) + fmt.Printf("gotKclModStr: '%v'\n", gotKclModStr) + expectKclModStr := utils.RmNewline(string(expectKclMod)) + fmt.Printf("expectKclModStr: '%v'\n", expectKclModStr) + expectKclModReverseStr := utils.RmNewline(string(expectKclModReverse)) + fmt.Printf("expectKclModReverseStr: '%v'\n", expectKclModReverseStr) + assert.Equal(t, - (utils.RmNewline(string(gotKclMod)) == utils.RmNewline(string(expectKclMod))) || (utils.RmNewline(string(gotKclMod)) == utils.RmNewline(string(expectKclModReverse))), + (gotKclModStr == expectKclModStr || gotKclModStr == expectKclModReverseStr), true, ) } @@ -150,8 +159,16 @@ func TestUpdateKclModAndLock(t *testing.T) { assert.Equal(t, len(kclPkg.modFile.Deps), 2) expectKclModLock, _ := os.ReadFile(filepath.Join(expectDir, "kcl.mod.lock")) expectKclModLockReverse, _ := os.ReadFile(filepath.Join(expectDir, "kcl.mod.reverse.lock")) + + gotKclModLockStr := utils.RmNewline(string(gotKclModLock)) + fmt.Printf("gotKclModLockStr: '%v'\n", gotKclModLockStr) + expectKclModLockStr := utils.RmNewline(string(expectKclModLock)) + fmt.Printf("expectKclModLockStr: '%v'\n", expectKclModLockStr) + expectKclModLockReverseStr := utils.RmNewline(string(expectKclModLockReverse)) + fmt.Printf("expectKclModLockReverseStr: '%v'\n", expectKclModLockReverseStr) + assert.Equal(t, - (utils.RmNewline(string(gotKclModLock)) == utils.RmNewline(string(expectKclModLock))) || (utils.RmNewline(string(gotKclModLock)) == utils.RmNewline(string(expectKclModLockReverse))), + (gotKclModLockStr == expectKclModLockStr) || (gotKclModLockStr == expectKclModLockReverseStr), true, ) } diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index af5025b6..1ee145b4 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -120,3 +120,13 @@ func TestDefaultKpmHome(t *testing.T) { assert.Equal(t, kpmHome, filePath) assert.Equal(t, DirExists(kpmHome), true) } + +func TestJoinPath(t *testing.T) { + assert.Equal(t, JoinPath("base", "elem"), "base/elem") + assert.Equal(t, JoinPath("base/", "elem"), "base/elem") + assert.Equal(t, JoinPath("base", "/elem"), "base/elem") + assert.Equal(t, JoinPath("", "/elem"), "/elem") + assert.Equal(t, JoinPath("", "elem"), "/elem") + assert.Equal(t, JoinPath("base/", ""), "base/") + assert.Equal(t, JoinPath("base", ""), "base/") +}