mr: retarget an open merge request onto another branch !121

merged merged by cmc on 2026-08-30 06:16 UTC · krz/gitbay:mr-retarget into main

9 files changed, +250 −6

Layout: unified · split

cmd/gitbay/main.go +1

	@@ -395,6 +395,7 @@ func mrCmd() *cobra.Command {
395	pass("close", "close without merging", passOpts{server: []string{"mr", "close"}, needsRepo: true}),	395	pass("close", "close without merging", passOpts{server: []string{"mr", "close"}, needsRepo: true}),
396	pass("edit", "edit title or body: <n> [--title <t>] [--body <b>\|--file -]", passOpts{server: []string{"mr", "edit"}, needsRepo: true, stdinOK: true}),	396	pass("edit", "edit title or body: <n> [--title <t>] [--body <b>\|--file -]", passOpts{server: []string{"mr", "edit"}, needsRepo: true, stdinOK: true}),
397	pass("milestone", "set or clear the milestone: <n> <title\|none>", passOpts{server: []string{"mr", "milestone"}, needsRepo: true}),	397	pass("milestone", "set or clear the milestone: <n> <title\|none>", passOpts{server: []string{"mr", "milestone"}, needsRepo: true}),
		398	pass("retarget", "retarget onto another branch: <n> <branch>", passOpts{server: []string{"mr", "retarget"}, needsRepo: true}),
398	)	399	)
399	}	400	}
400		401

