diff --git a/models/gituser/avatar_stack.go b/models/gituser/avatar_stack.go index d38c94bc7b..844bedca55 100644 --- a/models/gituser/avatar_stack.go +++ b/models/gituser/avatar_stack.go @@ -7,6 +7,7 @@ import ( "context" "gitea.dev/models/user" + "gitea.dev/modules/container" "gitea.dev/modules/git" "gitea.dev/modules/log" ) @@ -33,11 +34,18 @@ func BuildAvatarStackData(ctx context.Context, allParticipants []*git.CommitIden ret := &AvatarStackData{ Participants: make([]*CommitParticipant, 0, len(allParticipants)), } + uniqueUserIDs := make(container.Set[int64]) for _, p := range allParticipants { var giteaUser *user.User if emailUserMap != nil { giteaUser = emailUserMap.GetByEmail(p.Email) } + if giteaUser != nil { + // identities without a Gitea account can only be compared by their git identity + if !uniqueUserIDs.Add(giteaUser.ID) { + continue + } + } ret.Participants = append(ret.Participants, &CommitParticipant{GiteaUser: giteaUser, GitIdentity: p}) } return ret diff --git a/models/gituser/gituser.go b/models/gituser/gituser.go index 81a94a3a85..cad8b7fec8 100644 --- a/models/gituser/gituser.go +++ b/models/gituser/gituser.go @@ -39,8 +39,7 @@ func GetUserCommitsByGitCommits(ctx context.Context, gitCommits []*git.Commit, r emailSet := make(container.Set[string]) for _, c := range gitCommits { emailSet.Add(c.Author.Email) - emailSet.Add(c.Committer.Email) - for _, p := range c.AllParticipantIdentities() { + for _, p := range c.AllAuthorIdentities() { emailSet.Add(p.Email) } } @@ -55,7 +54,7 @@ func GetUserCommitsByGitCommits(ctx context.Context, gitCommits []*git.Commit, r uc := &UserCommit{ AuthorUser: emailUserMap.GetByEmail(c.Author.Email), // FIXME: why GetUserCommitsByGitCommits uses "Author", but ParseCommitsWithSignature uses "Committer"? GitCommit: c, - AvatarStackData: BuildAvatarStackData(ctx, c.AllParticipantIdentities(), emailUserMap), + AvatarStackData: BuildAvatarStackData(ctx, c.AllAuthorIdentities(), emailUserMap), } uc.AvatarStackData.SearchByEmailLink = searchByEmailLink userCommits = append(userCommits, uc) diff --git a/modules/git/commit_message.go b/modules/git/commit_message.go index 14c7b8f7e2..ce82a88ea0 100644 --- a/modules/git/commit_message.go +++ b/modules/git/commit_message.go @@ -39,9 +39,7 @@ type CommitMessage struct { trailerValues CommitMessageTrailerValues - allParticipants []*CommitIdentity - committerCoAuthorIdx int - committerCoAuthor *CommitIdentity + allAuthors []*CommitIdentity } func (c *CommitMessage) MessageUTF8() string { @@ -146,63 +144,50 @@ func CommitMessageParseTrailer(s string) CommitMessageTrailerValues { return ret } -// AllParticipantIdentities returns all the participants in the commit, the first one is the commit's author -func (c *Commit) AllParticipantIdentities() []*CommitIdentity { - if c.allParticipants != nil { - return c.allParticipants +// AllAuthorIdentities returns all the author and co-authors in the commit. Committer is not included: +// * 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) +func (c *Commit) AllAuthorIdentities() []*CommitIdentity { + if c.allAuthors != nil { + return c.allAuthors } - + trailerCoAuthors := c.MessageTrailer()["co-authored-by"] + c.allAuthors = make([]*CommitIdentity, 0, 1+len(trailerCoAuthors)) exclude := map[string]int{} - addParticipant := func(name, email string, role int) (existingRole int) { + addAuthor := func(name, email string, role int) { if name == "" && email == "" { - return 0 + return } - emailLower := strings.ToLower(email) - if existingRole = exclude[emailLower]; emailLower != "" && existingRole != 0 { - return existingRole + key := strings.ToLower(email) + if key == "" { + key = strings.ToLower(name) } - c.allParticipants = append(c.allParticipants, &CommitIdentity{Name: name, Email: email, role: role}) - exclude[emailLower] = role - return 0 + if existingRole := exclude[key]; key != "" && existingRole != 0 { + return + } + c.allAuthors = append(c.allAuthors, &CommitIdentity{Name: name, Email: email, role: role}) + exclude[key] = role } - 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"] { + addAuthor(c.Author.Name, c.Author.Email, commitIdentityRoleAuthor) + for _, coAuthorValue := range trailerCoAuthors { addr, err := mail.ParseAddress(coAuthorValue) coAuthorName, coAuthorEmail := coAuthorValue, "" if err == nil { 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} - } + addAuthor(coAuthorName, coAuthorEmail, commitIdentityRoleCoAuthor) } - return c.allParticipants + return c.allAuthors } -// 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 + all := c.AllAuthorIdentities() + if len(all) == 0 { + return nil } - if all[1].role != commitIdentityRoleCommitter { - return all[1:] // no committer, so all after author are co-authors + if all[0].role == commitIdentityRoleAuthor { + return all[1:] } - 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 + return all } diff --git a/modules/git/commit_message_test.go b/modules/git/commit_message_test.go index a602185d56..94c0317efc 100644 --- a/modules/git/commit_message_test.go +++ b/modules/git/commit_message_test.go @@ -52,41 +52,44 @@ func TestCommitMessageTrailer(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, r int) *CommitIdentity { return &CommitIdentity{n, e, r} } - roleAuthor, roleCommitter, roleCoAuthor := commitIdentityRoleAuthor, commitIdentityRoleCommitter, commitIdentityRoleCoAuthor + roleAuthor, _, roleCoAuthor := commitIdentityRoleAuthor, commitIdentityRoleCommitter, commitIdentityRoleCoAuthor type testCase struct { name string commit *Commit identities []*CommitIdentity } - t.Run("AllParticipants", func(t *testing.T) { + + t.Run("AllAuthors", func(t *testing.T) { cases := []testCase{ { - "DifferentUsers", + "CommitterExcluded", &Commit{ Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), - CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: x@m.com"}, + CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: Full Name "}, }, - []*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("c", "c@m.com", roleCommitter), idt("", "x@m.com", roleCoAuthor)}, + []*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("Full Name", "x@m.com", roleCoAuthor)}, }, { - "SameUser", + "AuthorIsCoAuthor", &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("a", "A@M.com"), - CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: a@m.com"}, + Author: sig("a", "a@m.com"), Committer: sig("c", "c@m.com"), + CommitMessage: CommitMessage{MessageRaw: "CO-Authored-BY: other-name "}, }, []*CommitIdentity{idt("a", "a@m.com", roleAuthor)}, }, { - "NoCommitter", + "EmptyAuthor", // synthesized commits (push feed) may have no author signature at all &Commit{ - Author: sig("a", "a@m.com"), Committer: sig("", ""), - CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: Full Name "}, + Author: sig("", ""), Committer: sig("", ""), + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: c "}, }, - []*CommitIdentity{idt("a", "a@m.com", roleAuthor), idt("Full Name", "X@M.com", roleCoAuthor)}, + // but if the commit message contains co-authors, the co-authors are still parsed for "all authors" + // if it is a problem, the caller should fix the problem (provide correct "author") + []*CommitIdentity{idt("c", "c@m.com", roleCoAuthor)}, }, } for _, c := range cases { - assert.Equal(t, c.identities, c.commit.AllParticipantIdentities(), "case: %s", c.name) + assert.Equal(t, c.identities, c.commit.AllAuthorIdentities(), "case: %s", c.name) } }) t.Run("CoAuthors", func(t *testing.T) { @@ -116,12 +119,12 @@ func TestCommitMessageParticipants(t *testing.T) { []*CommitIdentity{}, }, { - "CoAuthorCommitterNameWithIndex", // restore the committer co-author to the co-author list by the index with correct name + "CoAuthorNameOnlyAndDuplicate", &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 "}, + CommitMessage: CommitMessage{MessageRaw: "Co-authored-by: b\nCo-authored-by: b\nCo-authored-by: c"}, }, - []*CommitIdentity{idt("x", "x@m.com", roleCoAuthor), idt("c-other", "c@m.com", roleCoAuthor), idt("y", "y@m.com", roleCoAuthor)}, + []*CommitIdentity{idt("b", "", roleCoAuthor), idt("c", "", roleCoAuthor)}, }, } for _, c := range cases { diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 46f90be78a..f4a7b49832 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -370,7 +370,7 @@ func (ut *RenderUtils) AvatarStackPushCommit(pushCommit *repository.PushCommit) // there is no way to know the real committer, but the field can't be nil Committer: &git.Signature{Name: pushCommit.AuthorName, Email: pushCommit.AuthorEmail}, } - data := user_model.BuildAvatarStackData(ut.ctx, fakeGitCommit.AllParticipantIdentities(), nil) + data := user_model.BuildAvatarStackData(ut.ctx, fakeGitCommit.AllAuthorIdentities(), nil) return ut.AvatarStack(data) } diff --git a/routers/web/repo/blame.go b/routers/web/repo/blame.go index 457f9795a2..aa7a9fee43 100644 --- a/routers/web/repo/blame.go +++ b/routers/web/repo/blame.go @@ -223,7 +223,7 @@ func processBlameParts(ctx *context.Context, blameParts []*gitrepo.BlamePart) ma } func renderBlameFillFirstBlameRow(ctx *context.Context, repoLink string, part *gitrepo.BlamePart, commit *gituser.UserCommit, br *blameRow) { - br.AvatarStackData = gituser.BuildAvatarStackData(ctx, commit.GitCommit.AllParticipantIdentities(), nil) + br.AvatarStackData = gituser.BuildAvatarStackData(ctx, commit.GitCommit.AllAuthorIdentities(), nil) br.PreviousSha = part.PreviousSha br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(part.PreviousSha), util.PathEscapeSegments(part.PreviousPath)) br.CommitURL = fmt.Sprintf("%s/commit/%s", repoLink, url.PathEscape(part.Sha)) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 6df4e738c9..6caf829aa3 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -134,7 +134,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { return false } - avatarStackData := gituser.BuildAvatarStackData(ctx, latestCommit.AllParticipantIdentities(), nil) + avatarStackData := gituser.BuildAvatarStackData(ctx, latestCommit.AllAuthorIdentities(), nil) avatarStackData.SearchByEmailLink = gituser.RepoCommitSearchByEmailLink(ctx.Repo.RepoLink, ctx.Repo.RefFullName) ctx.Data["LatestCommitAvatarStackData"] = avatarStackData ctx.Data["LatestCommitVerification"] = verification diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 99f8222ca7..c2507b840c 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -109,7 +109,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ if c.Commit.Author != nil { emailSet.Add(c.Commit.Author.Email) } - for _, sig := range c.Commit.AllParticipantIdentities() { + for _, sig := range c.Commit.AllAuthorIdentities() { emailSet.Add(sig.Email) } } @@ -125,7 +125,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ } c.User = emailUserMap.GetByEmail(c.Commit.Author.Email) - c.AvatarStackData = gituser.BuildAvatarStackData(ctx, c.Commit.AllParticipantIdentities(), emailUserMap) + c.AvatarStackData = gituser.BuildAvatarStackData(ctx, c.Commit.AllAuthorIdentities(), emailUserMap) c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit)