Re: [PATCH v4 00/19] rebase: make the default backend configurable
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 17, 2020, 16:58 UTC
- Message-ID
- <47e7c9e6-7d83-185d-792d-f8e084c1a7a1@gmail.com>
- In-Reply-To
- <pull.679.v4.git.git.1579155273.gitgitgadget@gmail.com>
Hi Elijah
On 16/01/2020 06:14, Elijah Newren via GitGitGadget wrote:
Show 24 quoted lines
> This is a re-roll of en/rebase-backend which has been rebased on v2.25.0 and > updated to remove the dependence on ra/rebase-i-more-options, and also tries > to address feedback from Phillip, Junio, and Jonathan. This series does a > lot of work around making the default rebase backend configurable, and > switches the default from the am backend to the merge/interactive one. > > Changes since v3: > > * Rebased on origin/master and updated to remove the dependence on > ra/rebase-i-more-options. > * Added two new patches at the start of the series. > * Split the old first patch into two, while modifying them based on > Phillip's feedback (though slightly differently than discussed on the > list; instead of making --keep-empty a synonym for --empty=keep, I > instead kept backward compatibility by making it a no-op). > * I noted the post-commit hook in the differences between backends. Emily > is investigating what changes need to happen there, so I merely > documented the existing differences. > * dropped '-i' from the reflog messages; now they just refer to 'rebase' > > Changes possibly missing from this version, for discussion: > > * I did not remove the --am option as suggested by Phillip, since Junio and > Phillip were still discussing whether it is wanted/needed. > * I did not address the last two items Jonathan brought up as I
couldn't
> find enough information to reproduce or understand the problems.
I think I've got a fix for the failure with --committer-date-is-author-date that Jonathan reported which I'll post next week - the bug was not in this series, it was just exposed by it. I'll try and read through this series next week as well.
Best Wishes
Phillip
Show 1038 quoted lines
>
> Elijah Newren (19):
> git-rebase.txt: update description of --allow-empty-message
> t3404: directly test the behavior of interest
> rebase (interactive-backend): make --keep-empty the default
> rebase (interactive-backend): fix handling of commits that become
> empty
> t3406: simplify an already simple test
> rebase, sequencer: remove the broken GIT_QUIET handling
> rebase: make sure to pass along the quiet flag to the sequencer
> rebase: fix handling of restrict_revision
> t3432: make these tests work with either am or merge backends
> rebase: allow more types of rebases to fast-forward
> git-rebase.txt: add more details about behavioral differences of
> backends
> rebase: move incompatibility checks between backend options a bit
> earlier
> rebase: add an --am option
> git-prompt: change the prompt for interactive-based rebases
> rebase: drop '-i' from the reflog for interactive-based rebases
> rebase tests: mark tests specific to the am-backend with --am
> rebase tests: repeat some tests using the merge backend instead of am
> rebase: make the backend configurable via config setting
> rebase: change the default backend from "am" to "merge"
>
> Documentation/config/rebase.txt | 8 ++
> Documentation/git-rebase.txt | 150 +++++++++++++++++---
> builtin/rebase.c | 186 +++++++++++++++++++------
> contrib/completion/git-prompt.sh | 6 +-
> rebase-interactive.c | 7 +-
> rebase-interactive.h | 2 +-
> sequencer.c | 84 ++++++-----
> sequencer.h | 3 +-
> t/t3400-rebase.sh | 36 ++++-
> t/t3401-rebase-and-am-rename.sh | 4 +-
> t/t3404-rebase-interactive.sh | 19 +--
> t/t3406-rebase-message.sh | 19 ++-
> t/t3407-rebase-abort.sh | 6 +-
> t/t3420-rebase-autostash.sh | 2 +-
> t/t3421-rebase-topology-linear.sh | 16 +--
> t/t3424-rebase-empty.sh | 108 ++++++++++++++
> t/t3425-rebase-topology-merges.sh | 8 +-
> t/t3427-rebase-subtree.sh | 12 +-
> t/t3432-rebase-fast-forward.sh | 54 ++++---
> t/t5407-post-rewrite-hook.sh | 12 +-
> t/t5520-pull.sh | 27 +++-
> t/t6047-diff3-conflict-markers.sh | 13 +-
> t/t7512-status-help.sh | 12 +-
> t/t9106-git-svn-commit-diff-clobber.sh | 3 +-
> t/t9903-bash-prompt.sh | 8 +-
> 25 files changed, 595 insertions(+), 210 deletions(-)
> create mode 100755 t/t3424-rebase-empty.sh
>
>
> base-commit: d0654dc308b0ba76dd8ed7bbb33c8d8f7aacd783
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-679%2Fnewren%2Frebase-fixes-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-679/newren/rebase-fixes-v4
> Pull-Request: https://github.com/git/git/pull/679
>
> Range-diff vs v3:
>
> -: ---------- > 1: 3ea48d5394 git-rebase.txt: update description of --allow-empty-message
> -: ---------- > 2: 10fdd162a0 t3404: directly test the behavior of interest
> 1: 1c2b77e94d ! 3: 179f82ab83 rebase: extend the options for handling of empty commits
> @@ -1,49 +1,46 @@
> Author: Elijah Newren <newren@gmail.com>
>
> - rebase: extend the options for handling of empty commits
> + rebase (interactive-backend): make --keep-empty the default
>
> - Extend the interactive machinery with the ability to handle the full
> - spread of options for how to handle commits that either start or become
> - empty (by "become empty" I mean the changes in a commit are a subset of
> - changes that exist upstream, so the net effect of applying the commit is
> - no changes). Introduce a new command line flag for selecting the
> - desired behavior:
> - --empty={drop,keep,ask}
> - with the definitions:
> - drop: drop empty commits
> - keep: keep empty commits
> - ask: provide the user a chance to interact and pick what to do with
> - empty commits on a case-by-case basis
> + Different rebase backends have different treatment for commits which
> + start empty (i.e. have no changes relative to their parent), and the
> + --keep-empty option was added at some point to allow adjusting behavior
> + for the interactive backend. The handling of commits which start empty
> + is actually quite similar to commit b00bf1c9a8dd (git-rebase: make
> + --allow-empty-message the default, 2018-06-27), which pointed out that
> + the behavior for various backends is often more happenstance than
> + design. The specific change made in that commit is actually quite
> + relevant as well and much of the logic there directly applies here.
>
> - Note that traditionally, am-based rebases have always dropped commits
> - that either started or became empty, while interactive-based rebases
> - have defaulted to ask (and provided an option to keep commits that
> - started empty). This difference made sense since users of an am-based
> - rebase just wanted to quickly batch apply a sequence of commits, while
> - users editing a todo list will likely want the chance to interact and
> - handle unusual cases on a case-by-case basis. However, not all rebases
> - using the interactive machinery are explicitly interactive anymore. In
> - particular --merge was always meant to behave more like --am: just
> - rebase a batch of commits without popping up a todo list.
> + It makes a lot of sense in 'git commit' to error out on the creation of
> + empty commits, unless an override flag is provided. However, once
> + someone determines that there is a rare case that merits using the
> + manual override to create such a commit, it is somewhere between
> + annoying and harmful to have to take extra steps to keep such
> + intentional commits around. Granted, empty commits are quite rare,
> + which is why handling of them doesn't get considered much and folks tend
> + to defer to existing (accidental) behavior and assume there was a reason
> + for it, leading them to just add flags (--keep-empty in this case) that
> + allow them to override the bad defaults. Fix the interactive backend so
> + that --keep-empty is the default, much like we did with
> + --allow-empty-message. The am backend should also be fixed to have
> + --keep-empty semantics for commits that start empty, but that is not
> + included in this patch other than a testcase documenting the failure.
>
> - If the --empty flag is not specified, pick defaults as follows:
> - explicitly interactive: ask
> - --exec: keep (exec is about checking existing commits, and often
> - used without actually changing the base. Thus the
> - expectation is that the user doesn't necessarily want
> - anything to change; they just want to test).
> - otherwise: drop
> -
> - Also, this commit makes --keep-empty just imply --empty=keep, and hides
> - it from help so that we aren't confusing users with different ways to do
> - the same thing. (I could have added a --drop-empty flag, but then that
> - invites users to specify both --keep-empty and --drop-empty and we have
> - to add sanity checking around that; it seems cleaner to have a single
> - multi-valued option.) This actually fixes --keep-empty too; previously,
> - it only meant to sometimes keep empty commits, in particular commits
> - which started empty would be kept. But it would still error out and ask
> - the user what to do with commits that became empty. Now it keeps empty
> - commits, as instructed.
> + Note that there was one test in t3421 which appears to have been written
> + expecting --keep-empty to not be the default as correct behavior. This
> + test was introduced in commit 00b8be5a4d38 ("add tests for rebasing of
> + empty commits", 2013-06-06), which was part of a series focusing on
> + rebase topology and which had an interesting original cover letter at
> + https://lore.kernel.org/git/1347949878-12578-1-git-send-email-martinvonz@gmail.com/
> + which noted
> + Your input especially appreciated on whether you agree with the
> + intent of the test cases.
> + and then went into a long example about how one of the many tests added
> + had several questions about whether it was correct. As such, I believe
> + most the tests in that series were about testing rebase topology with as
> + many different flags as possible and were not trying to state in general
> + how those flags should behave otherwise.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
>
> @@ -51,126 +48,62 @@
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@
> - original branch. The index and working tree are also left
> unchanged as a result.
>
> -+--empty={drop,keep,ask}::
> -+ How to handle commits that become empty (because they contain a
> -+ subset of already upstream changes) or start empty. With drop
> -+ (the default), commits that start or become empty are dropped.
> -+ With keep (implied by --exec), such commits are kept. With ask
> -+ (implied by --interactive), the rebase will halt when an empty
> -+ commit is applied allowing you to choose whether to drop it or
> -+ commit it. Also with ask, if the rebase is interactive then
> -+ commits which start empty will be commented out in the todo
> -+ action list (giving you a chance to uncomment).
> -++
> -+Note that this has no effect on commits which are already upstream (as
> -+can be checked via `git log --cherry-mark ...`), which are always
> -+dropped by rebase.
> -++
> -+See also INCOMPATIBLE OPTIONS below.
> -+
> --keep-empty::
> - Keep the commits that do not change anything from its
> - parents in the result.
> -+ Deprecated alias for what is now known as --empty=keep.
> ++ No-op. Rebasing commits that started empty (had no change
> ++ relative to their parent) used to fail and this option would
> ++ override that behavior, allowing commits with empty changes to
> ++ be rebased. Now commits with no changes do not cause rebasing
> ++ to halt.
> +
> - See also INCOMPATIBLE OPTIONS below.
> -
> -@@
> - * --interactive
> - * --exec
> - * --keep-empty
> -+ * --empty=
> - * --edit-todo
> - * --root when used in combination with --onto
> -
> -@@
> - * --preserve-merges and --ignore-whitespace
> - * --preserve-merges and --committer-date-is-author-date
> - * --preserve-merges and --ignore-date
> -+ * --preserve-merges and --empty=
> - * --keep-base and --onto
> - * --keep-base and --root
> +-See also INCOMPATIBLE OPTIONS below.
> ++See also BEHAVIORAL DIFFERENCES and INCOMPATIBLE OPTIONS below.
>
> + --allow-empty-message::
> + No-op. Rebasing commits with an empty message used to fail
> @@
> + Empty commits
> + ~~~~~~~~~~~~~
>
> - There are some subtle differences how the backends behave.
> -
> --Empty commits
> --~~~~~~~~~~~~~
> --
> -The am backend drops any "empty" commits, regardless of whether the
> -commit started empty (had no changes relative to its parent to
> -start with) or ended empty (all changes were already applied
> -upstream in other commits).
> --
> ++The am backend unfortunately drops intentionally empty commits, i.e.
> ++commits that started empty, though these are rare in practice. It
> ++also drops commits that become empty and has no option for controlling
> ++this behavior.
> +
> -The interactive backend drops commits by default that
> -started empty and halts if it hits a commit that ended up empty.
> -The `--keep-empty` option exists for the interactive backend to allow
> -it to keep commits that started empty.
> --
> ++The interactive backend keeps intentionally empty commits.
> ++Unfortunately, it always halts whenever it runs across a commit that
> ++becomes empty, even when the rebase is not explicitly interactive.
> +
> Directory rename detection
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
> -
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> -@@
> - REBASE_PRESERVE_MERGES
> - };
> -
> -+enum empty_type {
> -+ EMPTY_UNSPECIFIED = -1,
> -+ EMPTY_DROP,
> -+ EMPTY_KEEP,
> -+ EMPTY_ASK
> -+};
> -+
> - struct rebase_options {
> - enum rebase_type type;
> -+ enum empty_type empty;
> - const char *state_dir;
> - struct commit *upstream;
> - const char *upstream_name;
> @@
> const char *action;
> int signoff;
> int allow_rerere_autoupdate;
> - int keep_empty;
> int autosquash;
> - int ignore_whitespace;
> char *gpg_sign_opt;
> -@@
> -
> - #define REBASE_OPTIONS_INIT { \
> - .type = REBASE_UNSPECIFIED, \
> -+ .empty = EMPTY_UNSPECIFIED, \
> - .flags = REBASE_NO_QUIET, \
> - .git_am_opts = ARGV_ARRAY_INIT, \
> - .git_format_patch_opt = STRBUF_INIT \
> -@@
> - replay.allow_rerere_auto = opts->allow_rerere_autoupdate;
> - replay.allow_empty = 1;
> - replay.allow_empty_message = opts->allow_empty_message;
> -+ replay.drop_redundant_commits = (opts->empty == EMPTY_DROP);
> -+ replay.keep_redundant_commits = (opts->empty == EMPTY_KEEP);
> -+ replay.ask_on_initially_empty = (opts->empty == EMPTY_ASK &&
> -+ !(opts->flags & REBASE_INTERACTIVE_EXPLICIT));
> - replay.verbose = opts->flags & REBASE_VERBOSE;
> - replay.reschedule_failed_exec = opts->reschedule_failed_exec;
> - replay.committer_date_is_author_date =
> + int autostash;
> @@
>
> git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
>
> - flags |= opts->keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
> -+ flags |= (opts->empty == EMPTY_DROP) ? TODO_LIST_DROP_EMPTY : 0;
> -+ flags |= (opts->empty == EMPTY_ASK &&
> -+ opts->flags & REBASE_INTERACTIVE_EXPLICIT) ?
> -+ TODO_LIST_ASK_EMPTY : 0;
> flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
> flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
> flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
> @@ -183,10 +116,8 @@
> +{
> + struct rebase_options *opts = opt->value;
> +
> -+ BUG_ON_OPT_NEG(unset);
> + BUG_ON_OPT_ARG(arg);
> +
> -+ opts->empty = EMPTY_KEEP;
> + opts->type = REBASE_INTERACTIVE;
> + return 0;
> +}
> @@ -201,62 +132,28 @@
> - OPT_BOOL(0, "keep-empty", &opts.keep_empty, N_("keep empty commits")),
> + { OPTION_CALLBACK, 'k', "keep-empty", &options, NULL,
> + N_("(DEPRECATED) keep empty commits"),
> -+ PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
> ++ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
> + parse_opt_keep_empty },
> - OPT_BOOL(0, "allow-empty-message", &opts.allow_empty_message,
> - N_("allow commits with empty messages")),
> - OPT_BOOL(0, "rebase-merges", &opts.rebase_merges, N_("rebase merge commits")),
> + OPT_BOOL_F(0, "allow-empty-message", &opts.allow_empty_message,
> + N_("allow commits with empty messages"),
> + PARSE_OPT_HIDDEN),
> @@
> opts->allow_rerere_autoupdate ?
> opts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ?
> "--rerere-autoupdate" : "--no-rerere-autoupdate" : "");
> - add_var(&script_snippet, "keep_empty", opts->keep_empty ? "yes" : "");
> -+ add_var(&script_snippet, "empty", opts->empty == EMPTY_KEEP ? "yes" : "");
> add_var(&script_snippet, "autosquash", opts->autosquash ? "t" : "");
> add_var(&script_snippet, "gpg_sign_opt", opts->gpg_sign_opt);
> add_var(&script_snippet, "cmd", opts->cmd);
> -@@
> - return 0;
> - }
> -
> -+static enum empty_type parse_empty_value(const char *value)
> -+{
> -+ if (!strcasecmp(value, "drop"))
> -+ return EMPTY_DROP;
> -+ else if (!strcasecmp(value, "keep"))
> -+ return EMPTY_KEEP;
> -+ else if (!strcasecmp(value, "ask"))
> -+ return EMPTY_ASK;
> -+
> -+ die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"ask\"."), value);
> -+}
> -+
> -+static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
> -+{
> -+ struct rebase_options *options = opt->value;
> -+ enum empty_type value = parse_empty_value(arg);
> -+
> -+ BUG_ON_OPT_NEG(unset);
> -+
> -+ options->empty = value;
> -+ return 0;
> -+}
> -+
> - static void NORETURN error_on_missing_default_upstream(void)
> - {
> - struct branch *current_branch = branch_get(NULL);
> @@
> "ignoring them"),
> REBASE_PRESERVE_MERGES, PARSE_OPT_HIDDEN),
> OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
> - OPT_BOOL('k', "keep-empty", &options.keep_empty,
> - N_("preserve empty commits during rebase")),
> -+ OPT_CALLBACK_F(0, "empty", &options, N_("{drop,keep,ask}"),
> -+ N_("how to handle empty commits"),
> -+ PARSE_OPT_NONEG, parse_opt_empty),
> + { OPTION_CALLBACK, 'k', "keep-empty", &options, NULL,
> + N_("(DEPRECATED) keep empty commits"),
> -+ PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,
> ++ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN,
> + parse_opt_keep_empty },
> OPT_BOOL(0, "autosquash", &options.autosquash,
> N_("move commits that begin with "
> @@ -267,26 +164,10 @@
>
> - if (options.keep_empty)
> - imply_interactive(&options, "--keep-empty");
> -+ if (options.empty != EMPTY_UNSPECIFIED)
> -+ imply_interactive(&options, "--empty");
> -
> +-
> if (gpg_sign) {
> free(options.gpg_sign_opt);
> -@@
> - break;
> - }
> -
> -+ if (options.empty == EMPTY_UNSPECIFIED) {
> -+ if (options.flags & REBASE_INTERACTIVE_EXPLICIT)
> -+ options.empty = EMPTY_ASK;
> -+ else if (exec.nr > 0)
> -+ options.empty = EMPTY_KEEP;
> -+ else
> -+ options.empty = EMPTY_DROP;
> -+ }
> - if (reschedule_failed_exec > 0 && !is_interactive(&options))
> - die(_("--reschedule-failed-exec requires "
> - "--exec or --interactive"));
> + options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
>
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> --- a/rebase-interactive.c
> @@ -296,19 +177,22 @@
> }
>
> -void append_todo_help(unsigned keep_empty, int command_count,
> -+void append_todo_help(unsigned no_ask_empty, int command_count,
> ++void append_todo_help(int command_count,
> const char *shortrevisions, const char *shortonto,
> struct strbuf *buf)
> {
> @@
> + "the rebase will be aborted.\n\n");
>
> strbuf_add_commented_lines(buf, msg, strlen(msg));
> -
> +-
> - if (!keep_empty) {
> -+ if (!no_ask_empty) {
> - msg = _("Note that empty commits are commented out");
> - strbuf_add_commented_lines(buf, msg, strlen(msg));
> - }
> +- msg = _("Note that empty commits are commented out");
> +- strbuf_add_commented_lines(buf, msg, strlen(msg));
> +- }
> + }
> +
> + int edit_todo_list(struct repository *r, struct todo_list *todo_list,
>
> diff --git a/rebase-interactive.h b/rebase-interactive.h
> --- a/rebase-interactive.h
> @@ -318,7 +202,7 @@
> struct todo_list;
>
> -void append_todo_help(unsigned keep_empty, int command_count,
> -+void append_todo_help(unsigned no_ask_empty, int command_count,
> ++void append_todo_help(int command_count,
> const char *shortrevisions, const char *shortonto,
> struct strbuf *buf);
> int edit_todo_list(struct repository *r, struct todo_list *todo_list,
> @@ -327,157 +211,51 @@
> --- a/sequencer.c
> +++ b/sequencer.c
> @@
> - static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
> - static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
> - static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedule-failed-exec")
> -+static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
> -+static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
> -+static GIT_PATH_FUNC(rebase_path_ask_on_initially_empty, "rebase-merge/ask_on_initially_empty")
> -
> - static int git_sequencer_config(const char *k, const char *v, void *cb)
> + struct replay_opts *opts,
> + struct commit *commit)
> {
> +- int index_unchanged, empty_commit;
> ++ int index_unchanged, originally_empty;
> +
> + /*
> + * Three cases:
> @@
> - empty_commit = is_original_commit_empty(commit);
> - if (empty_commit < 0)
> - return empty_commit;
> + if (opts->keep_redundant_commits)
> + return 1;
> +
> +- empty_commit = is_original_commit_empty(commit);
> +- if (empty_commit < 0)
> +- return empty_commit;
> - if (!empty_commit)
> -+ if (!empty_commit || opts->ask_on_initially_empty)
> ++ originally_empty = is_original_commit_empty(commit);
> ++ if (originally_empty < 0)
> ++ return originally_empty;
> ++ if (!originally_empty)
> return 0;
> else
> return 1;
> -@@
> - char *author = NULL;
> - struct commit_message msg = { NULL, NULL, NULL, NULL };
> - struct strbuf msgbuf = STRBUF_INIT;
> -- int res, unborn = 0, reword = 0, allow;
> -+ int res, unborn = 0, reword = 0, allow, drop_commit;
> -
> - if (opts->no_commit) {
> - /*
> -@@
> - goto leave;
> - }
> -
> -- allow = allow_empty(r, opts, commit);
> -- if (allow < 0) {
> -- res = allow;
> -- goto leave;
> -- } else if (allow)
> -- flags |= ALLOW_EMPTY;
> -- if (!opts->no_commit) {
> -+ drop_commit = 0;
> -+ if (opts->drop_redundant_commits && is_index_unchanged(r)) {
> -+ drop_commit = 1;
> -+ fprintf(stderr, _("No changes -- Patch already applied."));
> -+ } else {
> -+ allow = allow_empty(r, opts, commit);
> -+ if (allow < 0) {
> -+ res = allow;
> -+ goto leave;
> -+ } else if (allow) {
> -+ flags |= ALLOW_EMPTY;
> -+ }
> -+ }
> -+ if (!opts->no_commit && !drop_commit) {
> - if (author || command == TODO_REVERT || (flags & AMEND_MSG))
> - res = do_commit(r, msg_file, author, opts, flags);
> - else
> -@@
> - else if (!strcmp(key, "options.allow-empty-message"))
> - opts->allow_empty_message =
> - git_config_bool_or_int(key, value, &error_flag);
> -+ else if (!strcmp(key, "options.drop-redundant-commits"))
> -+ opts->drop_redundant_commits =
> -+ git_config_bool_or_int(key, value, &error_flag);
> - else if (!strcmp(key, "options.keep-redundant-commits"))
> - opts->keep_redundant_commits =
> - git_config_bool_or_int(key, value, &error_flag);
> -+ else if (!strcmp(key, "options.ask_on_initially_empty"))
> -+ opts->ask_on_initially_empty =
> -+ git_config_bool_or_int(key, value, &error_flag);
> - else if (!strcmp(key, "options.signoff"))
> - opts->signoff = git_config_bool_or_int(key, value, &error_flag);
> - else if (!strcmp(key, "options.record-origin"))
> -@@
> - if (file_exists(rebase_path_reschedule_failed_exec()))
> - opts->reschedule_failed_exec = 1;
> -
> -+ if (file_exists(rebase_path_drop_redundant_commits()))
> -+ opts->drop_redundant_commits = 1;
> -+
> -+ if (file_exists(rebase_path_keep_redundant_commits()))
> -+ opts->keep_redundant_commits = 1;
> -+
> -+ if (file_exists(rebase_path_ask_on_initially_empty()))
> -+ opts->ask_on_initially_empty = 1;
> -+
> - read_strategy_opts(opts, &buf);
> - strbuf_release(&buf);
> -
> -@@
> - write_file(rebase_path_cdate_is_adate(), "%s", "");
> - if (opts->ignore_date)
> - write_file(rebase_path_ignore_date(), "%s", "");
> -+ if (opts->drop_redundant_commits)
> -+ write_file(rebase_path_drop_redundant_commits(), "%s", "");
> -+ if (opts->keep_redundant_commits)
> -+ write_file(rebase_path_keep_redundant_commits(), "%s", "");
> -+ if (opts->ask_on_initially_empty)
> -+ write_file(rebase_path_ask_on_initially_empty(), "%s", "");
> - if (opts->reschedule_failed_exec)
> - write_file(rebase_path_reschedule_failed_exec(), "%s", "");
> -
> -@@
> - if (opts->allow_empty_message)
> - res |= git_config_set_in_file_gently(opts_file,
> - "options.allow-empty-message", "true");
> -+ if (opts->drop_redundant_commits)
> -+ res |= git_config_set_in_file_gently(opts_file,
> -+ "options.drop-redundant-commits", "true");
> - if (opts->keep_redundant_commits)
> - res |= git_config_set_in_file_gently(opts_file,
> - "options.keep-redundant-commits", "true");
> -+ if (opts->ask_on_initially_empty)
> -+ res |= git_config_set_in_file_gently(opts_file,
> -+ "options.ask_on_initially_empty", "true");
> - if (opts->signoff)
> - res |= git_config_set_in_file_gently(opts_file,
> - "options.signoff", "true");
> @@
> struct rev_info *revs, struct strbuf *out,
> unsigned flags)
> {
> - int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
> -+ int drop_empty = flags & TODO_LIST_DROP_EMPTY;
> -+ int ask_empty = flags & TODO_LIST_ASK_EMPTY;
> int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
> int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
> struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
> -@@
> - is_empty = is_original_commit_empty(commit);
> - if (!is_empty && (commit->object.flags & PATCHSAME))
> - continue;
> -+ if (is_empty && drop_empty)
> -+ continue;
> -
> - strbuf_reset(&oneline);
> - pretty_print_commit(pp, commit, &oneline);
> @@
> if (!to_merge) {
> /* non-merge commit: easy case */
> strbuf_reset(&buf);
> - if (!keep_empty && is_empty)
> -+ if (is_empty && ask_empty)
> - strbuf_addf(&buf, "%c ", comment_line_char);
> +- strbuf_addf(&buf, "%c ", comment_line_char);
> strbuf_addf(&buf, "%s %s %s", cmd_pick,
> oid_to_hex(&commit->object.oid),
> + oneline.buf);
> @@
> struct pretty_print_context pp = {0};
> struct rev_info revs;
> struct commit *commit;
> - int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
> -+ int drop_empty = flags & TODO_LIST_DROP_EMPTY;
> -+ int ask_empty = flags & TODO_LIST_ASK_EMPTY;
> const char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? "p" : "pick";
> int rebase_merges = flags & TODO_LIST_REBASE_MERGES;
>
> @@ -491,19 +269,16 @@
> if (!is_empty && (commit->object.flags & PATCHSAME))
> continue;
> - if (!keep_empty && is_empty)
> -+ if (is_empty && drop_empty)
> -+ continue;
> -+ if (is_empty && ask_empty)
> - strbuf_addf(out, "%c ", comment_line_char);
> +- strbuf_addf(out, "%c ", comment_line_char);
> strbuf_addf(out, "%s %s ", insn,
> oid_to_hex(&commit->object.oid));
> + pretty_print_commit(&pp, commit, out);
> @@
>
> todo_list_to_strbuf(r, todo_list, &buf, num, flags);
> if (flags & TODO_LIST_APPEND_TODO_HELP)
> - append_todo_help(flags & TODO_LIST_KEEP_EMPTY, count_commands(todo_list),
> -+ append_todo_help(!(flags & TODO_LIST_ASK_EMPTY),
> -+ count_commands(todo_list),
> ++ append_todo_help(count_commands(todo_list),
> shortrevisions, shortonto, &buf);
>
> res = write_message(buf.buf, buf.len, file, 0);
> @@ -511,16 +286,6 @@
> diff --git a/sequencer.h b/sequencer.h
> --- a/sequencer.h
> +++ b/sequencer.h
> -@@
> - int allow_rerere_auto;
> - int allow_empty;
> - int allow_empty_message;
> -+ int drop_redundant_commits;
> - int keep_redundant_commits;
> -+ int ask_on_initially_empty;
> - int verbose;
> - int quiet;
> - int reschedule_failed_exec;
> @@
> int sequencer_skip(struct repository *repo, struct replay_opts *opts);
> int sequencer_remove_state(struct replay_opts *opts);
> @@ -530,19 +295,34 @@
> #define TODO_LIST_SHORTEN_IDS (1U << 1)
> #define TODO_LIST_ABBREVIATE_CMDS (1U << 2)
> #define TODO_LIST_REBASE_MERGES (1U << 3)
> -@@
> - * `--onto`, we do not want to re-generate the root commits.
> - */
> - #define TODO_LIST_ROOT_WITH_ONTO (1U << 6)
> -+#define TODO_LIST_DROP_EMPTY (1U << 7)
> -+#define TODO_LIST_ASK_EMPTY (1U << 8)
> -
> -
> - int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
>
> diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh
> --- a/t/t3421-rebase-topology-linear.sh
> +++ b/t/t3421-rebase-topology-linear.sh
> +@@
> + test_run_rebase () {
> + result=$1
> + shift
> +- test_expect_$result "rebase $* drops empty commit" "
> ++ test_expect_$result "rebase $* keeps begin-empty commits" "
> + reset_rebase &&
> +- git rebase $* c l &&
> +- test_cmp_rev c HEAD~2 &&
> +- test_linear_range 'd l' c..
> ++ git rebase $* j l &&
> ++ test_cmp_rev c HEAD~4 &&
> ++ test_linear_range 'j d k l' c..
> + "
> + }
> +-test_run_rebase success ''
> ++test_run_rebase failure ''
> + test_run_rebase success -m
> + test_run_rebase success -i
> +-test_have_prereq !REBASE_P || test_run_rebase success -p
> ++test_have_prereq !REBASE_P || test_run_rebase failure -p
> +
> + test_run_rebase () {
> + result=$1
> @@
> test_run_rebase success ''
> test_run_rebase success -m
> @@ -603,31 +383,22 @@
> + git commit -m "Five letters ought to be enough for anybody"
> +'
> +
> -+test_expect_success 'rebase --merge --empty=drop' '
> ++test_expect_failure 'rebase (am-backend) with a variety of empty commits' '
> ++ test_when_finished "git rebase --abort" &&
> + git checkout -B testing localmods &&
> -+ git rebase --merge --empty=drop upstream &&
> -+
> -+ test_write_lines C B A >expect &&
> -+ git log --format=%s >actual &&
> -+ test_cmp expect actual
> -+'
> ++ # rebase (--am) should not drop commits that start empty
> ++ git rebase upstream &&
> +
> -+test_expect_success 'rebase --merge --empty=keep' '
> -+ git checkout -B testing localmods &&
> -+ git rebase --merge --empty=keep upstream &&
> -+
> -+ test_write_lines D C2 C B A >expect &&
> ++ test_write_lines D C B A >expect &&
> + git log --format=%s >actual &&
> + test_cmp expect actual
> +'
> +
> -+test_expect_success 'rebase --merge --empty=ask' '
> ++test_expect_failure 'rebase --merge with a variety of empty commits' '
> ++ test_when_finished "git rebase --abort" &&
> + git checkout -B testing localmods &&
> -+ test_must_fail git rebase --merge --empty=ask upstream &&
> -+
> -+ test_must_fail git rebase --skip &&
> -+ git commit --allow-empty &&
> -+ git rebase --continue &&
> ++ # rebase --merge should not halt on the commit that becomes empty
> ++ git rebase --merge upstream &&
> +
> + test_write_lines D C B A >expect &&
> + git log --format=%s >actual &&
> @@ -636,25 +407,17 @@
> +
> +GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR
> +
> -+test_expect_success 'rebase --interactive --empty=drop' '
> ++test_expect_success 'rebase --interactive with a variety of empty commits' '
> + git checkout -B testing localmods &&
> -+ git rebase --interactive --empty=drop upstream &&
> ++ test_must_fail git rebase --interactive upstream &&
> +
> -+ test_write_lines C B A >expect &&
> -+ git log --format=%s >actual &&
> -+ test_cmp expect actual
> -+'
> -+
> -+test_expect_success 'rebase --interactive --empty=keep' '
> -+ git checkout -B testing localmods &&
> -+ git rebase --interactive --empty=keep upstream &&
> ++ git rebase --skip &&
> +
> -+ test_write_lines D C2 C B A >expect &&
> ++ test_write_lines D C B A >expect &&
> + git log --format=%s >actual &&
> + test_cmp expect actual
> +'
> +
> -+
> +test_done
>
> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
> @@ -665,32 +428,28 @@
> '
>
> -test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
> -+test_expect_success 'Rebase -Xsubtree --empty=ask --onto commit' '
> ++test_expect_success 'Rebase -Xsubtree --onto commit' '
> reset_rebase &&
> git checkout -b rebase-onto to-rebase &&
> - test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
> -+ test_must_fail git rebase -Xsubtree=files_subtree --empty=ask --onto files-master master &&
> ++ test_must_fail git rebase -Xsubtree=files_subtree --onto files-master master &&
> : first pick results in no changes &&
> - git rebase --continue &&
> -+ test_must_fail git rebase --skip &&
> -+ : last pick was an empty commit that has no changes, but we want to keep it &&
> -+ git commit --allow-empty &&
> ++ git rebase --skip &&
> verbose test "$(commit_message HEAD~2)" = "master4" &&
> verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
> verbose test "$(commit_message HEAD)" = "Empty commit"
> '
>
> -test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' '
> -+test_expect_success 'Rebase -Xsubtree --empty=ask --rebase-merges --onto commit' '
> ++test_expect_success 'Rebase -Xsubtree --rebase-merges --onto commit' '
> reset_rebase &&
> git checkout -b rebase-merges-onto to-rebase &&
> - test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root &&
> -+ test_must_fail git rebase -Xsubtree=files_subtree --empty=ask --rebase-merges --onto files-master --root &&
> ++ test_must_fail git rebase -Xsubtree=files_subtree --rebase-merges --onto files-master --root &&
> : first pick results in no changes &&
> - git rebase --continue &&
> -+ test_must_fail git rebase --skip &&
> -+ : last pick was an empty commit that has no changes, but we want to keep it &&
> -+ git commit --allow-empty &&
> ++ git rebase --skip &&
> verbose test "$(commit_message HEAD~2)" = "master4" &&
> verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
> verbose test "$(commit_message HEAD)" = "Empty commit"
> -: ---------- > 4: c9542a2abe rebase (interactive-backend): fix handling of commits that become empty
> 2: bd3c5ec155 = 5: 9f66229d5c t3406: simplify an already simple test
> 3: 49388b79fd = 6: 8d731fa39c rebase, sequencer: remove the broken GIT_QUIET handling
> 4: 478479358f ! 7: b6b6597eef rebase: make sure to pass along the quiet flag to the sequencer
> @@ -8,13 +8,13 @@
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@
> + replay.allow_empty_message = opts->allow_empty_message;
> + replay.drop_redundant_commits = (opts->empty == EMPTY_DROP);
> replay.keep_redundant_commits = (opts->empty == EMPTY_KEEP);
> - replay.ask_on_initially_empty = (opts->empty == EMPTY_ASK &&
> - !(opts->flags & REBASE_INTERACTIVE_EXPLICIT));
> + replay.quiet = !(opts->flags & REBASE_NO_QUIET);
> replay.verbose = opts->flags & REBASE_VERBOSE;
> replay.reschedule_failed_exec = opts->reschedule_failed_exec;
> - replay.committer_date_is_author_date =
> + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
> @@
> N_("allow pre-rebase hook to run")),
> OPT_NEGBIT('q', "quiet", &options.flags,
> 5: ee26f5a161 = 8: 0acefa988b rebase: fix handling of restrict_revision
> 6: 34a69def33 = 9: 8c5b5b5133 t3432: make these tests work with either am or merge backends
> 7: f2c92853b4 ! 10: b8c087d6fb rebase: allow more types of rebases to fast-forward
> @@ -35,8 +35,8 @@
> OPT_STRING(0, "onto", &options.onto_name,
> N_("revision"),
> @@
> - options.ignore_date)
> - options.flags |= REBASE_FORCE;
> + state_dir_base, cmd_live_rebase, buf.buf);
> + }
>
> + if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
> + (action != ACTION_NONE) ||
> @@ -47,7 +47,9 @@
> +
> for (i = 0; i < options.git_am_opts.argc; i++) {
> const char *option = options.git_am_opts.argv[i], *p;
> - if (!strcmp(option, "--whitespace=fix") ||
> + if (!strcmp(option, "--committer-date-is-author-date") ||
> + !strcmp(option, "--ignore-date") ||
> + !strcmp(option, "--whitespace=fix") ||
> !strcmp(option, "--whitespace=strip"))
> - options.flags |= REBASE_FORCE;
> + allow_preemptive_ff = 0;
> 8: b307340f7c ! 11: b50a1741e0 git-rebase.txt: add more details about behavioral differences of backends
> @@ -8,23 +8,24 @@
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@
> - with `--keep-base` in order to drop those commits from your branch.
>
> --ignore-whitespace::
> -- Behaves differently depending on which backend is selected.
> --+
> --'am' backend: When applying a patch, ignore changes in whitespace in
> --context lines if necessary.
> --+
> --'interactive' backend: Treat lines with only whitespace changes as
> --unchanged for the sake of a three-way merge.
> -+ Ignore whitespace-only changes in the commits being rebased,
> -+ which may avoid "unnecessary" conflicts. (Both backends
> -+ currently have differing edgecase bugs with this option; see
> -+ BEHAVIORAL DIFFERENCES.)
> -
> --whitespace=<option>::
> - This flag is passed to the 'git apply' program
> +- These flag are passed to the 'git apply' program
> ++ These flags are passed to the 'git apply' program
> + (see linkgit:git-apply[1]) that applies the patch.
> + +
> + See also INCOMPATIBLE OPTIONS below.
> +@@
> +
> + * --committer-date-is-author-date
> + * --ignore-date
> +- * --whitespace
> + * --ignore-whitespace
> ++ * --whitespace
> + * -C
> +
> + are incompatible with the following options:
> @@
> Directory rename detection
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
> @@ -82,24 +83,14 @@
> +The interactive backend works with the full commits on both sides of
> +history and thus has no such limitations.
> +
> -+--ignore-whitespace
> -+~~~~~~~~~~~~~~~~~~~
> ++Hooks
> ++~~~~~
> +
> -+The --ignore-whitespace option is supposed to ignore whitespace-only
> -+changes if it allows the code to merge cleanly. Unfortunately, the
> -+different backends implement this differently, and both have different
> -+edge case bugs.
> -++
> -+'am' backend: When applying a patch, ignore changes in whitespace in
> -+context lines if necessary. (Which implies that if the whitespace
> -+change was not in the context lines but on a line with a real change,
> -+then the rebase will still fail with "unnecessary" content conflicts.)
> -++
> -+'interactive' backend: Treat lines with only whitespace changes as
> -+unchanged for the sake of a three-way merge. This means that if one
> -+side made no changes and the commits being rebased had whitespace-only
> -+changes, those whitespaces fixups will be discarded despite the fact
> -+that they present no content conflict.
> ++The am backend has not traditionally called the post-commit hook,
> ++while the merge/interactive backend has. However, this was by
> ++accident of implementation rather than by design. Both backends
> ++should have the same behavior, though it is not clear which one is
> ++correct.
> +
> +Miscellaneous differences
> +~~~~~~~~~~~~~~~~~~~~~~~~~
> @@ -123,19 +114,3 @@
>
> include::merge-strategies.txt[]
>
> -
> - diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh
> - --- a/t/t3433-rebase-options-compatibility.sh
> - +++ b/t/t3433-rebase-options-compatibility.sh
> -@@
> - GIT_AUTHOR_DATE="1999-04-02T08:03:20+05:30"
> - export GIT_AUTHOR_DATE
> -
> --# This is a special case in which both am and interactive backends
> --# provide the same output. It was done intentionally because
> --# both the backends fall short of optimal behaviour.
> -+# This is a common case in which both am and interactive backends
> -+# provide the same output with --ignore-whitespace.
> - test_expect_success 'setup' '
> - git checkout -b topic &&
> - q_to_tab >file <<-\EOF &&
> 9: 7c3f2e07f3 = 12: 58e6e4ffb3 rebase: move incompatibility checks between backend options a bit earlier
> 10: 1df11f0b51 ! 13: 5478c730ac rebase: add an --am option
> @@ -24,8 +24,8 @@
> +See also INCOMPATIBLE OPTIONS below.
> +
> --empty={drop,keep,ask}::
> - How to handle commits that become empty (because they contain a
> - subset of already upstream changes) or start empty. With drop
> + How to handle commits that are not empty to start and are not
> + clean cherry-picks of any upstream commit, but which become
> @@
> Ensure at least <n> lines of surrounding context match before
> and after each change. When fewer lines of surrounding
> @@ -37,7 +37,7 @@
>
> @@
> --whitespace=<option>::
> - This flag is passed to the 'git apply' program
> + These flags are passed to the 'git apply' program
> (see linkgit:git-apply[1]) that applies the patch.
> + Implies --am.
> +
> @@ -48,9 +48,9 @@
> The following options:
>
> + * --am
> - * --whitespace
> - * -C
> -
> + * --committer-date-is-author-date
> + * --ignore-date
> + * --ignore-whitespace
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> --- a/builtin/rebase.c
> 11: ff43593211 = 14: db5e29bd81 git-prompt: change the prompt for interactive-based rebases
> -: ---------- > 15: 413e190ac9 rebase: drop '-i' from the reflog for interactive-based rebases
> 12: 99388f24e5 ! 16: 170be283a8 rebase tests: mark tests specific to the am-backend with --am
> @@ -228,40 +228,6 @@
> test_rebase_same_head_ $status_f $what_f $cmp_f " --merge --no-ff" "$*"
> }
>
> - diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh
> - --- a/t/t3433-rebase-options-compatibility.sh
> - +++ b/t/t3433-rebase-options-compatibility.sh
> -@@
> - new line 2
> - line 3
> - EOF
> -- test_must_fail git rebase main side &&
> -+ test_must_fail git rebase --am main side &&
> - git rebase --abort &&
> -- git rebase --ignore-whitespace main side &&
> -+ git rebase --am --ignore-whitespace main side &&
> - test_cmp expect file
> - '
> -
> -@@
> -
> - test_expect_success '--committer-date-is-author-date works with am backend' '
> - git commit --amend &&
> -- git rebase --committer-date-is-author-date HEAD^ &&
> -+ git rebase --am --committer-date-is-author-date HEAD^ &&
> - git show HEAD --pretty="format:%ai" >authortime &&
> - git show HEAD --pretty="format:%ci" >committertime &&
> - test_cmp authortime committertime
> -@@
> - # sets to +0530.
> - test_expect_success '--ignore-date works with am backend' '
> - git commit --amend --date="$GIT_AUTHOR_DATE" &&
> -- git rebase --ignore-date HEAD^ &&
> -+ git rebase --am --ignore-date HEAD^ &&
> - git show HEAD --pretty="format:%ai" >authortime &&
> - grep "+0000" authortime
> - '
> -
> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
> --- a/t/t5407-post-rewrite-hook.sh
> +++ b/t/t5407-post-rewrite-hook.sh
> 13: c2ba6317bf = 17: 1e3d4066c4 rebase tests: repeat some tests using the merge backend instead of am
> 14: 8bec6df51a = 18: 9b4ac83d2d rebase: make the backend configurable via config setting
> 15: 044853fd61 = 19: 859a4a94d7 rebase: change the default backend from "am" to "merge"
>