From 3dd66b99f0d7e9fee5295c02e0a357060942e107 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 17 Aug 2022 15:13:47 +0800 Subject: [PATCH] make CheckPath logic clear --- modules/git/repo_attribute.go | 38 ++++++++++------------ modules/git/repo_language_stats_gogit.go | 9 +++-- modules/git/repo_language_stats_nogogit.go | 10 ++++-- services/gitdiff/gitdiff.go | 12 ++++--- 4 files changed, 38 insertions(+), 31 deletions(-) diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 1305e6f22494e..1964a5ea92601 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -190,24 +190,18 @@ func (c *CheckAttributeReader) Run() error { // CheckPath check attr for given path func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err error) { - defer func() { - if err != nil { - log.Error("CheckPath returns error: %v", err) - } - }() + rs = make(map[string]string) select { case <-c.ctx.Done(): - return nil, c.ctx.Err() + return default: } if _, err = c.stdinWriter.Write([]byte(path + "\x00")); err != nil { - defer c.Close() return nil, err } - rs = make(map[string]string) for range c.Attributes { select { case attr, ok := <-c.stdOut.ReadAttribute(): @@ -216,7 +210,7 @@ func (c *CheckAttributeReader) CheckPath(path string) (rs map[string]string, err } rs[attr.Attribute] = attr.Value case <-c.ctx.Done(): - return nil, c.ctx.Err() + return rs, nil } } return rs, nil @@ -394,10 +388,10 @@ func (wr *lineSeparatedAttributeWriter) Close() error { } // Create a check attribute reader for the current repository and provided commit ID -func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeReader, context.CancelFunc) { +func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeReader, context.CancelFunc, error) { indexFilename, worktree, deleteTemporaryFile, err := repo.ReadTreeToTemporaryIndex(commitID) if err != nil { - return nil, func() {} + return nil, func() {}, err } checker := &CheckAttributeReader{ @@ -408,21 +402,23 @@ func (repo *Repository) CheckAttributeReader(commitID string) (*CheckAttributeRe } ctx, cancel := context.WithCancel(repo.Ctx) if err := checker.Init(ctx); err != nil { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } else { - go func() { - err := checker.Run() - if err != nil && err != ctx.Err() { - log.Error("Unable to open checker for %s. Error: %v", commitID, err) - } - cancel() - }() + cancel() + return nil, nil, fmt.Errorf("unable to open checker for %s. Error: %v", commitID, err) } + + go func() { + err := checker.Run() + if err != nil && err != ctx.Err() { + log.Error("Unable to open checker for %s. Error: %v", commitID, err) + } + cancel() + }() + deferable := func() { _ = checker.Close() cancel() deleteTemporaryFile() } - return checker, deferable + return checker, deferable, nil } diff --git a/modules/git/repo_language_stats_gogit.go b/modules/git/repo_language_stats_gogit.go index 34b0dc45d3749..81f9e042533ad 100644 --- a/modules/git/repo_language_stats_gogit.go +++ b/modules/git/repo_language_stats_gogit.go @@ -41,7 +41,10 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, err } - checker, deferable := repo.CheckAttributeReader(commitID) + checker, deferable, err := repo.CheckAttributeReader(commitID) + if err != nil { + return nil, err + } defer deferable() sizes := make(map[string]int64) @@ -55,7 +58,9 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err if checker != nil { attrs, err := checker.CheckPath(f.Name) - if err == nil { + if err != nil { + log.Error("CheckPath returns error: %v", err) + } else { if vendored, has := attrs["linguist-vendored"]; has { if vendored == "set" || vendored == "true" { return nil diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index d237924f92a4c..3ad6802b5eac1 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/modules/git/repo_language_stats_nogogit.go @@ -62,7 +62,10 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err return nil, err } - checker, deferable := repo.CheckAttributeReader(commitID) + checker, deferable, err := repo.CheckAttributeReader(commitID) + if err != nil { + return nil, err + } defer deferable() contentBuf := bytes.Buffer{} @@ -87,7 +90,9 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err if checker != nil { attrs, err := checker.CheckPath(f.Name()) - if err == nil { + if err != nil { + log.Error("CheckPath returns error: %v", err) + } else { if vendored, has := attrs["linguist-vendored"]; has { if vendored == "set" || vendored == "true" { continue @@ -125,7 +130,6 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err continue } } - } } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 9844992f5b11d..3ccdc63474695 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1147,16 +1147,20 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff } diff.Start = opts.SkipTo - checker, deferable := gitRepo.CheckAttributeReader(opts.AfterCommitID) + checker, deferable, err := gitRepo.CheckAttributeReader(opts.AfterCommitID) + if err != nil { + return nil, err + } defer deferable() for _, diffFile := range diff.Files { - gotVendor := false gotGenerated := false if checker != nil { attrs, err := checker.CheckPath(diffFile.Name) - if err == nil { + if err != nil { + log.Error("CheckPath returns error: %v", err) + } else { if vendored, has := attrs["linguist-vendored"]; has { if vendored == "set" || vendored == "true" { diffFile.IsVendored = true @@ -1178,8 +1182,6 @@ func GetDiff(gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff } else if language, has := attrs["gitlab-language"]; has && language != "unspecified" && language != "" { diffFile.Language = language } - } else { - log.Error("Unexpected error: %v", err) } }