Merge pull request #8294 from dolthub/fulghum/rebase

Feature: Data conflict resolution during interactive rebase

go/cmd/dolt/commands/rebase.go

@@ -31,6 +31,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/rebase"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dprocedures"
+	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
 	"github.com/dolthub/dolt/go/libraries/utils/config"
 	"github.com/dolthub/dolt/go/libraries/utils/editor"
@@ -95,6 +96,12 @@ func (cmd RebaseCmd) Exec(ctx context.Context, commandStr string, args []string,
 		defer closeFunc()
 	}
 
+	// Set @@dolt_allow_commit_conflicts in case there are data conflicts that need to be resolved by the caller.
+	// Without this, the conflicts can't be committed to the branch working set, and the caller can't access them.
+	if _, err = GetRowsForSql(queryist, sqlCtx, "set @@dolt_allow_commit_conflicts=1;"); err != nil {
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	}
+
 	branchName, err := getActiveBranchName(sqlCtx, queryist)
 	if err != nil {
 		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
@@ -118,65 +125,84 @@ func (cmd RebaseCmd) Exec(ctx context.Context, commandStr string, args []string,
 		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(errors.New("error: "+rows[0][1].(string))), usage)
 	}
 
+	// If the rebase was successful, or if it was aborted, print out the message and
+	// ensure the branch being rebased is checked out in the CLI
 	message := rows[0][1].(string)
-	if strings.Contains(message, dprocedures.SuccessfulRebaseMessage) {
-		cli.Println(dprocedures.SuccessfulRebaseMessage + branchName)
-	} else if strings.Contains(message, dprocedures.RebaseAbortedMessage) {
-		cli.Println(dprocedures.RebaseAbortedMessage)
-	} else {
-		rebasePlan, err := getRebasePlan(cliCtx, sqlCtx, queryist, apr.Arg(0), branchName)
+	if strings.Contains(message, dprocedures.SuccessfulRebaseMessage) ||
+		strings.Contains(message, dprocedures.RebaseAbortedMessage) {
+		cli.Println(message)
+		if err = syncCliBranchToSqlSessionBranch(sqlCtx, dEnv); err != nil {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+		}
+		return 0
+	}
+
+	rebasePlan, err := getRebasePlan(cliCtx, sqlCtx, queryist, apr.Arg(0), branchName)
+	if err != nil {
+		// attempt to abort the rebase
+		_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	}
+
+	// if all uncommented lines are deleted in the editor, abort the rebase
+	if rebasePlan == nil || rebasePlan.Steps == nil || len(rebasePlan.Steps) == 0 {
+		rows, err := GetRowsForSql(queryist, sqlCtx, "CALL DOLT_REBASE('--abort');")
+		if err != nil {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+		}
+		status, err := getInt64ColAsInt64(rows[0][0])
 		if err != nil {
-			// attempt to abort the rebase
-			_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
 			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
 		}
+		if status == 1 {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(errors.New("error: "+rows[0][1].(string))), usage)
+		}
 
-		// if all uncommented lines are deleted in the editor, abort the rebase
-		if rebasePlan == nil || rebasePlan.Steps == nil || len(rebasePlan.Steps) == 0 {
-			rows, err := GetRowsForSql(queryist, sqlCtx, "CALL DOLT_REBASE('--abort');")
-			if err != nil {
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
-			}
-			status, err := getInt64ColAsInt64(rows[0][0])
-			if err != nil {
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
-			}
-			if status == 1 {
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(errors.New("error: "+rows[0][1].(string))), usage)
-			}
+		cli.Println(dprocedures.RebaseAbortedMessage)
+		return 0
+	}
 
-			cli.Println(dprocedures.RebaseAbortedMessage)
-		} else {
-			err = insertRebasePlanIntoDoltRebaseTable(rebasePlan, sqlCtx, queryist)
-			if err != nil {
-				// attempt to abort the rebase
-				_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
-			}
+	err = insertRebasePlanIntoDoltRebaseTable(rebasePlan, sqlCtx, queryist)
+	if err != nil {
+		// attempt to abort the rebase
+		_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	}
 
-			rows, err := GetRowsForSql(queryist, sqlCtx, "CALL DOLT_REBASE('--continue');")
-			if err != nil {
-				// attempt to abort the rebase
-				_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
-			}
-			status, err := getInt64ColAsInt64(rows[0][0])
-			if err != nil {
-				// attempt to abort the rebase
-				_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	rows, err = GetRowsForSql(queryist, sqlCtx, "CALL DOLT_REBASE('--continue');")
+	if err != nil {
+		// If the error is a data conflict, don't abort the rebase, but let the caller resolve the conflicts
+		if dprocedures.ErrRebaseDataConflict.Is(err) || strings.Contains(err.Error(), dprocedures.ErrRebaseDataConflict.Message[:40]) {
+			if checkoutErr := syncCliBranchToSqlSessionBranch(sqlCtx, dEnv); checkoutErr != nil {
+				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(checkoutErr), usage)
 			}
-			if status == 1 {
-				// attempt to abort the rebase
-				_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
-				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(errors.New("error: "+rows[0][1].(string))), usage)
+		} else {
+			_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
+			if checkoutErr := syncCliBranchToSqlSessionBranch(sqlCtx, dEnv); checkoutErr != nil {
+				return HandleVErrAndExitCode(errhand.VerboseErrorFromError(checkoutErr), usage)
 			}
+		}
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	}
 
-			cli.Println(dprocedures.SuccessfulRebaseMessage + branchName)
+	status, err = getInt64ColAsInt64(rows[0][0])
+	if err != nil {
+		_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
+		if err = syncCliBranchToSqlSessionBranch(sqlCtx, dEnv); err != nil {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
 		}
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+	}
+	if status == 1 {
+		_, _, _, _ = queryist.Query(sqlCtx, "CALL DOLT_REBASE('--abort');")
+		if err = syncCliBranchToSqlSessionBranch(sqlCtx, dEnv); err != nil {
+			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
+		}
+		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(errors.New("error: "+rows[0][1].(string))), usage)
 	}
 
