Fix commit retrieval by tag (#21804)

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: https://github.com/go-gitea/gitea/issues/21687
Replaces: https://github.com/go-gitea/gitea/pull/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>
This commit is contained in:
silverwind 2023-03-02 06:32:21 +01:00 committed by GitHub
parent 0945bf63d3
commit ea1d09718c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 26 additions and 58 deletions

View file

@ -7,7 +7,6 @@
package git package git
import ( import (
"fmt"
"strings" "strings"
"github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing"
@ -67,38 +66,6 @@ func (repo *Repository) IsCommitExist(name string) bool {
return err == nil 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) { func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
var tagObject *object.Tag var tagObject *object.Tag
@ -122,12 +89,6 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
commit := convertCommit(gogitCommit) commit := convertCommit(gogitCommit)
commit.repo = repo commit.repo = repo
if tagObject != nil {
commit.CommitMessage = strings.TrimSpace(tagObject.Message)
commit.Author = &tagObject.Tagger
commit.Signature = convertPGPSignatureForTag(tagObject)
}
tree, err := gogitCommit.Tree() tree, err := gogitCommit.Tree()
if err != nil { if err != nil {
return nil, err return nil, err

View file

@ -107,10 +107,6 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
return nil, err return nil, err
} }
commit.CommitMessage = strings.TrimSpace(tag.Message)
commit.Author = tag.Tagger
commit.Signature = tag.Signature
return commit, nil return commit, nil
case "commit": case "commit":
commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size)) commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size))

View file

@ -43,12 +43,13 @@ func TestGetTagCommitWithSignature(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
defer bareRepo1.Close() 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.NoError(t, err)
assert.NotNil(t, commit) assert.NotNil(t, commit)
assert.NotNil(t, commit.Signature) assert.NotNil(t, commit.Signature)
// test that signature is not in message // 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) { func TestGetCommitWithBadCommitID(t *testing.T) {

View file

@ -19,13 +19,14 @@ func TestRepository_GetRefs(t *testing.T) {
refs, err := bareRepo1.GetRefs() refs, err := bareRepo1.GetRefs()
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, refs, 5) assert.Len(t, refs, 6)
expectedRefs := []string{ expectedRefs := []string{
BranchPrefix + "branch1", BranchPrefix + "branch1",
BranchPrefix + "branch2", BranchPrefix + "branch2",
BranchPrefix + "master", BranchPrefix + "master",
TagPrefix + "test", TagPrefix + "test",
TagPrefix + "signed-tag",
NotesRef, NotesRef,
} }
@ -43,9 +44,12 @@ func TestRepository_GetRefsFiltered(t *testing.T) {
refs, err := bareRepo1.GetRefsFiltered(TagPrefix) refs, err := bareRepo1.GetRefsFiltered(TagPrefix)
assert.NoError(t, err) assert.NoError(t, err)
if assert.Len(t, refs, 1) { if assert.Len(t, refs, 2) {
assert.Equal(t, TagPrefix+"test", refs[0].Name) assert.Equal(t, TagPrefix+"signed-tag", refs[0].Name)
assert.Equal(t, "tag", refs[0].Type) 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())
} }
} }

View file

@ -24,9 +24,9 @@ func TestRepository_GetCodeActivityStats(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.NotNil(t, code) 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, 3, code.AuthorCount)
assert.EqualValues(t, 9, code.CommitCountInAllBranches) assert.EqualValues(t, 10, code.CommitCountInAllBranches)
assert.EqualValues(t, 10, code.Additions) assert.EqualValues(t, 10, code.Additions)
assert.EqualValues(t, 1, code.Deletions) assert.EqualValues(t, 1, code.Deletions)
assert.Len(t, code.Authors, 3) assert.Len(t, code.Authors, 3)

View file

@ -25,11 +25,14 @@ func TestRepository_GetTags(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
return return
} }
assert.Len(t, tags, 1) assert.Len(t, tags, 2)
assert.Equal(t, len(tags), total) assert.Equal(t, len(tags), total)
assert.EqualValues(t, "test", tags[0].Name) assert.EqualValues(t, "signed-tag", tags[0].Name)
assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[0].ID.String()) assert.EqualValues(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", tags[0].ID.String())
assert.EqualValues(t, "tag", tags[0].Type) 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) { func TestRepository_GetTag(t *testing.T) {

View file

@ -14,10 +14,10 @@ func TestGetLatestCommitTime(t *testing.T) {
bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
lct, err := GetLatestCommitTime(DefaultContext, bareRepo1Path) lct, err := GetLatestCommitTime(DefaultContext, bareRepo1Path)
assert.NoError(t, err) 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 // which is the time of commit
// feaf4ba6bc635fec442f46ddd4512416ec43c2c2 (refs/heads/master) // ce064814f4a0d337b333e646ece456cd39fab612 (refs/heads/master)
assert.EqualValues(t, 1563741793, lct.Unix()) assert.EqualValues(t, 1668354014, lct.Unix())
} }
func TestRepoIsEmpty(t *testing.T) { func TestRepoIsEmpty(t *testing.T) {

Binary file not shown.

View file

@ -1 +1,2 @@
37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push 37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push
feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind <me@silverwind.io> 1668354026 +0100 push

View file

@ -1 +1,2 @@
37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push 37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind <me@silverwind.io> 1563741799 +0200 push
feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind <me@silverwind.io> 1668354026 +0100 push

View file

@ -1 +1 @@
feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612

View file

@ -0,0 +1 @@
36f97d9a96457e2bab511db30fe2db03893ebc64