From 6c16c58d98284a36f39ad84c3e2b480cea02c2f5 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 8 Sep 2023 17:49:50 -0700 Subject: [PATCH 01/25] updated bats --- integration-tests/bats/replication.bats | 43 +++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/integration-tests/bats/replication.bats b/integration-tests/bats/replication.bats index aa7668e58e..f69487ea60 100644 --- a/integration-tests/bats/replication.bats +++ b/integration-tests/bats/replication.bats @@ -678,7 +678,7 @@ SQL [ "$status" -eq 0 ] } -@test "replication: non-fast-forward pull fails replication" { +@test "replication: non-fast-forward pull fails with force turned off" { dolt clone file://./rem1 clone1 cd clone1 dolt sql -q "create table t1 (a int primary key)" @@ -690,6 +690,7 @@ SQL cd ../repo1 dolt config --local --add sqlserver.global.dolt_read_replica_remote remote1 dolt config --local --add sqlserver.global.dolt_replicate_heads main + dolt sql -q "set @@persist.dolt_read_replica_force_pull = off" run dolt sql -q "show tables" [ "$status" -eq 0 ] @@ -698,12 +699,15 @@ SQL cd ../clone1 dolt checkout -b new-main HEAD~ dolt sql -q "create table t1 (a int primary key)" - dolt sql -q "insert into t1 values (1), (2), (3);" + dolt sql -q "insert into t1 values (1);" dolt add . dolt commit -am "new commit" dolt push -f origin new-main:main cd ../repo1 + + # with dolt_read_replica_force_pull set to false (not default), this fails with a replication + # error run dolt sql -q "show tables" [ "$status" -ne 0 ] [[ "$output" =~ "replication" ]] || false @@ -721,7 +725,6 @@ SQL cd ../repo1 dolt config --local --add sqlserver.global.dolt_read_replica_remote remote1 dolt config --local --add sqlserver.global.dolt_replicate_heads main - dolt config --local --add sqlserver.global.dolt_read_replica_force_pull 1 run dolt sql -q "select sum(a) from t1" [ "$status" -eq 0 ] @@ -822,3 +825,37 @@ SQL [ "$status" -eq 0 ] [ "${#lines[@]}" -eq 1 ] } + +@test "replication: commit --amend" { + mkdir test_commit_amend_replication_primary + dolt init --fun + + dolt remote add origin file://./test_commit_amend_replication + dolt push origin main + + dolt sql -q "set @@persist.dolt_replicate_all_heads = 1" + dolt sql -q "set @@persist.dolt_replicate_to_remote = 'origin'" + + dolt sql << SQL +create table foo (pk int primary key, c1 int); +insert into foo values (1,1); +SQL + + dolt commit -Am "Created Table" + + mkdir clone && cd clone + dolt clone file://../test_commit_amend_replication + cd test_commit_amend_replication + dolt sql -q "set @@persist.dolt_replicate_heads = 'main'" + dolt sql -q "set @@persist.dolt_read_replica_remote = 'origin'" + dolt sql -q "select * from foo" + + cd ../../ + dolt commit --amend -m "inserted 0,0. amended" + dolt push origin main + + cd clone/test_commit_amend_replication + run dolt sql -q "select * from foo" -r csv + [ "$status" -eq 0 ] + [[ "$output" =~ "1,1" ]] || false +} From 1216488006ede6eb65aa8cd330b2319ca918833c Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 8 Sep 2023 17:50:14 -0700 Subject: [PATCH 02/25] Bug fixes for type of sys var, persisting bools --- go/libraries/doltcore/sqle/dsess/session.go | 5 +++++ go/libraries/doltcore/sqle/system_variables.go | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/session.go b/go/libraries/doltcore/sqle/dsess/session.go index 8172d984e4..e12a8b61f1 100644 --- a/go/libraries/doltcore/sqle/dsess/session.go +++ b/go/libraries/doltcore/sqle/dsess/session.go @@ -1575,6 +1575,11 @@ func setPersistedValue(conf config.WritableConfig, key string, value interface{} case string: return config.SetString(conf, key, v) case bool: + if v { + return config.SetInt(conf, key, 1) + } else { + return config.SetInt(conf, key, 0) + } return sql.ErrInvalidType.New(v) default: return sql.ErrInvalidType.New(v) diff --git a/go/libraries/doltcore/sqle/system_variables.go b/go/libraries/doltcore/sqle/system_variables.go index a18ec75336..923834cc01 100644 --- a/go/libraries/doltcore/sqle/system_variables.go +++ b/go/libraries/doltcore/sqle/system_variables.go @@ -56,11 +56,11 @@ func AddDoltSystemVariables() { }, { Name: dsess.ReadReplicaForcePull, - Scope: sql.SystemVariableScope_Global, + Scope: sql.SystemVariableScope_Persist, Dynamic: true, SetVarHintApplies: false, - Type: types.NewSystemStringType(dsess.ReadReplicaForcePull), - Default: int8(0), + Type: types.NewSystemBoolType(dsess.ReadReplicaForcePull), + Default: int8(1), }, { Name: dsess.SkipReplicationErrors, From 722a454b75e959f5258db12832a35da5470df6aa Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 8 Sep 2023 18:13:34 -0700 Subject: [PATCH 03/25] Server test for persisting variables --- .../sqle/enginetest/dolt_server_test.go | 120 +++++++++++++++++- 1 file changed, 118 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go index 905e779e44..3b6374591e 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go @@ -421,6 +421,57 @@ var DropDatabaseMultiSessionScriptTests = []queries.ScriptTest{ }, } +var PersistVariableTests = []queries.ScriptTest{ + { + Name: "set persisted variables with on and off", + SetUpScript: []string{ + "set @@persist.dolt_skip_replication_errors = on;", + "set @@persist.dolt_read_replica_force_pull = off;", + }, + }, + { + Name: "retrieve persisted variables", + Assertions: []queries.ScriptTestAssertion{ + { + Query: "select @@dolt_skip_replication_errors", + Expected: []sql.Row{ + {1}, + }, + }, + { + Query: "select @@dolt_read_replica_force_pull", + Expected: []sql.Row{ + {0}, + }, + }, + }, + }, + { + Name: "set persisted variables with 1 and 0", + SetUpScript: []string{ + "set @@persist.dolt_skip_replication_errors = 0;", + "set @@persist.dolt_read_replica_force_pull = 1;", + }, + }, + { + Name: "retrieve persisted variables", + Assertions: []queries.ScriptTestAssertion{ + { + Query: "select @@dolt_skip_replication_errors", + Expected: []sql.Row{ + {0}, + }, + }, + { + Query: "select @@dolt_read_replica_force_pull", + Expected: []sql.Row{ + {1}, + }, + }, + }, + }, +} + // TestDoltMultiSessionBehavior runs tests that exercise multi-session logic on a running SQL server. Statements // are sent through the server, from out of process, instead of directly to the in-process engine API. func TestDoltMultiSessionBehavior(t *testing.T) { @@ -433,6 +484,11 @@ func TestDropDatabaseMultiSessionBehavior(t *testing.T) { testMultiSessionScriptTests(t, DropDatabaseMultiSessionScriptTests) } +// TestPersistVariable tests persisting variables across server starts +func TestPersistVariable(t *testing.T) { + testSerialSessionScriptTests(t, PersistVariableTests) +} + func testMultiSessionScriptTests(t *testing.T, tests []queries.ScriptTest) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { @@ -490,6 +546,62 @@ func testMultiSessionScriptTests(t *testing.T, tests []queries.ScriptTest) { } } +// testSerialSessionScriptTests creates an environment, then for each script starts a server and runs assertions, +// stopping the server in between scripts. Unlike other script test executors, scripts may influence later scripts in +// the block. +func testSerialSessionScriptTests(t *testing.T, tests []queries.ScriptTest) { + dEnv := dtestutils.CreateTestEnv() + serverConfig := sqlserver.DefaultServerConfig() + rand.Seed(time.Now().UnixNano()) + port := 15403 + rand.Intn(25) + serverConfig = serverConfig.WithPort(port) + defer dEnv.DoltDB.Close() + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + sc, serverConfig := startServerOnEnv(t, serverConfig, dEnv) + err := sc.WaitForStart() + require.NoError(t, err) + + conn1, sess1 := newConnection(t, serverConfig) + + t.Run(test.Name, func(t *testing.T) { + for _, setupStatement := range test.SetUpScript { + _, err := sess1.Exec(setupStatement) + require.NoError(t, err) + } + + for _, assertion := range test.Assertions { + t.Run(assertion.Query, func(t *testing.T) { + activeSession := sess1 + rows, err := activeSession.Query(assertion.Query) + + if len(assertion.ExpectedErrStr) > 0 { + require.EqualError(t, err, assertion.ExpectedErrStr) + } else if assertion.ExpectedErr != nil { + require.True(t, assertion.ExpectedErr.Is(err), "expected error %v, got %v", assertion.ExpectedErr, err) + } else if assertion.Expected != nil { + require.NoError(t, err) + assertResultsEqual(t, assertion.Expected, rows) + } else { + require.Fail(t, "unsupported ScriptTestAssertion property: %v", assertion) + } + if rows != nil { + require.NoError(t, rows.Close()) + } + }) + } + }) + + require.NoError(t, conn1.Close()) + + sc.StopServer() + err = sc.WaitForClose() + require.NoError(t, err) + }) + } +} + func makeDestinationSlice(t *testing.T, columnTypes []*gosql.ColumnType) []interface{} { dest := make([]any, len(columnTypes)) for i, columnType := range columnTypes { @@ -548,7 +660,6 @@ func assertResultsEqual(t *testing.T, expected []sql.Row, rows *gosql.Rows) { func startServer(t *testing.T, withPort bool, host string, unixSocketPath string) (*env.DoltEnv, *sqlserver.ServerController, sqlserver.ServerConfig) { dEnv := dtestutils.CreateTestEnv() serverConfig := sqlserver.DefaultServerConfig() - if withPort { rand.Seed(time.Now().UnixNano()) port := 15403 + rand.Intn(25) @@ -561,6 +672,11 @@ func startServer(t *testing.T, withPort bool, host string, unixSocketPath string serverConfig = serverConfig.WithSocket(unixSocketPath) } + onEnv, config := startServerOnEnv(t, serverConfig, dEnv) + return dEnv, onEnv, config +} + +func startServerOnEnv(t *testing.T, serverConfig sqlserver.ServerConfig, dEnv *env.DoltEnv, ) (*sqlserver.ServerController, sqlserver.ServerConfig) { sc := sqlserver.NewServerController() go func() { _, _ = sqlserver.Serve(context.Background(), "0.0.0", serverConfig, sc, dEnv) @@ -568,7 +684,7 @@ func startServer(t *testing.T, withPort bool, host string, unixSocketPath string err := sc.WaitForStart() require.NoError(t, err) - return dEnv, sc, serverConfig + return sc, serverConfig } // newConnection takes sqlserver.serverConfig and opens a connection, and will return that connection with a new session From be5a44ffa99ddbdf73dcd4a8802da918858194b1 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 8 Sep 2023 18:15:06 -0700 Subject: [PATCH 04/25] dead code --- go/libraries/doltcore/sqle/dsess/session.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/dsess/session.go b/go/libraries/doltcore/sqle/dsess/session.go index e12a8b61f1..086af221d6 100644 --- a/go/libraries/doltcore/sqle/dsess/session.go +++ b/go/libraries/doltcore/sqle/dsess/session.go @@ -1580,7 +1580,6 @@ func setPersistedValue(conf config.WritableConfig, key string, value interface{} } else { return config.SetInt(conf, key, 0) } - return sql.ErrInvalidType.New(v) default: return sql.ErrInvalidType.New(v) } From 2720246d6545bd4af724859126dd66fecee247be Mon Sep 17 00:00:00 2001 From: zachmu Date: Sat, 9 Sep 2023 01:27:04 +0000 Subject: [PATCH 05/25] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/sqle/enginetest/dolt_server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go index 3b6374591e..75a54598f0 100755 --- a/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_server_test.go @@ -556,7 +556,7 @@ func testSerialSessionScriptTests(t *testing.T, tests []queries.ScriptTest) { port := 15403 + rand.Intn(25) serverConfig = serverConfig.WithPort(port) defer dEnv.DoltDB.Close() - + for _, test := range tests { t.Run(test.Name, func(t *testing.T) { sc, serverConfig := startServerOnEnv(t, serverConfig, dEnv) @@ -676,7 +676,7 @@ func startServer(t *testing.T, withPort bool, host string, unixSocketPath string return dEnv, onEnv, config } -func startServerOnEnv(t *testing.T, serverConfig sqlserver.ServerConfig, dEnv *env.DoltEnv, ) (*sqlserver.ServerController, sqlserver.ServerConfig) { +func startServerOnEnv(t *testing.T, serverConfig sqlserver.ServerConfig, dEnv *env.DoltEnv) (*sqlserver.ServerController, sqlserver.ServerConfig) { sc := sqlserver.NewServerController() go func() { _, _ = sqlserver.Serve(context.Background(), "0.0.0", serverConfig, sc, dEnv) From e02a9efd02b2e1a112faa64d2321555e7d5a5e4f Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 10:16:20 -0700 Subject: [PATCH 06/25] Revert variable to global scope, fix tests --- go/libraries/doltcore/sqle/dsess/dolt_session_test.go | 4 ++-- go/libraries/doltcore/sqle/system_variables.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/dolt_session_test.go b/go/libraries/doltcore/sqle/dsess/dolt_session_test.go index 8b3fd1f923..4de8feea82 100644 --- a/go/libraries/doltcore/sqle/dsess/dolt_session_test.go +++ b/go/libraries/doltcore/sqle/dsess/dolt_session_test.go @@ -144,12 +144,12 @@ func TestSetPersistedValue(t *testing.T) { { Name: "bool", Value: true, - Err: sql.ErrInvalidType, + ExpectedRes: "1", }, { Name: "bool", Value: false, - Err: sql.ErrInvalidType, + ExpectedRes: "0", }, { Value: complex64(7), diff --git a/go/libraries/doltcore/sqle/system_variables.go b/go/libraries/doltcore/sqle/system_variables.go index 923834cc01..6f74e548e1 100644 --- a/go/libraries/doltcore/sqle/system_variables.go +++ b/go/libraries/doltcore/sqle/system_variables.go @@ -56,7 +56,7 @@ func AddDoltSystemVariables() { }, { Name: dsess.ReadReplicaForcePull, - Scope: sql.SystemVariableScope_Persist, + Scope: sql.SystemVariableScope_Global, Dynamic: true, SetVarHintApplies: false, Type: types.NewSystemBoolType(dsess.ReadReplicaForcePull), From 9f5c74ff9c051f5882c7c21fbdde361e6e74131c Mon Sep 17 00:00:00 2001 From: zachmu Date: Mon, 11 Sep 2023 17:27:52 +0000 Subject: [PATCH 07/25] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/sqle/dsess/dolt_session_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/libraries/doltcore/sqle/dsess/dolt_session_test.go b/go/libraries/doltcore/sqle/dsess/dolt_session_test.go index 4de8feea82..f13f910913 100644 --- a/go/libraries/doltcore/sqle/dsess/dolt_session_test.go +++ b/go/libraries/doltcore/sqle/dsess/dolt_session_test.go @@ -142,13 +142,13 @@ func TestSetPersistedValue(t *testing.T) { Value: "7", }, { - Name: "bool", - Value: true, + Name: "bool", + Value: true, ExpectedRes: "1", }, { - Name: "bool", - Value: false, + Name: "bool", + Value: false, ExpectedRes: "0", }, { From 1a0de31f7bc85ff71ff034f5c8f8a434640913d6 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 12:20:21 -0700 Subject: [PATCH 08/25] implement force pull --- go/cmd/dolt/cli/arg_parser_helpers.go | 2 +- go/cmd/dolt/commands/pull.go | 14 ++++++++++++-- go/libraries/doltcore/env/remotes.go | 7 ++++++- .../doltcore/sqle/dprocedures/dolt_pull.go | 12 +++++++++++- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 3e532c11fd..8a743a0489 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -217,7 +217,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(ForceFlag, "f", "Ignore any foreign key warnings and proceed with the commit.") + ap.SupportsFlag(ForceFlag, "f", "Update from the remote HEAD even if there are errors or the local branch has diverged.") 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.") ap.SupportsFlag(NoEditFlag, "", "Use an auto-generated commit message when creating a merge commit. The default for interactive CLI sessions is to open an editor.") diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 612465df16..49db7ff424 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -16,6 +16,7 @@ package commands import ( "context" + "errors" "fmt" "github.com/dolthub/go-mysql-server/sql" @@ -160,7 +161,16 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, } err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) - if err != nil { + if errors.Is(err, datas.ErrMergeNeeded) && pullSpec.Force { + h, err := srcDBCommit.HashOf() + if err != nil { + return err + } + err = dEnv.DoltDB.SetHead(ctx, branchRef, h) + if err != nil { + return err + } + } else if err != nil { return fmt.Errorf("fetch failed; %w", err) } @@ -178,7 +188,7 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, name, email, configErr := env.GetNameAndEmail(dEnv.Config) // If the name and email aren't set we can set them to empty values for now. This is only valid for ff - // merges which detect for later. + // merges which we detect later. if configErr != nil { if pullSpec.Noff { return configErr diff --git a/go/libraries/doltcore/env/remotes.go b/go/libraries/doltcore/env/remotes.go index f7e85cd3cc..435d2f225e 100644 --- a/go/libraries/doltcore/env/remotes.go +++ b/go/libraries/doltcore/env/remotes.go @@ -406,7 +406,12 @@ type PullSpec struct { // NewPullSpec returns PullSpec object using arguments passed into this function, which are remoteName, remoteRefName, // squash, noff, noCommit, noEdit, refSpecs, force and remoteOnly. This function validates remote and gets remoteRef // for given remoteRefName; if it's not defined, it uses current branch to get its upstream branch if it exists. -func NewPullSpec(_ context.Context, rsr RepoStateReader, remoteName, remoteRefName string, squash, noff, noCommit, noEdit, force, remoteOnly bool) (*PullSpec, error) { +func NewPullSpec( + _ context.Context, + rsr RepoStateReader, + remoteName, remoteRefName string, + squash, noff, noCommit, noEdit, force, remoteOnly bool, +) (*PullSpec, error) { refSpecs, err := GetRefSpecs(rsr, remoteName) if err != nil { return nil, err diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index 3d0c7ce20e..02ad505ec6 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -20,6 +20,7 @@ import ( "fmt" "sync" + "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -129,7 +130,16 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { // TODO: this could be replaced with a canFF check to test for error err = dbData.Ddb.FastForward(ctx, remoteTrackRef, srcDBCommit) - if err != nil { + if errors.Is(err, datas.ErrMergeNeeded) && pullSpec.Force { + h, err := srcDBCommit.HashOf() + if err != nil { + return noConflictsOrViolations, threeWayMerge, err + } + err = dbData.Ddb.SetHead(ctx, branchRef, h) + if err != nil { + return noConflictsOrViolations, threeWayMerge, err + } + } else if err != nil { return noConflictsOrViolations, threeWayMerge, fmt.Errorf("fetch failed; %w", err) } From 6f3beabf985ae4dad319ef69277e04b6c4a5327c Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 15:08:07 -0700 Subject: [PATCH 09/25] Attempt to perform merge of remote ref and tracking ref --- go/cmd/dolt/cli/arg_parser_helpers.go | 2 +- go/cmd/dolt/commands/pull.go | 82 ++++++++++++++++++++++++-- go/libraries/doltcore/doltdb/doltdb.go | 4 -- go/libraries/doltcore/merge/action.go | 10 +++- 4 files changed, 87 insertions(+), 11 deletions(-) diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 8a743a0489..33a578a72d 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -217,7 +217,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(ForceFlag, "f", "Update from the remote HEAD even if there are errors or the local branch has diverged.") + 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.") ap.SupportsFlag(NoEditFlag, "", "Use an auto-generated commit message when creating a merge commit. The default for interactive CLI sessions is to open an editor.") diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 49db7ff424..348ea55dad 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" + "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -160,21 +161,92 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, return err } + h, err := srcDBCommit.HashOf() + if err != nil { + return err + } + fmt.Println(h.String()) + err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) - if errors.Is(err, datas.ErrMergeNeeded) && pullSpec.Force { - h, err := srcDBCommit.HashOf() + if errors.Is(err, datas.ErrMergeNeeded) { + // merge the remote tracking ref before continuing to perform a merge of the local branch and working set + roots, err := dEnv.Roots(ctx) + if err != nil { + return err + } + name, email, _ := env.GetNameAndEmail(dEnv.Config) + + mergeSpec, err := merge.NewMergeSpec( + ctx, + dEnv.RepoStateReader(), + dEnv.DoltDB, + roots, + name, + email, + pullSpec.Msg, + remoteTrackRef.String(), + pullSpec.Squash, + pullSpec.Noff, + pullSpec.Force, + pullSpec.NoCommit, + pullSpec.NoEdit, + datas.CommitNowFunc(), + ) + if err != nil { + return err + } + + suggestedMsg := fmt.Sprintf( + pullSpec.Branch.GetPath(), + pullSpec.Remote.Url, + remoteTrackRef.GetPath(), + ) + + msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, "", suggestedMsg, mergeSpec.NoEdit, cliCtx) if err != nil { return err } - err = dEnv.DoltDB.SetHead(ctx, branchRef, h) + + tmpDir, err := dEnv.TempTableFilesDir() + if err != nil { + return err + } + opts := editor.Options{Deaf: dEnv.BulkDbEaFactory(), Tempdir: tmpDir} + + remoteTrackingRefCommit, err := dEnv.DoltDB.ResolveCommitRef(ctx, remoteTrackRef) + if err != nil { + return err + } + + result, err := merge.MergeCommits(sqlCtx, remoteTrackingRefCommit, srcDBCommit, opts) + if err != nil { + return err + } + + _, mergeRootHash, err := dEnv.DoltDB.WriteRootValue(ctx, result.Root) + if err != nil { + return err + } + + ts := datas.CommitNowFunc().Unix() + cm := &datas.CommitMeta{ + Name: name, + Email: email, + Timestamp: uint64(ts), + Description: msg, + UserTimestamp: ts, + } + + _, err = dEnv.DoltDB.Commit(ctx, mergeRootHash, remoteTrackRef, cm) if err != nil { return err } } else if err != nil { - return fmt.Errorf("fetch failed; %w", err) + return fmt.Errorf("fetch failed: %w", err) } // Merge iff branch is current branch and there is an upstream set (pullSpec.Branch is set to nil if there is no upstream) + // TODO: we should merge every branch if branchRef != pullSpec.Branch { continue } @@ -196,7 +268,7 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, name, email = "", "" } - // Begin merge + // Begin merge of working and head with the remote head mergeSpec, err := merge.NewMergeSpec(ctx, dEnv.RepoStateReader(), dEnv.DoltDB, roots, name, email, pullSpec.Msg, remoteTrackRef.String(), pullSpec.Squash, pullSpec.Noff, pullSpec.Force, pullSpec.NoCommit, pullSpec.NoEdit, t) if err != nil { return err diff --git a/go/libraries/doltcore/doltdb/doltdb.go b/go/libraries/doltcore/doltdb/doltdb.go index 0838816b67..3cf36a3a13 100644 --- a/go/libraries/doltcore/doltdb/doltdb.go +++ b/go/libraries/doltcore/doltdb/doltdb.go @@ -579,10 +579,6 @@ func (ddb *DoltDB) ReadCommit(ctx context.Context, h hash.Hash) (*Commit, error) // Commit will update a branch's head value to be that of a previously committed root value hash func (ddb *DoltDB) Commit(ctx context.Context, valHash hash.Hash, dref ref.DoltRef, cm *datas.CommitMeta) (*Commit, error) { - if dref.GetType() != ref.BranchRefType { - panic("can't commit to ref that isn't branch atm. will probably remove this.") - } - return ddb.CommitWithParentSpecs(ctx, valHash, dref, nil, cm) } diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index 307ddbc1fc..17864822a3 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -57,7 +57,15 @@ type MergeSpec struct { // NewMergeSpec returns MergeSpec object using arguments passed into this function, which are doltdb.Roots, username, // user email, commit msg, commitSpecStr, to squash, to noff, to force, noCommit, noEdit and date. This function // resolves head and merge commit, and it gets current diffs between current head and working set if it exists. -func NewMergeSpec(ctx context.Context, rsr env.RepoStateReader, ddb *doltdb.DoltDB, roots doltdb.Roots, name, email, msg, commitSpecStr string, squash, noff, force, noCommit, noEdit bool, date time.Time) (*MergeSpec, error) { +func NewMergeSpec( + ctx context.Context, + rsr env.RepoStateReader, + ddb *doltdb.DoltDB, + roots doltdb.Roots, + name, email, msg, commitSpecStr string, + squash, noff, force, noCommit, noEdit bool, + date time.Time, +) (*MergeSpec, error) { headCS, err := doltdb.NewCommitSpec("HEAD") if err != nil { return nil, err From d9353ea1a3c8f0f573b1db1742b25d6796965b16 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 15:52:51 -0700 Subject: [PATCH 10/25] Cleanup --- go/cmd/dolt/commands/merge.go | 5 +- go/cmd/dolt/commands/pull.go | 180 +++++++++++------- go/libraries/doltcore/env/remotes.go | 8 +- go/libraries/doltcore/merge/action.go | 75 ++++++-- .../doltcore/sqle/dprocedures/dolt_merge.go | 20 +- 5 files changed, 190 insertions(+), 98 deletions(-) diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index 8f5e59b94b..a33ba48ec2 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -497,7 +497,7 @@ func validateMergeSpec(ctx context.Context, spec *merge.MergeSpec) errhand.Verbo if _, err := merge.MayHaveConstraintViolations(ctx, ancRoot, mergedRoot); err != nil { return errhand.VerboseErrorFromError(err) } - if !spec.Noff { + if !spec.NoFF { cli.Println("Fast-forward") } } else if err == doltdb.ErrUpToDate || err == doltdb.ErrIsAhead { @@ -728,7 +728,7 @@ func performMerge(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryis if ok, err := spec.HeadC.CanFastForwardTo(ctx, spec.MergeC); err != nil && !errors.Is(err, doltdb.ErrUpToDate) { return nil, err } else if ok { - if spec.Noff { + if spec.NoFF { return executeNoFFMergeAndCommit(ctx, sqlCtx, queryist, dEnv, spec, suggestedMsg, cliCtx) } return nil, merge.ExecuteFFMerge(ctx, dEnv, spec) @@ -771,7 +771,6 @@ func executeNoFFMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryis pendingCommit, err := actions.GetCommitStaged(ctx, roots, ws, mergeParentCommits, dEnv.DbData().Ddb, actions.CommitStagedProps{ Message: msg, Date: spec.Date, - AllowEmpty: spec.AllowEmpty, Force: spec.Force, Name: spec.Name, Email: spec.Email, diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 348ea55dad..377721af6f 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -169,75 +169,7 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { - // merge the remote tracking ref before continuing to perform a merge of the local branch and working set - roots, err := dEnv.Roots(ctx) - if err != nil { - return err - } - name, email, _ := env.GetNameAndEmail(dEnv.Config) - - mergeSpec, err := merge.NewMergeSpec( - ctx, - dEnv.RepoStateReader(), - dEnv.DoltDB, - roots, - name, - email, - pullSpec.Msg, - remoteTrackRef.String(), - pullSpec.Squash, - pullSpec.Noff, - pullSpec.Force, - pullSpec.NoCommit, - pullSpec.NoEdit, - datas.CommitNowFunc(), - ) - if err != nil { - return err - } - - suggestedMsg := fmt.Sprintf( - pullSpec.Branch.GetPath(), - pullSpec.Remote.Url, - remoteTrackRef.GetPath(), - ) - - msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, "", suggestedMsg, mergeSpec.NoEdit, cliCtx) - if err != nil { - return err - } - - tmpDir, err := dEnv.TempTableFilesDir() - if err != nil { - return err - } - opts := editor.Options{Deaf: dEnv.BulkDbEaFactory(), Tempdir: tmpDir} - - remoteTrackingRefCommit, err := dEnv.DoltDB.ResolveCommitRef(ctx, remoteTrackRef) - if err != nil { - return err - } - - result, err := merge.MergeCommits(sqlCtx, remoteTrackingRefCommit, srcDBCommit, opts) - if err != nil { - return err - } - - _, mergeRootHash, err := dEnv.DoltDB.WriteRootValue(ctx, result.Root) - if err != nil { - return err - } - - ts := datas.CommitNowFunc().Unix() - cm := &datas.CommitMeta{ - Name: name, - Email: email, - Timestamp: uint64(ts), - Description: msg, - UserTimestamp: ts, - } - - _, err = dEnv.DoltDB.Commit(ctx, mergeRootHash, remoteTrackRef, cm) + err := mergeRemoteTrackingBranch(ctx, sqlCtx, queryist, dEnv, pullSpec, cliCtx, remoteTrackRef, srcDBCommit) if err != nil { return err } @@ -246,7 +178,6 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, } // Merge iff branch is current branch and there is an upstream set (pullSpec.Branch is set to nil if there is no upstream) - // TODO: we should merge every branch if branchRef != pullSpec.Branch { continue } @@ -262,14 +193,25 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, // If the name and email aren't set we can set them to empty values for now. This is only valid for ff // merges which we detect later. if configErr != nil { - if pullSpec.Noff { + if pullSpec.NoFF { return configErr } name, email = "", "" } // Begin merge of working and head with the remote head - mergeSpec, err := merge.NewMergeSpec(ctx, dEnv.RepoStateReader(), dEnv.DoltDB, roots, name, email, pullSpec.Msg, remoteTrackRef.String(), pullSpec.Squash, pullSpec.Noff, pullSpec.Force, pullSpec.NoCommit, pullSpec.NoEdit, t) + mergeSpec, err := merge.NewMergeSpec( + ctx, + dEnv.RepoStateReader(), + dEnv.DoltDB, + roots, + name, + email, + pullSpec.Msg, + remoteTrackRef.String(), + t, + merge.WithPullSpecOpts(pullSpec), + ) if err != nil { return err } @@ -299,7 +241,12 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, return err } - suggestedMsg := fmt.Sprintf("Merge branch '%s' of %s into %s", pullSpec.Branch.GetPath(), pullSpec.Remote.Url, headRef.GetPath()) + suggestedMsg := fmt.Sprintf( + "Merge branch '%s' of %s into %s", + pullSpec.Branch.GetPath(), + pullSpec.Remote.Url, + headRef.GetPath(), + ) tblStats, err := performMerge(ctx, sqlCtx, queryist, dEnv, mergeSpec, suggestedMsg, cliCtx) printSuccessStats(tblStats) if err != nil { @@ -325,3 +272,90 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, return nil } + +// mergeRemoteTrackingBranch merges the |srcDBCommit| commit into the remote tracking branch with the ref provided +func mergeRemoteTrackingBranch( + ctx context.Context, + sqlCtx *sql.Context, + queryist cli.Queryist, + dEnv *env.DoltEnv, + pullSpec *env.PullSpec, + cliCtx cli.CliContext, + remoteTrackRef ref.DoltRef, + srcDBCommit *doltdb.Commit, +) error { + // merge the remote tracking ref before continuing to perform a merge of the local branch and working set + name, email, _ := env.GetNameAndEmail(dEnv.Config) + + suggestedMsg := fmt.Sprintf( + "Merge branch '%s' of %s into %s", + pullSpec.Branch.GetPath(), + pullSpec.Remote.Url, + remoteTrackRef.GetPath(), + ) + msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, pullSpec.Msg, suggestedMsg, pullSpec.NoEdit, cliCtx) + if err != nil { + return err + } + + tmpDir, err := dEnv.TempTableFilesDir() + if err != nil { + return err + } + opts := editor.Options{Deaf: dEnv.BulkDbEaFactory(), Tempdir: tmpDir} + + remoteTrackingRefCommit, err := dEnv.DoltDB.ResolveCommitRef(ctx, remoteTrackRef) + if err != nil { + return err + } + + result, err := merge.MergeCommits(sqlCtx, remoteTrackingRefCommit, srcDBCommit, opts) + if err != nil { + return err + } + + _, mergeRootHash, err := dEnv.DoltDB.WriteRootValue(ctx, result.Root) + if err != nil { + return err + } + + ts := datas.CommitNowFunc().Unix() + cm := &datas.CommitMeta{ + Name: name, + Email: email, + Timestamp: uint64(ts), + Description: msg, + UserTimestamp: ts, + } + + remoteTrackingHash, err := remoteTrackingRefCommit.HashOf() + if err != nil { + return err + } + + firstParent, err := doltdb.NewCommitSpec(remoteTrackingHash.String()) + if err != nil { + return err + } + + srcCommitHash, err := srcDBCommit.HashOf() + if err != nil { + return err + } + + secondParent, err := doltdb.NewCommitSpec(srcCommitHash.String()) + if err != nil { + return err + } + + parentSpecs := []*doltdb.CommitSpec{ + firstParent, + secondParent, + } + + _, err = dEnv.DoltDB.CommitWithParentSpecs(ctx, mergeRootHash, remoteTrackRef, parentSpecs, cm) + if err != nil { + return err + } + return nil +} diff --git a/go/libraries/doltcore/env/remotes.go b/go/libraries/doltcore/env/remotes.go index 435d2f225e..023d804cda 100644 --- a/go/libraries/doltcore/env/remotes.go +++ b/go/libraries/doltcore/env/remotes.go @@ -392,9 +392,9 @@ func GetTrackingRef(branchRef ref.DoltRef, remote Remote) (ref.DoltRef, error) { type PullSpec struct { Msg string - Squash bool - Noff bool - NoCommit bool + Squash bool + NoFF bool + NoCommit bool NoEdit bool Force bool RemoteName string @@ -453,7 +453,7 @@ func NewPullSpec( return &PullSpec{ Squash: squash, - Noff: noff, + NoFF: noff, NoCommit: noCommit, NoEdit: noEdit, RemoteName: remoteName, diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index 17864822a3..195c09378a 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -28,10 +28,7 @@ import ( "github.com/dolthub/dolt/go/store/hash" ) -var ErrFailedToDetermineUnstagedDocs = errors.New("failed to determine unstaged docs") var ErrFailedToReadDatabase = errors.New("failed to read database") -var ErrMergeFailedToUpdateDocs = errors.New("failed to update docs to the new working root") -var ErrMergeFailedToUpdateRepoState = errors.New("unable to execute repo state update") var ErrFailedToDetermineMergeability = errors.New("failed to determine mergeability") type MergeSpec struct { @@ -43,17 +40,64 @@ type MergeSpec struct { StompedTblNames []string WorkingDiffs map[string]hash.Hash Squash bool - Msg string - Noff bool - NoCommit bool + Msg string + NoFF bool + NoCommit bool NoEdit bool Force bool - AllowEmpty bool Email string Name string Date time.Time } +type MergeSpecOpt func(*MergeSpec) + +func WithNoFf(noFF bool) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.NoFF = noFF + } +} + +func WithMsg(msg string) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.Msg = msg + } +} + +func WithNoCommit(noCommit bool) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.NoCommit = noCommit + } +} + +func WithNoEdit(noEdit bool) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.NoEdit = noEdit + } +} + +func WithForce(force bool) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.Force = force + } +} + +func WithSquash(squash bool) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.Squash = squash + } +} + +func WithPullSpecOpts(pullSpec *env.PullSpec) MergeSpecOpt { + return func(ms *MergeSpec) { + ms.NoEdit = pullSpec.NoEdit + ms.NoCommit = pullSpec.NoCommit + ms.Force = pullSpec.Force + ms.NoFF = pullSpec.NoFF + ms.Squash = pullSpec.Squash + } +} + // NewMergeSpec returns MergeSpec object using arguments passed into this function, which are doltdb.Roots, username, // user email, commit msg, commitSpecStr, to squash, to noff, to force, noCommit, noEdit and date. This function // resolves head and merge commit, and it gets current diffs between current head and working set if it exists. @@ -63,8 +107,8 @@ func NewMergeSpec( ddb *doltdb.DoltDB, roots doltdb.Roots, name, email, msg, commitSpecStr string, - squash, noff, force, noCommit, noEdit bool, date time.Time, + opts ...MergeSpecOpt, ) (*MergeSpec, error) { headCS, err := doltdb.NewCommitSpec("HEAD") if err != nil { @@ -107,7 +151,7 @@ func NewMergeSpec( return nil, fmt.Errorf("%w; %s", ErrFailedToDetermineMergeability, err.Error()) } - return &MergeSpec{ + spec := &MergeSpec{ HeadH: headH, MergeH: mergeH, HeadC: headCM, @@ -115,16 +159,17 @@ func NewMergeSpec( MergeC: mergeCM, StompedTblNames: stompedTblNames, WorkingDiffs: workingDiffs, - Squash: squash, Msg: msg, - Noff: noff, - NoCommit: noCommit, - NoEdit: noEdit, - Force: force, Email: email, Name: name, Date: date, - }, nil + } + + for _, opt := range opts { + opt(spec) + } + + return spec, nil } func ExecNoFFMerge(ctx context.Context, dEnv *env.DoltEnv, spec *MergeSpec) (map[string]*MergeStats, error) { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index c118ff174e..ea15e779be 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -195,7 +195,7 @@ func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, } if canFF { - if spec.Noff { + if spec.NoFF { var commit *doltdb.Commit ws, commit, err = executeNoFFMerge(ctx, sess, spec, dbName, ws, dbData, noCommit) if err == doltdb.ErrUnresolvedConflictsOrViolations { @@ -414,7 +414,6 @@ func executeNoFFMerge( pendingCommit, err := dSess.NewPendingCommit(ctx, dbName, roots, actions.CommitStagedProps{ Message: spec.Msg, Date: spec.Date, - AllowEmpty: spec.AllowEmpty, Force: spec.Force, Name: spec.Name, Email: spec.Email, @@ -474,7 +473,22 @@ 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") } - return merge.NewMergeSpec(ctx, dbData.Rsr, ddb, roots, name, email, msg, commitSpecStr, apr.Contains(cli.SquashParam), apr.Contains(cli.NoFFParam), apr.Contains(cli.ForceFlag), apr.Contains(cli.NoCommitFlag), apr.Contains(cli.NoEditFlag), t) + return merge.NewMergeSpec( + ctx, + dbData.Rsr, + ddb, + roots, + name, + email, + msg, + commitSpecStr, + t, + merge.WithSquash(apr.Contains(cli.SquashParam)), + merge.WithNoFf(apr.Contains(cli.NoFFParam)), + merge.WithForce(apr.Contains(cli.ForceFlag)), + merge.WithNoCommit(apr.Contains(cli.NoCommitFlag)), + merge.WithNoEdit(apr.Contains(cli.NoEditFlag)), + ) } func mergeRootToWorking( From 2e296758e3fd94e00bfc6aea0707f6da3cd8453a Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 16:14:52 -0700 Subject: [PATCH 11/25] Removed unused msg param --- go/cmd/dolt/commands/merge.go | 16 ++++---- go/cmd/dolt/commands/pull.go | 39 +++++++++++-------- go/libraries/doltcore/env/remotes.go | 6 +-- go/libraries/doltcore/merge/action.go | 14 +------ .../doltcore/sqle/dprocedures/dolt_merge.go | 1 - 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index a33ba48ec2..eac9700bb7 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -763,7 +763,7 @@ func executeNoFFMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryis mergeParentCommits = []*doltdb.Commit{ws.MergeState().Commit()} } - msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, spec.Msg, suggestedMsg, spec.NoEdit, cliCtx) + msg, err := getCommitMsgForMerge(sqlCtx, queryist, suggestedMsg, spec.NoEdit, cliCtx) if err != nil { return tblToStats, err } @@ -815,7 +815,7 @@ func executeMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryist cl return tblToStats, nil } - msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, spec.Msg, suggestedMsg, spec.NoEdit, cliCtx) + msg, err := getCommitMsgForMerge(sqlCtx, queryist, suggestedMsg, spec.NoEdit, cliCtx) if err != nil { return tblToStats, err } @@ -831,11 +831,13 @@ func executeMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryist cl } // getCommitMsgForMerge returns user defined message if exists; otherwise, get the commit message from editor. -func getCommitMsgForMerge(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, userDefinedMsg, suggestedMsg string, noEdit bool, cliCtx cli.CliContext) (string, error) { - if userDefinedMsg != "" { - return userDefinedMsg, nil - } - +func getCommitMsgForMerge( + sqlCtx *sql.Context, + queryist cli.Queryist, + suggestedMsg string, + noEdit bool, + cliCtx cli.CliContext, +) (string, error) { msg, err := getCommitMessageFromEditor(sqlCtx, queryist, suggestedMsg, "", noEdit, cliCtx) if err != nil { return msg, err diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 377721af6f..2114f3ecae 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -109,7 +109,19 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d return HandleVErrAndExitCode(verr, usage) } - pullSpec, err := env.NewPullSpec(ctx, dEnv.RepoStateReader(), remoteName, remoteRefName, apr.Contains(cli.SquashParam), apr.Contains(cli.NoFFParam), apr.Contains(cli.NoCommitFlag), apr.Contains(cli.NoEditFlag), apr.Contains(cli.ForceFlag), apr.NArg() == 1) + pullSpec, err := env.NewPullSpec( + ctx, + dEnv.RepoStateReader(), + remoteName, + remoteRefName, + apr.Contains(cli.SquashParam), + apr.Contains(cli.NoFFParam), + apr.Contains(cli.NoCommitFlag), + apr.Contains(cli.NoEditFlag), + apr.Contains(cli.ForceFlag), + apr.NArg() == 1, + ) + if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } @@ -122,7 +134,14 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d } // pullHelper splits pull into fetch, prepare merge, and merge to interleave printing -func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, dEnv *env.DoltEnv, pullSpec *env.PullSpec, cliCtx cli.CliContext) error { +func pullHelper( + ctx context.Context, + sqlCtx *sql.Context, + queryist cli.Queryist, + dEnv *env.DoltEnv, + pullSpec *env.PullSpec, + cliCtx cli.CliContext, +) error { srcDB, err := pullSpec.Remote.GetRemoteDBWithoutCaching(ctx, dEnv.DoltDB.ValueReadWriter().Format(), dEnv) if err != nil { return fmt.Errorf("failed to get remote db; %w", err) @@ -200,18 +219,7 @@ func pullHelper(ctx context.Context, sqlCtx *sql.Context, queryist cli.Queryist, } // Begin merge of working and head with the remote head - mergeSpec, err := merge.NewMergeSpec( - ctx, - dEnv.RepoStateReader(), - dEnv.DoltDB, - roots, - name, - email, - pullSpec.Msg, - remoteTrackRef.String(), - t, - merge.WithPullSpecOpts(pullSpec), - ) + mergeSpec, err := merge.NewMergeSpec(ctx, dEnv.RepoStateReader(), dEnv.DoltDB, roots, name, email, remoteTrackRef.String(), t, merge.WithPullSpecOpts(pullSpec)) if err != nil { return err } @@ -284,7 +292,6 @@ func mergeRemoteTrackingBranch( remoteTrackRef ref.DoltRef, srcDBCommit *doltdb.Commit, ) error { - // merge the remote tracking ref before continuing to perform a merge of the local branch and working set name, email, _ := env.GetNameAndEmail(dEnv.Config) suggestedMsg := fmt.Sprintf( @@ -293,7 +300,7 @@ func mergeRemoteTrackingBranch( pullSpec.Remote.Url, remoteTrackRef.GetPath(), ) - msg, err := getCommitMsgForMerge(ctx, sqlCtx, queryist, pullSpec.Msg, suggestedMsg, pullSpec.NoEdit, cliCtx) + msg, err := getCommitMsgForMerge(sqlCtx, queryist, suggestedMsg, pullSpec.NoEdit, cliCtx) if err != nil { return err } diff --git a/go/libraries/doltcore/env/remotes.go b/go/libraries/doltcore/env/remotes.go index 023d804cda..d05f2ec50f 100644 --- a/go/libraries/doltcore/env/remotes.go +++ b/go/libraries/doltcore/env/remotes.go @@ -391,7 +391,6 @@ func GetTrackingRef(branchRef ref.DoltRef, remote Remote) (ref.DoltRef, error) { } type PullSpec struct { - Msg string Squash bool NoFF bool NoCommit bool @@ -403,8 +402,9 @@ type PullSpec struct { Branch ref.DoltRef } -// NewPullSpec returns PullSpec object using arguments passed into this function, which are remoteName, remoteRefName, -// squash, noff, noCommit, noEdit, refSpecs, force and remoteOnly. This function validates remote and gets remoteRef +type PullSpecOpt func(*PullSpec) + +// NewPullSpec returns a PullSpec for the arguments given. This function validates remote and gets remoteRef // for given remoteRefName; if it's not defined, it uses current branch to get its upstream branch if it exists. func NewPullSpec( _ context.Context, diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index 195c09378a..36d51dc5f9 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -40,7 +40,6 @@ type MergeSpec struct { StompedTblNames []string WorkingDiffs map[string]hash.Hash Squash bool - Msg string NoFF bool NoCommit bool NoEdit bool @@ -58,12 +57,6 @@ func WithNoFf(noFF bool) MergeSpecOpt { } } -func WithMsg(msg string) MergeSpecOpt { - return func(ms *MergeSpec) { - ms.Msg = msg - } -} - func WithNoCommit(noCommit bool) MergeSpecOpt { return func(ms *MergeSpec) { ms.NoCommit = noCommit @@ -98,15 +91,13 @@ func WithPullSpecOpts(pullSpec *env.PullSpec) MergeSpecOpt { } } -// NewMergeSpec returns MergeSpec object using arguments passed into this function, which are doltdb.Roots, username, -// user email, commit msg, commitSpecStr, to squash, to noff, to force, noCommit, noEdit and date. This function -// resolves head and merge commit, and it gets current diffs between current head and working set if it exists. +// NewMergeSpec returns a MergeSpec with the arguments provided. func NewMergeSpec( ctx context.Context, rsr env.RepoStateReader, ddb *doltdb.DoltDB, roots doltdb.Roots, - name, email, msg, commitSpecStr string, + name, email, commitSpecStr string, date time.Time, opts ...MergeSpecOpt, ) (*MergeSpec, error) { @@ -159,7 +150,6 @@ func NewMergeSpec( MergeC: mergeCM, StompedTblNames: stompedTblNames, WorkingDiffs: workingDiffs, - Msg: msg, Email: email, Name: name, Date: date, diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index ea15e779be..b357a46584 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -480,7 +480,6 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a roots, name, email, - msg, commitSpecStr, t, merge.WithSquash(apr.Contains(cli.SquashParam)), From 9f6a0adaf3f5e513b70aeae61600021f3b6f9f4c Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 16:19:11 -0700 Subject: [PATCH 12/25] More cleanup of msg --- .../doltcore/sqle/dprocedures/dolt_merge.go | 40 ++++++++++--------- .../doltcore/sqle/dprocedures/dolt_pull.go | 2 +- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index b357a46584..73931c7203 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -155,7 +155,7 @@ func doDoltMerge(ctx *sql.Context, args []string) (string, int, int, error) { msg = userMsg } - ws, commit, conflicts, fastForward, err := performMerge(ctx, sess, roots, ws, dbName, mergeSpec, apr.Contains(cli.NoCommitFlag), msg) + ws, commit, conflicts, fastForward, err := performMerge(ctx, sess, ws, dbName, mergeSpec, apr.Contains(cli.NoCommitFlag), msg) if err != nil || conflicts != 0 || fastForward != 0 { return commit, conflicts, fastForward, err } @@ -169,7 +169,15 @@ func doDoltMerge(ctx *sql.Context, args []string) (string, int, int, error) { // fast-forward was performed. This commits the working set if merge is successful and // 'no-commit' flag is not defined. // TODO FF merging commit with constraint violations requires `constraint verify` -func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, ws *doltdb.WorkingSet, dbName string, spec *merge.MergeSpec, noCommit bool, msg string) (*doltdb.WorkingSet, string, int, int, error) { +func performMerge( + ctx *sql.Context, + sess *dsess.DoltSession, + ws *doltdb.WorkingSet, + dbName string, + spec *merge.MergeSpec, + noCommit bool, + msg string, +) (*doltdb.WorkingSet, string, int, int, error) { // todo: allow merges even when an existing merge is uncommitted if ws.MergeActive() { return ws, "", noConflictsOrViolations, threeWayMerge, doltdb.ErrMergeActive @@ -197,7 +205,7 @@ func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, if canFF { if spec.NoFF { var commit *doltdb.Commit - ws, commit, err = executeNoFFMerge(ctx, sess, spec, dbName, ws, dbData, noCommit) + ws, commit, err = executeNoFFMerge(ctx, sess, spec, msg, dbName, ws, noCommit) if err == doltdb.ErrUnresolvedConflictsOrViolations { // if there are unresolved conflicts, write the resulting working set back to the session and return an // error message @@ -366,13 +374,13 @@ func executeFFMerge(ctx *sql.Context, dbName string, squash bool, ws *doltdb.Wor } func executeNoFFMerge( - ctx *sql.Context, - dSess *dsess.DoltSession, - spec *merge.MergeSpec, - dbName string, - ws *doltdb.WorkingSet, - dbData env.DbData, - noCommit bool, + ctx *sql.Context, + dSess *dsess.DoltSession, + spec *merge.MergeSpec, + msg string, + dbName string, + ws *doltdb.WorkingSet, + noCommit bool, ) (*doltdb.WorkingSet, *doltdb.Commit, error) { mergeRoot, err := spec.MergeC.GetRootValue(ctx) if err != nil { @@ -410,9 +418,9 @@ func executeNoFFMerge( return ws.WithStagedRoot(roots.Staged), nil, nil } - + pendingCommit, err := dSess.NewPendingCommit(ctx, dbName, roots, actions.CommitStagedProps{ - Message: spec.Msg, + Message: msg, Date: spec.Date, Force: spec.Force, Name: spec.Name, @@ -438,13 +446,7 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a ddb, ok := sess.GetDoltDB(ctx, dbName) dbData, ok := sess.GetDbData(ctx, dbName) - - msg, ok := apr.GetValue(cli.MessageArg) - if !ok { - // TODO probably change, but we can't open editor so it'll have to be automated - msg = "automatic SQL merge" - } - + var err error var name, email string if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index 02ad505ec6..c93b918c11 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -172,7 +172,7 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { } msg := fmt.Sprintf("Merge branch '%s' of %s into %s", pullSpec.Branch.GetPath(), pullSpec.Remote.Url, headRef.GetPath()) - ws, _, conflicts, fastForward, err = performMerge(ctx, sess, roots, ws, dbName, mergeSpec, apr.Contains(cli.NoCommitFlag), msg) + ws, _, conflicts, fastForward, err = performMerge(ctx, sess, ws, dbName, mergeSpec, apr.Contains(cli.NoCommitFlag), msg) if err != nil && !errors.Is(doltdb.ErrUpToDate, err) { return conflicts, fastForward, err } From 717da1c923a47fdb6a94a87c0e54a6e846f6f652 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 16:25:33 -0700 Subject: [PATCH 13/25] Cleaned up pull spec --- go/cmd/dolt/commands/pull.go | 13 ++--- go/libraries/doltcore/env/remotes.go | 48 +++++++++++++++---- .../doltcore/sqle/dprocedures/dolt_pull.go | 14 +++++- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 2114f3ecae..dd2ad58eb3 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -109,17 +109,18 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d return HandleVErrAndExitCode(verr, usage) } + remoteOnly := apr.NArg() == 1 pullSpec, err := env.NewPullSpec( ctx, dEnv.RepoStateReader(), remoteName, remoteRefName, - apr.Contains(cli.SquashParam), - apr.Contains(cli.NoFFParam), - apr.Contains(cli.NoCommitFlag), - apr.Contains(cli.NoEditFlag), - apr.Contains(cli.ForceFlag), - apr.NArg() == 1, + remoteOnly, + env.WithSquash(apr.Contains(cli.SquashParam)), + env.WithNoFF(apr.Contains(cli.NoFFParam)), + env.WithNoCommit(apr.Contains(cli.NoCommitFlag)), + env.WithNoEdit(apr.Contains(cli.NoEditFlag)), + env.WithForce(apr.Contains(cli.ForceFlag)), ) if err != nil { diff --git a/go/libraries/doltcore/env/remotes.go b/go/libraries/doltcore/env/remotes.go index d05f2ec50f..20e5e3953d 100644 --- a/go/libraries/doltcore/env/remotes.go +++ b/go/libraries/doltcore/env/remotes.go @@ -404,13 +404,44 @@ type PullSpec struct { type PullSpecOpt func(*PullSpec) +func WithSquash(squash bool) PullSpecOpt { + return func(ps *PullSpec) { + ps.Squash = squash + } +} + +func WithNoFF(noff bool) PullSpecOpt { + return func(ps *PullSpec) { + ps.NoFF = noff + } +} + +func WithNoCommit(nocommit bool) PullSpecOpt { + return func(ps *PullSpec) { + ps.NoCommit = nocommit + } +} + +func WithNoEdit(noedit bool) PullSpecOpt { + return func(ps *PullSpec) { + ps.NoEdit = noedit + } +} + +func WithForce(force bool) PullSpecOpt { + return func(ps *PullSpec) { + ps.Force = force + } +} + // NewPullSpec returns a PullSpec for the arguments given. This function validates remote and gets remoteRef // for given remoteRefName; if it's not defined, it uses current branch to get its upstream branch if it exists. func NewPullSpec( _ context.Context, rsr RepoStateReader, remoteName, remoteRefName string, - squash, noff, noCommit, noEdit, force, remoteOnly bool, + remoteOnly bool, + opts ...PullSpecOpt, ) (*PullSpec, error) { refSpecs, err := GetRefSpecs(rsr, remoteName) if err != nil { @@ -451,17 +482,18 @@ func NewPullSpec( remoteRef = ref.NewBranchRef(remoteRefName) } - return &PullSpec{ - Squash: squash, - NoFF: noff, - NoCommit: noCommit, - NoEdit: noEdit, + spec := &PullSpec{ RemoteName: remoteName, Remote: remote, RefSpecs: refSpecs, Branch: remoteRef, - Force: force, - }, nil + } + + for _, opt := range opts { + opt(spec) + } + + return spec, nil } func GetAbsRemoteUrl(fs filesys2.Filesys, cfg config.ReadableConfig, urlArg string) (string, string, error) { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index c93b918c11..8dba959480 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -76,7 +76,19 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { remoteRefName = apr.Arg(1) } - pullSpec, err := env.NewPullSpec(ctx, dbData.Rsr, remoteName, remoteRefName, apr.Contains(cli.SquashParam), apr.Contains(cli.NoFFParam), apr.Contains(cli.NoCommitFlag), apr.Contains(cli.NoEditFlag), apr.Contains(cli.ForceFlag), apr.NArg() == 1) + remoteOnly := apr.NArg() == 1 + pullSpec, err := env.NewPullSpec( + ctx, + dbData.Rsr, + remoteName, + remoteRefName, + remoteOnly, + env.WithSquash(apr.Contains(cli.SquashParam)), + env.WithNoFF(apr.Contains(cli.NoFFParam)), + env.WithNoCommit(apr.Contains(cli.NoCommitFlag)), + env.WithNoEdit(apr.Contains(cli.NoEditFlag)), + env.WithForce(apr.Contains(cli.ForceFlag)), + ) if err != nil { return noConflictsOrViolations, threeWayMerge, err } From c398225c62991dd5c8ab222c9b8eb9392a3d6da5 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 16:33:06 -0700 Subject: [PATCH 14/25] Bats tests for merging remote tracking branch --- integration-tests/bats/remotes.bats | 45 +++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/integration-tests/bats/remotes.bats b/integration-tests/bats/remotes.bats index 56b42d9436..d208ca219c 100644 --- a/integration-tests/bats/remotes.bats +++ b/integration-tests/bats/remotes.bats @@ -1057,6 +1057,43 @@ SQL [[ ! "$output" =~ "another test commit" ]] || false } +@test "remotes: merge with divergent head" { + dolt remote add test-remote http://localhost:50051/test-org/test-repo + dolt push test-remote main + dolt sql < Date: Mon, 11 Sep 2023 16:59:44 -0700 Subject: [PATCH 15/25] Dolt_pull() behavior and bats test --- .../doltcore/sqle/dprocedures/dolt_merge.go | 42 +++++--- .../doltcore/sqle/dprocedures/dolt_pull.go | 101 ++++++++++++++++-- integration-tests/bats/remotes.bats | 36 +++++++ 3 files changed, 157 insertions(+), 22 deletions(-) diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index 73931c7203..9aa2b7f532 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -310,7 +310,17 @@ func abortMerge(ctx *sql.Context, workingSet *doltdb.WorkingSet, roots doltdb.Ro return workingSet, nil } -func executeMerge(ctx *sql.Context, sess *dsess.DoltSession, dbName string, squash bool, head, cm *doltdb.Commit, cmSpec string, ws *doltdb.WorkingSet, opts editor.Options, workingDiffs map[string]hash.Hash) (*doltdb.WorkingSet, error) { +func executeMerge( + ctx *sql.Context, + sess *dsess.DoltSession, + dbName string, + squash bool, + head, cm *doltdb.Commit, + cmSpec string, + ws *doltdb.WorkingSet, + opts editor.Options, + workingDiffs map[string]hash.Hash, +) (*doltdb.WorkingSet, error) { result, err := merge.MergeCommits(ctx, head, cm, opts) if err != nil { switch err { @@ -446,17 +456,10 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a ddb, ok := sess.GetDoltDB(ctx, dbName) dbData, ok := sess.GetDbData(ctx, dbName) - - var err error - var name, email string - if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { - name, email, err = cli.ParseAuthor(authorStr) - if err != nil { - return nil, err - } - } else { - name = ctx.Client().User - email = fmt.Sprintf("%s@%s", ctx.Client().User, ctx.Client().Address) + + name, email, err := getNameAndEmail(ctx, apr) + if err != nil { + return nil, err } t := ctx.QueryTime() @@ -492,6 +495,21 @@ func createMergeSpec(ctx *sql.Context, sess *dsess.DoltSession, dbName string, a ) } +func getNameAndEmail(ctx *sql.Context, apr *argparser.ArgParseResults) (string, string, error) { + var err error + var name, email string + if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { + name, email, err = cli.ParseAuthor(authorStr) + if err != nil { + return "", "", err + } + } else { + name = ctx.Client().User + email = fmt.Sprintf("%s@%s", ctx.Client().User, ctx.Client().Address) + } + return name, email, nil +} + func mergeRootToWorking( ctx *sql.Context, dSess *dsess.DoltSession, diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index 8dba959480..169c772d43 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -20,6 +20,7 @@ import ( "fmt" "sync" + "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/go-mysql-server/sql" @@ -140,14 +141,22 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { return noConflictsOrViolations, threeWayMerge, err } + headRef, err := dbData.Rsr.CWBHeadRef() + if err != nil { + return noConflictsOrViolations, threeWayMerge, err + } + + msg := fmt.Sprintf("Merge branch '%s' of %s into %s", pullSpec.Branch.GetPath(), pullSpec.Remote.Url, headRef.GetPath()) + // TODO: this could be replaced with a canFF check to test for error err = dbData.Ddb.FastForward(ctx, remoteTrackRef, srcDBCommit) - if errors.Is(err, datas.ErrMergeNeeded) && pullSpec.Force { - h, err := srcDBCommit.HashOf() + if errors.Is(err, datas.ErrMergeNeeded) { + name, email, err := getNameAndEmail(ctx, apr) if err != nil { return noConflictsOrViolations, threeWayMerge, err } - err = dbData.Ddb.SetHead(ctx, branchRef, h) + + err = mergeRemoteTrackingBranch(ctx, sess, dbName, remoteTrackRef, srcDBCommit, name, email, msg) if err != nil { return noConflictsOrViolations, threeWayMerge, err } @@ -169,12 +178,7 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { if err != nil { return noConflictsOrViolations, threeWayMerge, err } - - headRef, err := dbData.Rsr.CWBHeadRef() - if err != nil { - return noConflictsOrViolations, threeWayMerge, err - } - + uncommittedChanges, _, _, err := actions.RootHasUncommittedChanges(roots) if err != nil { return noConflictsOrViolations, threeWayMerge, err @@ -183,7 +187,6 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { return noConflictsOrViolations, threeWayMerge, ErrUncommittedChanges.New() } - msg := fmt.Sprintf("Merge branch '%s' of %s into %s", pullSpec.Branch.GetPath(), pullSpec.Remote.Url, headRef.GetPath()) ws, _, conflicts, fastForward, err = performMerge(ctx, sess, ws, dbName, mergeSpec, apr.Contains(cli.NoCommitFlag), msg) if err != nil && !errors.Is(doltdb.ErrUpToDate, err) { return conflicts, fastForward, err @@ -211,6 +214,84 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { return conflicts, fastForward, nil } +// mergeRemoteTrackingBranch merges the |srcDBCommit| commit into the remote tracking branch with the ref provided +// TODO: none of this is transactional +func mergeRemoteTrackingBranch( + ctx *sql.Context, + sess *dsess.DoltSession, + dbName string, + remoteTrackRef ref.DoltRef, + srcDBCommit *doltdb.Commit, + name, email, msg string, +) error { + dbState, ok, err := sess.LookupDbState(ctx, dbName) + if err != nil { + return err + } else if !ok { + return sql.ErrDatabaseNotFound.New(dbName) + } + + dbData, ok := sess.GetDbData(ctx, dbName) + if !ok { + return sql.ErrDatabaseNotFound.New(dbName) + } + + remoteTrackingRefCommit, err := dbData.Ddb.ResolveCommitRef(ctx, remoteTrackRef) + if err != nil { + return err + } + + result, err := merge.MergeCommits(ctx, remoteTrackingRefCommit, srcDBCommit, dbState.EditOpts()) + if err != nil { + return err + } + + _, mergeRootHash, err := dbData.Ddb.WriteRootValue(ctx, result.Root) + if err != nil { + return err + } + + ts := datas.CommitNowFunc().Unix() + cm := &datas.CommitMeta{ + Name: name, + Email: email, + Timestamp: uint64(ts), + Description: msg, + UserTimestamp: ts, + } + + remoteTrackingHash, err := remoteTrackingRefCommit.HashOf() + if err != nil { + return err + } + + firstParent, err := doltdb.NewCommitSpec(remoteTrackingHash.String()) + if err != nil { + return err + } + + srcCommitHash, err := srcDBCommit.HashOf() + if err != nil { + return err + } + + secondParent, err := doltdb.NewCommitSpec(srcCommitHash.String()) + if err != nil { + return err + } + + parentSpecs := []*doltdb.CommitSpec{ + firstParent, + secondParent, + } + + _, err = dbData.Ddb.CommitWithParentSpecs(ctx, mergeRootHash, remoteTrackRef, parentSpecs, cm) + if err != nil { + return err + } + return nil +} + // TODO: remove this as it does not do anything useful func pullerProgFunc(ctx context.Context, statsCh <-chan pull.Stats) { for { diff --git a/integration-tests/bats/remotes.bats b/integration-tests/bats/remotes.bats index d208ca219c..3c9c476ec6 100644 --- a/integration-tests/bats/remotes.bats +++ b/integration-tests/bats/remotes.bats @@ -1057,6 +1057,42 @@ SQL [[ ! "$output" =~ "another test commit" ]] || false } +@test "remotes: dolt_pull() with divergent head" { + dolt remote add test-remote http://localhost:50051/test-org/test-repo + dolt push test-remote main + dolt sql < Date: Mon, 11 Sep 2023 17:35:15 -0700 Subject: [PATCH 16/25] Making sure divergent head pulls aren't a fast-forward --- integration-tests/bats/remotes.bats | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/remotes.bats b/integration-tests/bats/remotes.bats index 3c9c476ec6..cde038250e 100644 --- a/integration-tests/bats/remotes.bats +++ b/integration-tests/bats/remotes.bats @@ -1084,8 +1084,9 @@ SQL cd "dolt-repo-clones/test-repo" - run dolt sql -q "call dolt_pull('origin')" + run dolt sql -q "call dolt_pull('origin')" -r csv [ "$status" -eq 0 ] + [[ "$output" =~ '0,0' ]] || false run dolt log --oneline -n 1 [ "$status" -eq 0 ] @@ -1122,8 +1123,8 @@ SQL run dolt pull origin [ "$status" -eq 0 ] + [[ ! "$output" =~ "Fast-forward" ]] || false - dolt log --oneline -n 2 run dolt log --oneline -n 1 [ "$status" -eq 0 ] [[ "$output" =~ "Merge branch 'main' of" ]] || false From 83584088910699a7522a7376dc722eba30502d47 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 11 Sep 2023 17:36:14 -0700 Subject: [PATCH 17/25] Much simpler version with correct fast-forward output, thanks aaron --- go/cmd/dolt/commands/pull.go | 96 ++----------------- .../doltcore/sqle/dprocedures/dolt_pull.go | 86 +---------------- 2 files changed, 11 insertions(+), 171 deletions(-) diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index dd2ad58eb3..d144dd86f8 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" - "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -189,7 +188,12 @@ func pullHelper( err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { - err := mergeRemoteTrackingBranch(ctx, sqlCtx, queryist, dEnv, pullSpec, cliCtx, remoteTrackRef, srcDBCommit) + // If the remote tracking branch has diverged from the local copy, we just overwrite it + h, err := srcDBCommit.HashOf() + if err != nil { + return err + } + err = dEnv.DoltDB.SetHead(ctx, remoteTrackRef, h) if err != nil { return err } @@ -280,90 +284,4 @@ func pullHelper( } return nil -} - -// mergeRemoteTrackingBranch merges the |srcDBCommit| commit into the remote tracking branch with the ref provided -func mergeRemoteTrackingBranch( - ctx context.Context, - sqlCtx *sql.Context, - queryist cli.Queryist, - dEnv *env.DoltEnv, - pullSpec *env.PullSpec, - cliCtx cli.CliContext, - remoteTrackRef ref.DoltRef, - srcDBCommit *doltdb.Commit, -) error { - name, email, _ := env.GetNameAndEmail(dEnv.Config) - - suggestedMsg := fmt.Sprintf( - "Merge branch '%s' of %s into %s", - pullSpec.Branch.GetPath(), - pullSpec.Remote.Url, - remoteTrackRef.GetPath(), - ) - msg, err := getCommitMsgForMerge(sqlCtx, queryist, suggestedMsg, pullSpec.NoEdit, cliCtx) - if err != nil { - return err - } - - tmpDir, err := dEnv.TempTableFilesDir() - if err != nil { - return err - } - opts := editor.Options{Deaf: dEnv.BulkDbEaFactory(), Tempdir: tmpDir} - - remoteTrackingRefCommit, err := dEnv.DoltDB.ResolveCommitRef(ctx, remoteTrackRef) - if err != nil { - return err - } - - result, err := merge.MergeCommits(sqlCtx, remoteTrackingRefCommit, srcDBCommit, opts) - if err != nil { - return err - } - - _, mergeRootHash, err := dEnv.DoltDB.WriteRootValue(ctx, result.Root) - if err != nil { - return err - } - - ts := datas.CommitNowFunc().Unix() - cm := &datas.CommitMeta{ - Name: name, - Email: email, - Timestamp: uint64(ts), - Description: msg, - UserTimestamp: ts, - } - - remoteTrackingHash, err := remoteTrackingRefCommit.HashOf() - if err != nil { - return err - } - - firstParent, err := doltdb.NewCommitSpec(remoteTrackingHash.String()) - if err != nil { - return err - } - - srcCommitHash, err := srcDBCommit.HashOf() - if err != nil { - return err - } - - secondParent, err := doltdb.NewCommitSpec(srcCommitHash.String()) - if err != nil { - return err - } - - parentSpecs := []*doltdb.CommitSpec{ - firstParent, - secondParent, - } - - _, err = dEnv.DoltDB.CommitWithParentSpecs(ctx, mergeRootHash, remoteTrackRef, parentSpecs, cm) - if err != nil { - return err - } - return nil -} +} \ No newline at end of file diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index 169c772d43..b0c62ce1bd 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -20,7 +20,6 @@ import ( "fmt" "sync" - "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/go-mysql-server/sql" @@ -151,12 +150,13 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { // TODO: this could be replaced with a canFF check to test for error err = dbData.Ddb.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { - name, email, err := getNameAndEmail(ctx, apr) + // If the remote tracking branch has diverged from the local copy, we just overwrite it + // TODO: none of this is transactional + h, err := srcDBCommit.HashOf() if err != nil { return noConflictsOrViolations, threeWayMerge, err } - - err = mergeRemoteTrackingBranch(ctx, sess, dbName, remoteTrackRef, srcDBCommit, name, email, msg) + err = dbData.Ddb.SetHead(ctx, remoteTrackRef, h) if err != nil { return noConflictsOrViolations, threeWayMerge, err } @@ -214,84 +214,6 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { return conflicts, fastForward, nil } -// mergeRemoteTrackingBranch merges the |srcDBCommit| commit into the remote tracking branch with the ref provided -// TODO: none of this is transactional -func mergeRemoteTrackingBranch( - ctx *sql.Context, - sess *dsess.DoltSession, - dbName string, - remoteTrackRef ref.DoltRef, - srcDBCommit *doltdb.Commit, - name, email, msg string, -) error { - dbState, ok, err := sess.LookupDbState(ctx, dbName) - if err != nil { - return err - } else if !ok { - return sql.ErrDatabaseNotFound.New(dbName) - } - - dbData, ok := sess.GetDbData(ctx, dbName) - if !ok { - return sql.ErrDatabaseNotFound.New(dbName) - } - - remoteTrackingRefCommit, err := dbData.Ddb.ResolveCommitRef(ctx, remoteTrackRef) - if err != nil { - return err - } - - result, err := merge.MergeCommits(ctx, remoteTrackingRefCommit, srcDBCommit, dbState.EditOpts()) - if err != nil { - return err - } - - _, mergeRootHash, err := dbData.Ddb.WriteRootValue(ctx, result.Root) - if err != nil { - return err - } - - ts := datas.CommitNowFunc().Unix() - cm := &datas.CommitMeta{ - Name: name, - Email: email, - Timestamp: uint64(ts), - Description: msg, - UserTimestamp: ts, - } - - remoteTrackingHash, err := remoteTrackingRefCommit.HashOf() - if err != nil { - return err - } - - firstParent, err := doltdb.NewCommitSpec(remoteTrackingHash.String()) - if err != nil { - return err - } - - srcCommitHash, err := srcDBCommit.HashOf() - if err != nil { - return err - } - - secondParent, err := doltdb.NewCommitSpec(srcCommitHash.String()) - if err != nil { - return err - } - - parentSpecs := []*doltdb.CommitSpec{ - firstParent, - secondParent, - } - - _, err = dbData.Ddb.CommitWithParentSpecs(ctx, mergeRootHash, remoteTrackRef, parentSpecs, cm) - if err != nil { - return err - } - return nil -} - // TODO: remove this as it does not do anything useful func pullerProgFunc(ctx context.Context, statsCh <-chan pull.Stats) { for { From 7c71476bab4e4fe0939db265d5c884a70a562134 Mon Sep 17 00:00:00 2001 From: zachmu Date: Tue, 12 Sep 2023 00:47:49 +0000 Subject: [PATCH 18/25] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/merge.go | 20 +++---- go/cmd/dolt/commands/pull.go | 18 +++--- go/libraries/doltcore/env/remotes.go | 20 +++---- go/libraries/doltcore/merge/action.go | 24 ++++---- .../doltcore/sqle/dprocedures/dolt_merge.go | 58 +++++++++---------- .../doltcore/sqle/dprocedures/dolt_pull.go | 4 +- 6 files changed, 72 insertions(+), 72 deletions(-) diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index eac9700bb7..46a08580a8 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -769,11 +769,11 @@ func executeNoFFMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryis } pendingCommit, err := actions.GetCommitStaged(ctx, roots, ws, mergeParentCommits, dEnv.DbData().Ddb, actions.CommitStagedProps{ - Message: msg, - Date: spec.Date, - Force: spec.Force, - Name: spec.Name, - Email: spec.Email, + Message: msg, + Date: spec.Date, + Force: spec.Force, + Name: spec.Name, + Email: spec.Email, }) headRef, err := dEnv.RepoStateReader().CWBHeadRef() @@ -832,11 +832,11 @@ func executeMergeAndCommit(ctx context.Context, sqlCtx *sql.Context, queryist cl // getCommitMsgForMerge returns user defined message if exists; otherwise, get the commit message from editor. func getCommitMsgForMerge( - sqlCtx *sql.Context, - queryist cli.Queryist, - suggestedMsg string, - noEdit bool, - cliCtx cli.CliContext, + sqlCtx *sql.Context, + queryist cli.Queryist, + suggestedMsg string, + noEdit bool, + cliCtx cli.CliContext, ) (string, error) { msg, err := getCommitMessageFromEditor(sqlCtx, queryist, suggestedMsg, "", noEdit, cliCtx) if err != nil { diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index d144dd86f8..6ad1691ef6 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -121,7 +121,7 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d env.WithNoEdit(apr.Contains(cli.NoEditFlag)), env.WithForce(apr.Contains(cli.ForceFlag)), ) - + if err != nil { return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage) } @@ -135,12 +135,12 @@ func (cmd PullCmd) Exec(ctx context.Context, commandStr string, args []string, d // pullHelper splits pull into fetch, prepare merge, and merge to interleave printing func pullHelper( - ctx context.Context, - sqlCtx *sql.Context, - queryist cli.Queryist, - dEnv *env.DoltEnv, - pullSpec *env.PullSpec, - cliCtx cli.CliContext, + ctx context.Context, + sqlCtx *sql.Context, + queryist cli.Queryist, + dEnv *env.DoltEnv, + pullSpec *env.PullSpec, + cliCtx cli.CliContext, ) error { srcDB, err := pullSpec.Remote.GetRemoteDBWithoutCaching(ctx, dEnv.DoltDB.ValueReadWriter().Format(), dEnv) if err != nil { @@ -185,7 +185,7 @@ func pullHelper( return err } fmt.Println(h.String()) - + err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { // If the remote tracking branch has diverged from the local copy, we just overwrite it @@ -284,4 +284,4 @@ func pullHelper( } return nil -} \ No newline at end of file +} diff --git a/go/libraries/doltcore/env/remotes.go b/go/libraries/doltcore/env/remotes.go index 20e5e3953d..f5045addff 100644 --- a/go/libraries/doltcore/env/remotes.go +++ b/go/libraries/doltcore/env/remotes.go @@ -391,9 +391,9 @@ func GetTrackingRef(branchRef ref.DoltRef, remote Remote) (ref.DoltRef, error) { } type PullSpec struct { - Squash bool - NoFF bool - NoCommit bool + Squash bool + NoFF bool + NoCommit bool NoEdit bool Force bool RemoteName string @@ -437,11 +437,11 @@ func WithForce(force bool) PullSpecOpt { // NewPullSpec returns a PullSpec for the arguments given. This function validates remote and gets remoteRef // for given remoteRefName; if it's not defined, it uses current branch to get its upstream branch if it exists. func NewPullSpec( - _ context.Context, - rsr RepoStateReader, - remoteName, remoteRefName string, - remoteOnly bool, - opts ...PullSpecOpt, + _ context.Context, + rsr RepoStateReader, + remoteName, remoteRefName string, + remoteOnly bool, + opts ...PullSpecOpt, ) (*PullSpec, error) { refSpecs, err := GetRefSpecs(rsr, remoteName) if err != nil { @@ -488,11 +488,11 @@ func NewPullSpec( RefSpecs: refSpecs, Branch: remoteRef, } - + for _, opt := range opts { opt(spec) } - + return spec, nil } diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index 36d51dc5f9..f3ecbd4478 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -40,8 +40,8 @@ type MergeSpec struct { StompedTblNames []string WorkingDiffs map[string]hash.Hash Squash bool - NoFF bool - NoCommit bool + NoFF bool + NoCommit bool NoEdit bool Force bool Email string @@ -52,7 +52,7 @@ type MergeSpec struct { type MergeSpecOpt func(*MergeSpec) func WithNoFf(noFF bool) MergeSpecOpt { - return func(ms *MergeSpec) { + return func(ms *MergeSpec) { ms.NoFF = noFF } } @@ -93,13 +93,13 @@ func WithPullSpecOpts(pullSpec *env.PullSpec) MergeSpecOpt { // NewMergeSpec returns a MergeSpec with the arguments provided. func NewMergeSpec( - ctx context.Context, - rsr env.RepoStateReader, - ddb *doltdb.DoltDB, - roots doltdb.Roots, - name, email, commitSpecStr string, - date time.Time, - opts ...MergeSpecOpt, + ctx context.Context, + rsr env.RepoStateReader, + ddb *doltdb.DoltDB, + roots doltdb.Roots, + name, email, commitSpecStr string, + date time.Time, + opts ...MergeSpecOpt, ) (*MergeSpec, error) { headCS, err := doltdb.NewCommitSpec("HEAD") if err != nil { @@ -154,11 +154,11 @@ func NewMergeSpec( Name: name, Date: date, } - + for _, opt := range opts { opt(spec) } - + return spec, nil } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index 9aa2b7f532..e864c7bfdb 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -170,13 +170,13 @@ func doDoltMerge(ctx *sql.Context, args []string) (string, int, int, error) { // 'no-commit' flag is not defined. // TODO FF merging commit with constraint violations requires `constraint verify` func performMerge( - ctx *sql.Context, - sess *dsess.DoltSession, - ws *doltdb.WorkingSet, - dbName string, - spec *merge.MergeSpec, - noCommit bool, - msg string, + ctx *sql.Context, + sess *dsess.DoltSession, + ws *doltdb.WorkingSet, + dbName string, + spec *merge.MergeSpec, + noCommit bool, + msg string, ) (*doltdb.WorkingSet, string, int, int, error) { // todo: allow merges even when an existing merge is uncommitted if ws.MergeActive() { @@ -311,15 +311,15 @@ func abortMerge(ctx *sql.Context, workingSet *doltdb.WorkingSet, roots doltdb.Ro } func executeMerge( - ctx *sql.Context, - sess *dsess.DoltSession, - dbName string, - squash bool, - head, cm *doltdb.Commit, - cmSpec string, - ws *doltdb.WorkingSet, - opts editor.Options, - workingDiffs map[string]hash.Hash, + ctx *sql.Context, + sess *dsess.DoltSession, + dbName string, + squash bool, + head, cm *doltdb.Commit, + cmSpec string, + ws *doltdb.WorkingSet, + opts editor.Options, + workingDiffs map[string]hash.Hash, ) (*doltdb.WorkingSet, error) { result, err := merge.MergeCommits(ctx, head, cm, opts) if err != nil { @@ -384,13 +384,13 @@ func executeFFMerge(ctx *sql.Context, dbName string, squash bool, ws *doltdb.Wor } func executeNoFFMerge( - ctx *sql.Context, - dSess *dsess.DoltSession, - spec *merge.MergeSpec, - msg string, - dbName string, - ws *doltdb.WorkingSet, - noCommit bool, + ctx *sql.Context, + dSess *dsess.DoltSession, + spec *merge.MergeSpec, + msg string, + dbName string, + ws *doltdb.WorkingSet, + noCommit bool, ) (*doltdb.WorkingSet, *doltdb.Commit, error) { mergeRoot, err := spec.MergeC.GetRootValue(ctx) if err != nil { @@ -428,13 +428,13 @@ func executeNoFFMerge( return ws.WithStagedRoot(roots.Staged), nil, nil } - + pendingCommit, err := dSess.NewPendingCommit(ctx, dbName, roots, actions.CommitStagedProps{ - Message: msg, - Date: spec.Date, - Force: spec.Force, - Name: spec.Name, - Email: spec.Email, + Message: msg, + Date: spec.Date, + Force: spec.Force, + Name: spec.Name, + Email: spec.Email, }) if err != nil { return nil, nil, err diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go index b0c62ce1bd..04c8d0e8d9 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_pull.go @@ -20,7 +20,6 @@ import ( "fmt" "sync" - "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/dolt/go/cmd/dolt/cli" @@ -30,6 +29,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/doltcore/ref" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" + "github.com/dolthub/dolt/go/store/datas" "github.com/dolthub/dolt/go/store/datas/pull" ) @@ -178,7 +178,7 @@ func doDoltPull(ctx *sql.Context, args []string) (int, int, error) { if err != nil { return noConflictsOrViolations, threeWayMerge, err } - + uncommittedChanges, _, _, err := actions.RootHasUncommittedChanges(roots) if err != nil { return noConflictsOrViolations, threeWayMerge, err From 39cf00505f8b63aeb2dd72c00c4bb429f0ccac5b Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 10:12:57 -0700 Subject: [PATCH 19/25] Removed redundant tests left over from stored procedure migration --- integration-tests/bats/sql-pull.bats | 153 +-------------------------- 1 file changed, 2 insertions(+), 151 deletions(-) diff --git a/integration-tests/bats/sql-pull.bats b/integration-tests/bats/sql-pull.bats index f1fcf5e5c6..3884afe21a 100644 --- a/integration-tests/bats/sql-pull.bats +++ b/integration-tests/bats/sql-pull.bats @@ -49,17 +49,7 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull main" { - cd repo2 - dolt sql -q "CALL dolt_pull('origin')" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - -@test "sql-pull: CALL dpull main" { +@test "sql-pull: dpull main" { cd repo2 dolt sql -q "CALL dpull('origin')" run dolt sql -q "show tables" -r csv @@ -79,16 +69,6 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull custom remote" { - cd repo2 - dolt sql -q "CALL dolt_pull('test-remote')" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - @test "sql-pull: dolt_pull default origin" { cd repo2 dolt remote remove test-remote @@ -100,17 +80,6 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull default origin" { - cd repo2 - dolt remote remove test-remote - dolt sql -q "CALL dolt_pull()" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - @test "sql-pull: dolt_pull default custom remote" { cd repo2 dolt remote remove origin @@ -122,17 +91,6 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull default custom remote" { - cd repo2 - dolt remote remove origin - dolt sql -q "CALL dolt_pull()" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - @test "sql-pull: dolt_pull up to date does not error" { cd repo2 dolt sql -q "call dolt_pull('origin')" @@ -144,17 +102,6 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull up to date does not error" { - cd repo2 - dolt sql -q "CALL dolt_pull('origin')" - dolt sql -q "CALL dolt_pull('origin')" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - @test "sql-pull: dolt_pull unknown remote fails" { cd repo2 run dolt sql -q "call dolt_pull('unknown')" @@ -163,14 +110,6 @@ teardown() { [[ ! "$output" =~ "panic" ]] || false } -@test "sql-pull: CALL dolt_pull unknown remote fails" { - cd repo2 - run dolt sql -q "CALL dolt_pull('unknown')" - [ "$status" -eq 1 ] - [[ "$output" =~ "unknown remote" ]] || false - [[ ! "$output" =~ "panic" ]] || false -} - @test "sql-pull: dolt_pull unknown feature branch fails" { cd repo2 dolt checkout feature @@ -180,15 +119,6 @@ teardown() { [[ ! "$output" =~ "panic" ]] || false } -@test "sql-pull: CALL dolt_pull unknown feature branch fails" { - cd repo2 - dolt checkout feature - run dolt sql -q "CALL dolt_pull('origin')" - [ "$status" -eq 1 ] - [[ "$output" =~ "You asked to pull from the remote 'origin', but did not specify a branch" ]] || false - [[ ! "$output" =~ "panic" ]] || false -} - @test "sql-pull: dolt_pull feature branch" { cd repo1 dolt checkout feature @@ -211,29 +141,7 @@ teardown() { [[ "$output" =~ "t1" ]] || false } -@test "sql-pull: CALL dolt_pull feature branch" { - cd repo1 - dolt checkout feature - dolt push --set-upstream origin feature - - cd ../repo2 - dolt checkout feature - dolt push --set-upstream origin feature - - cd ../repo1 - dolt merge main - dolt push - - cd ../repo2 - dolt sql -q "CALL dolt_pull('origin')" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - -@test "sql-pull: CALL dolt_checkout after dolt_fetch a new feature branch" { +@test "sql-pull: dolt_checkout after dolt_fetch a new feature branch" { cd repo1 dolt checkout -b feature2 dolt sql -q "create table t2 (i int primary key);" @@ -264,36 +172,6 @@ teardown() { dolt commit -am "2.1 commit" dolt push -f origin main - cd ../repo2 - run dolt sql -q "call dolt_pull('origin')" - [ "$status" -eq 1 ] - [[ ! "$output" =~ "panic" ]] || false - [[ "$output" =~ "fetch failed; dataset head is not ancestor of commit" ]] || false - - dolt sql -q "call dolt_pull('-f', 'origin')" - - run dolt log -n 1 - [ "$status" -eq 0 ] - [[ "$output" =~ "2.1 commit" ]] || false - - run dolt sql -q "show tables" -r csv - [ "${#lines[@]}" -eq 4 ] - [[ "$output" =~ "t3" ]] || false -} - -@test "sql-pull: CALL dolt_pull force" { - skip "todo: support dolt pull --force (cli too)" - cd repo2 - dolt sql -q "create table t2 (a int)" - dolt commit -am "2.0 commit" - dolt push origin main - - cd ../repo1 - dolt sql -q "create table t2 (a int primary key)" - dolt sql -q "create table t3 (a int primary key)" - dolt commit -am "2.1 commit" - dolt push -f origin main - cd ../repo2 run dolt sql -q "CALL dolt_pull('origin')" [ "$status" -eq 1 ] @@ -311,17 +189,6 @@ teardown() { [[ "$output" =~ "t3" ]] || false } -@test "sql-pull: dolt_pull squash" { - skip "todo: support dolt pull --squash (cli too)" - cd repo2 - dolt sql -q "call dolt_pull('--squash', 'origin')" - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - @test "sql-pull: CALL dolt_pull squash" { skip "todo: support dolt pull --squash (cli too)" cd repo2 @@ -334,22 +201,6 @@ teardown() { } @test "sql-pull: dolt_pull --noff flag" { - cd repo2 - dolt sql -q "call dolt_pull('--no-ff', 'origin')" - dolt status - run dolt log -n 1 - [ "$status" -eq 0 ] - # TODO change the default message name - [[ "$output" =~ "automatic SQL merge" ]] || false - - run dolt sql -q "show tables" -r csv - [ "$status" -eq 0 ] - [ "${#lines[@]}" -eq 2 ] - [[ "$output" =~ "Table" ]] || false - [[ "$output" =~ "t1" ]] || false -} - -@test "sql-pull: CALL dolt_pull --noff flag" { cd repo2 dolt sql -q "CALL dolt_pull('--no-ff', 'origin')" dolt status From 8aa8ebd3370b101e9aa62e06e12721b1a9fd5a25 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 10:19:30 -0700 Subject: [PATCH 20/25] Fix test --- integration-tests/bats/sql-pull.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/bats/sql-pull.bats b/integration-tests/bats/sql-pull.bats index 3884afe21a..e914ccbf31 100644 --- a/integration-tests/bats/sql-pull.bats +++ b/integration-tests/bats/sql-pull.bats @@ -204,10 +204,10 @@ teardown() { cd repo2 dolt sql -q "CALL dolt_pull('--no-ff', 'origin')" dolt status + run dolt log -n 1 [ "$status" -eq 0 ] - # TODO change the default message name - [[ "$output" =~ "automatic SQL merge" ]] || false + [[ "$output" =~ "Merge branch 'main'" ]] || false run dolt sql -q "show tables" -r csv [ "$status" -eq 0 ] From b38b0029309f4e7af5bbfd803d11743ac542ae30 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 11:18:07 -0700 Subject: [PATCH 21/25] Update go/libraries/doltcore/sqle/dprocedures/dolt_merge.go Co-authored-by: Aaron Son --- go/libraries/doltcore/sqle/dprocedures/dolt_merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index e864c7bfdb..a470ecc0c1 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -488,7 +488,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.WithNoFF(apr.Contains(cli.NoFFParam)), merge.WithForce(apr.Contains(cli.ForceFlag)), merge.WithNoCommit(apr.Contains(cli.NoCommitFlag)), merge.WithNoEdit(apr.Contains(cli.NoEditFlag)), From 1358aa845f8cb08b8ccb9e583c3250868a3a8ff8 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 11:18:12 -0700 Subject: [PATCH 22/25] Update go/libraries/doltcore/merge/action.go Co-authored-by: Aaron Son --- go/libraries/doltcore/merge/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/merge/action.go b/go/libraries/doltcore/merge/action.go index f3ecbd4478..02fbc7654c 100644 --- a/go/libraries/doltcore/merge/action.go +++ b/go/libraries/doltcore/merge/action.go @@ -51,7 +51,7 @@ type MergeSpec struct { type MergeSpecOpt func(*MergeSpec) -func WithNoFf(noFF bool) MergeSpecOpt { +func WithNoFF(noFF bool) MergeSpecOpt { return func(ms *MergeSpec) { ms.NoFF = noFF } From a59ba569e22ed20808c893e86125884c7931f71b Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 11:18:19 -0700 Subject: [PATCH 23/25] Update go/cmd/dolt/commands/pull.go Co-authored-by: Aaron Son --- go/cmd/dolt/commands/pull.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 6ad1691ef6..05858756c7 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -184,7 +184,6 @@ func pullHelper( if err != nil { return err } - fmt.Println(h.String()) err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { From b0b898d837f0749b4731b64fa9b635965f2706d7 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 12 Sep 2023 11:36:09 -0700 Subject: [PATCH 24/25] Remove dead code --- go/cmd/dolt/commands/pull.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 05858756c7..2deba12d78 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -179,12 +179,7 @@ func pullHelper( if err != nil { return err } - - h, err := srcDBCommit.HashOf() - if err != nil { - return err - } - + err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { // If the remote tracking branch has diverged from the local copy, we just overwrite it From 004d5b904666e3f49c0f044bff3513ede9714054 Mon Sep 17 00:00:00 2001 From: zachmu Date: Tue, 12 Sep 2023 18:53:22 +0000 Subject: [PATCH 25/25] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/cmd/dolt/commands/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/pull.go b/go/cmd/dolt/commands/pull.go index 2deba12d78..d228e78783 100644 --- a/go/cmd/dolt/commands/pull.go +++ b/go/cmd/dolt/commands/pull.go @@ -179,7 +179,7 @@ func pullHelper( if err != nil { return err } - + err = dEnv.DoltDB.FastForward(ctx, remoteTrackRef, srcDBCommit) if errors.Is(err, datas.ErrMergeNeeded) { // If the remote tracking branch has diverged from the local copy, we just overwrite it