Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 55 additions & 3 deletions modules/git/blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"io"
"os"
"regexp"

"code.gitea.io/gitea/modules/util"
)

// BlamePart represents block of blame - continuous lines with one sha
Expand All @@ -26,6 +28,11 @@ type BlameReader struct {
bufferedReader *bufio.Reader
done chan error
lastSha *string
ignoreRevsFile *os.File
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to store a closed File here. Maybe using ignoreRevsFileName string is good enough?

Otherwise it's not easy to understand when the file should be closed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used os.File here to have an explicit nil check. Sure a != "" check works too but my intent was to have a real file linked to the object. How about changing this member to os.FileInfo? That should be the best of both worlds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but my intent was to have a real file linked to the object.

But the file has been closed, the object itself can't do other things except accessing the f.Name(), which is simply { return f.name }

}

func (r *BlameReader) UsesIgnoreRevs() bool {
return r.ignoreRevsFile != nil
}

var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
Expand Down Expand Up @@ -98,17 +105,32 @@ func (r *BlameReader) Close() error {
r.bufferedReader = nil
_ = r.reader.Close()
_ = r.output.Close()
if r.ignoreRevsFile != nil {
_ = util.Remove(r.ignoreRevsFile.Name())
}
return err
}

// CreateBlameReader creates reader for given repository, commit and file
func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*BlameReader, error) {
cmd := NewCommandContextNoGlobals(ctx, "blame", "--porcelain").
AddDynamicArguments(commitID).
func CreateBlameReader(ctx context.Context, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (*BlameReader, error) {
cmd := NewCommandContextNoGlobals(ctx, "blame", "--porcelain")

var ignoreRevsFile *os.File
if !bypassBlameIgnore {
ignoreRevsFile = tryCreateBlameIgnoreRevsFile(commit)
if ignoreRevsFile != nil {
cmd.AddOptionValues("--ignore-revs-file", ignoreRevsFile.Name())
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to use --ignore-revs-file /dev/stdin and pass the content of the .git-blame-ignore-revs file but there is no Windows equivalent for /dev/stdin.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could think about creating a unix and windows variant where the windows variant uses an actual file while the UNIX variant uses stdin…

}

cmd.AddDynamicArguments(commit.ID.String()).
AddDashesAndList(file).
SetDescription(fmt.Sprintf("GetBlame [repo_path: %s]", repoPath))
reader, stdout, err := os.Pipe()
if err != nil {
if ignoreRevsFile != nil {
_ = util.Remove(ignoreRevsFile.Name())
}
return nil, err
}

Expand All @@ -134,5 +156,35 @@ func CreateBlameReader(ctx context.Context, repoPath, commitID, file string) (*B
reader: reader,
bufferedReader: bufferedReader,
done: done,
ignoreRevsFile: ignoreRevsFile,
}, nil
}

func tryCreateBlameIgnoreRevsFile(commit *Commit) *os.File {
entry, err := commit.GetTreeEntryByPath(".git-blame-ignore-revs")
if err != nil {
return nil
}

r, err := entry.Blob().DataAsync()
if err != nil {
return nil
}
defer r.Close()

f, err := os.CreateTemp("", "gitea_git-blame-ignore-revs")
if err != nil {
return nil
}
defer f.Close()

_, err = io.Copy(f, r)
if err != nil {
defer func() {
_ = util.Remove(f.Name())
}()
return nil
}

return f
}
144 changes: 122 additions & 22 deletions modules/git/blame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,127 @@ func TestReadingBlameOutput(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo5_pulls", "f32b0a9dfd09a60f616f29158f772cedd89942d2", "README.md")
assert.NoError(t, err)
defer blameReader.Close()

parts := []*BlamePart{
{
"72866af952e98d02a73003501836074b286a78f6",
[]string{
"# test_repo",
"Test repository for testing migration from github to gitea",
},
},
{
"f32b0a9dfd09a60f616f29158f772cedd89942d2",
[]string{"", "Do not make any changes to this repo it is used for unit testing"},
},
}

for _, part := range parts {
actualPart, err := blameReader.NextPart()
t.Run("Without .git-blame-ignore-revs", func(t *testing.T) {
repo, err := OpenRepository(ctx, "./tests/repos/repo5_pulls")
assert.NoError(t, err)
assert.Equal(t, part, actualPart)
}
defer repo.Close()

commit, err := repo.GetCommit("f32b0a9dfd09a60f616f29158f772cedd89942d2")
assert.NoError(t, err)

parts := []*BlamePart{
{
"72866af952e98d02a73003501836074b286a78f6",
[]string{
"# test_repo",
"Test repository for testing migration from github to gitea",
},
},
{
"f32b0a9dfd09a60f616f29158f772cedd89942d2",
[]string{"", "Do not make any changes to this repo it is used for unit testing"},
},
}

for _, bypass := range []bool{false, true} {
blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo5_pulls", commit, "README.md", bypass)
assert.NoError(t, err)
assert.NotNil(t, blameReader)
defer blameReader.Close()

assert.False(t, blameReader.UsesIgnoreRevs())

for _, part := range parts {
actualPart, err := blameReader.NextPart()
assert.NoError(t, err)
assert.Equal(t, part, actualPart)
}

// make sure all parts have been read
actualPart, err := blameReader.NextPart()
assert.Nil(t, actualPart)
assert.NoError(t, err)
}
})

t.Run("With .git-blame-ignore-revs", func(t *testing.T) {
repo, err := OpenRepository(ctx, "./tests/repos/repo6_blame")
assert.NoError(t, err)
defer repo.Close()

full := []*BlamePart{
{
"af7486bd54cfc39eea97207ca666aa69c9d6df93",
[]string{"line", "line"},
},
{
"45fb6cbc12f970b04eacd5cd4165edd11c8d7376",
[]string{"changed line"},
},
{
"af7486bd54cfc39eea97207ca666aa69c9d6df93",
[]string{"line", "line", ""},
},
}

cases := []struct {
CommitID string
UsesIgnoreRevs bool
Bypass bool
Parts []*BlamePart
}{
{
CommitID: "544d8f7a3b15927cddf2299b4b562d6ebd71b6a7",
UsesIgnoreRevs: true,
Bypass: false,
Parts: []*BlamePart{
{
"af7486bd54cfc39eea97207ca666aa69c9d6df93",
[]string{"line", "line", "changed line", "line", "line", ""},
},
},
},
{
CommitID: "544d8f7a3b15927cddf2299b4b562d6ebd71b6a7",
UsesIgnoreRevs: false,
Bypass: true,
Parts: full,
},
{
CommitID: "45fb6cbc12f970b04eacd5cd4165edd11c8d7376",
UsesIgnoreRevs: false,
Bypass: false,
Parts: full,
},
{
CommitID: "45fb6cbc12f970b04eacd5cd4165edd11c8d7376",
UsesIgnoreRevs: false,
Bypass: false,
Parts: full,
},
}

for _, c := range cases {
commit, err := repo.GetCommit(c.CommitID)
assert.NoError(t, err)

blameReader, err := CreateBlameReader(ctx, "./tests/repos/repo6_blame", commit, "blame.txt", c.Bypass)
assert.NoError(t, err)
assert.NotNil(t, blameReader)
defer blameReader.Close()

assert.Equal(t, c.UsesIgnoreRevs, blameReader.UsesIgnoreRevs())

for _, part := range c.Parts {
actualPart, err := blameReader.NextPart()
assert.NoError(t, err)
assert.Equal(t, part, actualPart)
}

// make sure all parts have been read
actualPart, err := blameReader.NextPart()
assert.Nil(t, actualPart)
assert.NoError(t, err)
}
})
}
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo6_blame/HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ref: refs/heads/master
4 changes: 4 additions & 0 deletions modules/git/tests/repos/repo6_blame/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[core]
repositoryformatversion = 0
filemode = true
bare = true
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo6_blame/refs/heads/master
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
544d8f7a3b15927cddf2299b4b562d6ebd71b6a7
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,7 @@ delete_preexisting = Delete pre-existing files
delete_preexisting_content = Delete files in %s
delete_preexisting_success = Deleted unadopted files in %s
blame_prior = View blame prior to this change
blame.ignore_revs = Ignoring revisions in .git-blame-ignore-revs.
author_search_tooltip = Shows a maximum of 30 users

transfer.accept = Accept Transfer
Expand Down
11 changes: 5 additions & 6 deletions routers/web/repo/blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"net/url"
"strings"

repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/charset"
"code.gitea.io/gitea/modules/context"
Expand Down Expand Up @@ -45,10 +44,6 @@ func RefBlame(ctx *context.Context) {
return
}

userName := ctx.Repo.Owner.Name
repoName := ctx.Repo.Repository.Name
commitID := ctx.Repo.CommitID

branchLink := ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()
treeLink := branchLink
rawLink := ctx.Repo.RepoLink + "/raw/" + ctx.Repo.BranchNameSubURL()
Expand Down Expand Up @@ -101,13 +96,17 @@ func RefBlame(ctx *context.Context) {
return
}

blameReader, err := git.CreateBlameReader(ctx, repo_model.RepoPath(userName, repoName), commitID, fileName)
bypassBlameIgnore := ctx.FormString("bypass-blame-ignore") == "true"

blameReader, err := git.CreateBlameReader(ctx, ctx.Repo.Repository.RepoPath(), ctx.Repo.Commit, fileName, bypassBlameIgnore)
if err != nil {
ctx.NotFound("CreateBlameReader", err)
return
}
defer blameReader.Close()

ctx.Data["UsesIgnoreRevs"] = blameReader.UsesIgnoreRevs()

blameParts := make([]git.BlamePart, 0)

for {
Expand Down
5 changes: 5 additions & 0 deletions templates/repo/blame.tmpl
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
{{if .UsesIgnoreRevs}}
<div class="ui info message">
<p><a href="{{.RepoLink}}/src/{{.BranchNameSubURL}}/.git-blame-ignore-revs">{{.locale.Tr "repo.blame.ignore_revs"}}</a></p>
</div>
{{end}}
<div class="{{TabSizeClass .Editorconfig .FileName}} non-diff-file-content">
<h4 class="file-header ui top attached header gt-df gt-ac gt-sb gt-fw">
<div class="file-header-left gt-df gt-ac gt-py-3 gt-pr-4">
Expand Down