Skip to content

Commit ba0eb9d

Browse files
skarimskarim
committed
Preserve legacy rebase recovery until completion
1 parent 9931fd7 commit ba0eb9d

4 files changed

Lines changed: 416 additions & 67 deletions

File tree

‎cmd/rebase.go‎

Lines changed: 111 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"path/filepath"
99
"strings"
1010

11+
cligit "github.com/cli/cli/v2/git"
1112
"github.com/github/gh-stack/internal/config"
1213
"github.com/github/gh-stack/internal/git"
1314
"github.com/github/gh-stack/internal/modify"
@@ -350,6 +351,9 @@ func continueRebase(cfg *config.Config, gitDir string) error {
350351
if state.Worktrees != nil {
351352
return continueWorktreeRebase(cfg, gitDir, state)
352353
}
354+
if err := validateLegacyRebasePhase(state, true); err != nil {
355+
return err
356+
}
353357

354358
sf, err := stack.Load(gitDir)
355359
if err != nil {
@@ -366,6 +370,9 @@ func continueRebase(cfg *config.Config, gitDir string) error {
366370
if s == nil {
367371
return fmt.Errorf("no stack found for branch %s", state.OriginalBranch)
368372
}
373+
if state.Phase == "complete" {
374+
return finishLegacyRebase(cfg, gitDir, state, sf, s)
375+
}
369376
trunkRef := state.TrunkRef
370377
if trunkRef == "" {
371378
trunkRef = s.Trunk.Branch
@@ -454,10 +461,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {
454461

455462
if result.Err != nil {
456463
cfg.Errorf("%v", result.Err)
457-
if err := restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs); err != nil {
458-
return err
459-
}
460-
if err := clearRebaseState(gitDir); err != nil {
464+
if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil {
461465
return err
462466
}
463467
return ErrSilent
@@ -466,13 +470,15 @@ func continueRebase(cfg *config.Config, gitDir string) error {
466470
if result.Conflicted {
467471
cfg.Warningf("Rebasing %s onto %s — conflict", result.ConflictBranch, result.ConflictBase)
468472

473+
state.Phase = "conflict"
469474
state.CurrentBranchIndex = result.ConflictIdx
470475
state.ConflictBranch = result.ConflictBranch
471476
state.RemainingBranches = result.Remaining
472477
state.UseOnto = result.NeedsOnto
473478
state.OntoOldBase = result.OntoOldBase
474479
if err := saveRebaseState(gitDir, state); err != nil {
475-
cfg.Warningf("failed to save rebase state: %s", err)
480+
cfg.Errorf("failed to save rebase state: %s", err)
481+
return errors.Join(ErrSilent, err)
476482
}
477483

478484
printConflictDetails(cfg, result.ConflictBase)
@@ -485,8 +491,6 @@ func continueRebase(cfg *config.Config, gitDir string) error {
485491
}
486492
}
487493

488-
_ = git.CheckoutBranch(state.OriginalBranch)
489-
490494
verifyStart, verifyEnd := state.StartIndex, state.EndIndex
491495
if verifyEnd <= verifyStart {
492496
verifyStart, verifyEnd = 0, len(s.Branches)
@@ -496,38 +500,13 @@ func continueRebase(cfg *config.Config, gitDir string) error {
496500
}
497501
if unstacked := verifyStacked(s, trunkBase, verifyStart, verifyEnd); len(unstacked) > 0 {
498502
reportUnstacked(cfg, trunkRef, unstacked)
499-
if err := restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs); err != nil {
500-
return err
501-
}
502-
if err := clearRebaseState(gitDir); err != nil {
503+
if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil {
503504
return err
504505
}
505506
return ErrSilent
506507
}
507508

508-
updateBaseSHAsWithTrunk(s, state.TrunkSHA)
509-
510-
_ = syncStackPRs(cfg, s)
511-
512-
if err := stack.Save(gitDir, sf); err != nil {
513-
return handleSaveError(cfg, err)
514-
}
515-
if err := clearRebaseState(gitDir); err != nil {
516-
cfg.Errorf("rebase completed but recovery state could not be cleared: %v", err)
517-
return ErrSilent
518-
}
519-
520-
if state.NoTrunk {
521-
cfg.Printf("All branches in stack rebased locally (without trunk)")
522-
} else if state.TrunkSHA != "" {
523-
cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA))
524-
} else {
525-
cfg.Printf("All branches in stack rebased locally with %s", trunkRef)
526-
}
527-
cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`",
528-
cfg.ColorCyan("gh stack submit"))
529-
530-
return nil
509+
return finishLegacyRebase(cfg, gitDir, state, sf, s)
531510
}
532511

533512
func abortRebase(cfg *config.Config, gitDir string) error {
@@ -547,55 +526,130 @@ func abortRebase(cfg *config.Config, gitDir string) error {
547526
cfg.Successf("Rebase aborted and branches restored")
548527
return nil
549528
}
529+
if err := validateLegacyRebasePhase(state, false); err != nil {
530+
return err
531+
}
532+
if err := rollbackLegacyRebase(cfg, gitDir, state); err != nil {
533+
return err
534+
}
535+
cfg.Successf("Rebase aborted and branches restored")
536+
return nil
537+
}
550538

539+
func validateLegacyRebasePhase(state *rebaseState, cont bool) error {
540+
switch state.Phase {
541+
case "", "conflict", "complete":
542+
return nil
543+
case "applying", "restoring":
544+
if !cont {
545+
return nil
546+
}
547+
return fmt.Errorf("legacy rebase is in phase %q; run `gh stack rebase --abort` to restore the stack", state.Phase)
548+
default:
549+
return fmt.Errorf("unknown legacy rebase phase %q; recovery state was retained", state.Phase)
550+
}
551+
}
552+
553+
func rollbackLegacyRebase(cfg *config.Config, gitDir string, state *rebaseState) error {
551554
inProgress, err := git.IsRebaseInProgress()
552555
if err != nil {
553556
return fmt.Errorf("checking rebase state: %w", err)
554557
}
555558
for branch := range state.OriginalRefs {
556-
if _, err := git.BranchExists(branch); err != nil {
559+
exists, err := git.BranchExists(branch)
560+
if err != nil {
557561
return fmt.Errorf("checking branch %s before restoring: %w", branch, err)
558562
}
563+
if exists {
564+
if _, err := git.RevParse(branch); err != nil {
565+
return fmt.Errorf("reading branch %s before restoring: %w", branch, err)
566+
}
567+
}
568+
}
569+
root, err := git.RootDir()
570+
if err != nil {
571+
return fmt.Errorf("finding recovery worktree: %w", err)
572+
}
573+
if !inProgress || state.Phase == "complete" {
574+
if err := worktree.CheckClean(git.CurrentOps(), root); err != nil {
575+
return err
576+
}
577+
}
578+
state.Phase = "restoring"
579+
if err := saveRebaseState(gitDir, state); err != nil {
580+
return err
559581
}
560582
if inProgress {
561583
if err := git.RebaseAbort(); err != nil {
562584
cfg.Errorf("aborting rebase; recovery state was retained: %s", err)
563585
return errors.Join(ErrSilent, err)
564586
}
565587
}
566-
root, err := git.RootDir()
567-
if err != nil {
568-
cfg.Errorf("finding recovery worktree: %s", err)
569-
return ErrSilent
570-
}
571588
if err := worktree.CheckClean(git.CurrentOps(), root); err != nil {
572589
cfg.Errorf("recovery state was retained: %s", err)
573590
return errors.Join(ErrSilent, err)
574591
}
575592

576-
var restoreErrors []string
577-
for branch, sha := range state.OriginalRefs {
578-
if err := git.CheckoutBranch(branch); err != nil {
579-
restoreErrors = append(restoreErrors, fmt.Sprintf("checkout %s: %s", branch, err))
580-
continue
581-
}
582-
if err := git.ResetHard(sha); err != nil {
583-
restoreErrors = append(restoreErrors, fmt.Sprintf("reset %s: %s", branch, err))
584-
}
593+
if err := restoreRebaseRefs(cfg, state.OriginalBranch, state.OriginalRefs); err != nil {
594+
return err
585595
}
596+
return clearRebaseState(gitDir)
597+
}
586598

587-
_ = git.CheckoutBranch(state.OriginalBranch)
588-
clearRebaseState(gitDir)
599+
func restoreRebaseCheckout(branch string) error {
600+
current, err := git.CurrentBranch()
601+
if err != nil && !errors.Is(err, cligit.ErrNotOnAnyBranch) {
602+
return fmt.Errorf("reading original checkout: %w", err)
603+
}
604+
if err == nil && current == branch {
605+
return nil
606+
}
607+
root, err := git.RootDir()
608+
if err != nil {
609+
return err
610+
}
611+
if err := worktree.CheckClean(git.CurrentOps(), root); err != nil {
612+
return err
613+
}
614+
if err := git.CheckoutBranch(branch); err != nil {
615+
return fmt.Errorf("restoring original checkout %s: %w", branch, err)
616+
}
617+
return nil
618+
}
589619

590-
if len(restoreErrors) > 0 {
591-
cfg.Warningf("Rebase aborted but some branches could not be fully restored:")
592-
for _, e := range restoreErrors {
593-
cfg.Printf(" %s", e)
620+
func finishLegacyRebase(cfg *config.Config, gitDir string, state *rebaseState, sf *stack.StackFile, s *stack.Stack) error {
621+
if state.Phase != "complete" {
622+
state.Phase = "complete"
623+
state.RemainingBranches = nil
624+
if err := saveRebaseState(gitDir, state); err != nil {
625+
return err
594626
}
595-
return ErrSilent
627+
}
628+
if err := restoreRebaseCheckout(state.OriginalBranch); err != nil {
629+
return err
630+
}
631+
updateBaseSHAsWithTrunk(s, state.TrunkSHA)
632+
_ = syncStackPRs(cfg, s)
633+
if err := stack.Save(gitDir, sf); err != nil {
634+
return handleSaveError(cfg, err)
635+
}
636+
if err := clearRebaseState(gitDir); err != nil {
637+
return fmt.Errorf("rebase completed but recovery state could not be cleared: %w", err)
596638
}
597639

598-
cfg.Successf("Rebase aborted and branches restored")
640+
trunkRef := state.TrunkRef
641+
if trunkRef == "" {
642+
trunkRef = s.Trunk.Branch
643+
}
644+
if state.NoTrunk {
645+
cfg.Printf("All branches in stack rebased locally (without trunk)")
646+
} else if state.TrunkSHA != "" {
647+
cfg.Printf("All branches in stack rebased locally with %s (%s)", trunkRef, short(state.TrunkSHA))
648+
} else {
649+
cfg.Printf("All branches in stack rebased locally with %s", trunkRef)
650+
}
651+
cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`",
652+
cfg.ColorCyan("gh stack submit"))
599653
return nil
600654
}
601655

0 commit comments

Comments
 (0)