From 15b3bb9e54891c200a533c2fa835a7b74e4b15e9 Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Tue, 18 Jul 2023 15:36:05 +0800 Subject: [PATCH 1/2] fix: lock on global file package-cache when 'kpm add'. --- pkg/cmd/cmd_add.go | 40 +++++++++++++++++------------------ pkg/mod/modfile.go | 8 ------- pkg/package/package.go | 28 ++++-------------------- pkg/settings/settings.go | 8 +++---- pkg/settings/settings_test.go | 12 +++++++---- 5 files changed, 35 insertions(+), 61 deletions(-) diff --git a/pkg/cmd/cmd_add.go b/pkg/cmd/cmd_add.go index 7c36612a..1abb5340 100644 --- a/pkg/cmd/cmd_add.go +++ b/pkg/cmd/cmd_add.go @@ -3,13 +3,10 @@ package cmd import ( - "context" "fmt" "os" "strings" - "time" - "github.com/gofrs/flock" "github.com/urfave/cli/v2" "kcl-lang.io/kpm/pkg/env" "kcl-lang.io/kpm/pkg/errors" @@ -37,6 +34,25 @@ func NewAddCmd() *cli.Command { }, Action: func(c *cli.Context) error { + // 1. get settings from the global config file. + settings, err := settings.GetSettings() + if err != nil { + return err + } + + // 2. acquire the lock of the package cache. + err = settings.AcquirePackageCacheLock() + if err != nil { + return err + } + + defer func() { + // 3. release the lock of the package cache after the function returns. + releaseErr := settings.ReleasePackageCacheLock() + if releaseErr != nil && err == nil { + err = releaseErr + } + }() pwd, err := os.Getwd() @@ -69,24 +85,6 @@ func NewAddCmd() *cli.Command { return err } - // Lock the kcl.mod.lock. - fileLock := flock.New(kclPkg.GetLockFilePath()) - lockCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - locked, err := fileLock.TryLockContext(lockCtx, time.Second) - if err == nil && locked { - defer func() { - if unlockErr := fileLock.Unlock(); unlockErr != nil && err == nil { - err = errors.InternalBug - } - }() - } - if err != nil { - reporter.Report("kpm: sorry, the program encountered an issue while trying to add a dependency.") - reporter.Report("kpm: please try again later") - return err - } - err = kclPkg.AddDeps(addOpts) if err != nil { return err diff --git a/pkg/mod/modfile.go b/pkg/mod/modfile.go index a87ea0f9..0558c0aa 100644 --- a/pkg/mod/modfile.go +++ b/pkg/mod/modfile.go @@ -155,14 +155,6 @@ func (dep *Dependency) Download(localPath string) (*Dependency, error) { return nil, err } - // Creating symbolic links in a global cache is not an optimal solution. - // This allows kcl compiler to locate the package by default. - // This feature is unstable and will be removed soon. - err = utils.CreateSymlink(dep.LocalFullPath, filepath.Join(filepath.Dir(dep.LocalFullPath), dep.Name)) - if err != nil { - return nil, err - } - return dep, nil } diff --git a/pkg/package/package.go b/pkg/package/package.go index 6f83ddd5..91f279c0 100644 --- a/pkg/package/package.go +++ b/pkg/package/package.go @@ -15,7 +15,6 @@ import ( "kcl-lang.io/kpm/pkg/opt" "kcl-lang.io/kpm/pkg/reporter" "kcl-lang.io/kpm/pkg/runner" - "kcl-lang.io/kpm/pkg/settings" "kcl-lang.io/kpm/pkg/utils" ) @@ -250,36 +249,17 @@ func (kclPkg *KclPkg) CreateDefaultKclProgram() error { // AddDeps will add the dependencies to current kcl package and update kcl.mod and kcl.mod.lock. func (kclPkg *KclPkg) AddDeps(opt *opt.AddOptions) error { - // 1. get settings from the global config file. - settings, err := settings.GetSettings() - if err != nil { - return err - } - - // 2. acquire the lock of the package cache. - err = settings.AcquirePackageCacheLock() - if err != nil { - return err - } - - defer func() { - // 3. release the lock of the package cache after the function returns. - releaseErr := settings.ReleasePackageCacheLock() - if releaseErr != nil && err == nil { - err = releaseErr - } - }() - // 4. get the name and version of the repository from the input arguments. + // 1. get the name and version of the repository from the input arguments. d := modfile.ParseOpt(&opt.RegistryOpts) - // 5. download the dependency to the local path. - err = kclPkg.DownloadDep(d, opt.LocalPath) + // 2. download the dependency to the local path. + err := kclPkg.DownloadDep(d, opt.LocalPath) if err != nil { return err } - // 6. update the kcl.mod and kcl.mod.lock. + // 3. update the kcl.mod and kcl.mod.lock. err = kclPkg.UpdateModAndLockFile() if err != nil { return err diff --git a/pkg/settings/settings.go b/pkg/settings/settings.go index 600b13bb..adec490c 100644 --- a/pkg/settings/settings.go +++ b/pkg/settings/settings.go @@ -66,15 +66,15 @@ func (settings *Settings) AcquirePackageCacheLock() error { if err != nil { return err } - + firstTry := true // if failed to lock the 'package-cache' file, wait until it is unlocked. if !locked { - reporter.Report("kpm: waiting for package-cache lock...") for { // try to lock the 'package-cache' file locked, err = settings.PackageCacheLock.TryLock() - if err != nil { - return err + if err != nil && firstTry { + reporter.Report("kpm: waiting for package-cache lock...") + firstTry = false } // if locked, break the loop. if locked { diff --git a/pkg/settings/settings_test.go b/pkg/settings/settings_test.go index e3576318..ae02ae00 100644 --- a/pkg/settings/settings_test.go +++ b/pkg/settings/settings_test.go @@ -123,24 +123,28 @@ func TestPackageCacheLock(t *testing.T) { go func() { defer wg.Done() err := settings.AcquirePackageCacheLock() - assert.Equal(t, err, nil) + fmt.Printf("1: locked.") + fmt.Printf("err: %v\n", err) for i := 0; i < 10; i++ { gotlist = append(gotlist, fmt.Sprintf("goroutine 1: %d", i)) } err = settings.ReleasePackageCacheLock() - assert.Equal(t, err, nil) + fmt.Printf("err: %v\n", err) + fmt.Printf("1: released.") }() // goroutine 2: append "goroutine 2: %d" to the list go func() { defer wg.Done() err := settings.AcquirePackageCacheLock() - assert.Equal(t, err, nil) + fmt.Printf("2: locked.") + fmt.Printf("err: %v\n", err) for i := 0; i < 10; i++ { gotlist = append(gotlist, fmt.Sprintf("goroutine 2: %d", i)) } err = settings.ReleasePackageCacheLock() - assert.Equal(t, err, nil) + fmt.Printf("err: %v\n", err) + fmt.Printf("2: released.") }() wg.Wait() From 959db7d29e1e62de9e6333041c14cbb14555c1fb Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Tue, 18 Jul 2023 17:21:45 +0800 Subject: [PATCH 2/2] fix: fix CR comments. --- pkg/settings/settings.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/settings/settings.go b/pkg/settings/settings.go index adec490c..600b13bb 100644 --- a/pkg/settings/settings.go +++ b/pkg/settings/settings.go @@ -66,15 +66,15 @@ func (settings *Settings) AcquirePackageCacheLock() error { if err != nil { return err } - firstTry := true + // if failed to lock the 'package-cache' file, wait until it is unlocked. if !locked { + reporter.Report("kpm: waiting for package-cache lock...") for { // try to lock the 'package-cache' file locked, err = settings.PackageCacheLock.TryLock() - if err != nil && firstTry { - reporter.Report("kpm: waiting for package-cache lock...") - firstTry = false + if err != nil { + return err } // if locked, break the loop. if locked {