e2e/mr_test.go +6 −5

	@@ -13,11 +13,12 @@ import (
13	)	13	)
14		14
15	type mrShow struct {	15	type mrShow struct {
16	Number int64 `json:"number"`	16	Number int64 `json:"number"`
17	State string `json:"state"`	17	State string `json:"state"`
18	Source string `json:"source"`	18	Source string `json:"source"`
19	HeadSHA string `json:"head_sha"`	19	TargetRef string `json:"target_ref"`
20	Reviews []struct {	20	HeadSHA string `json:"head_sha"`
		21	Reviews []struct {
21	Reviewer string `json:"reviewer"`	22	Reviewer string `json:"reviewer"`
22	Verdict string `json:"verdict"`	23	Verdict string `json:"verdict"`
23	Stale bool `json:"stale"`	24	Stale bool `json:"stale"`

e2e/mrretarget_test.go added +127

@@ -0,0 +1,127 @@
	1	package e2e
	2
	3	import (
	4	"net/url"
	5	"os"
	6	"path/filepath"
	7	"strings"
	8	"testing"
	9	)
	10
	11	// TestMRRetarget moves an open merge request onto another branch, over
	12	// SSH and from the browser. Retargeting changes which diff a review was
	13	// of, so the existing approvals have to go stale with it.
	14	func TestMRRetarget(t *testing.T) {
	15	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
	16	aliceKey := inst.newKey(t, "alice")
	17	bobKey := inst.newKey(t, "bob")
	18	inst.admin(t, "admin", "user", "create", "alice",
	19	"--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
	20	inst.admin(t, "admin", "user", "create", "bob",
	21	"--key", bobKey+".pub", "--email", "bob@example.test", "--verified")
	22
	23	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
	24	t.Fatalf("repo create: %s", errOut)
	25	}
	26	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/lib", "bob", "write"); code != 0 {
	27	t.Fatalf("grant: %s", errOut)
	28	}
	29
	30	env := inst.gitEnv(aliceKey)
	31	work := t.TempDir()
	32	mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w")
	33	dir := filepath.Join(work, "w")
	34	os.WriteFile(filepath.Join(dir, "lib.txt"), []byte("v1\n"), 0o644)
	35	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
	36	mustGit(t, dir, env, "add", ".")
	37	mustGit(t, dir, env, "commit", "-q", "-m", "base")
	38	mustGit(t, dir, env, "push", "-q", "origin", "main")
	39	// A second long-lived branch to retarget onto.
	40	mustGit(t, dir, env, "checkout", "-q", "-b", "release")
	41	os.WriteFile(filepath.Join(dir, "release.txt"), []byte("1.0\n"), 0o644)
	42	mustGit(t, dir, env, "add", ".")
	43	mustGit(t, dir, env, "commit", "-q", "-m", "cut release")
	44	mustGit(t, dir, env, "push", "-q", "origin", "release")
	45	mustGit(t, dir, env, "checkout", "-q", "-b", "feat", "main")
	46	os.WriteFile(filepath.Join(dir, "feat.txt"), []byte("work\n"), 0o644)
	47	mustGit(t, dir, env, "add", ".")
	48	mustGit(t, dir, env, "commit", "-q", "-m", "add feat")
	49	mustGit(t, dir, env, "push", "-q", "origin", "feat")
	50
	51	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/lib",
	52	"--source", "feat", "--target", "main", "--title", "'feature'"); code != 0 {
	53	t.Fatalf("mr create: %s", errOut)
	54	}
	55	if _, errOut, code := inst.ssh(t, bobKey, "", "mr", "review", "alice/lib", "1", "--approve"); code != 0 {
	56	t.Fatalf("review: %s", errOut)
	57	}
	58
	59	// Refusals first: an unknown branch, the branch it already targets,
	60	// and its own source.
	61	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "retarget", "alice/lib", "1", "nope"); code != 3 \|\|
	62	!strings.Contains(errOut, "not found") {
	63	t.Fatalf("unknown branch: exit %d, %s", code, errOut)
	64	}
	65	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "retarget", "alice/lib", "1", "main"); code != 2 \|\|
	66	!strings.Contains(errOut, "already targets") {
	67	t.Fatalf("same branch: exit %d, %s", code, errOut)
	68	}
	69	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "retarget", "alice/lib", "1", "feat"); code != 2 \|\|
	70	!strings.Contains(errOut, "source branch") {
	71	t.Fatalf("source branch: exit %d, %s", code, errOut)
	72	}
	73	// Nobody outside the repo can move it.
	74	eveKey := inst.newKey(t, "eve")
	75	inst.admin(t, "admin", "user", "create", "eve", "--key", eveKey+".pub")
	76	if _, _, code := inst.ssh(t, eveKey, "", "mr", "retarget", "alice/lib", "1", "release"); code == 0 {
	77	t.Fatal("a stranger retargeted the merge request")
	78	}
	79
	80	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "retarget", "alice/lib", "1", "release"); code != 0 {
	81	t.Fatalf("retarget: %s", errOut)
	82	}
	83	show := inst.mrShow(t, aliceKey, "alice/lib", "1")
	84	if show.TargetRef != "release" {
	85	t.Fatalf("target not moved: %q", show.TargetRef)
	86	}
	87	if len(show.Reviews) != 1 \|\| !show.Reviews[0].Stale {
	88	t.Fatalf("approval survived the retarget: %+v", show.Reviews)
	89	}
	90	// The diff follows the new base: release.txt is on the target now, so
	91	// it is no longer part of the change.
	92	out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "diff", "alice/lib", "1")
	93	if code != 0 {
	94	t.Fatalf("mr diff: %s", errOut)
	95	}
	96	if !strings.Contains(out, "feat.txt") \|\| strings.Contains(out, "release.txt") {
	97	t.Fatalf("diff not rebased on the new target:\n%s", out)
	98	}
	99
	100	// The move is recorded on the conversation.
	101	if out, _, _ := inst.ssh(t, aliceKey, "", "mr", "show", "alice/lib", "1"); !strings.Contains(out, "retargeted from main to release") {
	102	t.Fatalf("no system comment for the move:\n%s", out)
	103	}
	104
	105	// And the browser can do it, through the same command.
	106	mrURL := inst.base() + "/alice/lib/mrs/1"
	107	alice := inst.login(t, aliceKey)
	108	_, body := browserGet(t, alice, mrURL)
	109	if !strings.Contains(body, `action="/alice/lib/mrs/1/retarget"`) {
	110	t.Fatalf("no retarget control on the MR page:\n%s", body)
	111	}
	112	if status, _ := browserPost(t, alice, mrURL+"/retarget", url.Values{"target": {"main"}}); status != 200 {
	113	t.Fatalf("retarget post: %d", status)
	114	}
	115	if got := inst.mrShow(t, aliceKey, "alice/lib", "1").TargetRef; got != "main" {
	116	t.Fatalf("web retarget did not land: %q", got)
	117	}
	118
	119	// A merged merge request is settled.
	120	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "1"); code != 0 {
	121	t.Fatalf("merge: %s", errOut)
	122	}
	123	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "retarget", "alice/lib", "1", "release"); code != 2 \|\|
	124	!strings.Contains(errOut, "only an open merge request") {
	125	t.Fatalf("merged MR retargeted: exit %d, %s", code, errOut)
	126	}
	127	}

