From c5c991b1a4446f9bb4113a8905a01f6a56a6ead3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 10 Jul 2026 18:52:00 +0800 Subject: [PATCH] fix: co-author detection (#38392) Committer can also be co-author, it should only not be included in the co-author list if it is not in the "Co-author-by" list. * Author & Co-author: they changed the code (attribution) * Committer: they submitted the commit but didn't change the code (e.g.: maintainer signed a commit) Fix #38384 --- modules/git/commit_message.go | 76 +++++++++------ modules/git/commit_message_test.go | 146 +++++++++++++++-------------- 2 files changed, 126 insertions(+), 96 deletions(-) diff --git a/modules/git/commit_message.go b/modules/git/commit_message.go index 9aa3a6966a3..e03bca3f5e7 100644 --- a/modules/git/commit_message.go +++ b/modules/git/commit_message.go @@ -10,16 +10,22 @@ import ( "sync" "gitea.dev/modules/charset" - "gitea.dev/modules/container" "gitea.dev/modules/util" ) // CoAuthoredByTrailer is the canonical token for the `Co-authored-by:` git trailer. const CoAuthoredByTrailer = "Co-authored-by" +const ( + commitIdentityRoleAuthor = 1 + commitIdentityRoleCommitter = 2 + commitIdentityRoleCoAuthor = 3 +) + type CommitIdentity struct { Name string Email string + role int } // CommitMessageTrailerValues keys are all in lower-case @@ -33,7 +39,9 @@ type CommitMessage struct { trailerValues CommitMessageTrailerValues - allParticipants []*CommitIdentity + allParticipants []*CommitIdentity + committerCoAuthorIdx int + committerCoAuthor *CommitIdentity } func (c *CommitMessage) MessageUTF8() string { @@ -105,43 +113,57 @@ func (c *Commit) AllParticipantIdentities() []*CommitIdentity { return c.allParticipants } - exclude := container.Set[string]{} - c.allParticipants = append(c.allParticipants, &CommitIdentity{Name: c.Author.Name, Email: c.Author.Email}) - exclude.Add(strings.ToLower(c.Author.Email)) - - addParticipant := func(name, email string) { + exclude := map[string]int{} + addParticipant := func(name, email string, role int) (existingRole int) { if name == "" && email == "" { - return + return 0 } emailLower := strings.ToLower(email) - if emailLower != "" && exclude.Contains(emailLower) { - return + if existingRole = exclude[emailLower]; emailLower != "" && existingRole != 0 { + return existingRole } - c.allParticipants = append(c.allParticipants, &CommitIdentity{Name: name, Email: email}) - exclude.Add(emailLower) + c.allParticipants = append(c.allParticipants, &CommitIdentity{Name: name, Email: email, role: role}) + exclude[emailLower] = role + return 0 } - addParticipant(c.Committer.Name, c.Committer.Email) + + c.committerCoAuthorIdx = -1 + addParticipant(c.Author.Name, c.Author.Email, commitIdentityRoleAuthor) + addParticipant(c.Committer.Name, c.Committer.Email, commitIdentityRoleCommitter) for _, coAuthorValue := range c.MessageTrailer()["co-authored-by"] { addr, err := mail.ParseAddress(coAuthorValue) + coAuthorName, coAuthorEmail := coAuthorValue, "" if err == nil { - addParticipant(addr.Name, addr.Address) - } else { - addParticipant(coAuthorValue, "") + coAuthorName, coAuthorEmail = addr.Name, addr.Address + } + existingRole := addParticipant(coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor) + if existingRole == commitIdentityRoleCommitter && c.committerCoAuthorIdx == -1 { + c.committerCoAuthorIdx = len(c.allParticipants) + c.committerCoAuthor = &CommitIdentity{coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor} } } return c.allParticipants } -// CoAuthorIdentities returns the commit's co-authors: the participants without the -// author and the committer, which are already displayed separately. This avoids -// surfacing the committer as a "co-author" (see issue #38384). -func (c *Commit) CoAuthorIdentities() []*CommitIdentity { - var ret []*CommitIdentity - for _, p := range c.AllParticipantIdentities()[1:] { // index 0 is always the author - if c.Committer.Email != "" && strings.EqualFold(p.Email, c.Committer.Email) { - continue - } - ret = append(ret, p) +// CoAuthorIdentities returns co-author identities defined by "Co-authored-by:" in the git message trailer +// Only the commit's author is excluded. If committer is declared as co-author, it will be included in the result. +// * Author & Co-author: they changed the code (attribution) +// * Committer: they submitted the commit but didn't change the code (e.g.: maintainer signed a commit) +// So, a committer can also be a co-author if they changed the code. +func (c *Commit) CoAuthorIdentities() (coAuthors []*CommitIdentity) { + all := c.AllParticipantIdentities() + if len(all) <= 1 { + return nil // no co-author list } - return ret + if all[1].role != commitIdentityRoleCommitter { + return all[1:] // no committer, so all after author are co-authors + } + if c.committerCoAuthorIdx == -1 { + return all[2:] // the committer is not in the co-author list, so just return the co-author list + } + // the committer is in the co-author list but de-duplicated, so include them as co-author again + coAuthors = append(coAuthors, all[2:c.committerCoAuthorIdx]...) + coAuthors = append(coAuthors, c.committerCoAuthor) + coAuthors = append(coAuthors, all[c.committerCoAuthorIdx:]...) + return coAuthors } diff --git a/modules/git/commit_message_test.go b/modules/git/commit_message_test.go index ba5927d9fd6..e16fa19e8b8 100644 --- a/modules/git/commit_message_test.go +++ b/modules/git/commit_message_test.go @@ -47,75 +47,83 @@ func TestCommitMessageTrailer(t *testing.T) { } } -func TestCommitMessageAllParticipantIdentities(t *testing.T) { +func TestCommitMessageParticipants(t *testing.T) { sig := func(n, e string) *Signature { return &Signature{Name: n, Email: e} } - idt := func(n, e string) *CommitIdentity { return &CommitIdentity{Name: n, Email: e} } - cases := []struct { - commit *Commit - participant []*CommitIdentity - }{ - { - &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), - CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: x@m.com"}, - }, - []*CommitIdentity{idt("a", "a@m.com"), idt("c", "c@m.com"), idt("", "x@m.com")}, - }, - { - &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("a", "A@M.com"), - CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: a@m.com"}, - }, - []*CommitIdentity{idt("a", "a@m.com")}, - }, - { - &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("", ""), - CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: Full Name "}, - }, - []*CommitIdentity{idt("a", "a@m.com"), idt("Full Name", "X@M.com")}, - }, - } - for _, c := range cases { - assert.Equal(t, c.participant, c.commit.AllParticipantIdentities()) - } -} - -func TestCommitMessageCoAuthorIdentities(t *testing.T) { - sig := func(n, e string) *Signature { return &Signature{Name: n, Email: e} } - idt := func(n, e string) *CommitIdentity { return &CommitIdentity{Name: n, Email: e} } - cases := []struct { - commit *Commit - coAuthors []*CommitIdentity - }{ - { - // a genuine co-author (neither author nor committer) is reported - &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), - CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: x "}, - }, - []*CommitIdentity{idt("x", "x@m.com")}, - }, - { - // the committer is shown separately as "committed by", so it must - // not appear as a co-author even though it is a participant - &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), - CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c "}, - }, - nil, - }, - { - // regression for #38384: a Co-authored-by trailer naming the author - // must not cause the committer to be surfaced as the co-author - &Commit{ - Author: sig("silverwind", "me@silverwind.io"), Committer: sig("bircni", "bircni@icloud.com"), - CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: silverwind "}, - }, - nil, - }, - } - for _, c := range cases { - assert.Equal(t, c.coAuthors, c.commit.CoAuthorIdentities()) + idt := func(n, e string, r int) *CommitIdentity { return &CommitIdentity{n, e, r} } + roleAuthor, roleCommitter, roleCoAuthor := commitIdentityRoleAuthor, commitIdentityRoleCommitter, commitIdentityRoleCoAuthor + type testCase struct { + name string + commit *Commit + identities []*CommitIdentity } + t.Run("AllParticipants", func(t *testing.T) { + cases := []testCase{ + { + "DifferentUsers", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: x@m.com"}, + }, + []*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("c", "c@m.com", roleCommitter), idt("", "x@m.com", roleCoAuthor)}, + }, + { + "SameUser", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("a", "A@M.com"), + CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: a@m.com"}, + }, + []*CommitIdentity{idt("a", "a@m.com", roleAuthor)}, + }, + { + "NoCommitter", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("", ""), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: Full Name "}, + }, + []*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("Full Name", "X@M.com", roleCoAuthor)}, + }, + } + for _, c := range cases { + assert.Equal(t, c.identities, c.commit.AllParticipantIdentities(), "case: %s", c.name) + } + }) + t.Run("CoAuthors", func(t *testing.T) { + cases := []testCase{ + { + "GenuineCoAuthor", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: x "}, + }, + []*CommitIdentity{idt("x", "x@m.com", roleCoAuthor)}, + }, + { + "CoAuthorIsCommitter", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c "}, + }, + []*CommitIdentity{idt("c", "c@m.com", roleCoAuthor)}, + }, + { + "CoAuthorIsAuthor", + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: a "}, + }, + []*CommitIdentity{}, + }, + { + "CoAuthorCommitterNameWithIndex", // restore the committer co-author to the co-author list by the index with correct name + &Commit{ + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: x \nCo-authored-by: c-other \nCo-authored-by: y "}, + }, + []*CommitIdentity{idt("x", "x@m.com", roleCoAuthor), idt("c-other", "c@m.com", roleCoAuthor), idt("y", "y@m.com", roleCoAuthor)}, + }, + } + for _, c := range cases { + assert.Equal(t, c.identities, c.commit.CoAuthorIdentities(), "case: %s", c.name) + } + }) }