Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Move followed requests when a landing moves the branch

e535ca55547e47705590df49b2ea571dc7b3fa61 · Keenan Nemetz · 4d ago · parent a32a859a

Move followed requests when a landing moves the branch

A landing moves its base branch inside the server, so post-receive never runs
and the open requests that follow that branch keep the pre-merge tip. A stacked
request then reviews code the branch no longer has: its page says "branch
moved", its checks are looked up for the old commit, and any approval it holds
names a commit that is no longer the head.

MarkMerged now moves those followers in the same metadata commit that marks the
request merged and closes its issues, so the landing and its followers cannot
disagree if the write is interrupted. The follower rule is one function shared
with SyncBranchHead instead of two copies.

Closes #196

metadata/request_write.go +47−23

@@ -329,30 +329,12 @@ func SyncBranchHead(g *git.Repo, branch, head, actor string) (int, error) {
329 329 updated := 0
330 330 _, err := git.CommitFiles(g, Ref, func(files map[string][]byte) error {
331 331 // CommitFiles re-runs this on a lost race, so the count is per attempt.
332 updated = 0
333 contents := make(map[string]string)
334 for path, raw := range files {
335 if strings.HasPrefix(path, RequestsDir+"/") {
336 contents[path] = string(raw)
337 }
338 }
339 list, err := ParseRequests(contents)
332 n, err := followBranch(files, branch, head)
340 333 if err != nil {
341 return fmt.Errorf("requests: cannot follow %s: %w", branch, err)
342 }
343 for _, r := range list {
344 if r.State != "open" || r.HeadRef != branch || r.Head == head {
345 continue
346 }
347 r.Head = head
348 raw, err := renderRequest(r)
349 if err != nil {
350 return err
351 }
352 files[fmt.Sprintf("%s/%s/%s", RequestsDir, r.ID, requestName)] = raw
353 updated++
354 }
355 if updated == 0 {
334 return err
335 }
336 updated = n
337 if n == 0 {
356 338 return ErrNoChange
357 339 }
358 340 return nil
@@ -366,6 +348,41 @@ func SyncBranchHead(g *git.Repo, branch, head, actor string) (int, error) {
366 348 return updated, nil
367 349 }
368 350
351 // followBranch moves every open request that follows branch onto head, writing
352 // the changed request files into an open commit. It is SyncBranchHead's body,
353 // shared with the merge mark: a landing moves its base branch without running
354 // post-receive, so it has to move the requests following that branch in the same
355 // commit that marks it merged.
356 func followBranch(files map[string][]byte, branch, head string) (int, error) {
357 if branch == "" || !shaRE.MatchString(head) {
358 return 0, nil
359 }
360 contents := make(map[string]string)
361 for path, raw := range files {
362 if strings.HasPrefix(path, RequestsDir+"/") {
363 contents[path] = string(raw)
364 }
365 }
366 list, err := ParseRequests(contents)
367 if err != nil {
368 return 0, fmt.Errorf("requests: cannot follow %s: %w", branch, err)
369 }
370 updated := 0
371 for _, r := range list {
372 if r.State != "open" || r.HeadRef != branch || r.Head == head {
373 continue
374 }
375 r.Head = head
376 raw, err := renderRequest(r)
377 if err != nil {
378 return 0, err
379 }
380 files[fmt.Sprintf("%s/%s/%s", RequestsDir, r.ID, requestName)] = raw
381 updated++
382 }
383 return updated, nil
384 }
385
369 386 // Approve records principal's approval of the request's current head and base.
370 387 //
371 388 // expectedBase and expectedHead are the target the approver was looking at. If a
@@ -517,6 +534,13 @@ func MarkMerged(g *git.Repo, id, landedBase, landedHead, reviewedHead, actor str
517 534 return err
518 535 }
519 536 closed = ids
537 // The landing moved r.Base, so the requests that follow that branch move
538 // with it. This is the rule post-receive applies to a push, applied to the
539 // merge that was not a push: without it a stacked request keeps reviewing
540 // the pre-merge tip. Same commit as the mark, so the two cannot disagree.
541 if _, err := followBranch(files, r.Base, landedHead); err != nil {
542 return err
543 }
520 544 out = r
521 545 return nil
522 546 }, fmt.Sprintf("request: %s merged %s", id, landedHead), actor)