Commit 049d728538
Verified · cmc ci/build: success ci/test: failure
Layout: unified · split
.gitbay/wiki/Users.org +6
| @@ -685,6 +685,12 @@ editor, and the server's merge is its only writer. Creating the branch | |||
| 685 | is still a push, since there is nothing to open a merge request against | 685 | is still a push, since there is nothing to open a merge request against |
| 686 | yet. | 686 | yet. |
| 687 | 687 | ||
| 688 | A merge request whose head reaches its target some other way — a direct | ||
| 689 | push, or inside another merge request that is merged — is recorded as | ||
| 690 | merged by whoever moved the branch, including one whose source branch | ||
| 691 | is gone. One whose head was on the target before the update is left | ||
| 692 | alone; =mr merge= on it records it as merged. | ||
| 693 | |||
| 688 | A merge request whose target is another open merge request's source | 694 | A merge request whose target is another open merge request's source |
| 689 | branch is stacked on it: =mr create= says so, =mr show= carries | 695 | branch is stacked on it: =mr create= says so, =mr show= carries |
| 690 | =stacked_on= and =stacked=, and merging the lower one retargets the | 696 | =stacked_on= and =stacked=, and merging the lower one retargets the |
internal/control/landed.go added +65
| @@ -0,0 +1,65 @@ | |||
| 1 | package control | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "fmt" | ||
| 5 | "io" | ||
| 6 | "log/slog" | ||
| 7 | |||
| 8 | "gitbay.org/gitbay/internal/config" | ||
| 9 | "gitbay.org/gitbay/internal/gitutil" | ||
| 10 | "gitbay.org/gitbay/internal/store" | ||
| 11 | ) | ||
| 12 | |||
| 13 | // RecordLandedMRs records as merged, by userID, each open or source_gone | ||
| 14 | // merge request targeting branch whose head the update from old to new | ||
| 15 | // brought onto it: commits that reached the target by a direct push, or | ||
| 16 | // inside another merge request. A head already on the branch before the | ||
| 17 | // update is left alone, so a merge request opened with nothing to merge | ||
| 18 | // is not closed by an unrelated push; `mr merge` records that case (#108). | ||
| 19 | func RecordLandedMRs(st *store.Store, cfg config.Config, repo store.Repo, userID int64, branch, old, new string) { | ||
| 20 | mrs, err := st.UnresolvedMRsByTarget(repo.ID, branch) | ||
| 21 | if err != nil { | ||
| 22 | slog.Error("landed MRs: listing", "repo", repo.Path(), "err", err) | ||
| 23 | return | ||
| 24 | } | ||
| 25 | dir := RepoDir(cfg.Server.Root, repo.OwnerName, repo.Name) | ||
| 26 | base := old | ||
| 27 | if gitutil.ZeroSHA(old) { | ||
| 28 | base = "" | ||
| 29 | } | ||
| 30 | var c *Ctx | ||
| 31 | for _, mr := range mrs { | ||
| 32 | if mr.HeadSHA == "" { | ||
| 33 | continue | ||
| 34 | } | ||
| 35 | if landed, err := gitutil.IsAncestor(dir, mr.HeadSHA, new); err != nil || !landed { | ||
| 36 | continue | ||
| 37 | } | ||
| 38 | if base != "" { | ||
| 39 | if before, err := gitutil.IsAncestor(dir, mr.HeadSHA, base); err != nil || before { | ||
| 40 | continue | ||
| 41 | } | ||
| 42 | } | ||
| 43 | if err := st.MarkMerged(mr.ID, base, userID, ""); err != nil { | ||
| 44 | slog.Error("landed MRs: recording", "repo", repo.Path(), "mr", mr.Number, "err", err) | ||
| 45 | continue | ||
| 46 | } | ||
| 47 | if mr.QueuedAt != "" { | ||
| 48 | st.DequeueMerge(mr.ID) | ||
| 49 | } | ||
| 50 | st.RecordEvent(repo.ID, userID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, new)) | ||
| 51 | if c == nil { | ||
| 52 | u, err := st.UserByID(userID) | ||
| 53 | if err != nil { | ||
| 54 | continue | ||
| 55 | } | ||
| 56 | c = &Ctx{User: u, Store: st, Cfg: cfg, Stdin: emptyReader{}, Stdout: io.Discard, Stderr: io.Discard} | ||
| 57 | } | ||
| 58 | if parts, err := st.MRParticipants(mr.ID); err == nil { | ||
| 59 | notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number, | ||
| 60 | subject: mrSubject(repo, mr.Number, mr.Title), | ||
| 61 | action: fmt.Sprintf("merged !%d into %s (pushed)", mr.Number, branch), | ||
| 62 | path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) | ||
| 63 | } | ||
| 64 | } | ||
| 65 | } | ||
internal/control/landed_test.go added +119
| @@ -0,0 +1,119 @@ | |||
| 1 | package control | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "os" | ||
| 5 | "path/filepath" | ||
| 6 | "strings" | ||
| 7 | "testing" | ||
| 8 | |||
| 9 | "gitbay.org/gitbay/internal/config" | ||
| 10 | "gitbay.org/gitbay/internal/policy" | ||
| 11 | "gitbay.org/gitbay/internal/protocol" | ||
| 12 | "gitbay.org/gitbay/internal/store" | ||
| 13 | ) | ||
| 14 | |||
| 15 | // A merge request whose head reaches its target by a push, or inside | ||
| 16 | // another merge request, is recorded as merged; one whose head was on the | ||
| 17 | // target already, or that is not on it, is left open. | ||
| 18 | func TestRecordLandedMRs(t *testing.T) { | ||
| 19 | st, repo, uid := newQueueTestRepo(t) | ||
| 20 | git := gitRunner(t) | ||
| 21 | root := t.TempDir() | ||
| 22 | src := filepath.Join(root, "src") | ||
| 23 | os.MkdirAll(src, 0o755) | ||
| 24 | git(root, "init", "-q", "-b", "main", "src") | ||
| 25 | commit := func(file string) string { | ||
| 26 | os.WriteFile(filepath.Join(src, file), []byte(file+"\n"), 0o644) | ||
| 27 | git(src, "add", ".") | ||
| 28 | git(src, "commit", "-q", "-m", file) | ||
| 29 | return strings.TrimSpace(git(src, "rev-parse", "HEAD")) | ||
| 30 | } | ||
| 31 | base := commit("base") | ||
| 32 | branch := func(name, from string, files ...string) string { | ||
| 33 | git(src, "checkout", "-q", "-b", name, from) | ||
| 34 | var head string | ||
| 35 | for _, f := range files { | ||
| 36 | head = commit(f) | ||
| 37 | } | ||
| 38 | return head | ||
| 39 | } | ||
| 40 | pushed := branch("pushed", base, "a") | ||
| 41 | gone := branch("gone", base, "b") | ||
| 42 | other := branch("other", base, "c") | ||
| 43 | |||
| 44 | dir := RepoDir(root, repo.OwnerName, repo.Name) | ||
| 45 | os.MkdirAll(filepath.Dir(dir), 0o755) | ||
| 46 | git(root, "clone", "-q", "--bare", src, dir) | ||
| 47 | git(dir, "symbolic-ref", "HEAD", "refs/heads/main") | ||
| 48 | open := func(source, head string) store.MR { | ||
| 49 | n, err := st.CreateMR(repo.ID, uid, repo.ID, source, "main", source, "", head, "md", false) | ||
| 50 | if err != nil { | ||
| 51 | t.Fatal(err) | ||
| 52 | } | ||
| 53 | git(dir, "update-ref", mrHeadRef(n), head) | ||
| 54 | mr, err := st.MRByNumber(repo.ID, n) | ||
| 55 | if err != nil { | ||
| 56 | t.Fatal(err) | ||
| 57 | } | ||
| 58 | return mr | ||
| 59 | } | ||
| 60 | mrPushed, mrEmpty, mrOther := open("pushed", pushed), open("empty", base), open("other", other) | ||
| 61 | mrGone := open("gone", gone) | ||
| 62 | st.SetMRState(mrGone.ID, "source_gone") | ||
| 63 | |||
| 64 | cfg := config.Default() | ||
| 65 | cfg.Server.Root = root | ||
| 66 | state := func(mr store.MR) store.MR { | ||
| 67 | t.Helper() | ||
| 68 | m, err := st.MRByID(mr.ID) | ||
| 69 | if err != nil { | ||
| 70 | t.Fatal(err) | ||
| 71 | } | ||
| 72 | return m | ||
| 73 | } | ||
| 74 | push := func(old, new string) { | ||
| 75 | git(dir, "update-ref", "refs/heads/main", new, old) | ||
| 76 | RefsUpdated(st, cfg, repo.ID, uid, "full", []policy.RefUpdate{{Ref: "refs/heads/main", Old: old, New: new}}) | ||
| 77 | } | ||
| 78 | |||
| 79 | push(base, pushed) | ||
| 80 | if m := state(mrPushed); m.State != "merged" || m.MergedBase != base { | ||
| 81 | t.Errorf("pushed head: state %s, base %.7s", m.State, m.MergedBase) | ||
| 82 | } | ||
| 83 | if m := state(mrEmpty); m.State != "open" { | ||
| 84 | t.Errorf("head already on main before the push: %s", m.State) | ||
| 85 | } | ||
| 86 | if m := state(mrOther); m.State != "open" { | ||
| 87 | t.Errorf("head not on main: %s", m.State) | ||
| 88 | } | ||
| 89 | |||
| 90 | git(src, "checkout", "-q", "main") | ||
| 91 | git(src, "merge", "-q", "--ff-only", pushed) | ||
| 92 | git(src, "merge", "-q", "--no-ff", "-m", "merge gone", "gone") | ||
| 93 | merged := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | ||
| 94 | git(src, "push", "-q", dir, "main:refs/heads/tmp") | ||
| 95 | git(dir, "update-ref", "-d", "refs/heads/tmp") | ||
| 96 | push(pushed, merged) | ||
| 97 | if m := state(mrGone); m.State != "merged" { | ||
| 98 | t.Errorf("source_gone head merged in: %s", m.State) | ||
| 99 | } | ||
| 100 | |||
| 101 | // Merging outer fast-forwards main over inner's head too. | ||
| 102 | inner := branch("inner", merged, "d") | ||
| 103 | outer := branch("outer", inner, "e") | ||
| 104 | git(src, "push", "-q", dir, "inner", "outer") | ||
| 105 | mrInner, mrOuter := open("inner", inner), open("outer", outer) | ||
| 106 | c, errOut := pruneCtx(st, root, store.User{ID: uid, Username: "alice"}) | ||
| 107 | if code := mergeMR(c, repo, mrOuter, "ff"); code != protocol.ExitOK { | ||
| 108 | t.Fatalf("merge outer: exit %d: %s", code, errOut.String()) | ||
| 109 | } | ||
| 110 | if m := state(mrOuter); m.State != "merged" { | ||
| 111 | t.Errorf("outer: %s", m.State) | ||
| 112 | } | ||
| 113 | if m := state(mrInner); m.State != "merged" || m.MergedBase != merged { | ||
| 114 | t.Errorf("inner carried by outer: state %s, base %.7s", m.State, m.MergedBase) | ||
| 115 | } | ||
| 116 | if m := state(mrOther); m.State != "open" { | ||
| 117 | t.Errorf("unrelated MR after merging outer: %s", m.State) | ||
| 118 | } | ||
| 119 | } | ||
internal/control/mr.go +1
| @@ -1674,6 +1674,7 @@ func mergeMR(c *Ctx, repo store.Repo, mr store.MR, strategy string) int { | |||
| 1674 | QueueBranchBuilds(c.Store, c.Cfg.Server.Root, c.Cfg.Server.SiteURL, | 1674 | QueueBranchBuilds(c.Store, c.Cfg.Server.Root, c.Cfg.Server.SiteURL, |
| 1675 | repo, c.User.ID, mr.TargetRef, targetSHA, newSHA, time.Now()) | 1675 | repo, c.User.ID, mr.TargetRef, targetSHA, newSHA, time.Now()) |
| 1676 | c.Store.MarkMirrorsDirty(repo.ID, "push") | 1676 | c.Store.MarkMirrorsDirty(repo.ID, "push") |
| 1677 | RecordLandedMRs(c.Store, c.Cfg, repo, c.User.ID, mr.TargetRef, targetSHA, newSHA) | ||
| 1677 | if parts, err := c.Store.MRParticipants(mr.ID); err == nil { | 1678 | if parts, err := c.Store.MRParticipants(mr.ID); err == nil { |
| 1678 | notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number, | 1679 | notify(c, parts, notice{repo: repo, kind: "mr", number: mr.Number, |
| 1679 | subject: mrSubject(repo, mr.Number, mr.Title), | 1680 | subject: mrSubject(repo, mr.Number, mr.Title), |
internal/control/refsupdated.go +1
| @@ -58,6 +58,7 @@ func RefsUpdated(st *store.Store, cfg config.Config, repoID, userID int64, scope | |||
| 58 | if pushedRepoErr == nil && !u.IsDelete { | 58 | if pushedRepoErr == nil && !u.IsDelete { |
| 59 | QueueBranchBuilds(st, cfg.Server.Root, cfg.Server.SiteURL, | 59 | QueueBranchBuilds(st, cfg.Server.Root, cfg.Server.SiteURL, |
| 60 | pushedRepo, userID, branch, u.Old, u.New, time.Now()) | 60 | pushedRepo, userID, branch, u.Old, u.New, time.Now()) |
| 61 | RecordLandedMRs(st, cfg, pushedRepo, userID, branch, u.Old, u.New) | ||
| 61 | } | 62 | } |
| 62 | if u.IsForce { | 63 | if u.IsForce { |
| 63 | st.Audit(userID, "push.forced", map[string]any{ | 64 | st.Audit(userID, "push.forced", map[string]any{ |
internal/store/mrs.go +21
| @@ -281,6 +281,27 @@ func (s *Store) OpenMRsBySource(sourceRepoID int64, sourceRef string) ([]MR, err | |||
| 281 | return out, rows.Err() | 281 | return out, rows.Err() |
| 282 | } | 282 | } |
| 283 | 283 | ||
| 284 | // UnresolvedMRsByTarget returns the open and source_gone MRs in repoID | ||
| 285 | // targeting targetRef, oldest first. | ||
| 286 | func (s *Store) UnresolvedMRsByTarget(repoID int64, targetRef string) ([]MR, error) { | ||
| 287 | rows, err := s.DB.Query( | ||
| 288 | mrSelect+" WHERE m.repo_id = ? AND m.target_ref = ? AND m.state IN ('open','source_gone') ORDER BY m.number", | ||
| 289 | repoID, targetRef) | ||
| 290 | if err != nil { | ||
| 291 | return nil, err | ||
| 292 | } | ||
| 293 | defer rows.Close() | ||
| 294 | var out []MR | ||
| 295 | for rows.Next() { | ||
| 296 | m, err := scanMR(rows) | ||
| 297 | if err != nil { | ||
| 298 | return nil, err | ||
| 299 | } | ||
| 300 | out = append(out, m) | ||
| 301 | } | ||
| 302 | return out, rows.Err() | ||
| 303 | } | ||
| 304 | |||
| 284 | // MarkMerged records the merge along with the target tip it landed on, so | 305 | // MarkMerged records the merge along with the target tip it landed on, so |
| 285 | // the MR's diff stays reconstructable after fast-forwards. actorID 0 and an | 306 | // the MR's diff stays reconstructable after fast-forwards. actorID 0 and an |
| 286 | // empty at leave the merger unknown and stamp the current time, which is | 307 | // empty at leave the merger unknown and stamp the current time, which is |