Commit b6b09c54da
Verified · cmc
Layout: unified · split
e2e/mr_test.go +10
| @@ -248,6 +248,13 @@ func TestMergeRequests(t *testing.T) { | |||
| 248 | t.Fatalf("merge after fork deletion: %s", errOut) | 248 | t.Fatalf("merge after fork deletion: %s", errOut) |
| 249 | } | 249 | } |
| 250 | 250 | ||
| 251 | // Merged MRs keep their historical diff: after the fast-forward the | ||
| 252 | // live merge-base equals the head, so the recorded base must be used. | ||
| 253 | diffOut2, _, code := inst.ssh(t, aliceKey, "", "mr", "diff", "alice/lib", "1") | ||
| 254 | if code != 0 || !strings.Contains(diffOut2, "feature.txt") { | ||
| 255 | t.Fatalf("post-merge diff empty: %d\n%s", code, diffOut2) | ||
| 256 | } | ||
| 257 | |||
| 251 | // Web read views. | 258 | // Web read views. |
| 252 | status, body := inst.get(t, "/alice/lib/mrs?state=all") | 259 | status, body := inst.get(t, "/alice/lib/mrs?state=all") |
| 253 | if status != 200 || !strings.Contains(body, "add feature") || !strings.Contains(body, "second") { | 260 | if status != 200 || !strings.Contains(body, "add feature") || !strings.Contains(body, "second") { |
| @@ -257,4 +264,7 @@ func TestMergeRequests(t *testing.T) { | |||
| 257 | if status != 200 || !strings.Contains(body, "stale") || !strings.Contains(body, "merged") { | 264 | if status != 200 || !strings.Contains(body, "stale") || !strings.Contains(body, "merged") { |
| 258 | t.Fatalf("mr detail: %d\n%s", status, body) | 265 | t.Fatalf("mr detail: %d\n%s", status, body) |
| 259 | } | 266 | } |
| 267 | if !strings.Contains(body, "feature.txt") { | ||
| 268 | t.Fatalf("merged MR web diff empty:\n%s", body) | ||
| 269 | } | ||
| 260 | } | 270 | } |
internal/control/mr.go +10 −4
| @@ -352,9 +352,15 @@ func runMRDiff(c *Ctx, args []string) int { | |||
| 352 | } | 352 | } |
| 353 | dir := RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name) | 353 | dir := RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name) |
| 354 | head := mrHeadRef(mr.Number) | 354 | head := mrHeadRef(mr.Number) |
| 355 | base, err := gitutil.MergeBase(dir, "refs/heads/"+mr.TargetRef, head) | 355 | // After a merge (especially fast-forward) the live merge-base equals |
| 356 | if err != nil { | 356 | // the head and the diff would vanish; use the recorded base instead. |
| 357 | return c.fail(protocol.ExitFailure, "%v", err) | 357 | base := mr.MergedBase |
| 358 | if base == "" { | ||
| 359 | b, err := gitutil.MergeBase(dir, "refs/heads/"+mr.TargetRef, head) | ||
| 360 | if err != nil { | ||
| 361 | return c.fail(protocol.ExitFailure, "%v", err) | ||
| 362 | } | ||
| 363 | base = b | ||
| 358 | } | 364 | } |
| 359 | patch, err := gitutil.Diff(dir, base, head, 4<<20) | 365 | patch, err := gitutil.Diff(dir, base, head, 4<<20) |
| 360 | if err != nil { | 366 | if err != nil { |
| @@ -663,7 +669,7 @@ func runMRMerge(c *Ctx, args []string) int { | |||
| 663 | if err := gitutil.UpdateRefCAS(dir, targetRef, newSHA, targetSHA); err != nil { | 669 | if err := gitutil.UpdateRefCAS(dir, targetRef, newSHA, targetSHA); err != nil { |
| 664 | return c.fail(protocol.ExitFailure, "target branch moved during merge; retry: %v", err) | 670 | return c.fail(protocol.ExitFailure, "target branch moved during merge; retry: %v", err) |
| 665 | } | 671 | } |
| 666 | if err := c.Store.SetMRState(mr.ID, "merged"); err != nil { | 672 | if err := c.Store.MarkMerged(mr.ID, targetSHA); err != nil { |
| 667 | return c.fail(protocol.ExitFailure, "%v", err) | 673 | return c.fail(protocol.ExitFailure, "%v", err) |
| 668 | } | 674 | } |
| 669 | c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA)) | 675 | c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA)) |
internal/httpd/web.go +7 −1
| @@ -642,7 +642,13 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) { | |||
| 642 | 642 | ||
| 643 | headRef := fmt.Sprintf("refs/merge-requests/%d/head", m.Number) | 643 | headRef := fmt.Sprintf("refs/merge-requests/%d/head", m.Number) |
| 644 | var lines []diffLine | 644 | var lines []diffLine |
| 645 | if base, err := gitutil.MergeBase(p.Dir, "refs/heads/"+m.TargetRef, headRef); err == nil { | 645 | base := m.MergedBase |
| 646 | if base == "" { | ||
| 647 | if b, err := gitutil.MergeBase(p.Dir, "refs/heads/"+m.TargetRef, headRef); err == nil { | ||
| 648 | base = b | ||
| 649 | } | ||
| 650 | } | ||
| 651 | if base != "" { | ||
| 646 | if patch, err := gitutil.Diff(p.Dir, base, headRef, 4<<20); err == nil { | 652 | if patch, err := gitutil.Diff(p.Dir, base, headRef, 4<<20); err == nil { |
| 647 | lines = classifyDiff(patch) | 653 | lines = classifyDiff(patch) |
| 648 | } | 654 | } |
internal/store/migrations/0006_merged_base.down.sql added +1
| @@ -0,0 +1 @@ | |||
| 1 | ALTER TABLE merge_requests DROP COLUMN merged_base; | ||
internal/store/migrations/0006_merged_base.up.sql added +1
| @@ -0,0 +1 @@ | |||
| 1 | ALTER TABLE merge_requests ADD COLUMN merged_base TEXT NOT NULL DEFAULT ''; | ||
internal/store/mrs.go +12 −2
| @@ -18,6 +18,7 @@ type MR struct { | |||
| 18 | Body string | 18 | Body string |
| 19 | State string // open | merged | closed | source_gone | 19 | State string // open | merged | closed | source_gone |
| 20 | HeadSHA string | 20 | HeadSHA string |
| 21 | MergedBase string // target tip at merge time; base for historical diffs | ||
| 21 | CreatedAt string | 22 | CreatedAt string |
| 22 | UpdatedAt string | 23 | UpdatedAt string |
| 23 | } | 24 | } |
| @@ -57,7 +58,7 @@ const mrSelect = ` | |||
| 57 | COALESCE(m.source_repo_id, 0), | 58 | COALESCE(m.source_repo_id, 0), |
| 58 | COALESCE(COALESCE(su.username, so.name) || '/' || sr.name, ''), | 59 | COALESCE(COALESCE(su.username, so.name) || '/' || sr.name, ''), |
| 59 | m.source_ref, m.target_ref, m.title, m.body, m.state, m.head_sha, | 60 | m.source_ref, m.target_ref, m.title, m.body, m.state, m.head_sha, |
| 60 | m.created_at, m.updated_at | 61 | m.merged_base, m.created_at, m.updated_at |
| 61 | FROM merge_requests m | 62 | FROM merge_requests m |
| 62 | JOIN users u ON u.id = m.author_id | 63 | JOIN users u ON u.id = m.author_id |
| 63 | LEFT JOIN repos sr ON sr.id = m.source_repo_id | 64 | LEFT JOIN repos sr ON sr.id = m.source_repo_id |
| @@ -67,7 +68,7 @@ const mrSelect = ` | |||
| 67 | func scanMR(row interface{ Scan(...any) error }) (MR, error) { | 68 | func scanMR(row interface{ Scan(...any) error }) (MR, error) { |
| 68 | var m MR | 69 | var m MR |
| 69 | err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath, | 70 | err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath, |
| 70 | &m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.State, &m.HeadSHA, &m.CreatedAt, &m.UpdatedAt) | 71 | &m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.State, &m.HeadSHA, &m.MergedBase, &m.CreatedAt, &m.UpdatedAt) |
| 71 | return m, err | 72 | return m, err |
| 72 | } | 73 | } |
| 73 | 74 | ||
| @@ -124,6 +125,15 @@ func (s *Store) OpenMRsBySource(sourceRepoID int64, sourceRef string) ([]MR, err | |||
| 124 | return out, rows.Err() | 125 | return out, rows.Err() |
| 125 | } | 126 | } |
| 126 | 127 | ||
| 128 | // MarkMerged records the merge along with the target tip it landed on, so | ||
| 129 | // the MR's diff stays reconstructable after fast-forwards. | ||
| 130 | func (s *Store) MarkMerged(mrID int64, baseSHA string) error { | ||
| 131 | _, err := s.DB.Exec( | ||
| 132 | "UPDATE merge_requests SET state = 'merged', merged_base = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?", | ||
| 133 | baseSHA, mrID) | ||
| 134 | return err | ||
| 135 | } | ||
| 136 | |||
| 127 | func (s *Store) SetMRState(mrID int64, state string) error { | 137 | func (s *Store) SetMRState(mrID int64, state string) error { |
| 128 | res, err := s.DB.Exec( | 138 | res, err := s.DB.Exec( |
| 129 | "UPDATE merge_requests SET state = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?", | 139 | "UPDATE merge_requests SET state = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?", |