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.

Test the merge result before landing it

e5478085e5b53a2892d0649d6f07c6f0a5653378 · Keenan Nemetz · 11d ago · parent db5e4382

Test the merge result before landing it

A protected branch could receive a non-fast-forward landing whose tree was
never built: the gate asked for checks on the request head, and the merge
commit was made at landing time. A pushed merge with the right parents but
extra content was accepted too.

Publish the merge commit under refs/vole/candidates/, test it as the base
branch, and make the gate require its results. Approvals name the head and
the base branch, so a moved base retests without a new approval. A merger
who clicks while that run is still going waits for it instead of clicking
again. A pushed merge commit now has to carry exactly the tree a clean merge
would produce.

Closes: 01M61QQ56TESQ1HTMB1Z5C3733

server/web/request_view.go +104−15

@@ -128,12 +128,29 @@ type requestSidebarView struct {
128 128 // is narrowed to the viewer's own namespace for a contributor.
129 129 BaseOptions []branchOption
130 130 HeadOptions []branchOption
131 // CanMerge is the merge control: true only when the gate would accept the
132 // landing now, so the button never offers something the handler would refuse.
131 // CanMerge is the merge control: true when the caller may merge and the gate
132 // would accept the request's approvals. The button also covers "merge when
133 // ready", so a combined result that is still running does not hide it.
133 134 CanMerge bool
135 // MergePending is true when the only thing between the request and a landing is
136 // a combined result that is queued or running.
137 MergePending bool
138 // MergeArmed is true when a merger asked for the request to land as soon as the
139 // combined result passes.
140 MergeArmed bool
134 141 // MergeBlocked is the gate's reason when the caller may merge in principle but
135 // the request is not ready (missing approval, failing check, conflict).
136 MergeBlocked string
142 // the request cannot land (missing approval, failing check, conflict).
143 MergeBlocked string
144 // CombinedNeeded is true when landing needs a merge commit, so there is a tree
145 // that only exists as a combined result.
146 CombinedNeeded bool
147 // CombinedSHA is that commit and CombinedRuns are the newest runs recorded for
148 // it. CombinedState is passed, testing, failed or conflict.
149 CombinedSHA string
150 CombinedRuns []*runs.Run
151 CombinedState string
152 // Notice is a non-error message a write wants beside the controls.
153 Notice string
137 154 CanDiscuss bool
138 155 ContributorCI *requestCI
139 156 Error string
@@ -227,20 +244,86 @@ func (w *Web) requestSidebar(r *http.Request, m *repo.Meta, req *metadata.Reques
227 244 // Preflight the landing rather than guessing from the approval count: the same
228 245 // gate the handler calls is asked here, so the button and the write agree about
229 246 // approvals, checks and conflicts.
230 var mergeable bool
247 var mergeable, mergePending, mergeArmed bool
231 248 var mergeBlocked string
232 if req.Open() && canMerge(acl, who) {
233 // Prepare is what Land runs, so the control and the write agree about
234 // approvals, checks and conflicts. A conflict is already called out with its
235 // paths in the Review section, so it does not repeat a sentence here.
249 var combinedNeeded bool
250 var combinedSHA, combinedState string
251 var combinedRuns []*runs.Run
252 showPlan := canMerge(acl, who) ||
253 (acl != nil && acl.RequiresChecks() && acl.IsProtected("refs/heads/"+req.Base))
254 if req.Open() && showPlan {
255 // Prepare publishes the combined result when one is needed. It is a derived
256 // ref, not review state, so it is safe to make on a read; the reconciler
257 // does the same work in the background.
236 258 plan, err := landing.Prepare(w.landGate(m, acl, who), req.ID, who, time.Now().UTC())
237 259 switch {
238 260 case err != nil:
239 mergeBlocked = err.Error()
240 case len(plan.Conflicts) > 0:
241 mergeBlocked = "" // the Review section already names the paths
242 default:
243 mergeable = true
261 if canMerge(acl, who) {
262 mergeBlocked = err.Error()
263 }
264 case plan != nil:
265 combinedNeeded = plan.MergeCommit && plan.Checks
266 combinedSHA = plan.NewTip
267 switch {
268 case len(plan.Conflicts) > 0:
269 combinedState = "conflict"
270 case !plan.MergeCommit:
271 combinedState = ""
272 case !plan.Checks:
273 combinedState = "passed"
274 case len(plan.Gaps) == 0:
275 combinedState = "passed"
276 case plan.Pending:
277 combinedState = "testing"
278 default:
279 combinedState = "failed"
280 }
281 if w.Runs != nil && plan.NewTip != "" && plan.MergeCommit {
282 if res, err := w.Runs.ForSHA(m.Name, plan.NewTip); err == nil {
283 for _, run := range res {
284 combinedRuns = append(combinedRuns, run)
285 }
286 sort.Slice(combinedRuns, func(i, j int) bool {
287 if combinedRuns[i].Workflow != combinedRuns[j].Workflow {
288 return combinedRuns[i].Workflow < combinedRuns[j].Workflow
289 }
290 return combinedRuns[i].Job < combinedRuns[j].Job
291 })
292 }
293 }
294 if canMerge(acl, who) {
295 switch {
296 case plan.Blocked != "":
297 mergeBlocked = plan.Blocked
298 case len(plan.Conflicts) > 0:
299 mergeBlocked = "" // the Review section already names the paths
300 default:
301 mergeable = true
302 if !plan.Ready {
303 if plan.Pending || len(plan.Gaps) == 0 {
304 mergePending = true
305 } else {
306 mergeBlocked = "The combined result failed. " +
307 "The runs below name the job; pushing a fix starts a new review."
308 }
309 }
310 }
311 }
312 // Someone is looking at this request, so a missing combined result is
313 // work worth queueing even before the first approval. This is the
314 // "recently viewed" half of the base-move rule; the reconciler handles
315 // the head push. Anonymous and read-only viewers do not get to spend
316 // runner time by loading a page.
317 if plan.MergeCommit && len(plan.Gaps) > 0 && w.EnsureCandidate != nil && w.canDiscuss(r, acl) {
318 if err := w.EnsureCandidate(m.Name, req.ID); err != nil && w.Logger != nil {
319 w.Logger("web: cannot prepare the combined result for %s: %v", req.ID, err)
320 }
321 }
322 }
323 }
324 if req.Open() && canMerge(acl, who) && w.Runs != nil {
325 if arm, err := w.Runs.AutoMerge(m.Name, req.ID); err == nil && arm != nil {
326 mergeArmed = true
244 327 }
245 328 }
246 329 var contributorCI *requestCI
@@ -259,7 +342,10 @@ func (w *Web) requestSidebar(r *http.Request, m *repo.Meta, req *metadata.Reques
259 342 Approvals: approvals, Checks: checks, Required: required, Current: current,
260 343 CanApprove: approvable, CanClose: canClose, HeadRefNote: headRefNote,
261 344 CanRetarget: canRetarget, BaseOptions: baseOptions, HeadOptions: headOptions,
262 CanMerge: mergeable, MergeBlocked: mergeBlocked,
345 CanMerge: mergeable, MergePending: mergePending, MergeArmed: mergeArmed,
346 MergeBlocked: mergeBlocked,
347 CombinedNeeded: combinedNeeded, CombinedSHA: combinedSHA,
348 CombinedRuns: combinedRuns, CombinedState: combinedState,
263 349 CanDiscuss: w.canDiscuss(r, acl),
264 350 ContributorCI: contributorCI,
265 351 Error: errMsg,
@@ -400,6 +486,9 @@ func (w *Web) renderRequestPage(rw http.ResponseWriter, r *http.Request, errMsg
400 486
401 487 sidebar := w.requestSidebar(r, m, req, m.Name+" · "+req.Title, errMsg).
402 488 withLanding(w.landingFrom(m, req, files, alreadyMerged))
489 if r.URL.Query().Get("merging") == "1" {
490 sidebar.Notice = "Merging when the combined result passes. Use Cancel merge to stop waiting."
491 }
403 492 w.render(rw, "request", requestPageView{
404 493 requestSidebarView: sidebar,
405 494 Tab: "conversation",