fix: fix redownload when the remote repo has no mod file (#542)

* fix: fix redownload when the remote repo has no mod file Signed-off-by: zongz <[email protected]> * fix: move test case into global lock Signed-off-by: zongz <[email protected]> * fix: fix test case Signed-off-by: zongz <[email protected]> * fix: fix test case TestGraph Signed-off-by: zongz <[email protected]> * fix: fix test case Signed-off-by: zongz <[email protected]> * refactor: refactor test utils to create a separate kpm home for testing Signed-off-by: zongz <[email protected]> * fix: move some test into separate kpm home Signed-off-by: zongz <[email protected]> --------- Signed-off-by: zongz <[email protected]>
kcl-lang · Nov 13, 2024 · 6b2c011 · 6b2c011
1 parent e828ddd
commit 6b2c011
Show file tree

Hide file tree

Showing 18 changed files with 302 additions and 159 deletions.
diff --git a/pkg/client/add_test.go b/pkg/client/add_test.go
diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go
@@ -27,7 +27,6 @@ import (
 	"kcl-lang.io/kcl-go/pkg/kcl"
 	"kcl-lang.io/kpm/pkg/downloader"
 	"kcl-lang.io/kpm/pkg/env"
-	"kcl-lang.io/kpm/pkg/features"
 	"kcl-lang.io/kpm/pkg/opt"
 	pkg "kcl-lang.io/kpm/pkg/package"
 	"kcl-lang.io/kpm/pkg/reporter"
@@ -36,25 +35,6 @@ import (
 	"kcl-lang.io/kpm/pkg/utils"
 )
 
-const testDataDir = "test_data"
-
-func getTestDir(subDir string) string {
-	pwd, _ := os.Getwd()
-	testDir := filepath.Join(pwd, testDataDir)
-	testDir = filepath.Join(testDir, subDir)
-
-	return testDir
-}
-
-func initTestDir(subDir string) string {
-	testDir := getTestDir(subDir)
-	// clean the test data
-	_ = os.RemoveAll(testDir)
-	_ = os.Mkdir(testDir, 0755)
-
-	return testDir
-}
-
 func TestWithGlobalLock(t *testing.T) {
 	test.RunTestWithGlobalLock(t, "TestUpdateWithKclMod", testUpdateWithKclMod)
 	test.RunTestWithGlobalLock(t, "TestUpdateWithKclModlock", testUpdateWithKclModlock)
@@ -83,7 +63,6 @@ func TestWithGlobalLock(t *testing.T) {
 	test.RunTestWithGlobalLock(t, "TestDownloadGitWithPackage", testDownloadGitWithPackage)
 	test.RunTestWithGlobalLock(t, "TestModandLockFilesWithGitPackageDownload", testModandLockFilesWithGitPackageDownload)
 	test.RunTestWithGlobalLock(t, "TestDependencyGraph", testDependencyGraph)
-	test.RunTestWithGlobalLock(t, "TestAddWithModSpec", testAddWithModSpec)
 	test.RunTestWithGlobalLock(t, "TestRunRemoteWithArgsInvalid", testRunRemoteWithArgsInvalid)
 	test.RunTestWithGlobalLock(t, "TestRunRemoteWithArgs", testRunRemoteWithArgs)
 	test.RunTestWithGlobalLock(t, "TestRunWithNoSumCheck", testRunWithGitPackage)
@@ -93,18 +72,9 @@ func TestWithGlobalLock(t *testing.T) {
 	test.RunTestWithGlobalLock(t, "TestPull", testPull)
 	test.RunTestWithGlobalLock(t, "TestPullWithInsecureSkipTLSverify", testPullWithInsecureSkipTLSverify)
 	test.RunTestWithGlobalLock(t, "TestPullWithModSpec", testPullWithModSpec)
-	test.RunTestWithGlobalLock(t, "testAddWithOnlyModSpec", testAddWithOnlyModSpec)
-	test.RunTestWithGlobalLock(t, "testAddRenameWithModSpec", testAddRenameWithModSpec)
-	test.RunTestWithGlobalLock(t, "testAddRenameWithNoSpec", testAddRenameWithNoSpec)
 	test.RunTestWithGlobalLock(t, "testPullWithOnlySpec", testPullWithOnlySpec)
 	test.RunTestWithGlobalLock(t, "TestRunWithModSpecVersion", testRunWithModSpecVersion)
 	test.RunTestWithGlobalLock(t, "TestGraph", testGraph)
-
-	features.Enable(features.SupportNewStorage)
-	test.RunTestWithGlobalLock(t, "testAddWithModSpec", testAddWithModSpec)
-	test.RunTestWithGlobalLock(t, "testAddWithOnlyModSpec", testAddWithOnlyModSpec)
-	test.RunTestWithGlobalLock(t, "testAddRenameWithModSpec", testAddRenameWithModSpec)
-	test.RunTestWithGlobalLock(t, "testAddRenameWithNoSpec", testAddRenameWithNoSpec)
 }
 
 // TestDownloadOci test download from oci registry.

diff --git a/pkg/client/graph_test.go b/pkg/client/graph_test.go
@@ -4,8 +4,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/mod/module"
-	"gotest.tools/v3/assert"
 	pkg "kcl-lang.io/kpm/pkg/package"
 	"kcl-lang.io/kpm/pkg/utils"
 )
@@ -43,5 +43,7 @@ func testGraph(t *testing.T) {
 		t.Fatalf("failed to display graph: %v", err)
 	}
 
-	assert.Equal(t, utils.RmNewline(graStr), "[email protected] [email protected]@0.0.1 [email protected]@0.0.1 [email protected]")
+	assert.Contains(t, utils.RmNewline(graStr), "[email protected] [email protected]")
+	assert.Contains(t, utils.RmNewline(graStr), "[email protected] [email protected]")
+	assert.Contains(t, utils.RmNewline(graStr), "[email protected] [email protected]")
 }
diff --git a/pkg/client/test.go b/pkg/client/test.go
@@ -0,0 +1,60 @@
+package client
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+const testDataDir = "test_data"
+
+func getTestDir(subDir string) string {
+	pwd, _ := os.Getwd()
+	testDir := filepath.Join(pwd, testDataDir)
+	testDir = filepath.Join(testDir, subDir)
+
+	return testDir
+}
+
+func initTestDir(subDir string) string {
+	testDir := getTestDir(subDir)
+	// clean the test data
+	_ = os.RemoveAll(testDir)
+	_ = os.Mkdir(testDir, 0755)
+
+	return testDir
+}
+
+// Use a global variable to store the kpmcli instance.
+func RunTestWithGlobalLockAndKpmCli(t *testing.T, name string, testFunc func(t *testing.T, kpmcli *KpmClient)) {
+	t.Run(name, func(t *testing.T) {
+		kpmcli, err := NewKpmClient()
+		if err != nil {
+			t.Errorf("Error acquiring lock: %v", err)
+		}
+		err = kpmcli.AcquirePackageCacheLock()
+		if err != nil {
+			t.Errorf("Error acquiring lock: %v", err)
+		}
+
+		defer func() {
+			err = kpmcli.ReleasePackageCacheLock()
+			if err != nil {
+				t.Errorf("Error acquiring lock: %v", err)
+			}
+		}()
+
+		// create a tmp dir as kpm home for test
+		tmpDir, err := os.MkdirTemp("", "")
+		if err != nil {
+			t.Errorf("Error acquiring lock: %v", err)
+		}
+		// clean the temp dir.
+		defer os.RemoveAll(tmpDir)
+		kpmcli.SetHomePath(tmpDir)
+
+		testFunc(t, kpmcli)
+		fmt.Printf("%s completed\n", name)
+	})
+}
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.bk b/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.bk
@@ -0,0 +1,4 @@
+[package]
+name = "git"
+edition = "v0.10.0"
+version = "0.0.1"
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.expect b/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.expect
@@ -0,0 +1,7 @@
+[package]
+name = "git"
+edition = "v0.10.0"
+version = "0.0.1"
+
+[dependencies]
+cc = { git = "https://github.com/kcl-lang/flask-demo-kcl-manifests.git", commit = "8308200", version = "0.0.1" }
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.lock.bk b/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.lock.bk
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.lock.expect b/pkg/client/test_data/add_with_mod_spec/git_mod_0/kcl.mod.lock.expect
@@ -0,0 +1,7 @@
+[dependencies]
+  [dependencies.cc]
+    name = "cc"
+    full_name = "cc_0.0.1"
+    version = "0.0.1"
+    url = "https://github.com/kcl-lang/flask-demo-kcl-manifests.git"
+    commit = "8308200"
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_0/main.k b/pkg/client/test_data/add_with_mod_spec/git_mod_0/main.k
@@ -0,0 +1 @@
+The_first_kcl_program = 'Hello World!'
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.bk b/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.bk
@@ -0,0 +1,4 @@
+[package]
+name = "git"
+edition = "v0.10.0"
+version = "0.0.1"
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.expect b/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.expect
@@ -0,0 +1,7 @@
+[package]
+name = "git"
+edition = "v0.10.0"
+version = "0.0.1"
+
+[dependencies]
+cc = { git = "https://github.com/kcl-lang/flask-demo-kcl-manifests.git", commit = "5ab0fff", version = "0.0.1" }
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.lock.bk b/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.lock.bk
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.lock.expect b/pkg/client/test_data/add_with_mod_spec/git_mod_1/kcl.mod.lock.expect
@@ -0,0 +1,7 @@
+[dependencies]
+  [dependencies.cc]
+    name = "cc"
+    full_name = "cc_0.0.1"
+    version = "0.0.1"
+    url = "https://github.com/kcl-lang/flask-demo-kcl-manifests.git"
+    commit = "5ab0fff"
diff --git a/pkg/client/test_data/add_with_mod_spec/git_mod_1/main.k b/pkg/client/test_data/add_with_mod_spec/git_mod_1/main.k
@@ -0,0 +1 @@
+The_first_kcl_program = 'Hello World!'
diff --git a/pkg/downloader/downloader.go b/pkg/downloader/downloader.go
@@ -275,7 +275,7 @@ func (d *DepDownloader) Download(opts *DownloadOptions) error {
 	localPath := opts.LocalPath
 	cacheFullPath := opts.CachePath
 	if ok, err := features.Enabled(features.SupportNewStorage); err == nil && !ok && opts.EnableCache {
-		if utils.DirExists(cacheFullPath) && utils.DirExists(filepath.Join(cacheFullPath, constants.KCL_MOD)) &&
+		if utils.DirExists(cacheFullPath) &&
 			// If the version in modspec is empty, meanings the latest version is needed.
 			// The latest version should be requested first and the cache should be updated.
 			((opts.Source.ModSpec != nil && opts.Source.ModSpec.Version != "") || opts.Source.ModSpec == nil) {

diff --git a/pkg/package/modfile.go b/pkg/package/modfile.go
@@ -571,7 +571,7 @@ func ParseOpt(opt *opt.RegistryOptions) (*Dependency, error) {
 					Version: opt.Registry.Tag,
 					Name:    opt.Registry.Ref,
 				},
-				Oci:      &ociSource,
+				Oci: &ociSource,
 			},
 			Version: opt.Registry.Tag,
 		}, nil

diff --git a/pkg/package/test_data/test_rename_pkg/kcl.mod.lock b/pkg/package/test_data/test_rename_pkg/kcl.mod.lock
@@ -0,0 +1,8 @@
+[dependencies]
+  [dependencies.newpkg]
+    name = "newpkg"
+    full_name = "newpkg_0.0.1"
+    version = "0.0.1"
+    reg = "ghcr.io"
+    repo = "kcl-lang/helloworld"
+    oci_tag = "0.1.4"
diff --git a/pkg/visitor/visitor.go b/pkg/visitor/visitor.go
@@ -222,13 +222,6 @@ func (rv *RemoteVisitor) Visit(s *downloader.Source, v visitFunc) error {
 		defer os.RemoveAll(tmpDir)
 	}
 
-	if !utils.DirExists(modFullPath) {
-		err := os.MkdirAll(modFullPath, 0755)
-		if err != nil {
-			return err
-		}
-	}
-
 	credCli, err = downloader.LoadCredentialFile(rv.Settings.CredentialsFile)
 	if err != nil {
 		return err