Collapse a contributor's several addresses into one row !127

merged merged by cmc on 2026-08-30 23:52 UTC · krz/gitbay:fix-contributor-identity into main

4 files changed, +137 −15

Layout: unified · split

e2e/facts_test.go +89
@@ -1,8 +1,10 @@
11package e2e
22
33import (
4 "fmt"
45 "os"
56 "path/filepath"
7 "strconv"
68 "strings"
79 "testing"
810)
@@ -94,3 +96,90 @@ purpose with or without fee is hereby granted.
9496THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
9597WITH REGARD TO THIS SOFTWARE.
9698`
99
100// TestContributorIdentity covers the two ways several addresses fold into
101// one contributor: the repository's own .mailmap, and the addresses an
102// account has verified here. Addresses with neither stay distinct, even
103// when they carry the same name.
104func TestContributorIdentity(t *testing.T) {
105 smtp := startFakeSMTP(t)
106 inst := startInstanceWith(t, fmt.Sprintf(
107 "[mail]\nsmtp_host = %q\nfrom = \"noreply@gitbay.test\"\n", smtp.addr))
108 aliceKey := inst.newKey(t, "alice")
109 inst.admin(t, "admin", "user", "create", "alice",
110 "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
111 // A second address on the same account, verified without the mail
112 // round trip — this test is about what the addresses mean, not how
113 // they got proven.
114 if _, errOut, code := inst.ssh(t, aliceKey, "", "email", "add", "alice2@example.test"); code != 0 {
115 t.Fatalf("email add: %s", errOut)
116 }
117 inst.admin(t, "admin", "email", "verify", "alice", "alice2@example.test")
118 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/ident"); code != 0 {
119 t.Fatalf("repo create: %s", errOut)
120 }
121
122 work := t.TempDir()
123 env := inst.gitEnv(aliceKey)
124 as := func(name, email string) []string {
125 return append(append([]string{}, env...),
126 "GIT_AUTHOR_NAME="+name, "GIT_AUTHOR_EMAIL="+email)
127 }
128 mustGit(t, work, env, "clone", inst.sshURL("alice/ident"), "w")
129 dir := filepath.Join(work, "w")
130 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
131
132 // The mailmap claims the old address; nothing else does.
133 os.WriteFile(filepath.Join(dir, ".mailmap"),
134 []byte("Alice <alice@example.test> <alice@old.test>\n"), 0o644)
135 mustGit(t, dir, env, "add", ".mailmap")
136 mustGit(t, dir, as("Alice", "alice@example.test"), "commit", "-q", "-m", "mailmap")
137
138 commits := []struct{ name, email string }{
139 {"Alice", "alice@example.test"},
140 {"Alice", "alice@example.test"},
141 {"A. Cleberg", "alice2@example.test"}, // verified here, not in the mailmap
142 {"A. Cleberg", "alice2@example.test"},
143 {"Alice Old", "alice@old.test"}, // in the mailmap, no account
144 {"Alice Old", "alice@old.test"},
145 {"Pat", "pat1@example.test"}, // same name, different people
146 {"Pat", "pat2@example.test"},
147 }
148 for i, c := range commits {
149 mustGit(t, dir, as(c.name, c.email),
150 "commit", "-q", "--allow-empty", "-m", strconv.Itoa(i))
151 }
152 mustGit(t, dir, env, "push", "-q", "origin", "main")
153
154 status, body := inst.get(t, "/alice/ident")
155 if status != 200 {
156 t.Fatalf("repo home: %d", status)
157 }
158 // Scope the positives to the contributors bar: the tip bar names an
159 // author too, and links it to the same profile.
160 _, contribs, ok := strings.Cut(body, `<p class="contribs">`)
161 if !ok {
162 t.Fatal("no contributors bar")
163 }
164 contribs, _, _ = strings.Cut(contribs, "</p>")
165 for _, want := range []string{
166 "3 contributors",
167 // Three addresses, one account, one row, named by the account.
168 `title="alice@example.test · 7 commits">alice</a>`,
169 // One commit is one commit.
170 `title="pat1@example.test · 1 commit"`,
171 `title="pat2@example.test · 1 commit"`,
172 } {
173 if !strings.Contains(contribs, want) {
174 t.Errorf("contributors bar missing %q", want)
175 }
176 }
177 if n := strings.Count(contribs, `href="/alice"`); n != 1 {
178 t.Errorf("account listed as %d contributors, want 1", n)
179 }
180 for _, gone := range []string{"alice@old.test", "alice2@example.test", "A. Cleberg", "Alice Old"} {
181 if strings.Contains(body, gone) {
182 t.Errorf("collapsed identity %q still shown", gone)
183 }
184 }
185}
internal/gitutil/facts.go +5 −6
@@ -27,9 +27,11 @@ type Contributor struct {
2727
2828// Contributors summarises authorship reachable from ref, most commits
2929// first. Identities are keyed by email, since that is what the forge can
30// tie back to an account.
31func Contributors(dir, ref string, max int) []Contributor {
32 out, err := exec.Command("git", "-C", dir, "log", "--format=%an%x01%ae", ref).Output()
30// tie back to an account, after the repository's own .mailmap has had its
31// say — a bare repo resolves that from HEAD:.mailmap with no config.
32func Contributors(dir, ref string) []Contributor {
33 out, err := exec.Command("git", "-C", dir, "log",
34 "--use-mailmap", "--format=%aN%x01%aE", ref).Output()
3335 if err != nil {
3436 return nil
3537 }
@@ -53,9 +55,6 @@ func Contributors(dir, ref string, max int) []Contributor {
5355 list = append(list, *byEmail[e])
5456 }
5557 sort.SliceStable(list, func(i, j int) bool { return list[i].Commits > list[j].Commits })
56 if max > 0 && len(list) > max {
57 list = list[:max]
58 }
5958 return list
6059}
6160
internal/httpd/facts.go +42 −8
@@ -1,6 +1,9 @@
11package httpd
22
33import (
4 "fmt"
5 "sort"
6
47 "gitbay.org/gitbay/internal/control"
58 "gitbay.org/gitbay/internal/gitutil"
69)
@@ -21,12 +24,49 @@ type repoFacts struct {
2124type factContributor struct {
2225 Name string
2326 User string // account, when the email is verified here
24 Email string
27 Email string // the account's busiest address, when several collapsed
2528 Commits int
2629}
2730
31// Title is the hover text: which address the commits carry, and how many.
32func (c factContributor) Title() string {
33 if c.Commits == 1 {
34 return c.Email + " · 1 commit"
35 }
36 return fmt.Sprintf("%s · %d commits", c.Email, c.Commits)
37}
38
2839const maxContributors = 12
2940
41// collapseContributors folds the addresses of one account into a single
42// row. .mailmap has already merged whatever the repository claims about
43// its own history; this merges what an account has proven here, which a
44// repository cannot know about. Addresses with no account stay distinct —
45// two people sharing a git name are not one contributor.
46func (s *Server) collapseContributors(cs []gitutil.Contributor) []factContributor {
47 names := s.authorNames()
48 var out []factContributor
49 byUser := map[string]int{}
50 for _, c := range cs {
51 user, known := names.account(c.Email)
52 if known {
53 if i, seen := byUser[user]; seen {
54 out[i].Commits += c.Commits
55 continue
56 }
57 byUser[user] = len(out)
58 }
59 out = append(out, factContributor{
60 Name: names.name(c.Email, c.Name), User: user, Email: c.Email, Commits: c.Commits,
61 })
62 }
63 sort.SliceStable(out, func(i, j int) bool { return out[i].Commits > out[j].Commits })
64 if len(out) > maxContributors {
65 out = out[:maxContributors]
66 }
67 return out
68}
69
3070func (s *Server) factsFor(p repoPage) repoFacts {
3171 f := repoFacts{
3272 Commits: gitutil.CountCommits(p.Dir, p.Ref),
@@ -40,13 +80,7 @@ func (s *Server) factsFor(p repoPage) repoFacts {
4080 }
4181 f.Languages = gitutil.Languages(p.Dir, p.Ref, langOf)
4282
43 names := s.authorNames()
44 for _, c := range gitutil.Contributors(p.Dir, p.Ref, maxContributors) {
45 user, _ := names.account(c.Email)
46 f.Contributors = append(f.Contributors, factContributor{
47 Name: names.name(c.Email, c.Name), User: user, Email: c.Email, Commits: c.Commits,
48 })
49 }
83 f.Contributors = s.collapseContributors(gitutil.Contributors(p.Dir, p.Ref))
5084
5185 if rels, err := s.st.ListReleases(p.Repo.ID); err == nil && len(rels) > 0 {
5286 f.Release = rels[0].Tag
internal/web/templates/tree.html +1 −1
@@ -21,7 +21,7 @@
2121 </p>
2222 {{with .Facts.Languages}}<p class="langbar" aria-hidden="true">{{range .}}<span class="lang lang-{{slug .Name}}" style="width:{{pct .Percent}}%"></span>{{end}}</p>
2323 <p class="langs">{{range .}}<span class="lang-name"><span class="dot lang-{{slug .Name}}"></span>{{.Name}} <span class="muted">{{pct .Percent}}%</span></span>{{end}}</p>{{end}}
24 {{with .Facts.Contributors}}<p class="contribs"><span class="label">{{len .}} contributor{{if ne (len .) 1}}s{{end}}</span>{{range .}}{{template "authorname" dict "Name" .Name "User" .User "Email" (printf "%s · %d commits" .Email .Commits)}}{{end}}</p>{{end}}
24 {{with .Facts.Contributors}}<p class="contribs"><span class="label">{{len .}} contributor{{if ne (len .) 1}}s{{end}}</span>{{range .}}{{template "authorname" dict "Name" .Name "User" .User "Email" .Title}}{{end}}</p>{{end}}
2525</div>{{end}}
2626{{with .Tip}}{{if .SHA}}<div class="tipbar">
2727 <span class="who">{{template "authorname" dict "Name" .Author "User" .User "Email" .Email}}</span>