| @@ -567,6 +567,13 @@ func runMRShow(c *Ctx, args []string) int { |
| 567 | d := MRShow{mrOut: mrToOut(repo, mr, true), Checks: checks, Combined: combined, |
567 | d := MRShow{mrOut: mrToOut(repo, mr, true), Checks: checks, Combined: combined, |
| 568 | UnresolvedThreads: unresolved, Commits: commits, Comments: cs, Reviews: rs} |
568 | UnresolvedThreads: unresolved, Commits: commits, Comments: cs, Reviews: rs} |
| 569 | d.StackedOn, d.Stacked = stackOf(c, repo, mr) |
569 | d.StackedOn, d.Stacked = stackOf(c, repo, mr) |
| |
570 | if mr.State == "open" || mr.State == "source_gone" { |
| |
571 | if targetSHA, err := gitutil.ResolveRef(dir, "refs/heads/"+mr.TargetRef); err == nil { |
| |
572 | if g, err := MergeGates(c.Store, repo, mr, dir, targetSHA, mr.HeadSHA); err == nil { |
| |
573 | d.Gates = &g |
| |
574 | } |
| |
575 | } |
| |
576 | } |
| 570 | return c.emit(d, func(w io.Writer) { |
577 | return c.emit(d, func(w io.Writer) { |
| 571 | state := d.State |
578 | state := d.State |
| 572 | if d.Draft { |
579 | if d.Draft { |
| @@ -604,6 +611,20 @@ func runMRShow(c *Ctx, args []string) int { |
| 604 | if d.UnresolvedThreads > 0 { |
611 | if d.UnresolvedThreads > 0 { |
| 605 | fmt.Fprintf(w, "unresolved threads: %d\n", d.UnresolvedThreads) |
612 | fmt.Fprintf(w, "unresolved threads: %d\n", d.UnresolvedThreads) |
| 606 | } |
613 | } |
| |
614 | if g := d.Gates; g != nil { |
| |
615 | ff := "fast-forward possible" |
| |
616 | if !g.FastForward { |
| |
617 | ff = "fast-forward not possible" |
| |
618 | } |
| |
619 | if len(g.Unmet) == 0 { |
| |
620 | fmt.Fprintf(w, "gates: met; %s\n", ff) |
| |
621 | } else { |
| |
622 | fmt.Fprintf(w, "gates: %d unmet; %s\n", len(g.Unmet), ff) |
| |
623 | for _, u := range g.Unmet { |
| |
624 | fmt.Fprintf(w, "gate: %s\n", u) |
| |
625 | } |
| |
626 | } |
| |
627 | } |
| 607 | for _, r := range rs { |
628 | for _, r := range rs { |
| 608 | stale := "" |
629 | stale := "" |
| 609 | if r.Stale { |
630 | if r.Stale { |
| @@ -800,11 +821,17 @@ func runMRReview(c *Ctx, args []string) int { |
| 800 | action: reviewAction(mr.Number, verdict, published), |
821 | action: reviewAction(mr.Number, verdict, published), |
| 801 | path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) |
822 | path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) |
| 802 | } |
823 | } |
| 803 | return c.emit(map[string]any{"number": mr.Number, "verdict": verdict, "published": published}, func(w io.Writer) { |
824 | // Whether the merge gates will count this verdict, said now rather |
| |
825 | // than at the refusal (#199). |
| |
826 | counts := ReviewersWhoCount(c.Store, repo, []store.MRReview{{Reviewer: c.User.Username}})[c.User.Username] |
| |
827 | return c.emit(map[string]any{"number": mr.Number, "verdict": verdict, "published": published, "counts": counts}, func(w io.Writer) { |
| 804 | fmt.Fprintf(w, "reviewed %s!%d: %s", repo.Path(), mr.Number, verdict) |
828 | fmt.Fprintf(w, "reviewed %s!%d: %s", repo.Path(), mr.Number, verdict) |
| 805 | if published > 0 { |
829 | if published > 0 { |
| 806 | fmt.Fprintf(w, " (%d comment(s))", published) |
830 | fmt.Fprintf(w, " (%d comment(s))", published) |
| 807 | } |
831 | } |
| |
832 | if !counts { |
| |
833 | fmt.Fprintf(w, " (advisory: no write access on %s, so the merge gates do not count it)", repo.Path()) |
| |
834 | } |
| 808 | fmt.Fprintln(w) |
835 | fmt.Fprintln(w) |
| 809 | }) |
836 | }) |
| 810 | } |
837 | } |
| @@ -928,31 +955,8 @@ func runMRMerge(c *Ctx, args []string) int { |
| 928 | return c.fail(protocol.ExitFailure, "MR head ref: %v", err) |
955 | return c.fail(protocol.ExitFailure, "MR head ref: %v", err) |
| 929 | } |
956 | } |
| 930 | |
957 | |
| 931 | // Check gate: with require_checks, the MR head must carry statuses |
958 | // Merge gates: draft, checks, approvals, CODEOWNERS, resolved threads, |
| 932 | // and every one of them must be green. |
959 | // all reported at once. |
| 933 | if repo.Settings.RequireChecks { |
| |
| 934 | statuses, err := c.Store.ListCommitStatuses(repo.ID, headSHA) |
| |
| 935 | if err != nil { |
| |
| 936 | return c.fail(protocol.ExitFailure, "%v", err) |
| |
| 937 | } |
| |
| 938 | switch store.CombinedStatus(statuses) { |
| |
| 939 | case "success": |
| |
| 940 | case "": |
| |
| 941 | return c.fail(protocol.ExitDenied, |
| |
| 942 | "%s requires green checks and none were reported on %.10s", repo.Path(), headSHA) |
| |
| 943 | default: |
| |
| 944 | var bad []string |
| |
| 945 | for _, st := range statuses { |
| |
| 946 | if st.State != "success" { |
| |
| 947 | bad = append(bad, st.Context+"="+st.State) |
| |
| 948 | } |
| |
| 949 | } |
| |
| 950 | return c.fail(protocol.ExitDenied, |
| |
| 951 | "%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", ")) |
| |
| 952 | } |
| |
| 953 | } |
| |
| 954 | |
| |
| 955 | // Review gates: approvals, CODEOWNERS, resolved threads. |
| |
| 956 | if code := c.reviewGates(repo, mr, dir, targetSHA, headSHA); code >= 0 { |
960 | if code := c.reviewGates(repo, mr, dir, targetSHA, headSHA); code >= 0 { |
| 957 | return code |
961 | return code |
| 958 | } |
962 | } |
| @@ -1225,20 +1229,62 @@ func runMRMerge(c *Ctx, args []string) int { |
| 1225 | }) |
1229 | }) |
| 1226 | } |
1230 | } |
| 1227 | |
1231 | |
| 1228 | // reviewGates enforces require_approvals (fresh, non-author, latest review |
1232 | // reviewGates refuses a merge whose gates are not all met, naming every |
| 1229 | // per reviewer; a fresh request-changes blocks), require_codeowners, and |
1233 | // unmet one. Returns -1 to proceed. |
| 1230 | // require_resolved. Returns -1 to proceed. |
| |
| 1231 | func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int { |
1234 | func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int { |
| |
1235 | g, err := MergeGates(c.Store, repo, mr, dir, targetSHA, headSHA) |
| |
1236 | if err != nil { |
| |
1237 | return c.fail(protocol.ExitFailure, "%v", err) |
| |
1238 | } |
| |
1239 | if len(g.Unmet) > 0 { |
| |
1240 | return c.fail(protocol.ExitDenied, "%s", strings.Join(g.Unmet, "; ")) |
| |
1241 | } |
| |
1242 | return -1 |
| |
1243 | } |
| |
1244 | |
| |
1245 | // MergeGates computes where a merge request stands against its |
| |
1246 | // repository's gates: draft, require_checks, require_approvals (fresh, |
| |
1247 | // non-author, latest review per reviewer from someone who can write; a |
| |
1248 | // fresh request-changes blocks), require_codeowners and require_resolved. |
| |
1249 | // Unmet carries one sentence per gate not passed. Fast-forward is |
| |
1250 | // reported, not gated: whether it matters depends on the strategy. |
| |
1251 | func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) (GatesOut, error) { |
| |
1252 | set := repo.Settings |
| |
1253 | g := GatesOut{Draft: mr.Draft, ApprovalsRequired: set.RequireApprovals, |
| |
1254 | CodeownersRequired: set.RequireCodeowners, ResolvedRequired: set.RequireResolved, |
| |
1255 | ChecksRequired: set.RequireChecks} |
| 1232 | // A draft is open but not asking. This gate is unconditional — no |
1256 | // A draft is open but not asking. This gate is unconditional — no |
| 1233 | // setting turns it off — because the author said so themselves. |
1257 | // setting turns it off — because the author said so themselves. |
| 1234 | if mr.Draft { |
1258 | if mr.Draft { |
| 1235 | return c.fail(protocol.ExitDenied, |
1259 | g.Unmet = append(g.Unmet, fmt.Sprintf("!%d is a draft; `gitbay mr ready %s %d` first", mr.Number, repo.Path(), mr.Number)) |
| 1236 | "!%d is a draft; `gitbay mr ready %s %d` first", mr.Number, repo.Path(), mr.Number) |
| |
| 1237 | } |
1260 | } |
| 1238 | set := repo.Settings |
1261 | |
| 1239 | reviews, err := c.Store.ListMRReviews(mr.ID) |
1262 | // Checks: with require_checks, the head must carry statuses and every |
| |
1263 | // one of them must be green. |
| |
1264 | statuses, err := st.ListCommitStatuses(repo.ID, headSHA) |
| 1240 | if err != nil { |
1265 | if err != nil { |
| 1241 | return c.fail(protocol.ExitFailure, "%v", err) |
1266 | return g, err |
| |
1267 | } |
| |
1268 | g.Checks = store.CombinedStatus(statuses) |
| |
1269 | if set.RequireChecks { |
| |
1270 | switch g.Checks { |
| |
1271 | case "success": |
| |
1272 | case "": |
| |
1273 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks and none were reported on %.10s", repo.Path(), headSHA)) |
| |
1274 | default: |
| |
1275 | var bad []string |
| |
1276 | for _, st := range statuses { |
| |
1277 | if st.State != "success" { |
| |
1278 | bad = append(bad, st.Context+"="+st.State) |
| |
1279 | } |
| |
1280 | } |
| |
1281 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", "))) |
| |
1282 | } |
| |
1283 | } |
| |
1284 | |
| |
1285 | reviews, err := st.ListMRReviews(mr.ID) |
| |
1286 | if err != nil { |
| |
1287 | return g, err |
| 1242 | } |
1288 | } |
| 1243 | // Latest fresh review per reviewer decides their stance — but only |
1289 | // Latest fresh review per reviewer decides their stance — but only |
| 1244 | // from someone the repository trusts to write to it. Reviewing is |
1290 | // from someone the repository trusts to write to it. Reviewing is |
| @@ -1246,7 +1292,7 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA |
| 1246 | // public change possible; deciding a merge gate is not the same |
1292 | // public change possible; deciding a merge gate is not the same |
| 1247 | // thing, and counting every verdict let anyone with an account |
1293 | // thing, and counting every verdict let anyone with an account |
| 1248 | // satisfy require_approvals or block a merge indefinitely (#147). |
1294 | // satisfy require_approvals or block a merge indefinitely (#147). |
| 1249 | counts := ReviewersWhoCount(c.Store, repo, reviews) |
1295 | counts := ReviewersWhoCount(st, repo, reviews) |
| 1250 | latest := map[string]string{} |
1296 | latest := map[string]string{} |
| 1251 | for _, r := range reviews { |
1297 | for _, r := range reviews { |
| 1252 | if r.Stale || r.Reviewer == mr.Author || !counts[r.Reviewer] { |
1298 | if r.Stale || r.Reviewer == mr.Author || !counts[r.Reviewer] { |
| @@ -1254,26 +1300,22 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA |
| 1254 | } |
1300 | } |
| 1255 | latest[r.Reviewer] = r.Verdict |
1301 | latest[r.Reviewer] = r.Verdict |
| 1256 | } |
1302 | } |
| 1257 | var approvers []string |
| |
| 1258 | var blockers []string |
| |
| 1259 | for who, verdict := range latest { |
1303 | for who, verdict := range latest { |
| 1260 | switch verdict { |
1304 | switch verdict { |
| 1261 | case "approve": |
1305 | case "approve": |
| 1262 | approvers = append(approvers, who) |
1306 | g.Approvals = append(g.Approvals, who) |
| 1263 | case "request_changes": |
1307 | case "request_changes": |
| 1264 | blockers = append(blockers, who) |
1308 | g.ChangesRequested = append(g.ChangesRequested, who) |
| 1265 | } |
1309 | } |
| 1266 | } |
1310 | } |
| 1267 | |
1311 | slices.Sort(g.Approvals) |
| |
1312 | slices.Sort(g.ChangesRequested) |
| 1268 | if set.RequireApprovals > 0 { |
1313 | if set.RequireApprovals > 0 { |
| 1269 | if len(blockers) > 0 { |
1314 | if len(g.ChangesRequested) > 0 { |
| 1270 | slices.Sort(blockers) |
1315 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requested changes on !%d; resolve their review before merging", strings.Join(g.ChangesRequested, ", "), mr.Number)) |
| 1271 | return c.fail(protocol.ExitDenied, |
| |
| 1272 | "%s requested changes on !%d; resolve their review before merging", strings.Join(blockers, ", "), mr.Number) |
| |
| 1273 | } |
1316 | } |
| 1274 | if len(approvers) < set.RequireApprovals { |
1317 | if len(g.Approvals) < set.RequireApprovals { |
| 1275 | return c.fail(protocol.ExitDenied, |
1318 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires %d fresh approval(s); !%d has %d", repo.Path(), set.RequireApprovals, mr.Number, len(g.Approvals))) |
| 1276 | "%s requires %d fresh approval(s); !%d has %d", repo.Path(), set.RequireApprovals, mr.Number, len(approvers)) |
| |
| 1277 | } |
1319 | } |
| 1278 | } |
1320 | } |
| 1279 | |
1321 | |
| @@ -1287,65 +1329,72 @@ func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA |
| 1287 | content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20) |
1329 | content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20) |
| 1288 | } |
1330 | } |
| 1289 | if err != nil || len(content) == 0 { |
1331 | if err != nil || len(content) == 0 { |
| 1290 | return c.fail(protocol.ExitDenied, |
1332 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires CODEOWNERS approval but %s carries no CODEOWNERS file", repo.Path(), mr.TargetRef)) |
| 1291 | "%s requires CODEOWNERS approval but %s carries no CODEOWNERS file", |
1333 | } else { |
| 1292 | repo.Path(), mr.TargetRef) |
1334 | rules := policy.ParseCodeowners(string(content)) |
| 1293 | } |
1335 | base, err := gitutil.MergeBase(dir, targetSHA, headSHA) |
| 1294 | rules := policy.ParseCodeowners(string(content)) |
1336 | if err != nil { |
| 1295 | base, err := gitutil.MergeBase(dir, targetSHA, headSHA) |
1337 | return g, err |
| 1296 | if err != nil { |
| |
| 1297 | return c.fail(protocol.ExitFailure, "%v", err) |
| |
| 1298 | } |
| |
| 1299 | files, err := gitutil.DiffFiles(dir, base, headSHA) |
| |
| 1300 | if err != nil { |
| |
| 1301 | return c.fail(protocol.ExitFailure, "%v", err) |
| |
| 1302 | } |
| |
| 1303 | approved := map[string]bool{} |
| |
| 1304 | for _, a := range approvers { |
| |
| 1305 | approved[a] = true |
| |
| 1306 | } |
| |
| 1307 | missing := map[string][]string{} // owner-set key -> example paths |
| |
| 1308 | for _, f := range files { |
| |
| 1309 | owners := policy.OwnersFor(rules, f) |
| |
| 1310 | if owners == nil { |
| |
| 1311 | continue |
| |
| 1312 | } |
1338 | } |
| 1313 | ok := false |
1339 | files, err := gitutil.DiffFiles(dir, base, headSHA) |
| 1314 | for _, o := range owners { |
1340 | if err != nil { |
| 1315 | if approved[o] { |
1341 | return g, err |
| 1316 | ok = true |
1342 | } |
| 1317 | break |
1343 | approved := map[string]bool{} |
| 1318 | } |
1344 | for _, a := range g.Approvals { |
| |
1345 | approved[a] = true |
| 1319 | } |
1346 | } |
| 1320 | if !ok { |
1347 | missing := map[string][]string{} // owner-set key -> paths |
| 1321 | key := strings.Join(owners, ",") |
1348 | var keys []string |
| 1322 | if len(missing[key]) < 3 { |
1349 | for _, f := range files { |
| |
1350 | owners := policy.OwnersFor(rules, f) |
| |
1351 | if owners == nil { |
| |
1352 | continue |
| |
1353 | } |
| |
1354 | ok := false |
| |
1355 | for _, o := range owners { |
| |
1356 | if approved[o] { |
| |
1357 | ok = true |
| |
1358 | break |
| |
1359 | } |
| |
1360 | } |
| |
1361 | if !ok { |
| |
1362 | key := strings.Join(owners, ",") |
| |
1363 | if _, seen := missing[key]; !seen { |
| |
1364 | keys = append(keys, key) |
| |
1365 | } |
| 1323 | missing[key] = append(missing[key], f) |
1366 | missing[key] = append(missing[key], f) |
| 1324 | } |
1367 | } |
| 1325 | } |
1368 | } |
| 1326 | } |
1369 | if len(missing) > 0 { |
| 1327 | if len(missing) > 0 { |
1370 | slices.Sort(keys) |
| 1328 | var parts []string |
1371 | var parts []string |
| 1329 | for owners, paths := range missing { |
1372 | for _, key := range keys { |
| 1330 | parts = append(parts, fmt.Sprintf("%s (owned by %s)", strings.Join(paths, ", "), owners)) |
1373 | paths := missing[key] |
| |
1374 | g.OwnersOutstanding = append(g.OwnersOutstanding, OwnersOut{Files: paths, Owners: strings.Split(key, ",")}) |
| |
1375 | if len(paths) > 3 { |
| |
1376 | paths = paths[:3] |
| |
1377 | } |
| |
1378 | parts = append(parts, fmt.Sprintf("%s (owned by %s)", strings.Join(paths, ", "), key)) |
| |
1379 | } |
| |
1380 | g.Unmet = append(g.Unmet, "CODEOWNERS approval missing for: "+strings.Join(parts, "; ")) |
| 1331 | } |
1381 | } |
| 1332 | slices.Sort(parts) |
| |
| 1333 | return c.fail(protocol.ExitDenied, |
| |
| 1334 | "CODEOWNERS approval missing for: %s", strings.Join(parts, "; ")) |
| |
| 1335 | } |
1382 | } |
| 1336 | } |
1383 | } |
| 1337 | |
1384 | |
| 1338 | if set.RequireResolved { |
1385 | n, err := st.UnresolvedThreadCount(mr.ID) |
| 1339 | n, err := c.Store.UnresolvedThreadCount(mr.ID) |
1386 | if err != nil { |
| 1340 | if err != nil { |
1387 | return g, err |
| 1341 | return c.fail(protocol.ExitFailure, "%v", err) |
| |
| 1342 | } |
| |
| 1343 | if n > 0 { |
| |
| 1344 | return c.fail(protocol.ExitDenied, |
| |
| 1345 | "%s requires review threads resolved; !%d has %d open (mr threads %s %d)", repo.Path(), mr.Number, n, repo.Path(), mr.Number) |
| |
| 1346 | } |
| |
| 1347 | } |
1388 | } |
| 1348 | return -1 |
1389 | g.OpenThreads = n |
| |
1390 | if set.RequireResolved && n > 0 { |
| |
1391 | g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires review threads resolved; !%d has %d open (mr threads %s %d)", repo.Path(), mr.Number, n, repo.Path(), mr.Number)) |
| |
1392 | } |
| |
1393 | |
| |
1394 | if ff, err := gitutil.IsAncestor(dir, targetSHA, headSHA); err == nil { |
| |
1395 | g.FastForward = ff |
| |
1396 | } |
| |
1397 | return g, nil |
| 1349 | } |
1398 | } |
| 1350 | |
1399 | |
| 1351 | func runMRDraft(c *Ctx, args []string) int { return setMRDraft(c, args, true) } |
1400 | func runMRDraft(c *Ctx, args []string) int { return setMRDraft(c, args, true) } |