Skip to content

Commit

Permalink
Fix commit retrieval by tag (go-gitea#21804)
Browse files Browse the repository at this point in the history
It is not correct to return tag data when commit data is requested, so
remove the hacky code that overwrote parts of a commit with parts of a
tag.

This fixes commit retrieval by tag for both the latest commit in the UI
and the commit info on tag webhook events.

Fixes: go-gitea#21687
Replaces: go-gitea#21693

<img width="324" alt="Screenshot 2022-11-13 at 15 26 37"
src="https://user-images.githubusercontent.com/115237/201526975-736c6ea7-ad6a-467a-a823-9a63d6ecb718.png">

<img width="789" alt="image"
src="https://user-images.githubusercontent.com/115237/201526876-90a13ffc-1e5c-4d76-911b-f1ae51e8eaab.png">

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
silverwind and lunny authored Mar 2, 2023
1 parent 0945bf6 commit ea1d097
Show file tree
Hide file tree
Showing 19 changed files with 26 additions and 58 deletions.
39 changes: 0 additions & 39 deletions modules/git/repo_commit_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
package git

import (
"fmt"
"strings"

"github.com/go-git/go-git/v5/plumbing"
Expand Down Expand Up @@ -67,38 +66,6 @@ func (repo *Repository) IsCommitExist(name string) bool {
return err == nil
}

func convertPGPSignatureForTag(t *object.Tag) *CommitGPGSignature {
if t.PGPSignature == "" {
return nil
}

var w strings.Builder
var err error

if _, err = fmt.Fprintf(&w,
"object %s\ntype %s\ntag %s\ntagger ",
t.Target.String(), t.TargetType.Bytes(), t.Name); err != nil {
return nil
}

if err = t.Tagger.Encode(&w); err != nil {
return nil
}

if _, err = fmt.Fprintf(&w, "\n\n"); err != nil {
return nil
}

if _, err = fmt.Fprintf(&w, t.Message); err != nil {
return nil
}

return &CommitGPGSignature{
Signature: t.PGPSignature,
Payload: strings.TrimSpace(w.String()) + "\n",
}
}

func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
var tagObject *object.Tag

Expand All @@ -122,12 +89,6 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
commit := convertCommit(gogitCommit)
commit.repo = repo

if tagObject != nil {
commit.CommitMessage = strings.TrimSpace(tagObject.Message)
commit.Author = &tagObject.Tagger
commit.Signature = convertPGPSignatureForTag(tagObject)
}

tree, err := gogitCommit.Tree()
if err != nil {
return nil, err
Expand Down
4 changes: 0 additions & 4 deletions modules/git/repo_commit_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
return nil, err
}

commit.CommitMessage = strings.TrimSpace(tag.Message)
commit.Author = tag.Tagger
commit.Signature = tag.Signature

return commit, nil
case "commit":
commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size))
Expand Down
5 changes: 3 additions & 2 deletions modules/git/repo_commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ func TestGetTagCommitWithSignature(t *testing.T) {
assert.NoError(t, err)
defer bareRepo1.Close()

commit, err := bareRepo1.GetCommit("3ad28a9149a2864384548f3d17ed7f38014c9e8a")
// both the tag and the commit are signed here, this validates only the commit signature
commit, err := bareRepo1.GetCommit("28b55526e7100924d864dd89e35c1ea62e7a5a32")
assert.NoError(t, err)
assert.NotNil(t, commit)
assert.NotNil(t, commit.Signature)
// test that signature is not in message
assert.Equal(t, "tag", commit.CommitMessage)
assert.Equal(t, "signed-commit\n", commit.CommitMessage)
}