internal/control/mr.go +59

	@@ -38,6 +38,8 @@ func init() {
38	register(Command{Path: []string{"mr", "edit"},	38	register(Command{Path: []string{"mr", "edit"},
39	Summary: "edit title or body: mr edit <owner/name> <n> [--title <t>] [--body <b> \| --file -] [--format md\|org]",	39	Summary: "edit title or body: mr edit <owner/name> <n> [--title <t>] [--body <b> \| --file -] [--format md\|org]",
40	ReadsStdin: true, Run: runMREdit})	40	ReadsStdin: true, Run: runMREdit})
		41	register(Command{Path: []string{"mr", "retarget"},
		42	Summary: "retarget onto another branch: mr retarget <owner/name> <n> <branch>", Run: runMRRetarget})
41	register(Command{Path: []string{"mr", "comment"},	43	register(Command{Path: []string{"mr", "comment"},
42	Summary: "comment: mr comment <owner/name> <n> [--message <m> \| --file -] [--format md\|org]",	44	Summary: "comment: mr comment <owner/name> <n> [--message <m> \| --file -] [--format md\|org]",
43	ReadsStdin: true, Run: runMRComment})	45	ReadsStdin: true, Run: runMRComment})
	@@ -550,6 +552,63 @@ func runMREdit(c *Ctx, args []string) int {
550	})	552	})
551	}	553	}
552		554
		555	// runMRRetarget moves an open merge request onto another branch of the
		556	// same repository.
		557	func runMRRetarget(c *Ctx, args []string) int {
		558	if len(args) != 3 {
		559	return c.fail(protocol.ExitUsage, "usage: mr retarget <owner/name> <n> <branch>")
		560	}
		561	repo, mr, code := mrRef(c, args[:2], policy.CanRead)
		562	if code >= 0 {
		563	return code
		564	}
		565	if code := refuseArchived(c, repo); code >= 0 {
		566	return code
		567	}
		568	grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
		569	if err != nil {
		570	return c.fail(protocol.ExitFailure, "%v", err)
		571	}
		572	if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
		573	return c.fail(protocol.ExitDenied, "only the author or users with write access can retarget this merge request")
		574	}
		575	if mr.State == "merged" \|\| mr.State == "closed" {
		576	return c.fail(protocol.ExitUsage, "!%d is %s; only an open merge request can be retargeted", mr.Number, mr.State)
		577	}
		578	target := args[2]
		579	if target == mr.TargetRef {
		580	return c.fail(protocol.ExitUsage, "!%d already targets %s", mr.Number, target)
		581	}
		582	if mr.SourceRepoID == repo.ID && target == mr.SourceRef {
		583	return c.fail(protocol.ExitUsage, "%s is the source branch of !%d", target, mr.Number)
		584	}
		585	dir := RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name)
		586	if _, err := gitutil.ResolveRef(dir, "refs/heads/"+target); err != nil {
		587	return c.fail(protocol.ExitNotFound, "branch %s not found in %s", target, repo.Path())
		588	}
		589	// The diff, the commit list and the merge gates all derive their base
		590	// from the target on every read, so the only thing to check here is
		591	// that a base exists at all: without one there is nothing to show and
		592	// nothing to merge.
		593	base, err := gitutil.MergeBase(dir, "refs/heads/"+target, mrHeadRef(mr.Number))
		594	if err != nil \|\| base == "" {
		595	return c.fail(protocol.ExitUsage, "%s shares no history with the head of !%d", target, mr.Number)
		596	}
		597	old := mr.TargetRef
		598	if err := c.Store.SetMRTarget(mr.ID, target); err != nil {
		599	return c.fail(protocol.ExitFailure, "%v", err)
		600	}
		601	c.Store.AddMRSystemComment(mr.ID, c.User.ID, fmt.Sprintf("retargeted from %s to %s", old, target))
		602	if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
		603	notifyUsers(c, parts, mrSubject(repo, mr.Number, mr.Title),
		604	notifyBody(c, fmt.Sprintf("retargeted !%d from %s to %s", mr.Number, old, target), "",
		605	fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)))
		606	}
		607	return c.emit(map[string]any{"number": mr.Number, "target_ref": target, "merge_base": base}, func(w io.Writer) {
		608	fmt.Fprintf(w, "retargeted %s!%d from %s to %s (base %.10s)\n", repo.Path(), mr.Number, old, target, base)
		609	})
		610	}
		611
