diff --git a/go/cmd/dolt/cli/arg_parser_helpers.go b/go/cmd/dolt/cli/arg_parser_helpers.go index 6257a230e0..bd13ef2ee8 100644 --- a/go/cmd/dolt/cli/arg_parser_helpers.go +++ b/go/cmd/dolt/cli/arg_parser_helpers.go @@ -77,6 +77,7 @@ func ParseAuthor(authorStr string) (string, string, error) { const ( AllowEmptyFlag = "allow-empty" + SkipEmptyFlag = "skip-empty" DateParam = "date" MessageArg = "message" AuthorParam = "author" @@ -140,7 +141,8 @@ var branchForceFlagDesc = "Reset {{.LessThan}}branchname{{.GreaterThan}} to {{.L func CreateCommitArgParser() *argparser.ArgParser { ap := argparser.NewArgParserWithMaxArgs("commit", 0) ap.SupportsString(MessageArg, "m", "msg", "Use the given {{.LessThan}}msg{{.GreaterThan}} as the commit message.") - ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety.") + ap.SupportsFlag(AllowEmptyFlag, "", "Allow recording a commit that has the exact same data as its sole parent. This is usually a mistake, so it is disabled by default. This option bypasses that safety. Cannot be used with --skip-empty.") + ap.SupportsFlag(SkipEmptyFlag, "", "Only create a commit if there are staged changes. If no changes are staged, the call to commit is a no-op. Cannot be used with --allow-empty.") ap.SupportsString(DateParam, "", "date", "Specify the date used in the commit. If not specified the current system time is used.") ap.SupportsFlag(ForceFlag, "f", "Ignores any foreign key warnings and proceeds with the commit.") ap.SupportsString(AuthorParam, "", "author", "Specify an explicit author using the standard A U Thor {{.LessThan}}author@example.com{{.GreaterThan}} format.") @@ -403,3 +405,13 @@ func VerifyNoAwsParams(apr *argparser.ArgParseResults) error { return nil } + +// VerifyCommitArgs validates the arguments in |apr| for `dolt commit` and returns an error +// if any validation problems were encountered. +func VerifyCommitArgs(apr *argparser.ArgParseResults) error { + if apr.Contains(AllowEmptyFlag) && apr.Contains(SkipEmptyFlag) { + return fmt.Errorf("error: cannot use both --allow-empty and --skip-empty") + } + + return nil +} diff --git a/go/cmd/dolt/commands/commit.go b/go/cmd/dolt/commands/commit.go index 7cf9cf1ae9..ad468cb0a4 100644 --- a/go/cmd/dolt/commands/commit.go +++ b/go/cmd/dolt/commands/commit.go @@ -77,41 +77,53 @@ func (cmd CommitCmd) ArgParser() *argparser.ArgParser { // Exec executes the command func (cmd CommitCmd) Exec(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv, cliCtx cli.CliContext) int { - res := performCommit(ctx, commandStr, args, dEnv) + res, skipped := performCommit(ctx, commandStr, args, dEnv) if res == 1 { return res } + if skipped { + iohelp.WriteLine(cli.CliOut, "Skipping empty commit") + return res + } + // if the commit was successful, print it out using the log command return LogCmd{}.Exec(ctx, "log", []string{"-n=1"}, dEnv, nil) } -func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) int { +// performCommit creates a new Dolt commit using the specified |commandStr| and |args| for the specified Dolt environment +// |dEnv|. The response is an integer status code indicating success or failure, as well as a boolean that indicates +// if the commit was skipped (e.g. because --skip-empty was specified as an argument). +func performCommit(ctx context.Context, commandStr string, args []string, dEnv *env.DoltEnv) (int, bool) { ap := cli.CreateCommitArgParser() help, usage := cli.HelpAndUsagePrinters(cli.CommandDocsForCommandString(commandStr, commitDocs, ap)) apr := cli.ParseArgsOrDie(ap, args, help) + if err := cli.VerifyCommitArgs(apr); err != nil { + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), help), false + } + allFlag := apr.Contains(cli.AllFlag) upperCaseAllFlag := apr.Contains(cli.UpperCaseAllFlag) if dEnv.IsLocked() { - return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help) + return HandleVErrAndExitCode(errhand.VerboseErrorFromError(env.ErrActiveServerLock.New(dEnv.LockFile())), help), false } roots, err := dEnv.Roots(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working root").AddCause(err).Build(), usage), false } if upperCaseAllFlag { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } else if allFlag { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return handleCommitErr(ctx, dEnv, err, help) + return handleCommitErr(ctx, dEnv, err, help), false } } @@ -125,13 +137,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * } else { // This command creates a commit, so we need user identity if !cli.CheckUserNameAndEmail(dEnv) { - return 1 + return 1, false } name, email, err = env.GetNameAndEmail(dEnv.Config) } if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } msg, msgOk := apr.GetValue(cli.MessageArg) @@ -140,13 +152,13 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { commitMeta, cmErr := headCommit.GetCommitMeta(ctx) if cmErr != nil { - return handleCommitErr(ctx, dEnv, cmErr, usage) + return handleCommitErr(ctx, dEnv, cmErr, usage), false } amendStr = commitMeta.Description } msg, err = getCommitMessageFromEditor(ctx, dEnv, "", amendStr, false) if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } } @@ -156,7 +168,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * t, err = cli.ParseDate(commitTimeStr) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("error: invalid date").AddCause(err).Build(), usage), false } } @@ -172,23 +184,23 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * newRoots, err := actions.ResetSoftToRef(ctx, dEnv.DbData(), "HEAD~1") if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } ws, err := dEnv.WorkingSet(ctx) if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } prevHash, err := ws.HashOf() if err != nil { - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't get working set").AddCause(err).Build(), usage), false } var mergeParentCommits []*doltdb.Commit @@ -202,6 +214,7 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag) || apr.Contains(cli.AmendFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, @@ -210,20 +223,26 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false + } + + // If no error was reported and there is no pending commit, then no commit was created, likely + // because of --skip-empty, so return a success code, but indicate that no commit was created. + if pendingCommit == nil { + return 0, true } headRef, err := dEnv.RepoStateReader().CWBHeadRef() if err != nil { - return handleCommitErr(ctx, dEnv, err, usage) + return handleCommitErr(ctx, dEnv, err, usage), false } _, err = dEnv.DoltDB.CommitWithWorkingSet( ctx, @@ -239,18 +258,18 @@ func performCommit(ctx context.Context, commandStr string, args []string, dEnv * if apr.Contains(cli.AmendFlag) { newRoots, errRes := actions.ResetSoftToRef(ctx, dEnv.DbData(), headHash.String()) if errRes != nil { - return handleResetError(errRes, usage) + return handleResetError(errRes, usage), false } err = dEnv.UpdateStagedRoot(ctx, newRoots.Staged) if err != nil { - return handleResetError(err, usage) + return handleResetError(err, usage), false } } - return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage) + return HandleVErrAndExitCode(errhand.BuildDError("Couldn't commit").AddCause(err).Build(), usage), false } - return 0 + return 0, false } func handleCommitErr(ctx context.Context, dEnv *env.DoltEnv, err error, usage cli.UsagePrinter) int { diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index 93651de708..bee68abe10 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -580,8 +580,8 @@ func executeMergeAndCommit(ctx context.Context, dEnv *env.DoltEnv, spec *merge.M author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - res := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) - if res != 0 { + res, skipped := performCommit(ctx, "commit", []string{"-m", msg, "--author", author}, dEnv) + if res != 0 || skipped { return nil, fmt.Errorf("dolt commit failed after merging") } diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index f348f71dc3..3fda0d3d9b 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -27,6 +27,7 @@ type CommitStagedProps struct { Message string Date time.Time AllowEmpty bool + SkipEmpty bool Amend bool Force bool Name string @@ -62,6 +63,10 @@ func GetCommitStaged( isEmpty := len(staged) == 0 allowEmpty := ws.MergeActive() || props.AllowEmpty || props.Amend + + if isEmpty && props.SkipEmpty { + return nil, nil + } if isEmpty && !allowEmpty { return nil, NothingStaged{notStaged} } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go index c73d138f4c..b5d691b638 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go @@ -93,7 +93,8 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, error) { return "", fmt.Errorf("dolt add failed") } - return doDoltCommit(ctx, []string{"-m", commitMsg}) + commitHash, _, err := doDoltCommit(ctx, []string{"-m", commitMsg}) + return commitHash, err } // cherryPick checks that the current working set is clean, verifies the cherry-pick commit is not a merge commit diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go index 6699687743..72fd7825cd 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_commit.go @@ -32,51 +32,65 @@ var hashType = types.MustCreateString(query.Type_TEXT, 32, sql.Collation_ascii_b // doltCommit is the stored procedure version for the CLI command `dolt commit`. func doltCommit(ctx *sql.Context, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - return rowToIter(res), nil + if skipped { + return nil, nil + } + return rowToIter(commitHash), nil } // doltCommitHashOut is the stored procedure version for the CLI function `commit`. The first parameter is the variable // to set the hash of. func doltCommitHashOut(ctx *sql.Context, outHash *string, args ...string) (sql.RowIter, error) { - res, err := doDoltCommit(ctx, args) + commitHash, skipped, err := doDoltCommit(ctx, args) if err != nil { return nil, err } - *outHash = res - return rowToIter(res), nil + if skipped { + return nil, nil + } + + *outHash = commitHash + return rowToIter(commitHash), nil } -func doDoltCommit(ctx *sql.Context, args []string) (string, error) { +// doDoltCommit creates a dolt commit using the specified command line |args| provided. The response is the commit hash +// of the new commit (or the empty string if the commit was skipped), a boolean that indicates if creating the commit +// was skipped (e.g. due to --skip-empty), and an error describing any error encountered. +func doDoltCommit(ctx *sql.Context, args []string) (string, bool, error) { if err := branch_control.CheckAccess(ctx, branch_control.Permissions_Write); err != nil { - return "", err + return "", false, err } // Get the information for the sql context. dbName := ctx.GetCurrentDatabase() apr, err := cli.CreateCommitArgParser().Parse(args) if err != nil { - return "", err + return "", false, err + } + + if err := cli.VerifyCommitArgs(apr); err != nil { + return "", false, err } dSess := dsess.DSessFromSess(ctx.Session) roots, ok := dSess.GetRoots(ctx, dbName) if !ok { - return "", fmt.Errorf("Could not load database %s", dbName) + return "", false, fmt.Errorf("Could not load database %s", dbName) } if apr.Contains(cli.UpperCaseAllFlag) { roots, err = actions.StageAllTables(ctx, roots, true) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } else if apr.Contains(cli.AllFlag) { roots, err = actions.StageModifiedAndDeletedTables(ctx, roots) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -84,7 +98,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if authorStr, ok := apr.GetValue(cli.AuthorParam); ok { name, email, err = cli.ParseAuthor(authorStr) if err != nil { - return "", err + return "", false, err } } else { // In SQL mode, use the current SQL user as the commit author, instead of the `dolt config` configured values. @@ -100,15 +114,15 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { if amend { commit, err := dSess.GetHeadCommit(ctx, dbName) if err != nil { - return "", err + return "", false, err } commitMeta, err := commit.GetCommitMeta(ctx) if err != nil { - return "", err + return "", false, err } msg = commitMeta.Description } else { - return "", fmt.Errorf("Must provide commit message.") + return "", false, fmt.Errorf("Must provide commit message.") } } @@ -118,7 +132,7 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { t, err = cli.ParseDate(commitTimeStr) if err != nil { - return "", fmt.Errorf(err.Error()) + return "", false, fmt.Errorf(err.Error()) } } @@ -126,31 +140,34 @@ func doDoltCommit(ctx *sql.Context, args []string) (string, error) { Message: msg, Date: t, AllowEmpty: apr.Contains(cli.AllowEmptyFlag), + SkipEmpty: apr.Contains(cli.SkipEmptyFlag), Amend: amend, Force: apr.Contains(cli.ForceFlag), Name: name, Email: email, }) if err != nil { - return "", err + return "", false, err } // Nothing to commit, and we didn't pass --allowEmpty - if pendingCommit == nil { - return "", errors.New("nothing to commit") + if pendingCommit == nil && apr.Contains(cli.SkipEmptyFlag) { + return "", true, nil + } else if pendingCommit == nil { + return "", false, errors.New("nothing to commit") } newCommit, err := dSess.DoltCommit(ctx, dbName, dSess.GetTransaction(), pendingCommit) if err != nil { - return "", err + return "", false, err } h, err := newCommit.HashOf() if err != nil { - return "", err + return "", false, err } - return h.String(), nil + return h.String(), false, nil } func getDoltArgs(ctx *sql.Context, row sql.Row, children []sql.Expression) ([]string, error) { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index a587de2328..b619fe1a13 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -222,7 +222,7 @@ func performMerge(ctx *sql.Context, sess *dsess.DoltSession, roots doltdb.Roots, if !noCommit { author := fmt.Sprintf("%s <%s>", spec.Name, spec.Email) - _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) + _, _, err = doDoltCommit(ctx, []string{"-m", msg, "--author", author}) if err != nil { return ws, noConflictsOrViolations, threeWayMerge, fmt.Errorf("dolt_commit failed") } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go index f930a6fb82..4124fdee75 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_revert.go @@ -128,7 +128,7 @@ func doDoltRevert(ctx *sql.Context, args []string) (int, error) { if err != nil { return 1, err } - _, err = doDoltCommit(ctx, commitArgs) + _, _, err = doDoltCommit(ctx, commitArgs) if err != nil { return 1, err } diff --git a/integration-tests/bats/commit.bats b/integration-tests/bats/commit.bats index 2f94b3669f..917c9bc295 100644 --- a/integration-tests/bats/commit.bats +++ b/integration-tests/bats/commit.bats @@ -51,4 +51,25 @@ teardown() { run dolt commit [ $status -eq 1 ] [[ "$output" =~ "Failed to open commit editor" ]] || false +} + +@test "commit: --skip-empty correctly skips committing when no changes are staged" { + dolt sql -q "create table t(pk int primary key);" + dolt add t + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt commit --allow-empty --skip-empty -m "commit message" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt commit --skip-empty -m "commit message" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt commit --skip-empty -m "commit message" + [ $new_head = $(get_head_commit) ] } \ No newline at end of file diff --git a/integration-tests/bats/helper/common.bash b/integration-tests/bats/helper/common.bash index 7e85640e3e..c252f2ecce 100644 --- a/integration-tests/bats/helper/common.bash +++ b/integration-tests/bats/helper/common.bash @@ -38,6 +38,11 @@ current_dolt_user_email() { dolt config --global --get user.email } +# get_head_commit returns the commit hash for the current HEAD +get_head_commit() { + dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +} + setup_no_dolt_init() { export PATH=$PATH:~/go/bin cd $BATS_TMPDIR diff --git a/integration-tests/bats/sql-commit.bats b/integration-tests/bats/sql-commit.bats index 80503fd257..78eb38f871 100644 --- a/integration-tests/bats/sql-commit.bats +++ b/integration-tests/bats/sql-commit.bats @@ -453,6 +453,21 @@ SQL [[ "$output" =~ 'error: no value for option `message' ]] || false } -get_head_commit() { - dolt log -n 1 | grep -m 1 commit | cut -c 13-44 +@test "sql-commit: --skip-empty correctly skips committing when no changes are staged" { + original_head=$(get_head_commit) + + # When --allow-empty and --skip-empty are both specified, the user should get an error + run dolt sql -q "CALL DOLT_COMMIT('--allow-empty', '--skip-empty', '-m', 'commit message');" + [ $status -eq 1 ] + [[ "$output" =~ 'error: cannot use both --allow-empty and --skip-empty' ]] || false + [ $original_head = $(get_head_commit) ] + + # When changes are staged, --skip-empty has no effect + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + new_head=$(get_head_commit) + [ $original_head != $new_head ] + + # When no changes are staged, --skip-empty skips creating the commit + dolt sql -q "CALL DOLT_COMMIT('--skip-empty', '-m', 'commit message');" + [ $new_head = $(get_head_commit) ] }