From 1d47e96e776eea7f9f54de6450ff17a7dfed4e65 Mon Sep 17 00:00:00 2001 From: Neil Macneale IV Date: Wed, 18 Feb 2026 23:41:54 +0000 Subject: [PATCH] Implement cherry-pick --continue --- go/cmd/dolt/cli/arg_parser_helpers.go | 1 + go/cmd/dolt/commands/cherry-pick.go | 55 ++++++++++- .../doltcore/cherry_pick/cherry_pick.go | 92 +++++++++++++++++++ .../sqle/dprocedures/dolt_cherry_pick.go | 8 ++ .../enginetest/dolt_queries_cherry_pick.go | 62 ++++++++++--- 5 files changed, 202 insertions(+), 16 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 3f345dba40..362efad127 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -193,6 +193,7 @@ func CreateCheckoutArgParser() *argparser.ArgParser { func CreateCherryPickArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("cherrypick", 1) ap.SupportsFlag(AbortParam, "", "Abort the current conflict resolution process, and revert all changes from the in-process cherry-pick operation.") + ap.SupportsFlag(ContinueFlag, "", "Continue the current cherry-pick operation after conflicts have been resolved.") ap.SupportsFlag(AllowEmptyFlag, "", "Allow empty commits to be cherry-picked. "+ "Note that use of this option only keeps commits that were initially empty. "+ "Commits which become empty, due to a previous commit, will cause cherry-pick to fail.") diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 25dc1d8cdd..8c18a687a6 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -47,7 +47,7 @@ If any data conflicts, schema conflicts, or constraint violations are detected d var ErrCherryPickConflictsOrViolations = errors.NewKind("error: Unable to apply commit cleanly due to conflicts " + "or constraint violations. Please resolve the conflicts and/or constraint violations, then use `dolt add` " + - "to add the tables to the staged set, and `dolt commit` to commit the changes and finish cherry-picking. \n" + + "to add the tables to the staged set, and `dolt cherry-pick --continue` to complete the cherry-pick. \n" + "To undo all changes from this cherry-pick operation, use `dolt cherry-pick --abort`.\n" + "For more information on handling conflicts, see: https://docs.dolthub.com/concepts/dolt/git/conflicts") @@ -97,7 +97,12 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str } if apr.Contains(cli.AbortParam) { - err = cherryPickAbort(queryist.Queryist, queryist.Context) + err = cherryPickAbort(queryist.Context, queryist.Queryist) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) + } + + if apr.Contains(cli.ContinueFlag) { + err = cherryPickContinue(queryist.Context, queryist.Queryist) return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } @@ -117,11 +122,11 @@ func (cmd CherryPickCmd) Exec(ctx context.Context, commandStr string, args []str return HandleVErrAndExitCode(errhand.BuildDError("cherry-picking multiple commits is not supported yet").SetPrintUsage().Build(), usage) } - err = cherryPick(queryist.Queryist, queryist.Context, apr, args) + err = cherryPick(queryist.Context, queryist.Queryist, apr, args) return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } -func cherryPick(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgParseResults, args []string) error { +func cherryPick(sqlCtx *sql.Context, queryist cli.Queryist, apr *argparser.ArgParseResults, args []string) error { cherryStr := apr.Arg(0) if len(cherryStr) == 0 { return fmt.Errorf("error: cannot cherry-pick empty string") @@ -220,7 +225,7 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re } } -func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { +func cherryPickAbort(sqlCtx *sql.Context, queryist cli.Queryist) error { query := "call dolt_cherry_pick('--abort')" _, err := cli.GetRowsForSql(queryist, sqlCtx, query) if err != nil { @@ -235,6 +240,46 @@ func cherryPickAbort(queryist cli.Queryist, sqlCtx *sql.Context) error { return nil } +func cherryPickContinue(sqlCtx *sql.Context, queryist cli.Queryist) error { + query := "call dolt_cherry_pick('--continue')" + rows, err := cli.GetRowsForSql(queryist, sqlCtx, query) + if err != nil { + return err + } + + if len(rows) != 1 { + return fmt.Errorf("error: unexpected number of rows returned from dolt_cherry_pick: %d", len(rows)) + } + + // Get the commit hash from the result + commitHash := "" + for _, row := range rows { + var ok bool + commitHash, ok, err = sql.Unwrap[string](sqlCtx, row[0]) + if err != nil { + return fmt.Errorf("Unable to parse commitHash column: %w", err) + } + if !ok { + return fmt.Errorf("Unexpected type for commitHash column, expected string, found %T", row[0]) + } + } + + // Print the commit info on successful continue + commit, err := getCommitInfo(queryist, sqlCtx, commitHash) + if commit == nil || err != nil { + return fmt.Errorf("error: failed to get commit metadata for ref '%s': %v", commitHash, err) + } + + cli.ExecuteWithStdioRestored(func() { + pager := outputpager.Start() + defer pager.Stop() + + PrintCommitInfo(pager, 0, false, false, "auto", commit) + }) + + return nil +} + func hasStagedAndUnstagedChanged(queryist cli.Queryist, sqlCtx *sql.Context) (hasStagedChanges bool, hasUnstagedChanges bool, err error) { stagedTables, unstagedTables, err := GetDoltStatus(queryist, sqlCtx) if err != nil { diff --git a/go/libraries/doltcore/cherry_pick/cherry_pick.go b/go/libraries/doltcore/cherry_pick/cherry_pick.go index e7fd3e6405..f706c14c69 100644 --- a/go/libraries/doltcore/cherry_pick/cherry_pick.go +++ b/go/libraries/doltcore/cherry_pick/cherry_pick.go @@ -231,6 +231,98 @@ func AbortCherryPick(ctx *sql.Context, dbName string) error { return doltSession.SetWorkingSet(ctx, dbName, newWs) } +// ContinueCherryPick continues a cherry-pick merge that was paused due to conflicts. +// It checks that conflicts have been resolved and creates the final commit with the +// original commit's metadata. +func ContinueCherryPick(ctx *sql.Context, dbName string) (string, int, int, int, error) { + doltSession := dsess.DSessFromSess(ctx.Session) + + ws, err := doltSession.WorkingSet(ctx, dbName) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("fatal: unable to load working set: %w", err) + } + + if !ws.MergeActive() { + return "", 0, 0, 0, fmt.Errorf("error: There is no cherry-pick merge to continue") + } + mergeState := ws.MergeState() + + // Check if there are any unresolved conflicts + hasConflicts, err := doltdb.HasConflicts(ctx, ws.WorkingRoot()) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: unable to check for conflicts: %w", err) + } + if hasConflicts { + return "", 0, 0, 0, fmt.Errorf("error: cannot continue cherry-pick with unresolved conflicts") + } + + // Check if there are unstaged changes (working != staged) + stagedRoot := ws.StagedRoot() + workingRoot := ws.WorkingRoot() + + isClean, err := rootsEqual(stagedRoot, workingRoot) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: unable to compare staged and working roots: %w", err) + } + if !isClean { + return "", 0, 0, 0, fmt.Errorf("error: cannot continue cherry-pick with unstaged changes") + } + + // Get the original commit metadata from the merge state + cherryCommit := mergeState.Commit() + if cherryCommit == nil { + return "", 0, 0, 0, fmt.Errorf("error: unable to get original commit from merge state") + } + + cherryCommitMeta, err := cherryCommit.GetCommitMeta(ctx) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: unable to get commit metadata: %w", err) + } + + // Create the commit with the original commit's metadata + commitProps := actions.CommitStagedProps{ + Message: cherryCommitMeta.Description, + Date: cherryCommitMeta.Time(), + AllowEmpty: false, + Name: cherryCommitMeta.Name, + Email: cherryCommitMeta.Email, + } + + // Get the roots from the working set for creating the commit + roots, ok := doltSession.GetRoots(ctx, dbName) + if !ok { + return "", 0, 0, 0, fmt.Errorf("fatal: unable to load roots for %s", dbName) + } + + pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, commitProps) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: failed to create pending commit: %w", err) + } + if pendingCommit == nil { + return "", 0, 0, 0, fmt.Errorf("error: no changes to commit") + } + + // Clear the merge state from the working set before committing + // This ensures DoltCommit sees a clean working set + clearedWs := ws.ClearMerge() + err = doltSession.SetWorkingSet(ctx, dbName, clearedWs) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: failed to clear merge state: %w", err) + } + + commit, err := doltSession.DoltCommit(ctx, dbName, doltSession.GetTransaction(), pendingCommit) + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: failed to execute commit: %w", err) + } + + commitHash, err := commit.HashOf() + if err != nil { + return "", 0, 0, 0, fmt.Errorf("error: failed to get commit hash: %w", err) + } + + return commitHash.String(), 0, 0, 0, nil +} + // cherryPick checks that the current working set is clean, verifies the cherry-pick commit is not a merge commit // or a commit without parent commit, performs merge and returns the new working set root value and // the commit message of cherry-picked commit as the commit message of the new commit created during this command. diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go index 58cbe5c364..ce6833490a 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go @@ -80,10 +80,18 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, int, int, int, e return "", 0, 0, 0, err } + if apr.Contains(cli.AbortParam) && apr.Contains(cli.ContinueFlag) { + return "", 0, 0, 0, fmt.Errorf("error: --continue and --abort are mutually exclusive") + } + if apr.Contains(cli.AbortParam) { return "", 0, 0, 0, cherry_pick.AbortCherryPick(ctx, dbName) } + if apr.Contains(cli.ContinueFlag) { + return cherry_pick.ContinueCherryPick(ctx, dbName) + } + // we only support cherry-picking a single commit for now. if apr.NArg() == 0 { return "", 0, 0, 0, ErrEmptyCherryPick diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_cherry_pick.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_cherry_pick.go index 9f10708dbe..3068963134 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_cherry_pick.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_cherry_pick.go @@ -15,12 +15,35 @@ package enginetest import ( + "time" + + "github.com/dolthub/go-mysql-server/enginetest" "github.com/dolthub/go-mysql-server/enginetest/queries" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/types" ) +// timeValidator validates that a value is a time.Time with the expected date/time +type timeValidator struct { + expectedTime time.Time +} + +var _ enginetest.CustomValueValidator = &timeValidator{} + +func (tv *timeValidator) Validate(val interface{}) (bool, error) { + t, ok := val.(time.Time) + if !ok { + return false, nil + } + return t.Equal(tv.expectedTime), nil +} + +func timeEquals(dateStr string) *timeValidator { + t, _ := time.Parse("2006-01-02T15:04:05Z", dateStr) + return &timeValidator{expectedTime: t} +} + var DoltCherryPickTests = []queries.ScriptTest{ { Name: "error cases: basic validation", @@ -660,8 +683,8 @@ var DoltCherryPickTests = []queries.ScriptTest{ Expected: []sql.Row{{1, "resolved_value"}}, }, { - Query: "select committer, message, date from dolt_log limit 1;", - Expected: []sql.Row{{"Test User ", "add row from branch1", "2022-01-01T12:00:00Z"}}, + Query: "select committer, email, message, date from dolt_log limit 1;", + Expected: []sql.Row{{"Test User", "test@example.com", "add row from branch1", timeEquals("2022-01-01T12:00:00Z")}}, }, }, }, @@ -709,20 +732,37 @@ var DoltCherryPickTests = []queries.ScriptTest{ Expected: []sql.Row{{"", 2, 0, 0}}, }, { - Query: "select table_name from dolt_conflicts order by table_name;", + Query: "select `table` from dolt_conflicts order by `table`;", Expected: []sql.Row{{"t1"}, {"t2"}}, }, { - Query: "update t1 set v = 'resolved_t1' where pk = 1;", - Expected: []sql.Row{{types.OkResult{RowsAffected: 1, Info: plan.UpdateInfo{Matched: 1, Updated: 1}}}}, + Query: "update t1 set v = 'resolved_t1' where pk = 1;", + SkipResultsCheck: true, }, { - Query: "update t2 set v = 'resolved_t2' where pk = 1;", - Expected: []sql.Row{{types.OkResult{RowsAffected: 1, Info: plan.UpdateInfo{Matched: 1, Updated: 1}}}}, + Query: "delete from dolt_conflicts_t1;", + SkipResultsCheck: true, }, { - Query: "call dolt_add('t1', 't2');", - Expected: []sql.Row{{0}}, + Query: "call dolt_add('t1');", + SkipResultsCheck: true, + }, + { + // Should still have one remaining conflict. + Query: "call dolt_cherry_pick('--continue');", + ExpectedErrStr: "error: cannot continue cherry-pick with unresolved conflicts", + }, + { + Query: "update t2 set v = 'resolved_t2' where pk = 1;", + SkipResultsCheck: true, + }, + { + Query: "delete from dolt_conflicts_t2;", + SkipResultsCheck: true, + }, + { + Query: "call dolt_add('t2');", + SkipResultsCheck: true, }, { Query: "call dolt_cherry_pick('--continue');", @@ -741,8 +781,8 @@ var DoltCherryPickTests = []queries.ScriptTest{ Expected: []sql.Row{}, }, { - Query: "select committer, message, date from dolt_log limit 1;", - Expected: []sql.Row{{"Branch User ", "add rows from branch1", "2022-02-01T10:30:00Z"}}, + Query: "select committer, email, message, date from dolt_log limit 1;", + Expected: []sql.Row{{"Branch User", "branch@example.com", "add rows from branch1", timeEquals("2022-02-01T10:30:00Z")}}, }, }, },