553	func runMRComment(c *Ctx, args []string) int {	612	func runMRComment(c *Ctx, args []string) int {
554	var rest []string	613	var rest []string
555	var message, file, format string	614	var message, file, format string

internal/httpd/mractions.go +14

	@@ -121,6 +121,20 @@ func (s Server) mrDiffCommentSubmit(w http.ResponseWriter, r http.Request, u s
121	s.mrDiffRedirect(w, r, msg)	121	s.mrDiffRedirect(w, r, msg)
122	}	122	}
123		123
		124	// mrRetargetSubmit moves the merge request onto another branch.
		125	func (s Server) mrRetargetSubmit(w http.ResponseWriter, r http.Request, u store.User) {
		126	target := strings.TrimSpace(r.FormValue("target"))
		127	if target == "" {
		128	s.mrRedirect(w, r, "pick a branch to retarget onto")
		129	return
		130	}
		131	_, msg, ok := s.runControl(u, mrArgs(r, "retarget", target))
		132	if ok {
		133	msg = ""
		134	}
		135	s.mrRedirect(w, r, msg)
		136	}
		137
124	// mrThreadSubmit resolves or reopens one review thread.	138	// mrThreadSubmit resolves or reopens one review thread.
125	func (s Server) mrThreadSubmit(w http.ResponseWriter, r http.Request, u store.User) {	139	func (s Server) mrThreadSubmit(w http.ResponseWriter, r http.Request, u store.User) {
126	verb := "resolve"	140	verb := "resolve"

internal/httpd/routes.go +2

 				Handler: s.checkOrigin(s.requireUser(s.mrMergeSubmit))},
 			Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/close", Mutating: true,
 				Handler: s.checkOrigin(s.requireUser(s.mrCloseSubmit))},
+			Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/retarget", Mutating: true,
+				Handler: s.checkOrigin(s.requireUser(s.mrRetargetSubmit))},
 			Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/thread", Mutating: true,
 				Handler: s.checkOrigin(s.requireUser(s.mrThreadSubmit))},
 			Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/diff-comment", Mutating: true,

internal/httpd/web.go +3 −1

 	// its own view rather than a fold at the foot of the conversation.
 	// A query parameter keeps this working without JavaScript.
 	unresolved, _ := s.st.UnresolvedThreadCount(m.ID)
