Commit 390e0c9531
390e0c95310e48a01cd45c2410ebf1713f102177
parent: b347d6c8c4
Verified · cmc ci/build: success ci/test: success
cmc <hello@cleberg.net> · 2026-09-11 19:40 UTC
control: repo transfer moves the directory before the record
A transfer into an org folds the repository's same-named labels and
milestones into the org's rows inside the owner-update transaction.
The directory used to move afterwards, and a move that failed
transferred ownership back without unfolding, leaving the folded rows
with the org. The directory now moves first, so a failed move changes
nothing; a record that then fails moves the directory back.
Closes #212
internal/control/repo.go
+12 −10
| @@ -426,22 +426,24 @@ func runRepoTransfer(c *Ctx, args []string) int { |
| 426 | 426 | if _, err := os.Stat(newDir); err == nil { |
| 427 | 427 | return c.fail(protocol.ExitFailure, "repository directory already exists at %s/%s", newOwner, repo.Name) |
| 428 | 428 | } |
| 429 | | if err := c.Store.TransferRepo(repo.ID, newKind, newID); err != nil { |
| 430 | | return c.failErr(err) |
| 431 | | } |
| 429 | // The directory moves before the record changes: a move that fails |
| 430 | // leaves nothing to undo, whereas the record's change into an org |
| 431 | // folds labels and milestones into the org's rows, which a revert |
| 432 | // cannot unfold (#212). A record that then fails moves the directory |
| 433 | // back, and says so if even that fails, since the operator then has |
| 434 | // a row pointing at a directory that is not there. |
| 432 | 435 | if err := os.MkdirAll(filepath.Dir(newDir), 0o750); err != nil { |
| 433 | | c.Store.TransferRepo(repo.ID, repo.OwnerKind, repo.OwnerID) |
| 434 | 436 | return c.fail(protocol.ExitFailure, "%v", err) |
| 435 | 437 | } |
| 436 | 438 | if err := os.Rename(oldDir, newDir); err != nil { |
| 437 | | // Keep name and disk consistent: revert the database change, and |
| 438 | | // say so if even that fails, since the operator then has a row |
| 439 | | // pointing at a directory that is not there. |
| 440 | | if rerr := c.Store.TransferRepo(repo.ID, repo.OwnerKind, repo.OwnerID); rerr != nil { |
| 441 | | return c.fail(protocol.ExitFailure, "moving repository: %v; and reverting the record failed: %v (the record now names %s but the directory is still %s)", err, rerr, newOwner+"/"+repo.Name, repo.Path()) |
| 442 | | } |
| 443 | 439 | return c.fail(protocol.ExitFailure, "moving repository: %v", err) |
| 444 | 440 | } |
| 441 | if err := c.Store.TransferRepo(repo.ID, newKind, newID); err != nil { |
| 442 | if rerr := os.Rename(newDir, oldDir); rerr != nil { |
| 443 | return c.fail(protocol.ExitFailure, "%v; and moving the directory back failed: %v (the record still names %s but the directory is now %s)", err, rerr, repo.Path(), newOwner+"/"+repo.Name) |
| 444 | } |
| 445 | return c.failErr(err) |
| 446 | } |
| 445 | 447 | newPath := newOwner + "/" + repo.Name |
| 446 | 448 | return c.emit(map[string]string{"repo": newPath, "was": repo.Path()}, func(w io.Writer) { |
| 447 | 449 | fmt.Fprintf(w, "transferred %s to %s — clone URLs now use %s\n", repo.Path(), newPath, newPath) |
internal/control/repotransfer_test.go
added
+70
| @@ -0,0 +1,70 @@ |
| 1 | package control |
| 2 | |
| 3 | import ( |
| 4 | "os" |
| 5 | "path/filepath" |
| 6 | "testing" |
| 7 | |
| 8 | "gitbay.org/gitbay/internal/protocol" |
| 9 | ) |
| 10 | |
| 11 | // A transfer into an org folds the repository's same-named labels into |
| 12 | // the org's rows. The directory moves first, so a move that fails leaves |
| 13 | // the record, and the fold, untouched (#212). |
| 14 | func TestRepoTransferMovesDirectoryBeforeRecord(t *testing.T) { |
| 15 | f := newOrgFixture(t) |
| 16 | root := t.TempDir() |
| 17 | if err := f.st.SetLabel(f.app, "bug", "#00ff00"); err != nil { |
| 18 | t.Fatal(err) |
| 19 | } |
| 20 | if _, err := f.st.SetOrgLabel(f.org, "bug", "#ff0000"); err != nil { |
| 21 | t.Fatal(err) |
| 22 | } |
| 23 | oldDir := RepoDir(root, "alice", "app") |
| 24 | newDir := RepoDir(root, "acme", "app") |
| 25 | if err := os.MkdirAll(oldDir, 0o750); err != nil { |
| 26 | t.Fatal(err) |
| 27 | } |
| 28 | // A file where the org's directory must go makes the move fail. |
| 29 | if err := os.WriteFile(filepath.Dir(newDir), nil, 0o600); err != nil { |
| 30 | t.Fatal(err) |
| 31 | } |
| 32 | repoLabels := func() int { |
| 33 | var n int |
| 34 | f.st.DB.QueryRow("SELECT COUNT(*) FROM labels WHERE repo_id = ?", f.app.ID).Scan(&n) |
| 35 | return n |
| 36 | } |
| 37 | run := func() (int, string) { |
| 38 | c, out := f.ctx(f.alice) |
| 39 | c.Cfg.Server.Root = root |
| 40 | return runRepoTransfer(c, []string{"alice/app", "acme"}), out.String() |
| 41 | } |
| 42 | if code, out := run(); code != protocol.ExitFailure { |
| 43 | t.Fatalf("failed move: exit %d %s", code, out) |
| 44 | } |
| 45 | if r, _ := f.st.RepoByID(f.app.ID); r.OwnerKind != "user" || r.OwnerID != f.alice { |
| 46 | t.Fatalf("owner changed after a failed move: %s/%s", r.OwnerKind, r.OwnerName) |
| 47 | } |
| 48 | if n := repoLabels(); n != 1 { |
| 49 | t.Fatalf("labels folded after a failed move: %d repo rows", n) |
| 50 | } |
| 51 | if _, err := os.Stat(oldDir); err != nil { |
| 52 | t.Fatalf("directory moved after a failed move: %v", err) |
| 53 | } |
| 54 | |
| 55 | if err := os.Remove(filepath.Dir(newDir)); err != nil { |
| 56 | t.Fatal(err) |
| 57 | } |
| 58 | if code, out := run(); code != protocol.ExitOK { |
| 59 | t.Fatalf("transfer: exit %d %s", code, out) |
| 60 | } |
| 61 | if r, _ := f.st.RepoByID(f.app.ID); r.OwnerKind != "org" || r.OwnerID != f.org { |
| 62 | t.Fatalf("owner after transfer: %s/%s", r.OwnerKind, r.OwnerName) |
| 63 | } |
| 64 | if n := repoLabels(); n != 0 { |
| 65 | t.Fatalf("labels not folded after transfer: %d repo rows", n) |
| 66 | } |
| 67 | if _, err := os.Stat(newDir); err != nil { |
| 68 | t.Fatalf("directory not moved: %v", err) |
| 69 | } |
| 70 | } |