-	return HandleVErrAndExitCode(nil, usage)
+	cli.Println(rows[0][1].(string))
+	return 0
 }
 
 // getRebasePlan opens an editor for users to edit the rebase plan and returns the parsed rebase plan from the editor.
@@ -313,3 +339,17 @@ func insertRebasePlanIntoDoltRebaseTable(plan *rebase.RebasePlan, sqlCtx *sql.Co
 
 	return nil
 }
+
+// syncCliBranchToSqlSessionBranch sets the current branch for the CLI (in repo_state.json) to the active branch
+// for the current session. This is needed during rebasing, since any conflicts need to be resolved while the
+// session is on the rebase working branch (e.g. dolt_rebase_t1) and after the rebase finishes, the session needs
+// to be back on the branch being rebased (e.g. t1).
+func syncCliBranchToSqlSessionBranch(ctx *sql.Context, dEnv *env.DoltEnv) error {
+	doltSession := dsess.DSessFromSess(ctx.Session)
+	currentBranch, err := doltSession.GetBranch()
+	if err != nil {
+		return err
+	}
+
+	return saveHeadBranch(dEnv.FS, currentBranch)
+}

go/libraries/doltcore/cherry_pick/cherry_pick.go

@@ -108,29 +108,15 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str
 		return "", mergeResult, nil
 	}
 
-	commitProps := actions.CommitStagedProps{
-		Date:    ctx.QueryTime(),
-		Name:    ctx.Client().User,
-		Email:   fmt.Sprintf("%s@%s", ctx.Client().User, ctx.Client().Address),
-		Message: commitMsg,
-	}
-
-	if options.CommitMessage != "" {
-		commitProps.Message = options.CommitMessage
-	}
-	if options.Amend {
-		commitProps.Amend = true
-	}
-	if options.EmptyCommitHandling == doltdb.KeepEmptyCommit {
-		commitProps.AllowEmpty = true
+	commitProps, err := CreateCommitStagedPropsFromCherryPickOptions(ctx, options)
+	if err != nil {
+		return "", nil, err
 	}
 
-	if options.CommitBecomesEmptyHandling == doltdb.DropEmptyCommit {
-		commitProps.SkipEmpty = true
-	} else if options.CommitBecomesEmptyHandling == doltdb.KeepEmptyCommit {
-		commitProps.AllowEmpty = true
-	} else if options.CommitBecomesEmptyHandling == doltdb.StopOnEmptyCommit {
-		return "", nil, fmt.Errorf("stop on empty commit is not currently supported")
+	// If no commit message was explicitly provided in the cherry-pick options,
+	// use the commit message from the cherry-picked commit.
+	if commitProps.Message == "" {
+		commitProps.Message = commitMsg
 	}
 
 	// NOTE: roots are old here (after staging the tables) and need to be refreshed
@@ -139,7 +125,7 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str
 		return "", nil, fmt.Errorf("failed to get roots for current session")
 	}
 
-	pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, commitProps)
+	pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, *commitProps)
 	if err != nil {
 		return "", nil, err
 	}
@@ -164,6 +150,36 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str
 	return h.String(), nil, nil
 }
 