func TestGetCommitWithBadCommitID(t *testing.T) {
Expand Down
12 changes: 8 additions & 4 deletions modules/git/repo_ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ func TestRepository_GetRefs(t *testing.T) {
refs, err := bareRepo1.GetRefs()

assert.NoError(t, err)
assert.Len(t, refs, 5)
assert.Len(t, refs, 6)

expectedRefs := []string{
BranchPrefix + "branch1",
BranchPrefix + "branch2",
BranchPrefix + "master",
TagPrefix + "test",
TagPrefix + "signed-tag",
NotesRef,
}

Expand All @@ -43,9 +44,12 @@ func TestRepository_GetRefsFiltered(t *testing.T) {
refs, err := bareRepo1.GetRefsFiltered(TagPrefix)

assert.NoError(t, err)
if assert.Len(t, refs, 1) {
assert.Equal(t, TagPrefix+"test", refs[0].Name)
if assert.Len(t, refs, 2) {
assert.Equal(t, TagPrefix+"signed-tag", refs[0].Name)
assert.Equal(t, "tag", refs[0].Type)
assert.Equal(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", refs[0].Object.String())
assert.Equal(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", refs[0].Object.String())
assert.Equal(t, TagPrefix+"test", refs[1].Name)
assert.Equal(t, "tag", refs[1].Type)
assert.Equal(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", refs[1].Object.String())
}
}
4 changes: 2 additions & 2 deletions modules/git/repo_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func TestRepository_GetCodeActivityStats(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, code)

assert.EqualValues(t, 9, code.CommitCount)
assert.EqualValues(t, 10, code.CommitCount)
assert.EqualValues(t, 3, code.AuthorCount)
assert.EqualValues(t, 9, code.CommitCountInAllBranches)
assert.EqualValues(t, 10, code.CommitCountInAllBranches)
assert.EqualValues(t, 10, code.Additions)
assert.EqualValues(t, 1, code.Deletions)
assert.Len(t, code.Authors, 3)
Expand Down
9 changes: 6 additions & 3 deletions modules/git/repo_tag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ func TestRepository_GetTags(t *testing.T) {
assert.NoError(t, err)
return
}
assert.Len(t, tags, 1)
assert.Len(t, tags, 2)
assert.Equal(t, len(tags), total)
assert.EqualValues(t, "test", tags[0].Name)
assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[0].ID.String())
assert.EqualValues(t, "signed-tag", tags[0].Name)
assert.EqualValues(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", tags[0].ID.String())
assert.EqualValues(t, "tag", tags[0].Type)
assert.EqualValues(t, "test", tags[1].Name)
assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[1].ID.String())
assert.EqualValues(t, "tag", tags[1].Type)
}

func TestRepository_GetTag(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions modules/git/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ func TestGetLatestCommitTime(t *testing.T) {
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
lct, err := GetLatestCommitTime(DefaultContext, bareRepo1Path)
assert.NoError(t, err)
// Time is Sun Jul 21 22:43:13 2019 +0200
// Time is Sun Nov 13 16:40:14 2022 +0100
// which is the time of commit
// feaf4ba6bc635fec442f46ddd4512416ec43c2c2 (refs/heads/master)
assert.EqualValues(t, 1563741793, lct.Unix())
// ce064814f4a0d337b333e646ece456cd39fab612 (refs/heads/master)
assert.EqualValues(t, 1668354014, lct.Unix())
}

func TestRepoIsEmpty(t *testing.T) {
Expand Down
Binary file added modules/git/tests/repos/repo1_bare/index
Binary file not shown.
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo1_bare/logs/HEAD
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push
feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind <me@silverwind.io> 1668354026 +0100 push
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo1_bare/logs/refs/heads/master
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push
feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind <me@silverwind.io> 1668354026 +0100 push
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.
2 changes: 1 addition & 1 deletion modules/git/tests/repos/repo1_bare/refs/heads/master
Original file line number Diff line number Diff line change
@@ -1 +1 @@
feaf4ba6bc635fec442f46ddd4512416ec43c2c2
ce064814f4a0d337b333e646ece456cd39fab612
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo1_bare/refs/tags/signed-tag
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
36f97d9a96457e2bab511db30fe2db03893ebc64

0 comments on commit ea1d097

Please sign in to comment.