+	branches, _ := gitutil.Refs(p.Dir, "heads")
 	view := r.URL.Query().Get("view")
 	if view != "commits" && view != "diff" {
 		view = "conversation"
 		DiffFiles       []diffFile
 		Stat            diffStat
 		Commits         []commitRow
+		Branches        []gitutil.Ref
 		CanEdit         bool
 		CanWrite        bool
 		Unresolved      int
 		Notice          string
 		DetachedThreads []diffThread
 	}{p, m, view, md(m.Body, m.BodyFormat), checks, store.CombinedStatus(checks), renderComments(comments, md),
-		reviews, files, stat, commits, s.canEditItem(r, p.Repo, m.Author),
+		reviews, files, stat, commits, branches, s.canEditItem(r, p.Repo, m.Author),
 		canWrite, unresolved, r.URL.Query().Get("e"), detachedThreads})
 }

internal/store/mrs.go +25

	@@ -183,6 +183,31 @@ func (s *Store) UpdateMRHead(mrID int64, headSHA string) error {
183	return tx.Commit()	183	return tx.Commit()
184	}	184	}
185		185
		186	// SetMRTarget retargets a merge request and marks every existing review
		187	// stale, in one transaction. The base of the diff is derived from the
		188	// target on every read, so nothing else has to move; an approval,
		189	// though, was of the diff against the old branch.
		190	func (s *Store) SetMRTarget(mrID int64, targetRef string) error {
		191	tx, err := s.DB.Begin()
		192	if err != nil {
		193	return err
		194	}
		195	defer tx.Rollback()
		196	res, err := tx.Exec(
		197	"UPDATE merge_requests SET target_ref = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?",
		198	targetRef, mrID)
		199	if err != nil {
		200	return err
		201	}
		202	if n, _ := res.RowsAffected(); n == 0 {
		203	return ErrNotFound
		204	}
		205	if _, err := tx.Exec("UPDATE mr_reviews SET stale = 1 WHERE mr_id = ?", mrID); err != nil {
		206	return err
		207	}
		208	return tx.Commit()
		209	}
		210
186	// MarkSourceGoneForRepo flags every open MR sourced from the repo; called	211	// MarkSourceGoneForRepo flags every open MR sourced from the repo; called
187	// when a fork is deleted. Head refs in the target repos are retained.	212	// when a fork is deleted. Head refs in the target repos are retained.
188	func (s *Store) MarkSourceGoneForRepo(sourceRepoID int64) error {	213	func (s *Store) MarkSourceGoneForRepo(sourceRepoID int64) error {

internal/web/templates/mr.html +13

	@@ -94,6 +94,19 @@
94	</form>	94	</form>
95	</div>	95	</div>
96	{{end}}	96	{{end}}
		97	{{if and .CanEdit (or (eq .MR.State "open") (eq .MR.State "source_gone"))}}
		98	<div class="grp">
		99	<h2>Target</h2>
		100	<form method="post" action="{{$base}}/retarget" class="actions">
		101	<label class="none" for="target">Branch</label>
		102	<select id="target" name="target">
		103	{{range .Branches}}<option value="{{.Name}}"{{if eq .Name $.MR.TargetRef}} selected{{end}}>{{.Name}}</option>{{end}}
		104	</select>
		105	<button type="submit">Retarget</button>
		106	</form>
		107	<p class="row none">Retargeting stales existing reviews.</p>
		108	</div>
		109	{{end}}
97	<div class="grp">	110	<div class="grp">
98	<h2>Reviews</h2>	111	<h2>Reviews</h2>
99	{{range .Reviews}}<p class="row"><span class="dot {{if eq .Verdict "approve"}}ok{{else}}pend{{end}}"></span><a href="/{{.Reviewer}}">{{.Reviewer}}</a> {{.Verdict}}{{if .Stale}} <span class="chip chip-stale">stale</span>{{end}}</p>	112	{{range .Reviews}}<p class="row"><span class="dot {{if eq .Verdict "approve"}}ok{{else}}pend{{end}}"></span><a href="/{{.Reviewer}}">{{.Reviewer}}</a> {{.Verdict}}{{if .Stale}} <span class="chip chip-stale">stale</span>{{end}}</p>