From 4725a884f8e8666708ef5943c2c774e7f3f641d6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 May 2025 14:12:14 -0700 Subject: [PATCH 1/7] Don't display error log when .git-blame-ignore-revs doesn't exist --- modules/git/blame.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 6eb583a6b9c44..e16aea714f9c9 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -132,7 +132,7 @@ func (r *BlameReader) Close() error { } // CreateBlameReader creates reader for given repository, commit and file -func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (*BlameReader, error) { +func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (rd *BlameReader, err error) { reader, stdout, err := os.Pipe() if err != nil { return nil, err @@ -141,9 +141,18 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath cmd := NewCommandNoGlobals("blame", "--porcelain") var ignoreRevsFileName string - var ignoreRevsFileCleanup func() // TODO: maybe it should check the returned err in a defer func to make sure the cleanup could always be executed correctly + var ignoreRevsFileCleanup func() + defer func() { + if err != nil && ignoreRevsFileCleanup != nil { + ignoreRevsFileCleanup() + } + }() + if DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore { - ignoreRevsFileName, ignoreRevsFileCleanup = tryCreateBlameIgnoreRevsFile(commit) + ignoreRevsFileName, ignoreRevsFileCleanup, err = tryCreateBlameIgnoreRevsFile(commit) + if err != nil { + return nil, err + } if ignoreRevsFileName != "" { // Possible improvement: use --ignore-revs-file /dev/stdin on unix // There is no equivalent on Windows. May be implemented if Gitea uses an external git backend. @@ -182,33 +191,32 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath }, nil } -func tryCreateBlameIgnoreRevsFile(commit *Commit) (string, func()) { +func tryCreateBlameIgnoreRevsFile(commit *Commit) (string, func(), error) { entry, err := commit.GetTreeEntryByPath(".git-blame-ignore-revs") if err != nil { - log.Error("Unable to get .git-blame-ignore-revs file: GetTreeEntryByPath: %v", err) - return "", nil + if IsErrNotExist(err) { + return "", nil, nil + } + return "", nil, err } r, err := entry.Blob().DataAsync() if err != nil { - log.Error("Unable to get .git-blame-ignore-revs file data: DataAsync: %v", err) - return "", nil + return "", nil, err } defer r.Close() f, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("git-blame-ignore-revs") if err != nil { - log.Error("Unable to get .git-blame-ignore-revs file data: CreateTempFileRandom: %v", err) - return "", nil + return "", nil, err } filename := f.Name() _, err = io.Copy(f, r) _ = f.Close() if err != nil { cleanup() - log.Error("Unable to get .git-blame-ignore-revs file data: Copy: %v", err) - return "", nil + return "", nil, err } - return filename, cleanup + return filename, cleanup, nil } From af1131fa0156f7d8d5f2901f471763df31c70467 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 13 May 2025 20:35:41 -0700 Subject: [PATCH 2/7] Fix bug --- modules/git/blame.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index e16aea714f9c9..ca4f8ec832611 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -133,13 +133,6 @@ func (r *BlameReader) Close() error { // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (rd *BlameReader, err error) { - reader, stdout, err := os.Pipe() - if err != nil { - return nil, err - } - - cmd := NewCommandNoGlobals("blame", "--porcelain") - var ignoreRevsFileName string var ignoreRevsFileCleanup func() defer func() { @@ -148,6 +141,8 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath } }() + cmd := NewCommandNoGlobals("blame", "--porcelain") + if DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore { ignoreRevsFileName, ignoreRevsFileCleanup, err = tryCreateBlameIgnoreRevsFile(commit) if err != nil { @@ -163,6 +158,11 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file) done := make(chan error, 1) + // use err1 to not override err + reader, stdout, err1 := os.Pipe() + if err1 != nil { + return nil, err1 + } go func() { stderr := bytes.Buffer{} // TODO: it doesn't work for directories (the directories shouldn't be "blamed"), and the "err" should be returned by "Read" but not by "Close" From 0d91124f454a840e3f6e75d8f6bbd411d24ba14a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 May 2025 11:50:09 -0700 Subject: [PATCH 3/7] rename err1 to errPipe --- modules/git/blame.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index ca4f8ec832611..65c536daa1012 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -159,9 +159,9 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath done := make(chan error, 1) // use err1 to not override err - reader, stdout, err1 := os.Pipe() - if err1 != nil { - return nil, err1 + reader, stdout, errPipe := os.Pipe() + if errPipe != nil { + return nil, errPipe } go func() { stderr := bytes.Buffer{} From 3dab8276e2d2362e39cccbab977399737a10ab4f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 May 2025 11:54:57 -0700 Subject: [PATCH 4/7] Don't handle IsNotExist error in the function --- modules/git/blame.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 65c536daa1012..c639514b5a893 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -145,7 +145,7 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath if DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore { ignoreRevsFileName, ignoreRevsFileCleanup, err = tryCreateBlameIgnoreRevsFile(commit) - if err != nil { + if err != nil && !IsErrNotExist(err) { return nil, err } if ignoreRevsFileName != "" { @@ -194,9 +194,6 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath func tryCreateBlameIgnoreRevsFile(commit *Commit) (string, func(), error) { entry, err := commit.GetTreeEntryByPath(".git-blame-ignore-revs") if err != nil { - if IsErrNotExist(err) { - return "", nil, nil - } return "", nil, err } From 7b25aae3f0c5527c8dce0c47c1746cb451044996 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 May 2025 11:58:24 -0700 Subject: [PATCH 5/7] update comment --- modules/git/blame.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index c639514b5a893..1ff91c2632abf 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -158,7 +158,7 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file) done := make(chan error, 1) - // use err1 to not override err + // Use errPipe to preserve the original err and avoid overwriting it. reader, stdout, errPipe := os.Pipe() if errPipe != nil { return nil, errPipe From 3dc618054e1e687f423e169aa2ecca92bbb72128 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 May 2025 18:42:01 -0700 Subject: [PATCH 6/7] Use err --- modules/git/blame.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index 1ff91c2632abf..f839f2ff2d014 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -159,9 +159,9 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath done := make(chan error, 1) // Use errPipe to preserve the original err and avoid overwriting it. - reader, stdout, errPipe := os.Pipe() - if errPipe != nil { - return nil, errPipe + reader, stdout, err := os.Pipe() + if err != nil { + return nil, err } go func() { stderr := bytes.Buffer{} From c8e150aba2e6efb28592441c574fd4929865cdbb Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 25 May 2025 23:20:06 -0700 Subject: [PATCH 7/7] Remove comment --- modules/git/blame.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/git/blame.go b/modules/git/blame.go index f839f2ff2d014..659dec34a1e78 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -158,7 +158,6 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file) done := make(chan error, 1) - // Use errPipe to preserve the original err and avoid overwriting it. reader, stdout, err := os.Pipe() if err != nil { return nil, err