+// CreateCommitStagedPropsFromCherryPickOptions converts the specified cherry-pick |options| into a CommitStagedProps
+// instance that can be used to create a pending commit.
+func CreateCommitStagedPropsFromCherryPickOptions(ctx *sql.Context, options CherryPickOptions) (*actions.CommitStagedProps, error) {
+	commitProps := actions.CommitStagedProps{
+		Date:  ctx.QueryTime(),
+		Name:  ctx.Client().User,
+		Email: fmt.Sprintf("%s@%s", ctx.Client().User, ctx.Client().Address),
+	}
+
+	if options.CommitMessage != "" {
+		commitProps.Message = options.CommitMessage
+	}
+	if options.Amend {
+		commitProps.Amend = true
+	}
+	if options.EmptyCommitHandling == doltdb.KeepEmptyCommit {
+		commitProps.AllowEmpty = true
+	}
+
+	if options.CommitBecomesEmptyHandling == doltdb.DropEmptyCommit {
+		commitProps.SkipEmpty = true
+	} else if options.CommitBecomesEmptyHandling == doltdb.KeepEmptyCommit {
+		commitProps.AllowEmpty = true
+	} else if options.CommitBecomesEmptyHandling == doltdb.StopOnEmptyCommit {
+		return nil, fmt.Errorf("stop on empty commit is not currently supported")
+	}
+
+	return &commitProps, nil
+}
+
 func previousCommitMessage(ctx *sql.Context) (string, error) {
 	doltSession := dsess.DSessFromSess(ctx.Session)
 	headCommit, err := doltSession.GetHeadCommit(ctx, ctx.GetCurrentDatabase())

go/libraries/doltcore/doltdb/workingset.go

@@ -66,6 +66,15 @@ type RebaseState struct {
 
 	// emptyCommitHandling specifies how to handle empty commits that contain no changes.
 	emptyCommitHandling EmptyCommitHandling
+
+	// lastAttemptedStep records the last rebase plan step that was attempted, whether it completed successfully, or
+	// resulted in conflicts for the user to manually resolve. This field is not valid unless rebasingStarted is set
+	// to true.
+	lastAttemptedStep float32
+
+	// rebasingStarted is true once the rebase plan has been started to execute. Once rebasingStarted is true, the
+	// value in lastAttemptedStep has been initialized and is valid to read.
+	rebasingStarted bool
 }
 
 // Branch returns the name of the branch being actively rebased. This is the branch that will be updated to point
@@ -93,6 +102,24 @@ func (rs RebaseState) CommitBecomesEmptyHandling() EmptyCommitHandling {
 	return rs.commitBecomesEmptyHandling
 }
 
+func (rs RebaseState) LastAttemptedStep() float32 {
+	return rs.lastAttemptedStep
+}
+
+func (rs RebaseState) WithLastAttemptedStep(step float32) *RebaseState {
+	rs.lastAttemptedStep = step
+	return &rs
+}
+
+func (rs RebaseState) RebasingStarted() bool {
+	return rs.rebasingStarted
+}
+
+func (rs RebaseState) WithRebasingStarted(rebasingStarted bool) *RebaseState {
+	rs.rebasingStarted = rebasingStarted
+	return &rs
+}
+
 type MergeState struct {
 	// the source commit
 	commit *Commit
@@ -517,6 +544,8 @@ func newWorkingSet(ctx context.Context, name string, vrw types.ValueReadWriter,
 			branch:                     dsws.RebaseState.Branch(ctx),
 			commitBecomesEmptyHandling: EmptyCommitHandling(dsws.RebaseState.CommitBecomesEmptyHandling(ctx)),
 			emptyCommitHandling:        EmptyCommitHandling(dsws.RebaseState.EmptyCommitHandling(ctx)),
+			lastAttemptedStep:          dsws.RebaseState.LastAttemptedStep(ctx),
+			rebasingStarted:            dsws.RebaseState.RebasingStarted(ctx),
 		}
 	}
 
@@ -613,7 +642,8 @@ func (ws *WorkingSet) writeValues(ctx context.Context, db *DoltDB, meta *datas.W
 		}
 
 		rebaseState = datas.NewRebaseState(preRebaseWorking.TargetHash(), dCommit.Addr(), ws.rebaseState.branch,
-			uint8(ws.rebaseState.commitBecomesEmptyHandling), uint8(ws.rebaseState.emptyCommitHandling))
+			uint8(ws.rebaseState.commitBecomesEmptyHandling), uint8(ws.rebaseState.emptyCommitHandling),
+			ws.rebaseState.lastAttemptedStep, ws.rebaseState.rebasingStarted)
 	}
 
 	return &datas.WorkingSetSpec{

go/libraries/doltcore/rebase/rebase.go

@@ -62,6 +62,13 @@ type RebasePlanStep struct {
 	CommitMsg   string
 }
 
+// RebaseOrderAsFloat returns the RebaseOrder as a float32. Float32 provides enough scale and precision to hold
+// rebase order values, since they are limited to two decimal points of precision and six total digits.
+func (rps *RebasePlanStep) RebaseOrderAsFloat() float32 {
+	f64, _ := rps.RebaseOrder.Float64()
+	return float32(f64)
+}
+
 // CreateDefaultRebasePlan creates and returns the default rebase plan for the commits between
 // |startCommit| and |upstreamCommit|, equivalent to the log of startCommit..upstreamCommit. The
 // default plan includes each of those commits, in the same order they were originally applied, and