Commit e917696f47
Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success
Layout: unified · split
.gitbay/wiki/Users.org +6 −1
| @@ -341,7 +341,12 @@ Semantics worth knowing: | |||
| 341 | *is* one (original commits and signatures land untouched). | 341 | *is* one (original commits and signatures land untouched). |
| 342 | - on =require_signed_commits= branches only fast-forwards of fully | 342 | - on =require_signed_commits= branches only fast-forwards of fully |
| 343 | verified commits merge; everything server-created is refused with | 343 | verified commits merge; everything server-created is refused with |
| 344 | instructions to rebase locally. | 344 | instructions to rebase locally. =gitbay mr rebase <n>= is those |
| 345 | instructions: in a clone of the target it replays the source branch | ||
| 346 | onto the target and force-pushes it, then =mr merge <n>= fast-forwards. | ||
| 347 | The replay is local, so the commits carry your signature and not the | ||
| 348 | server's — it holds no key. A branch living in a fork is refused, with | ||
| 349 | the repository to run it in; rebase it there. | ||
| 345 | 350 | ||
| 346 | Repo admins can gate merges (=repo settings ...=): =require-approvals | 351 | Repo admins can gate merges (=repo settings ...=): =require-approvals |
| 347 | <n>= (fresh, non-author approvals; each reviewer's latest review is | 352 | <n>= (fresh, non-author approvals; each reviewer's latest review is |
cmd/gitbay/local.go +96
| @@ -1,6 +1,7 @@ | |||
| 1 | package main | 1 | package main |
| 2 | 2 | ||
| 3 | import ( | 3 | import ( |
| 4 | "encoding/json" | ||
| 4 | "fmt" | 5 | "fmt" |
| 5 | "os" | 6 | "os" |
| 6 | "os/exec" | 7 | "os/exec" |
| @@ -317,3 +318,98 @@ func hasFlag(args []string, flag string) bool { | |||
| 317 | } | 318 | } |
| 318 | return false | 319 | return false |
| 319 | } | 320 | } |
| 321 | |||
| 322 | // cmdMRRebase implements `gitbay mr rebase <n>`: replay the merge | ||
| 323 | // request's source branch onto its target and re-push it. | ||
| 324 | // | ||
| 325 | // A repository requiring signed commits accepts only fast-forward merges, | ||
| 326 | // because a squash or merge commit is server-created and unsigned. The | ||
| 327 | // refusal names the manual procedure — rebase locally, re-push, merge | ||
| 328 | // again — and this is that procedure. The git work is local so the | ||
| 329 | // replayed commits are signed by whatever key the user's own git config | ||
| 330 | // signs with; the server is never asked to vouch for a commit it did not | ||
| 331 | // receive already signed (#175). | ||
| 332 | func cmdMRRebase(args []string) int { | ||
| 333 | if len(args) != 1 { | ||
| 334 | fmt.Fprintln(os.Stderr, "usage: gitbay mr rebase <n>") | ||
| 335 | return protocol.ExitUsage | ||
| 336 | } | ||
| 337 | n := args[0] | ||
| 338 | // Cheapest check first: a dirty tree stops the rebase anyway, and | ||
| 339 | // saying so costs no round trip. | ||
| 340 | if dirty, err := worktreeDirty(); err != nil { | ||
| 341 | fmt.Fprintln(os.Stderr, "gitbay:", err) | ||
| 342 | return protocol.ExitFailure | ||
| 343 | } else if dirty { | ||
| 344 | fmt.Fprintln(os.Stderr, "gitbay: working tree has uncommitted changes; commit or stash them first") | ||
| 345 | return protocol.ExitFailure | ||
| 346 | } | ||
| 347 | t, err := resolveTarget() | ||
| 348 | if err != nil { | ||
| 349 | fmt.Fprintln(os.Stderr, "gitbay:", err) | ||
| 350 | return protocol.ExitFailure | ||
| 351 | } | ||
| 352 | if t.repo == "" { | ||
| 353 | fmt.Fprintln(os.Stderr, "gitbay: run this in a clone of the repository the merge request targets") | ||
| 354 | return protocol.ExitUsage | ||
| 355 | } | ||
| 356 | out, code := captureSSH(t, []string{"mr", "show", t.repo, n, "--json"}) | ||
| 357 | if code != 0 { | ||
| 358 | return code | ||
| 359 | } | ||
| 360 | var env struct { | ||
| 361 | Data struct { | ||
| 362 | Source string `json:"source"` | ||
| 363 | TargetRef string `json:"target_ref"` | ||
| 364 | State string `json:"state"` | ||
| 365 | } `json:"data"` | ||
| 366 | } | ||
| 367 | if err := json.Unmarshal([]byte(out), &env); err != nil { | ||
| 368 | fmt.Fprintln(os.Stderr, "gitbay: reading merge request:", err) | ||
| 369 | return protocol.ExitProtocol | ||
| 370 | } | ||
| 371 | source, target, state := env.Data.Source, env.Data.TargetRef, env.Data.State | ||
| 372 | if state != "open" { | ||
| 373 | fmt.Fprintf(os.Stderr, "gitbay: !%s is %s\n", n, state) | ||
| 374 | return protocol.ExitUsage | ||
| 375 | } | ||
| 376 | // A fork's branch lives in a repository this clone does not push to, | ||
| 377 | // and guessing which remote that is would be worse than saying so. | ||
| 378 | if strings.Contains(source, ":") { | ||
| 379 | fmt.Fprintf(os.Stderr, | ||
| 380 | "gitbay: !%s comes from %s; rebase it in a clone of that repository and push there\n", n, source) | ||
| 381 | return protocol.ExitUsage | ||
| 382 | } | ||
| 383 | // git talks to origin here, so it needs the instance's ssh options the | ||
| 384 | // same way `repo clone` does — without them a configured key or port | ||
| 385 | // is used by the CLI and not by the fetch and push it runs. | ||
| 386 | if len(t.inst.SSHOptions) > 0 { | ||
| 387 | os.Setenv("GIT_SSH_COMMAND", "ssh "+strings.Join(quoteAll(t.inst.SSHOptions), " ")) | ||
| 388 | } | ||
| 389 | if code := runGitLocal("fetch", "origin"); code != 0 { | ||
| 390 | return code | ||
| 391 | } | ||
| 392 | // git rebase checks the branch out itself, so a conflict leaves the | ||
| 393 | // rebase in progress on the right branch for the person to finish. | ||
| 394 | if code := runGitLocal("rebase", "origin/"+target, source); code != 0 { | ||
| 395 | fmt.Fprintf(os.Stderr, | ||
| 396 | "gitbay: rebase stopped; resolve it, then: git push --force-with-lease origin %s\n", source) | ||
| 397 | return code | ||
| 398 | } | ||
| 399 | if code := runGitLocal("push", "--force-with-lease", "origin", source); code != 0 { | ||
| 400 | return code | ||
| 401 | } | ||
| 402 | fmt.Printf("rebased %s onto %s; merge with: gitbay mr merge %s\n", source, target, n) | ||
| 403 | return 0 | ||
| 404 | } | ||
| 405 | |||
| 406 | // worktreeDirty reports whether the working tree has changes a rebase | ||
| 407 | // would refuse to run over. | ||
| 408 | func worktreeDirty() (bool, error) { | ||
| 409 | cmd := exec.Command(toolpath.Look("git"), "status", "--porcelain") | ||
| 410 | out, err := cmd.Output() | ||
| 411 | if err != nil { | ||
| 412 | return false, fmt.Errorf("git status: %w", err) | ||
| 413 | } | ||
| 414 | return strings.TrimSpace(string(out)) != "", nil | ||
| 415 | } | ||
cmd/gitbay/main.go +1
| @@ -533,6 +533,7 @@ func mrCmd() *cobra.Command { | |||
| 533 | pass("show", "show a merge request", passOpts{server: []string{"mr", "show"}, needsRepo: true}), | 533 | pass("show", "show a merge request", passOpts{server: []string{"mr", "show"}, needsRepo: true}), |
| 534 | pass("diff", "show the diff", passOpts{server: []string{"mr", "diff"}, needsRepo: true}), | 534 | pass("diff", "show the diff", passOpts{server: []string{"mr", "diff"}, needsRepo: true}), |
| 535 | local("checkout", "fetch and check out the MR head locally: gitbay mr checkout <n>", cmdMRCheckout), | 535 | local("checkout", "fetch and check out the MR head locally: gitbay mr checkout <n>", cmdMRCheckout), |
| 536 | local("rebase", "replay the MR's branch onto its target and re-push: gitbay mr rebase <n>", cmdMRRebase), | ||
| 536 | pass("comment", "comment on a merge request", passOpts{server: []string{"mr", "comment"}, needsRepo: true, stdinOK: true, editor: "comment"}), | 537 | pass("comment", "comment on a merge request", passOpts{server: []string{"mr", "comment"}, needsRepo: true, stdinOK: true, editor: "comment"}), |
| 537 | pass("diff-comment", "comment on a diff line: --path <f> --line <l> [--old] [--pending] [--reply <id>]", passOpts{server: []string{"mr", "diff-comment"}, needsRepo: true, stdinOK: true, editor: "comment"}), | 538 | pass("diff-comment", "comment on a diff line: --path <f> --line <l> [--old] [--pending] [--reply <id>]", passOpts{server: []string{"mr", "diff-comment"}, needsRepo: true, stdinOK: true, editor: "comment"}), |
| 538 | pass("threads", "review threads on an MR", passOpts{server: []string{"mr", "threads"}, needsRepo: true}), | 539 | pass("threads", "review threads on an MR", passOpts{server: []string{"mr", "threads"}, needsRepo: true}), |
e2e/mrrebase_test.go added +145
| @@ -0,0 +1,145 @@ | |||
| 1 | package e2e | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "fmt" | ||
| 5 | "os" | ||
| 6 | "path/filepath" | ||
| 7 | "strings" | ||
| 8 | "testing" | ||
| 9 | ) | ||
| 10 | |||
| 11 | // `mr rebase` replays a merge request's branch onto its target and | ||
| 12 | // re-pushes it, which is what makes a fast-forward merge possible again | ||
| 13 | // after the target has moved. The git work is local, so the replayed | ||
| 14 | // commits are signed by whatever the user's git config signs with and the | ||
| 15 | // server is never asked to vouch for a commit it did not receive already | ||
| 16 | // signed (#175). | ||
| 17 | func TestCLIMRRebase(t *testing.T) { | ||
| 18 | inst := startInstance(t) | ||
| 19 | aliceKey := inst.newKey(t, "alice") | ||
| 20 | inst.admin(t, "admin", "user", "create", "alice", | ||
| 21 | "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified") | ||
| 22 | |||
| 23 | c := &cli{bin: buildGitbayCLI(t), configDir: t.TempDir(), inst: inst, key: aliceKey} | ||
| 24 | c.must(t, "", "", "remote", "add", "test", "127.0.0.1", | ||
| 25 | "--port", fmt.Sprint(inst.port), | ||
| 26 | "--ssh-option", "-i", "--ssh-option", aliceKey, | ||
| 27 | "--ssh-option", "-oIdentitiesOnly=yes", | ||
| 28 | "--ssh-option", "-oStrictHostKeyChecking=no", | ||
| 29 | "--ssh-option", "-oUserKnownHostsFile="+filepath.Join(inst.sshDir, "kh"), | ||
| 30 | "--ssh-option", "-oBatchMode=yes", | ||
| 31 | "--default") | ||
| 32 | c.must(t, "", "", "repo", "create", "alice/app") | ||
| 33 | |||
| 34 | env := inst.gitEnv(aliceKey) | ||
| 35 | work := t.TempDir() | ||
| 36 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 37 | dir := filepath.Join(work, "w") | ||
| 38 | os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644) | ||
| 39 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 40 | mustGit(t, dir, env, "add", ".") | ||
| 41 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 42 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 43 | |||
| 44 | // A branch off main, and a merge request for it. | ||
| 45 | mustGit(t, dir, env, "checkout", "-q", "-b", "feat") | ||
| 46 | os.WriteFile(filepath.Join(dir, "b.txt"), []byte("b\n"), 0o644) | ||
| 47 | mustGit(t, dir, env, "add", ".") | ||
| 48 | mustGit(t, dir, env, "commit", "-q", "-m", "the change") | ||
| 49 | mustGit(t, dir, env, "push", "-q", "origin", "feat") | ||
| 50 | c.must(t, dir, "", "mr", "create", "alice/app", | ||
| 51 | "--source", "feat", "--target", "main", "--title", "change") | ||
| 52 | |||
| 53 | // main moves on, so feat is no longer a fast-forward. | ||
| 54 | mustGit(t, dir, env, "checkout", "-q", "main") | ||
| 55 | os.WriteFile(filepath.Join(dir, "c.txt"), []byte("c\n"), 0o644) | ||
| 56 | mustGit(t, dir, env, "add", ".") | ||
| 57 | mustGit(t, dir, env, "commit", "-q", "-m", "moved on") | ||
| 58 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 59 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", "--strategy", "ff"); code == 0 { | ||
| 60 | t.Fatal("fast-forward merged a diverged branch") | ||
| 61 | } else if !strings.Contains(errOut, "fast-forward not possible") { | ||
| 62 | t.Fatalf("unexpected refusal: %s", errOut) | ||
| 63 | } | ||
| 64 | |||
| 65 | // Rebase, and the same merge now lands. | ||
| 66 | out, errOut, code := c.run(t, dir, "", "mr", "rebase", "1") | ||
| 67 | if code != 0 { | ||
| 68 | t.Fatalf("mr rebase: exit %d\n%s\n%s", code, out, errOut) | ||
| 69 | } | ||
| 70 | if !strings.Contains(out, "gitbay mr merge 1") { | ||
| 71 | t.Errorf("rebase does not name the next step: %s", out) | ||
| 72 | } | ||
| 73 | if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", "--strategy", "ff"); code != 0 { | ||
| 74 | t.Fatalf("fast-forward still refused after a rebase: %s", errOut) | ||
| 75 | } | ||
| 76 | |||
| 77 | // The rebase moved the branch rather than merging main into it: the | ||
| 78 | // change is one commit on top of what main had. | ||
| 79 | out, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/app", "1", "--json") | ||
| 80 | if !strings.Contains(out, `"state":"merged"`) { | ||
| 81 | t.Fatalf("merge request not merged:\n%s", out) | ||
| 82 | } | ||
| 83 | } | ||
| 84 | |||
| 85 | // The guards: a dirty tree, and a source in a fork this clone cannot push. | ||
| 86 | func TestCLIMRRebaseRefusals(t *testing.T) { | ||
| 87 | inst := startInstance(t) | ||
| 88 | aliceKey := inst.newKey(t, "alice") | ||
| 89 | bobKey := inst.newKey(t, "bob") | ||
| 90 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | ||
| 91 | inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub") | ||
| 92 | |||
| 93 | c := &cli{bin: buildGitbayCLI(t), configDir: t.TempDir(), inst: inst, key: aliceKey} | ||
| 94 | c.must(t, "", "", "remote", "add", "test", "127.0.0.1", | ||
| 95 | "--port", fmt.Sprint(inst.port), | ||
| 96 | "--ssh-option", "-i", "--ssh-option", aliceKey, | ||
| 97 | "--ssh-option", "-oIdentitiesOnly=yes", | ||
| 98 | "--ssh-option", "-oStrictHostKeyChecking=no", | ||
| 99 | "--ssh-option", "-oUserKnownHostsFile="+filepath.Join(inst.sshDir, "kh"), | ||
| 100 | "--ssh-option", "-oBatchMode=yes", | ||
| 101 | "--default") | ||
| 102 | c.must(t, "", "", "repo", "create", "alice/app") | ||
| 103 | |||
| 104 | env := inst.gitEnv(aliceKey) | ||
| 105 | work := t.TempDir() | ||
| 106 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 107 | dir := filepath.Join(work, "w") | ||
| 108 | os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a\n"), 0o644) | ||
| 109 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 110 | mustGit(t, dir, env, "add", ".") | ||
| 111 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 112 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 113 | |||
| 114 | // bob forks, pushes, and opens a merge request from the fork. | ||
| 115 | inst.ssh(t, bobKey, "", "repo", "fork", "alice/app") | ||
| 116 | benv := inst.gitEnv(bobKey) | ||
| 117 | bwork := t.TempDir() | ||
| 118 | mustGit(t, bwork, benv, "clone", inst.sshURL("bob/app"), "w") | ||
| 119 | bdir := filepath.Join(bwork, "w") | ||
| 120 | mustGit(t, bdir, benv, "checkout", "-q", "-b", "feat") | ||
| 121 | os.WriteFile(filepath.Join(bdir, "b.txt"), []byte("b\n"), 0o644) | ||
| 122 | mustGit(t, bdir, benv, "add", ".") | ||
| 123 | mustGit(t, bdir, benv, "commit", "-q", "-m", "from the fork") | ||
| 124 | mustGit(t, bdir, benv, "push", "-q", "origin", "feat") | ||
| 125 | inst.ssh(t, bobKey, "", "mr", "create", "alice/app", | ||
| 126 | "--source", "bob/app:feat", "--target", "main", "--title", "forked") | ||
| 127 | |||
| 128 | // Alice's clone of the target cannot rebase a branch that lives in | ||
| 129 | // bob's fork, and says which repository to do it in. | ||
| 130 | _, errOut, code := c.run(t, dir, "", "mr", "rebase", "1") | ||
| 131 | if code == 0 { | ||
| 132 | t.Fatal("rebased a fork's branch from the target's clone") | ||
| 133 | } | ||
| 134 | if !strings.Contains(errOut, "bob/app:feat") { | ||
| 135 | t.Errorf("refusal does not name the fork: %s", errOut) | ||
| 136 | } | ||
| 137 | |||
| 138 | // A dirty tree is refused before any round trip, so the merge request | ||
| 139 | // number never has to be valid for this one. | ||
| 140 | os.WriteFile(filepath.Join(dir, "a.txt"), []byte("edited\n"), 0o644) | ||
| 141 | if _, errOut, code := c.run(t, dir, "", "mr", "rebase", "1"); code == 0 || | ||
| 142 | !strings.Contains(errOut, "uncommitted changes") { | ||
| 143 | t.Errorf("dirty tree not refused: exit %d %s", code, errOut) | ||
| 144 | } | ||
| 145 | } | ||