diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 9aa11137f7..0f9fa4465f 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -151,7 +151,7 @@ func CreateCleanArgParser() *argparser.ArgParser { func CreateCheckoutArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithVariableArgs("checkout") - ap.SupportsString(CheckoutCoBranch, "", "branch", "Create a new branch named {{.LessThan}}new_branch{{.GreaterThan}} and start it at {{.LessThan}}start_point{{.GreaterThan}}.") + ap.SupportsString(CheckoutCreateBranch, "", "branch", "Create a new branch named {{.LessThan}}new_branch{{.GreaterThan}} and start it at {{.LessThan}}start_point{{.GreaterThan}}.") ap.SupportsFlag(ForceFlag, "f", "If there is any changes in working set, the force flag will wipe out the current changes and checkout the new branch.") ap.SupportsString(TrackFlag, "t", "", "When creating a new branch, set up 'upstream' configuration.") return ap diff --git a/go/cmd/dolt/cli/flags.go b/go/cmd/dolt/cli/flags.go index 16c6378817..a7ae9f6aea 100644 --- a/go/cmd/dolt/cli/flags.go +++ b/go/cmd/dolt/cli/flags.go @@ -17,53 +17,53 @@ package cli // Constants for command line flags names. These tend to be used in multiple places, so defining // them low in the package dependency tree makes sense. const ( - AbortParam = "abort" - AllFlag = "all" - AllowEmptyFlag = "allow-empty" - AmendFlag = "amend" - AuthorParam = "author" - BranchParam = "branch" - CachedFlag = "cached" - CheckoutCoBranch = "b" - CommitFlag = "commit" - CopyFlag = "copy" - DateParam = "date" - DecorateFlag = "decorate" - DeleteFlag = "delete" - DeleteForceFlag = "D" - DryRunFlag = "dry-run" - ForceFlag = "force" - HardResetParam = "hard" - HostFlag = "host" - ListFlag = "list" - MergesFlag = "merges" - MessageArg = "message" - MinParentsFlag = "min-parents" - MoveFlag = "move" - NoCommitFlag = "no-commit" - NoEditFlag = "no-edit" - NoFFParam = "no-ff" - NoPrettyFlag = "no-pretty" - NoTLSFlag = "no-tls" - NotFlag = "not" - NumberFlag = "number" - OneLineFlag = "oneline" - OursFlag = "ours" - OutputOnlyFlag = "output-only" - ParentsFlag = "parents" - PasswordFlag = "password" - PortFlag = "port" - PruneFlag = "prune" - RemoteParam = "remote" - SetUpstreamFlag = "set-upstream" - ShallowFlag = "shallow" - ShowIgnoredFlag = "ignored" - SkipEmptyFlag = "skip-empty" - SoftResetParam = "soft" - SquashParam = "squash" - TablesFlag = "tables" - TheirsFlag = "theirs" - TrackFlag = "track" - UpperCaseAllFlag = "ALL" - UserFlag = "user" + AbortParam = "abort" + AllFlag = "all" + AllowEmptyFlag = "allow-empty" + AmendFlag = "amend" + AuthorParam = "author" + BranchParam = "branch" + CachedFlag = "cached" + CheckoutCreateBranch = "b" + CommitFlag = "commit" + CopyFlag = "copy" + DateParam = "date" + DecorateFlag = "decorate" + DeleteFlag = "delete" + DeleteForceFlag = "D" + DryRunFlag = "dry-run" + ForceFlag = "force" + HardResetParam = "hard" + HostFlag = "host" + ListFlag = "list" + MergesFlag = "merges" + MessageArg = "message" + MinParentsFlag = "min-parents" + MoveFlag = "move" + NoCommitFlag = "no-commit" + NoEditFlag = "no-edit" + NoFFParam = "no-ff" + NoPrettyFlag = "no-pretty" + NoTLSFlag = "no-tls" + NotFlag = "not" + NumberFlag = "number" + OneLineFlag = "oneline" + OursFlag = "ours" + OutputOnlyFlag = "output-only" + ParentsFlag = "parents" + PasswordFlag = "password" + PortFlag = "port" + PruneFlag = "prune" + RemoteParam = "remote" + SetUpstreamFlag = "set-upstream" + ShallowFlag = "shallow" + ShowIgnoredFlag = "ignored" + SkipEmptyFlag = "skip-empty" + SoftResetParam = "soft" + SquashParam = "squash" + TablesFlag = "tables" + TheirsFlag = "theirs" + TrackFlag = "track" + UpperCaseAllFlag = "ALL" + UserFlag = "user" ) diff --git a/go/cmd/dolt/commands/checkout.go b/go/cmd/dolt/commands/checkout.go index 18ebb5b021..fb06c68937 100644 --- a/go/cmd/dolt/commands/checkout.go +++ b/go/cmd/dolt/commands/checkout.go @@ -106,15 +106,15 @@ func (cmd CheckoutCmd) Exec(ctx context.Context, commandStr string, args []strin return 1 } - branchOrTrack := apr.Contains(cli.CheckoutCoBranch) || apr.Contains(cli.TrackFlag) + branchOrTrack := apr.Contains(cli.CheckoutCreateBranch) || apr.Contains(cli.TrackFlag) if (branchOrTrack && apr.NArg() > 1) || (!branchOrTrack && apr.NArg() == 0) { usagePrt() return 1 } var branchName string - if apr.Contains(cli.CheckoutCoBranch) { - branchName, _ = apr.GetValue(cli.CheckoutCoBranch) + if apr.Contains(cli.CheckoutCreateBranch) { + branchName, _ = apr.GetValue(cli.CheckoutCreateBranch) } else if apr.Contains(cli.TrackFlag) { if apr.NArg() > 0 { usagePrt() diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go b/go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go index 72f7ebbcdc..27a82ea8c9 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_checkout.go @@ -71,7 +71,7 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes return 1, "", err } - branchOrTrack := apr.Contains(cli.CheckoutCoBranch) || apr.Contains(cli.TrackFlag) + branchOrTrack := apr.Contains(cli.CheckoutCreateBranch) || apr.Contains(cli.TrackFlag) if apr.Contains(cli.TrackFlag) && apr.NArg() > 0 { return 1, "", errors.New("Improper usage.") } @@ -85,6 +85,15 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes return 1, "", fmt.Errorf("Could not load database %s", currentDbName) } + // Prevent the -b option from being used to create new branches on read-only databases + readOnlyDatabase, err := isReadOnlyDatabase(ctx, currentDbName) + if err != nil { + return 1, "", err + } + if apr.Contains(cli.CheckoutCreateBranch) && readOnlyDatabase { + return 1, "", fmt.Errorf("unable to create new branch in a read-only database") + } + updateHead := apr.Contains(cli.MoveFlag) var rsc doltdb.ReplicationStatusController @@ -190,6 +199,19 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes return 0, successMessage, nil } +// isReadOnlyDatabase returns true if the named database is a read-only database. An error is returned +// if any issues are encountered while looking up the named database. +func isReadOnlyDatabase(ctx *sql.Context, dbName string) (bool, error) { + doltSession := dsess.DSessFromSess(ctx.Session) + db, err := doltSession.Provider().Database(ctx, dbName) + if err != nil { + return false, err + } + + rodb, ok := db.(sql.ReadOnlyDatabase) + return ok && rodb.IsReadOnly(), nil +} + // createWorkingSetForLocalBranch will make a new working set for a local // branch ref if one does not already exist. Can be used to fix up local branch // state when branches have been created without working sets in the past. @@ -324,7 +346,7 @@ func checkoutNewBranch(ctx *sql.Context, dbName string, dbData env.DbData, apr * newBranchName = remoteBranchName } - if newBranch, ok := apr.GetValue(cli.CheckoutCoBranch); ok { + if newBranch, ok := apr.GetValue(cli.CheckoutCreateBranch); ok { if len(newBranch) == 0 { return "", "", ErrEmptyBranchName } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_reset.go b/go/libraries/doltcore/sqle/dprocedures/dolt_reset.go index 3bd1884a12..f22e7f79d3 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_reset.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_reset.go @@ -62,19 +62,15 @@ func doDoltReset(ctx *sql.Context, args []string) (int, error) { return 1, fmt.Errorf("error: --%s and --%s are mutually exclusive options.", cli.HardResetParam, cli.SoftResetParam) } - provider := dSess.Provider() - db, err := provider.Database(ctx, dbName) - if err != nil { - return 1, err - } - // Disallow manipulating any roots for read-only databases – changing the branch // HEAD would allow changing data, and working set and index shouldn't ever have // any contents for a read-only database. - if rodb, ok := db.(sql.ReadOnlyDatabase); ok { - if rodb.IsReadOnly() { - return 1, fmt.Errorf("unable to reset HEAD in read-only databases") - } + isReadOnly, err := isReadOnlyDatabase(ctx, dbName) + if err != nil { + return 1, err + } + if isReadOnly { + return 1, fmt.Errorf("unable to reset HEAD in read-only databases") } // Get all the needed roots. diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 2846eab853..51cc932303 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -1701,6 +1701,17 @@ func TestDoltCheckout(t *testing.T) { enginetest.TestScript(t, h, script) }() } + + h := newDoltHarness(t) + defer h.Close() + engine, err := h.NewEngine(t) + require.NoError(t, err) + readOnlyEngine, err := h.NewReadOnlyEngine(engine.EngineAnalyzer().Catalog.DbProvider) + require.NoError(t, err) + + for _, script := range DoltCheckoutReadOnlyScripts { + enginetest.TestScriptWithEngine(t, readOnlyEngine, h, script) + } } func TestDoltCheckoutPrepared(t *testing.T) { @@ -1711,6 +1722,17 @@ func TestDoltCheckoutPrepared(t *testing.T) { enginetest.TestScriptPrepared(t, h, script) }() } + + h := newDoltHarness(t) + defer h.Close() + engine, err := h.NewEngine(t) + require.NoError(t, err) + readOnlyEngine, err := h.NewReadOnlyEngine(engine.EngineAnalyzer().Catalog.DbProvider) + require.NoError(t, err) + + for _, script := range DoltCheckoutReadOnlyScripts { + enginetest.TestScriptWithEnginePrepared(t, readOnlyEngine, h, script) + } } func TestDoltBranch(t *testing.T) { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go index 17978a72d5..8df70bccc0 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries.go @@ -2676,6 +2676,18 @@ var DoltCheckoutScripts = []queries.ScriptTest{ }, } +var DoltCheckoutReadOnlyScripts = []queries.ScriptTest{ + { + Name: "dolt checkout -b returns an error for read-only databases", + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_checkout('-b', 'newBranch');", + ExpectedErrStr: "unable to create new branch in a read-only database", + }, + }, + }, +} + var DoltInfoSchemaScripts = []queries.ScriptTest{ { Name: "info_schema changes with dolt_checkout", diff --git a/integration-tests/bats/sql-server.bats b/integration-tests/bats/sql-server.bats index 078f77140b..cd129c6d33 100644 --- a/integration-tests/bats/sql-server.bats +++ b/integration-tests/bats/sql-server.bats @@ -852,6 +852,11 @@ SQL [ $status -ne 0 ] [[ $output =~ "read-only" ]] || false + # dolt checkout can't create new branches on a read only database + run dolt sql-client --use-db "repo1/$hash" -u dolt -P $PORT -q "call dolt_checkout('-b', 'newBranch');" + [ $status -ne 0 ] + [[ $output =~ "unable to create new branch in a read-only database" ]] || false + # server should still be alive after an error run dolt sql-client --use-db "repo1/$hash" -u dolt -P $PORT -q "select count(*) from test" [ $status -eq 0 ]