diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 2bf10b18bdb..5e0a3e7c588 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -88,6 +88,7 @@ func CreateMergeArgParser() *argparser.ArgParser { return errors.New("Error: Dolt does not support merging from multiple commits. You probably meant to checkout one and then merge from the other.") } ap.SupportsFlag(NoFFParam, "", "Create a merge commit even when the merge resolves as a fast-forward.") + ap.SupportsFlag(FFOnlyParam, "", "Refuse to merge unless the current HEAD is already up to date or the merge can be resolved as a fast-forward.") ap.SupportsFlag(SquashParam, "", "Merge changes to the working set without updating the commit history") ap.SupportsString(MessageArg, "m", "msg", "Use the given {{.LessThan}}msg{{.GreaterThan}} as the commit message.") ap.SupportsFlag(AbortParam, "", "Abort the in-progress merge and return the working set to the state before the merge started.") @@ -218,6 +219,7 @@ func CreatePullArgParser() *argparser.ArgParser { ap.ArgListHelp = append(ap.ArgListHelp, [2]string{"remoteBranch", "The name of a branch on the specified remote to be merged into the current working set."}) ap.SupportsFlag(SquashParam, "", "Merge changes to the working set without updating the commit history") ap.SupportsFlag(NoFFParam, "", "Create a merge commit even when the merge resolves as a fast-forward.") + ap.SupportsFlag(FFOnlyParam, "", "Refuse to merge unless the current HEAD is already up to date or the merge can be resolved as a fast-forward.") ap.SupportsFlag(ForceFlag, "f", "Update from the remote HEAD even if there are errors.") ap.SupportsFlag(CommitFlag, "", "Perform the merge and commit the result. This is the default option, but can be overridden with the --no-commit flag. Note that this option does not affect fast-forward merges, which don't create a new merge commit, and if any merge conflicts or constraint violations are detected, no commit will be attempted.") ap.SupportsFlag(NoCommitFlag, "", "Perform the merge and stop just before creating a merge commit. Note this will not prevent a fast-forward merge; use the --no-ff arg together with the --no-commit arg to prevent both fast-forwards and merge commits.") diff --git a/go/cmd/dolt/cli/flags.go b/go/cmd/dolt/cli/flags.go index 62a2129c5eb..737ea9fc7cd 100644 --- a/go/cmd/dolt/cli/flags.go +++ b/go/cmd/dolt/cli/flags.go @@ -53,6 +53,7 @@ const ( NoCommitFlag = "no-commit" NoEditFlag = "no-edit" NoFFParam = "no-ff" + FFOnlyParam = "ff-only" NoPrettyFlag = "no-pretty" NoTLSFlag = "no-tls" NoJsonMergeFlag = "dont-merge-json" diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index a2b0dcc73e8..9c6a856ecb1 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -53,6 +53,7 @@ The second syntax ({{.LessThan}}dolt merge --abort{{.GreaterThan}}) can only be Synopsis: []string{ "[--squash] {{.LessThan}}branch{{.GreaterThan}}", "--no-ff [-m message] {{.LessThan}}branch{{.GreaterThan}}", + "--ff-only {{.LessThan}}branch{{.GreaterThan}}", "--abort", }, } @@ -197,6 +198,12 @@ func validateDoltMergeArgs(apr *argparser.ArgParseResults, usage cli.UsagePrinte if apr.ContainsAll(cli.SquashParam, cli.NoFFParam) { return HandleVErrAndExitCode(errhand.BuildDError(ErrConflictingFlags, cli.SquashParam, cli.NoFFParam).Build(), usage) } + if apr.ContainsAll(cli.FFOnlyParam, cli.NoFFParam) { + return HandleVErrAndExitCode(errhand.BuildDError(ErrConflictingFlags, cli.FFOnlyParam, cli.NoFFParam).Build(), usage) + } + if apr.ContainsAll(cli.FFOnlyParam, cli.SquashParam) { + return HandleVErrAndExitCode(errhand.BuildDError(ErrConflictingFlags, cli.FFOnlyParam, cli.SquashParam).Build(), usage) + } // This command may create a commit, so we need user identity if !cli.CheckUserNameAndEmail(cliCtx.Config()) { @@ -266,6 +273,8 @@ func constructInterpolatedDoltMergeQuery(apr *argparser.ArgParseResults, cliCtx params = append(params, apr.Arg(0)) } else if apr.Contains(cli.NoFFParam) { writeToBuffer("--no-ff", false) + } else if apr.Contains(cli.FFOnlyParam) { + writeToBuffer("--ff-only", false) } else if apr.Contains(cli.AbortParam) { writeToBuffer("--abort", false) } diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index da6bfb7a9b4..a905e2443c5 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -91,6 +91,14 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d verr := errhand.VerboseErrorFromError(errors.New(fmt.Sprintf(ErrConflictingFlags, cli.SquashParam, cli.NoFFParam))) return HandleVErrAndExitCode(verr, usage) } + if apr.ContainsAll(cli.FFOnlyParam, cli.NoFFParam) { + verr := errhand.VerboseErrorFromError(errors.New(fmt.Sprintf(ErrConflictingFlags, cli.FFOnlyParam, cli.NoFFParam))) + return HandleVErrAndExitCode(verr, usage) + } + if apr.ContainsAll(cli.FFOnlyParam, cli.SquashParam) { + verr := errhand.VerboseErrorFromError(errors.New(fmt.Sprintf(ErrConflictingFlags, cli.FFOnlyParam, cli.SquashParam))) + return HandleVErrAndExitCode(verr, usage) + } // This command may create a commit, so we need user identity if !cli.CheckUserNameAndEmail(cliCtx.Config()) { bdr := errhand.BuildDError("Could not determine name and/or email.") @@ -253,6 +261,9 @@ func constructInterpolatedDoltPullQuery(apr *argparser.ArgParseResults) (string, if apr.Contains(cli.NoFFParam) { args = append(args, "'--no-ff'") } + if apr.Contains(cli.FFOnlyParam) { + args = append(args, "'--ff-only'") + } if apr.Contains(cli.ForceFlag) { args = append(args, "'--force'") } diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index cffb9748697..3d69a1c095b 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -29,6 +29,14 @@ import ( var ErrFailedToDetermineMergeability = errors.New("failed to determine mergeability") +type FastForwardMode int + +const ( + FastForwardDefault FastForwardMode = iota + FastForwardOnly + NoFastForward +) + type MergeSpec struct { HeadH hash.Hash MergeH hash.Hash @@ -38,7 +46,7 @@ type MergeSpec struct { StompedTblNames []doltdb.TableName WorkingDiffs map[doltdb.TableName]hash.Hash Squash bool - NoFF bool + FFMode FastForwardMode NoCommit bool NoEdit bool Force bool @@ -49,9 +57,9 @@ type MergeSpec struct { type MergeSpecOpt func(*MergeSpec) -func WithNoFF(noFF bool) MergeSpecOpt { +func WithFastForwardMode(mode FastForwardMode) MergeSpecOpt { return func(ms *MergeSpec) { - ms.NoFF = noFF + ms.FFMode = mode } } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index ce3c0db9696..b695f695603 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -118,7 +118,13 @@ func doDoltMerge(ctx *sql.Context, args []string) (string, int, int, string, err } if apr.ContainsAll(cli.SquashParam, cli.NoFFParam) { - return "", noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together.\n", cli.SquashParam, cli.NoFFParam) + return "", noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together", cli.SquashParam, cli.NoFFParam) + } + if apr.ContainsAll(cli.FFOnlyParam, cli.NoFFParam) { + return "", noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together", cli.FFOnlyParam, cli.NoFFParam) + } + if apr.ContainsAll(cli.FFOnlyParam, cli.SquashParam) { + return "", noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together", cli.FFOnlyParam, cli.SquashParam) } ws, err := sess.WorkingSet(ctx, dbName) @@ -226,7 +232,7 @@ func performMerge( } if canFF { - if spec.NoFF { + if spec.FFMode == merge.NoFastForward { var commit *doltdb.Commit ws, commit, err = executeNoFFMerge(ctx, sess, spec, msg, dbName, ws, noCommit) if err == doltdb.ErrUnresolvedConflictsOrViolations { @@ -264,6 +270,10 @@ func performMerge( return ws, h.String(), noConflictsOrViolations, fastForwardMerge, "merge successful", nil } + if spec.FFMode == merge.FastForwardOnly { + return ws, "", noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("fatal: Not possible to fast-forward, aborting") + } + dbState, ok, err := sess.LookupDbState(ctx, dbName) if err != nil { return ws, "", noConflictsOrViolations, threeWayMerge, "", err @@ -482,6 +492,15 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a if apr.Contains(cli.NoCommitFlag) && apr.Contains(cli.CommitFlag) { return nil, errors.New("cannot define both 'commit' and 'no-commit' flags at the same time") } + + // Determine FastForwardMode based on flags. validation of mutually exclusive flags done earlier + var ffMode merge.FastForwardMode = merge.FastForwardDefault + if apr.Contains(cli.NoFFParam) { + ffMode = merge.NoFastForward + } else if apr.Contains(cli.FFOnlyParam) { + ffMode = merge.FastForwardOnly + } + return merge.NewMergeSpec( ctx, dbData.Rsr, @@ -492,7 +511,7 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a commitSpecStr, t, merge.WithSquash(apr.Contains(cli.SquashParam)), - merge.WithNoFF(apr.Contains(cli.NoFFParam)), + merge.WithFastForwardMode(ffMode), merge.WithForce(apr.Contains(cli.ForceFlag)), merge.WithNoCommit(apr.Contains(cli.NoCommitFlag)), merge.WithNoEdit(apr.Contains(cli.NoEditFlag)), diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index 5a1e3aae8c4..46f3a940b14 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -96,6 +96,14 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, string, error) { return noConflictsOrViolations, threeWayMerge, "", actions.ErrInvalidPullArgs } + // Validate conflicting flags + if apr.ContainsAll(cli.FFOnlyParam, cli.NoFFParam) { + return noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together", cli.FFOnlyParam, cli.NoFFParam) + } + if apr.ContainsAll(cli.FFOnlyParam, cli.SquashParam) { + return noConflictsOrViolations, threeWayMerge, "", fmt.Errorf("error: Flags '--%s' and '--%s' cannot be used together", cli.FFOnlyParam, cli.SquashParam) + } + var remoteName, remoteRefName string if apr.NArg() == 1 { remoteName = apr.Arg(0) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 6889ee027f2..c18b00a5968 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -2965,6 +2965,116 @@ var MergeScripts = []queries.ScriptTest{ }, }, }, + { + Name: "--ff-only flag success when fast-forward is possible", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "INSERT INTO t VALUES (1, 'main1'), (2, 'main2');", + "CALL dolt_commit('-Am', 'main commit');", + "CALL dolt_checkout('-b', 'feature');", + "INSERT INTO t VALUES (3, 'feature1');", + "CALL dolt_commit('-am', 'feature commit');", + "CALL dolt_checkout('main');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', 'feature');", + Expected: []sql.Row{{doltCommit, 1, 0, "merge successful"}}, + }, + { + Query: "SELECT * FROM t ORDER BY pk;", + Expected: []sql.Row{{1, "main1"}, {2, "main2"}, {3, "feature1"}}, + }, + }, + }, + { + Name: "--ff-only flag failure when fast-forward is not possible", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "INSERT INTO t VALUES (1, 'main1'), (2, 'main2');", + "CALL dolt_commit('-Am', 'main commit');", + "CALL dolt_checkout('-b', 'feature');", + "INSERT INTO t VALUES (3, 'feature1');", + "CALL dolt_commit('-am', 'feature commit');", + "CALL dolt_checkout('main');", + "INSERT INTO t VALUES (4, 'main3');", + "CALL dolt_commit('-am', 'main commit 2');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', 'feature');", + ExpectedErrStr: "fatal: Not possible to fast-forward, aborting", + }, + { + Query: "SELECT * FROM t ORDER BY pk;", + Expected: []sql.Row{{1, "main1"}, {2, "main2"}, {4, "main3"}}, // No changes + }, + }, + }, + { + Name: "--ff-only flag with already up-to-date branch", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "INSERT INTO t VALUES (1, 'main1'), (2, 'main2');", + "CALL dolt_commit('-Am', 'main commit');", + "CALL dolt_checkout('-b', 'feature');", + "CALL dolt_checkout('main');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', 'feature');", + Expected: []sql.Row{{"", 0, 0, "Everything up-to-date"}}, + }, + }, + }, + { + Name: "--ff-only conflicts with --no-ff", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "CALL dolt_commit('-Am', 'initial commit');", + "CALL dolt_checkout('-b', 'feature');", + "CALL dolt_checkout('main');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', '--no-ff', 'feature');", + ExpectedErrStr: "error: Flags '--ff-only' and '--no-ff' cannot be used together", + }, + }, + }, + { + Name: "--ff-only conflicts with --squash", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "CALL dolt_commit('-Am', 'initial commit');", + "CALL dolt_checkout('-b', 'feature');", + "CALL dolt_checkout('main');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', '--squash', 'feature');", + ExpectedErrStr: "error: Flags '--ff-only' and '--squash' cannot be used together", + }, + }, + }, + { + Name: "--ff-only with no-commit flag should work", + SetUpScript: []string{ + "CREATE TABLE t (pk int PRIMARY KEY, c1 varchar(20));", + "INSERT INTO t VALUES (1, 'main1');", + "CALL dolt_commit('-Am', 'main commit');", + "CALL dolt_checkout('-b', 'feature');", + "INSERT INTO t VALUES (2, 'feature1');", + "CALL dolt_commit('-am', 'feature commit');", + "CALL dolt_checkout('main');", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "CALL dolt_merge('--ff-only', '--no-commit', 'feature');", + Expected: []sql.Row{{doltCommit, 1, 0, "merge successful"}}, // Fast-forward merge with commit hash + }, + }, + }, } var KeylessMergeCVsAndConflictsScripts = []queries.ScriptTest{ diff --git a/integration-tests/bats/pull.bats b/integration-tests/bats/pull.bats index 9229928d79c..fb96fb4c899 100755 --- a/integration-tests/bats/pull.bats +++ b/integration-tests/bats/pull.bats @@ -581,3 +581,68 @@ SQL # Verify that the remote branch was deleted. [[ ! "$output" =~ "origin/new-branch" ]] || false } + +@test "pull: --ff-only succeeds when fast-forward is possible" { + cd repo2 + run dolt sql -q "show tables" -r csv + [ "$status" -eq 0 ] + [ "${#lines[@]}" -eq 1 ] # Only header, no tables + + run dolt pull --ff-only origin + [ "$status" -eq 0 ] + [[ "$output" =~ "Fast-forward" ]] || false + + run dolt sql -q "show tables" -r csv + [ "$status" -eq 0 ] + [ "${#lines[@]}" -eq 2 ] + [[ "$output" =~ "t1" ]] || false +} + +@test "pull: --ff-only fails when fast-forward is not possible" { + cd repo2 + + # Make a local commit that diverges from remote + dolt sql -q "create table divergent (id int primary key);" + dolt commit -Am "local divergent commit" + + run dolt pull --ff-only origin + [ "$status" -eq 1 ] + [[ "$output" =~ "fatal: Not possible to fast-forward, aborting" ]] || false +} + +@test "pull: --ff-only conflicts with --no-ff" { + cd repo2 + run dolt pull --ff-only --no-ff origin + [ "$status" -eq 1 ] + [[ "$output" =~ "Flags '--ff-only' and '--no-ff' cannot be used together" ]] || false +} + +@test "pull: --ff-only conflicts with --squash" { + cd repo2 + run dolt pull --ff-only --squash origin + [ "$status" -eq 1 ] + [[ "$output" =~ "Flags '--ff-only' and '--squash' cannot be used together" ]] || false +} + +@test "pull: --ff-only with already up-to-date branch" { + cd repo2 + # First pull normally to get up to date + dolt pull origin + + # Now pull again with --ff-only - should succeed with "up-to-date" message + run dolt pull --ff-only origin + [ "$status" -eq 0 ] + [[ "$output" =~ "Everything up-to-date" ]] || false +} + +@test "pull: --ff-only works with --no-commit" { + cd repo2 + + run dolt pull --ff-only --no-commit origin + [ "$status" -eq 0 ] + [[ "$output" =~ "Fast-forward" ]] || false + + run dolt sql -q "show tables" -r csv + [ "$status" -eq 0 ] + [[ "$output" =~ "t1" ]] || false +}