{"thread":{"id":"52496","subject":"[PATCH 00/15] rebase: make the default backend configurable","startedAt":"2019-12-20T17:09:53Z","lastAt":"2020-03-17T04:45:25Z","messageCount":161,"participants":["Elijah Newren via GitGitGadget","Alban Gruin","Elijah Newren","Junio C Hamano","SZEDER Gábor","Denton Liu","Phillip Wood","Johannes Schindelin","Jonathan Nieder","Emily Shaffer","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":15},"messages":[{"id":"388617","messageId":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":null,"subject":"[PATCH 00/15] rebase: make the default backend configurable","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:33Z","receivedAt":"2019-12-20T17:09:53Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series does a lot of work around making the default rebase backend\nconfigurable:\n\n * provide configurability of handling empty commits and iron out\n differences between backends\n * increase/fix capabilities of the merge/interactive backend to make it\n workable as the default\n * document the remaining differences in backends more thoroughly\n * add an --am option for explicitly requesting the am-backend\n * extend merge/interactive backend testing to put it on par with the am\n backend\n * add a 'rebase.backend' config option and:\n * switch the rebase.backend default value from 'am' to 'merge'.\n\nAreas I'd like reviewers to focus, in priority order:\n\n * Patch 15 (Are we ready to switch the default backend? Or should we leave\n this patch out for now? Junio suggested we may be ready or at least are\n close[1])\n * Patch 1 (Does my empty handling make sense? Do others agree I fixed \n --keep-empty, or do they view it as breaking it?)\n * Patch 8 (Do the updates to the documentation of behavioral differences\n make sense? Is it too long?) * Patch 11 (okay to change the\n git-completion shell prompt slightly, especially in light of patches\n 15 & 16? We did a prompt change previously when we merged the merge\n backend with the interactive one, so I assume so, but just want to\n make sure people have a chance to chime in.)\n \n \n\nIf it's too soon to switch from the 'am' to 'merge' backend, we can just\ndrop the last patch and I'll resubmit it later. I generated this series\nmostly through switching the default first and then watching what broke, but\nmoved the patch to the end to make it easy to drop.\n\nBriefly, reasons for switching the default backend boil down to the fact\nthat the am-backend drops information and thus limits what it can do. This\nmanifests in different ways:\n\n * lack of tree information that would allow us to warn users that new files\n in old directories might want to move along with the other files that\n were renamed with those directories[1]\n * incorrect application of patches in the presence of non-unique context\n lines[2], which could be avoided with access to the original files\n involved.\n * less information available to annotate conflict markers (since am creates\n fake ancestors and commits on top of them, and doesn't have access to the\n original commits)\n\n[1] https://lore.kernel.org/git/xmqqa78d2qmk.fsf@gitster-ct.c.googlers.com/\n[2] https://lore.kernel.org/git/xmqqh8jeh1id.fsf@gitster-ct.c.googlers.com/\n[3] \nhttps://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+h+t4Mn4A@mail.gmail.com/\n\nElijah Newren (15):\n rebase: extend the options for handling of empty commits\n t3406: simplify an already simple test\n rebase, sequencer: remove the broken GIT_QUIET handling\n rebase: make sure to pass along the quiet flag to the sequencer\n rebase: fix handling of restrict_revision\n t3432: make these tests work with either am or merge backends\n rebase: allow more types of rebases to fast-forward\n git-rebase.txt: add more details about behavioral differences of\n backends\n rebase: move incompatibility checks between backend options a bit\n earlier\n rebase: add an --am option\n contrib: change the prompt for am-based rebases\n rebase tests: mark tests specific to the am-backend with --am\n rebase tests: repeat some tests using the merge backend instead of am\n rebase: make the backend configurable via config setting\n rebase: change the default backend from \"am\" to \"merge\"\n\n Documentation/config/rebase.txt | 8 ++\n Documentation/git-rebase.txt | 150 ++++++++++++++++----\n builtin/rebase.c | 181 +++++++++++++++++++-----\n contrib/completion/git-prompt.sh | 2 +-\n rebase-interactive.c | 4 +-\n rebase-interactive.h | 2 +-\n sequencer.c | 80 ++++++++---\n sequencer.h | 6 +-\n t/t3400-rebase.sh | 36 ++++-\n t/t3401-rebase-and-am-rename.sh | 4 +-\n t/t3404-rebase-interactive.sh | 2 +-\n t/t3406-rebase-message.sh | 19 ++-\n t/t3407-rebase-abort.sh | 6 +-\n t/t3420-rebase-autostash.sh | 2 +-\n t/t3421-rebase-topology-linear.sh | 4 +-\n t/t3424-rebase-empty.sh | 89 ++++++++++++\n t/t3425-rebase-topology-merges.sh | 8 +-\n t/t3427-rebase-subtree.sh | 16 ++-\n t/t3432-rebase-fast-forward.sh | 59 ++++----\n t/t3433-rebase-options-compatibility.sh | 13 +-\n t/t5407-post-rewrite-hook.sh | 12 +-\n t/t5520-pull.sh | 27 +++-\n t/t6047-diff3-conflict-markers.sh | 13 +-\n t/t7512-status-help.sh | 12 +-\n t/t9106-git-svn-commit-diff-clobber.sh | 3 +-\n t/t9903-bash-prompt.sh | 6 +-\n 26 files changed, 582 insertions(+), 182 deletions(-)\n create mode 100755 t/t3424-rebase-empty.sh\n\n\nbase-commit: 12029dc57db23baef008e77db1909367599210ee\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-679%2Fnewren%2Frebase-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-679/newren/rebase-fixes-v1\nPull-Request: https://github.com/git/git/pull/679\n-- \ngitgitgadget\n"},{"id":"388618","messageId":"47ea99fb30190b96b05f90735335424f7a37a95e.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 02/15] t3406: simplify an already simple test","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:35Z","receivedAt":"2019-12-20T17:09:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen the merge backend was re-implemented on top of the interactive\nbackend, the output of rebase --merge changed a little. This change\nallowed this test to be simplified, though it wasn't noticed until now.\nSimplify the testcase a little.\n\nSigned-off-by: Elijah Newren \n---\n t/t3406-rebase-message.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh\nindex b393e1e9fe..0c2c569f95 100755\n--- a/t/t3406-rebase-message.sh\n+++ b/t/t3406-rebase-message.sh\n@@ -18,11 +18,8 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'rebase -m' '\n-\tgit rebase -m master >report &&\n-\t>expect &&\n-\tsed -n -e \"/^Already applied: /p\" \\\n-\t\t-e \"/^Committed: /p\" report >actual &&\n-\ttest_cmp expect actual\n+\tgit rebase -m master >actual &&\n+\ttest_must_be_empty actual\n '\n \n test_expect_success 'rebase against master twice' '\n-- \ngitgitgadget\n\n"},{"id":"388619","messageId":"b99feebae111814be75dd99b5547539f8a764339.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 03/15] rebase, sequencer: remove the broken GIT_QUIET handling","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:36Z","receivedAt":"2019-12-20T17:09:55Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe GIT_QUIET environment variable was used to signal the non-am\nbackends that the rebase should perform quietly. The preserve-merges\nbackend does not make use of the quiet flag anywhere (other than to\nwrite out its state whenever it writes state), and this mechanism was\nbroken in the conversion from shell to C. Since this environment\nvariable was specifically designed for scripts and the only backend that\nwould still use it is no longer a script, just gut this code.\n\nA subsequent commit will fix --quiet for the interactive/merge backend\nin a different way.\n\nSigned-off-by: Elijah Newren \n---\n builtin/rebase.c | 6 ++----\n sequencer.c | 6 ++----\n 2 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 96db10eaed..c71d169688 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -718,8 +718,8 @@ static int rebase_write_basic_state(struct rebase_options *opts)\n \t\t opts->onto ? oid_to_hex(&opts->onto->object.oid) : \"\");\n \twrite_file(state_dir_path(\"orig-head\", opts), \"%s\",\n \t\t oid_to_hex(&opts->orig_head));\n-\twrite_file(state_dir_path(\"quiet\", opts), \"%s\",\n-\t\t opts->flags & REBASE_NO_QUIET ? \"\" : \"t\");\n+\tif (!(opts->flags & REBASE_NO_QUIET))\n+\t\twrite_file(state_dir_path(\"quiet\", opts), \"%s\", \"\");\n \tif (opts->flags & REBASE_VERBOSE)\n \t\twrite_file(state_dir_path(\"verbose\", opts), \"%s\", \"\");\n \tif (opts->strategy)\n@@ -1178,8 +1178,6 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)\n \tadd_var(&script_snippet, \"revisions\", opts->revisions);\n \tadd_var(&script_snippet, \"restrict_revision\", opts->restrict_revision ?\n \t\toid_to_hex(&opts->restrict_revision->object.oid) : NULL);\n-\tadd_var(&script_snippet, \"GIT_QUIET\",\n-\t\topts->flags & REBASE_NO_QUIET ? \"\" : \"t\");\n \tsq_quote_argv_pretty(&buf, opts->git_am_opts.argv);\n \tadd_var(&script_snippet, \"git_am_opt\", buf.buf);\n \tstrbuf_release(&buf);\ndiff --git a/sequencer.c b/sequencer.c\nindex d2c11f34b7..71062212a5 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2688,8 +2688,6 @@ static void write_strategy_opts(struct replay_opts *opts)\n int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\t struct commit *onto, const char *orig_head)\n {\n-\tconst char *quiet = getenv(\"GIT_QUIET\");\n-\n \tif (head_name)\n \t\twrite_file(rebase_path_head_name(), \"%s\\n\", head_name);\n \tif (onto)\n@@ -2698,8 +2696,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,\n \tif (orig_head)\n \t\twrite_file(rebase_path_orig_head(), \"%s\\n\", orig_head);\n \n-\tif (quiet)\n-\t\twrite_file(rebase_path_quiet(), \"%s\\n\", quiet);\n+\tif (opts->quiet)\n+\t\twrite_file(rebase_path_quiet(), \"%s\", \"\");\n \tif (opts->verbose)\n \t\twrite_file(rebase_path_verbose(), \"%s\", \"\");\n \tif (opts->strategy)\n-- \ngitgitgadget\n\n"},{"id":"388620","messageId":"13e2056e780b00baf86d4020c0974b6b05ce115b.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 01/15] rebase: extend the options for handling of empty commits","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:34Z","receivedAt":"2019-12-20T17:09:56Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nExtend the interactive machinery with the ability to handle the full\nspread of options for how to handle commits that either start or become\nempty (by \"become empty\" I mean the changes in a commit are a subset of\nchanges that exist upstream, so the net effect of applying the commit is\nno changes). Introduce a new command line flag for selecting the\ndesired behavior:\n --empty={drop,keep,ask}\nwith the definitions:\n drop: drop empty commits\n keep: keep empty commits\n ask: provide the user a chance to interact and pick what to do with\n empty commits on a case-by-case basis\n\nNote that traditionally, am-based rebases have always dropped commits\nthat either started or became empty, while interactive-based rebases\nhave defaulted to ask (and provided an option to keep commits that\nstarted empty). This difference made sense since users of an am-based\nrebase just wanted to quickly batch apply a sequence of commits, while\nusers editing a todo list will likely want the chance to interact and\nhandle unusual cases on a case-by-case basis. However, not all rebases\nusing the interactive machinery are explicitly interactive anymore. In\nparticular --merge was always meant to behave more like --am: just\nrebase a batch of commits without popping up a todo list.\n\nIf the --empty flag is not specified, pick defaults as follows:\n explicitly interactive: ask\n --exec: keep (exec is about checking existing commits, and often\n used without actually changing the base. Thus the\n expectation is that the user doesn't necessarily want\n anything to change; they just want to test).\n otherwise: drop\n\nAlso, this commit makes --keep-empty just imply --empty=keep, and hides\nit from help so that we aren't confusing users with different ways to do\nthe same thing. (I could have added a --drop-empty flag, but then that\ninvites users to specify both --keep-empty and --drop-empty and we have\nto add sanity checking around that; it seems cleaner to have a single\nmulti-valued option.) This actually fixes --keep-empty too; previously,\nit only meant to sometimes keep empty commits, in particular commits\nwhich started empty would be kept. But it would still error out and ask\nthe user what to do with commits that became empty. Now it keeps empty\ncommits, as instructed.\n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-rebase.txt | 35 ++++++------\n builtin/rebase.c | 87 +++++++++++++++++++++++++++---\n rebase-interactive.c | 4 +-\n rebase-interactive.h | 2 +-\n sequencer.c | 74 +++++++++++++++++++------\n sequencer.h | 6 ++-\n t/t3421-rebase-topology-linear.sh | 4 +-\n t/t3424-rebase-empty.sh | 89 +++++++++++++++++++++++++++++++\n t/t3427-rebase-subtree.sh | 16 +++---\n 9 files changed, 267 insertions(+), 50 deletions(-)\n create mode 100755 t/t3424-rebase-empty.sh\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 1d0e2d27cc..ff32ca1080 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -258,9 +258,25 @@ See also INCOMPATIBLE OPTIONS below.\n \toriginal branch. The index and working tree are also left\n \tunchanged as a result.\n \n+--empty={drop,keep,ask}::\n+\tHow to handle commits that become empty (because they contain a\n+\tsubset of already upstream changes) or start empty. With drop\n+\t(the default), commits that start or become empty are dropped.\n+\tWith keep (implied by --exec), such commits are kept. With ask\n+\t(implied by --interactive), the rebase will halt when an empty\n+\tcommit is applied allowing you to choose whether to drop it or\n+\tcommit it. Also with ask, if the rebase is interactive then\n+\tcommits which start empty will be commented out in the todo\n+\taction list (giving you a chance to uncomment).\n++\n+Note that this has no effect on commits which are already upstream (as\n+can be checked via `git log --cherry-mark ...`), which are always\n+dropped by rebase.\n++\n+See also INCOMPATIBLE OPTIONS below.\n+\n --keep-empty::\n-\tKeep the commits that do not change anything from its\n-\tparents in the result.\n+\tDeprecated alias for what is now known as --empty=keep.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n@@ -569,6 +585,7 @@ are incompatible with the following options:\n * --interactive\n * --exec\n * --keep-empty\n+ * --empty=\n * --edit-todo\n * --root when used in combination with --onto\n \n@@ -580,6 +597,7 @@ In addition, the following pairs of options are incompatible:\n * --preserve-merges and --ignore-whitespace\n * --preserve-merges and --committer-date-is-author-date\n * --preserve-merges and --ignore-date\n+ * --preserve-merges and --empty=\n * --keep-base and --onto\n * --keep-base and --root\n \n@@ -588,19 +606,6 @@ BEHAVIORAL DIFFERENCES\n \n There are some subtle differences how the backends behave.\n \n-Empty commits\n-~~~~~~~~~~~~~\n-\n-The am backend drops any \"empty\" commits, regardless of whether the\n-commit started empty (had no changes relative to its parent to\n-start with) or ended empty (all changes were already applied\n-upstream in other commits).\n-\n-The interactive backend drops commits by default that\n-started empty and halts if it hits a commit that ended up empty.\n-The `--keep-empty` option exists for the interactive backend to allow\n-it to keep commits that started empty.\n-\n Directory rename detection\n ~~~~~~~~~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex ddf33bc9d4..96db10eaed 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -50,8 +50,16 @@ enum rebase_type {\n \tREBASE_PRESERVE_MERGES\n };\n \n+enum empty_type {\n+\tEMPTY_UNSPECIFIED = -1,\n+\tEMPTY_DROP,\n+\tEMPTY_KEEP,\n+\tEMPTY_ASK\n+};\n+\n struct rebase_options {\n \tenum rebase_type type;\n+\tenum empty_type empty;\n \tconst char *state_dir;\n \tstruct commit *upstream;\n \tconst char *upstream_name;\n@@ -77,7 +85,6 @@ struct rebase_options {\n \tconst char *action;\n \tint signoff;\n \tint allow_rerere_autoupdate;\n-\tint keep_empty;\n \tint autosquash;\n \tint ignore_whitespace;\n \tchar *gpg_sign_opt;\n@@ -95,6 +102,7 @@ struct rebase_options {\n \n #define REBASE_OPTIONS_INIT {\t\t\t \t\\\n \t\t.type = REBASE_UNSPECIFIED,\t \t\\\n+\t\t.empty = EMPTY_UNSPECIFIED,\t \t\\\n \t\t.flags = REBASE_NO_QUIET, \t\t\\\n \t\t.git_am_opts = ARGV_ARRAY_INIT,\t\t\\\n \t\t.git_format_patch_opt = STRBUF_INIT\t\\\n@@ -114,6 +122,10 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)\n \t\treplay.allow_rerere_auto = opts->allow_rerere_autoupdate;\n \treplay.allow_empty = 1;\n \treplay.allow_empty_message = opts->allow_empty_message;\n+\treplay.drop_redundant_commits = (opts->empty == EMPTY_DROP);\n+\treplay.keep_redundant_commits = (opts->empty == EMPTY_KEEP);\n+\treplay.ask_on_initially_empty = (opts->empty == EMPTY_ASK &&\n+\t\t\t\t\t !(opts->flags & REBASE_INTERACTIVE_EXPLICIT));\n \treplay.verbose = opts->flags & REBASE_VERBOSE;\n \treplay.reschedule_failed_exec = opts->reschedule_failed_exec;\n \treplay.committer_date_is_author_date =\n@@ -389,7 +401,10 @@ static int run_rebase_interactive(struct rebase_options *opts,\n \n \tgit_config_get_bool(\"rebase.abbreviatecommands\", &abbreviate_commands);\n \n-\tflags |= opts->keep_empty ? TODO_LIST_KEEP_EMPTY : 0;\n+\tflags |= (opts->empty == EMPTY_DROP) ? TODO_LIST_DROP_EMPTY : 0;\n+\tflags |= (opts->empty == EMPTY_ASK &&\n+\t\t opts->flags & REBASE_INTERACTIVE_EXPLICIT) ?\n+\t\t\tTODO_LIST_ASK_EMPTY : 0;\n \tflags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;\n \tflags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;\n \tflags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;\n@@ -453,6 +468,19 @@ static int run_rebase_interactive(struct rebase_options *opts,\n \treturn ret;\n }\n \n+static int parse_opt_keep_empty(const struct option *opt, const char *arg,\n+\t\t\t\tint unset)\n+{\n+\tstruct rebase_options *opts = opt->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n+\n+\topts->empty = EMPTY_KEEP;\n+\topts->type = REBASE_INTERACTIVE;\n+\treturn 0;\n+}\n+\n static const char * const builtin_rebase_interactive_usage[] = {\n \tN_(\"git rebase--interactive []\"),\n \tNULL\n@@ -466,7 +494,10 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_NEGBIT(0, \"ff\", &opts.flags, N_(\"allow fast-forward\"),\n \t\t\t REBASE_FORCE),\n-\t\tOPT_BOOL(0, \"keep-empty\", &opts.keep_empty, N_(\"keep empty commits\")),\n+\t\t{ OPTION_CALLBACK, 'k', \"keep-empty\", &options, NULL,\n+\t\t\tN_(\"(DEPRECATED) keep empty commits\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,\n+\t\t\tparse_opt_keep_empty },\n \t\tOPT_BOOL(0, \"allow-empty-message\", &opts.allow_empty_message,\n \t\t\t N_(\"allow commits with empty messages\")),\n \t\tOPT_BOOL(0, \"rebase-merges\", &opts.rebase_merges, N_(\"rebase merge commits\")),\n@@ -1166,7 +1197,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)\n \t\topts->allow_rerere_autoupdate ?\n \t\t\topts->allow_rerere_autoupdate == RERERE_AUTOUPDATE ?\n \t\t\t\"--rerere-autoupdate\" : \"--no-rerere-autoupdate\" : \"\");\n-\tadd_var(&script_snippet, \"keep_empty\", opts->keep_empty ? \"yes\" : \"\");\n+\tadd_var(&script_snippet, \"empty\", opts->empty == EMPTY_KEEP ? \"yes\" : \"\");\n \tadd_var(&script_snippet, \"autosquash\", opts->autosquash ? \"t\" : \"\");\n \tadd_var(&script_snippet, \"gpg_sign_opt\", opts->gpg_sign_opt);\n \tadd_var(&script_snippet, \"cmd\", opts->cmd);\n@@ -1360,6 +1391,33 @@ static int parse_opt_interactive(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static long parse_empty_value(const char *value)\n+{\n+\tif (!value)\n+\t\treturn EMPTY_UNSPECIFIED;\n+\telse if (!strcasecmp(value, \"drop\"))\n+\t\treturn EMPTY_DROP;\n+\telse if (!strcasecmp(value, \"keep\"))\n+\t\treturn EMPTY_KEEP;\n+\telse if (!strcasecmp(value, \"ask\"))\n+\t\treturn EMPTY_ASK;\n+\treturn EMPTY_UNSPECIFIED;\n+}\n+\n+static int parse_opt_empty(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct rebase_options *options = opt->value;\n+\tlong value = parse_empty_value(arg);\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tif (value < 0)\n+\t\treturn error(_(\"option empty accepts \\\"drop\\\", \"\n+\t\t\t \"\\\"keep\\\", and \\\"ask\\\"\"));\n+\n+\toptions->empty = value;\n+\treturn 0;\n+}\n+\n static void NORETURN error_on_missing_default_upstream(void)\n {\n \tstruct branch *current_branch = branch_get(NULL);\n@@ -1505,8 +1563,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t\t \"ignoring them\"),\n \t\t\t REBASE_PRESERVE_MERGES, PARSE_OPT_HIDDEN),\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n-\t\tOPT_BOOL('k', \"keep-empty\", &options.keep_empty,\n-\t\t\t N_(\"preserve empty commits during rebase\")),\n+\t\tOPT_CALLBACK_F(0, \"empty\", &options, N_(\"{drop,keep,ask}\"),\n+\t\t\t N_(\"how to handle empty commits\"),\n+\t\t\t PARSE_OPT_NONEG, parse_opt_empty),\n+\t\t{ OPTION_CALLBACK, 'k', \"keep-empty\", &options, NULL,\n+\t\t\tN_(\"(DEPRECATED) keep empty commits\"),\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN,\n+\t\t\tparse_opt_keep_empty },\n \t\tOPT_BOOL(0, \"autosquash\", &options.autosquash,\n \t\t\t N_(\"move commits that begin with \"\n \t\t\t \"squash!/fixup! under -i\")),\n@@ -1770,8 +1833,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (!(options.flags & REBASE_NO_QUIET))\n \t\targv_array_push(&options.git_am_opts, \"-q\");\n \n-\tif (options.keep_empty)\n-\t\timply_interactive(&options, \"--keep-empty\");\n+\tif (options.empty != EMPTY_UNSPECIFIED)\n+\t\timply_interactive(&options, \"--empty\");\n \n \tif (gpg_sign) {\n \t\tfree(options.gpg_sign_opt);\n@@ -1856,6 +1919,14 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n+\tif (options.empty == EMPTY_UNSPECIFIED) {\n+\t\tif (options.flags & REBASE_INTERACTIVE_EXPLICIT)\n+\t\t\toptions.empty = EMPTY_ASK;\n+\t\telse if (exec.nr > 0)\n+\t\t\toptions.empty = EMPTY_KEEP;\n+\t\telse\n+\t\t\toptions.empty = EMPTY_DROP;\n+\t}\n \tif (reschedule_failed_exec > 0 && !is_interactive(&options))\n \t\tdie(_(\"--reschedule-failed-exec requires \"\n \t\t \"--exec or --interactive\"));\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex aa18ae82b7..ad82bf77df 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -28,7 +28,7 @@ static enum missing_commit_check_level get_missing_commit_check_level(void)\n \treturn MISSING_COMMIT_CHECK_IGNORE;\n }\n \n-void append_todo_help(unsigned keep_empty, int command_count,\n+void append_todo_help(unsigned no_ask_empty, int command_count,\n \t\t const char *shortrevisions, const char *shortonto,\n \t\t struct strbuf *buf)\n {\n@@ -81,7 +81,7 @@ void append_todo_help(unsigned keep_empty, int command_count,\n \n \tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \n-\tif (!keep_empty) {\n+\tif (!no_ask_empty) {\n \t\tmsg = _(\"Note that empty commits are commented out\");\n \t\tstrbuf_add_commented_lines(buf, msg, strlen(msg));\n \t}\ndiff --git a/rebase-interactive.h b/rebase-interactive.h\nindex 44dbb06311..f531e00ba7 100644\n--- a/rebase-interactive.h\n+++ b/rebase-interactive.h\n@@ -5,7 +5,7 @@ struct strbuf;\n struct repository;\n struct todo_list;\n \n-void append_todo_help(unsigned keep_empty, int command_count,\n+void append_todo_help(unsigned no_ask_empty, int command_count,\n \t\t const char *shortrevisions, const char *shortonto,\n \t\t struct strbuf *buf);\n int edit_todo_list(struct repository *r, struct todo_list *todo_list,\ndiff --git a/sequencer.c b/sequencer.c\nindex 763ccbbc45..d2c11f34b7 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -160,6 +160,9 @@ static GIT_PATH_FUNC(rebase_path_strategy, \"rebase-merge/strategy\")\n static GIT_PATH_FUNC(rebase_path_strategy_opts, \"rebase-merge/strategy_opts\")\n static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, \"rebase-merge/allow_rerere_autoupdate\")\n static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, \"rebase-merge/reschedule-failed-exec\")\n+static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, \"rebase-merge/drop_redundant_commits\")\n+static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, \"rebase-merge/keep_redundant_commits\")\n+static GIT_PATH_FUNC(rebase_path_ask_on_initially_empty, \"rebase-merge/ask_on_initially_empty\")\n \n static int git_sequencer_config(const char *k, const char *v, void *cb)\n {\n@@ -1623,7 +1626,7 @@ static int allow_empty(struct repository *r,\n \tempty_commit = is_original_commit_empty(commit);\n \tif (empty_commit < 0)\n \t\treturn empty_commit;\n-\tif (!empty_commit)\n+\tif (!empty_commit || opts->ask_on_initially_empty)\n \t\treturn 0;\n \telse\n \t\treturn 1;\n@@ -1837,7 +1840,7 @@ static int do_pick_commit(struct repository *r,\n \tchar *author = NULL;\n \tstruct commit_message msg = { NULL, NULL, NULL, NULL };\n \tstruct strbuf msgbuf = STRBUF_INIT;\n-\tint res, unborn = 0, reword = 0, allow;\n+\tint res, unborn = 0, reword = 0, allow, drop_commit;\n \n \tif (opts->no_commit) {\n \t\t/*\n@@ -2042,13 +2045,20 @@ static int do_pick_commit(struct repository *r,\n \t\tgoto leave;\n \t}\n \n-\tallow = allow_empty(r, opts, commit);\n-\tif (allow < 0) {\n-\t\tres = allow;\n-\t\tgoto leave;\n-\t} else if (allow)\n-\t\tflags |= ALLOW_EMPTY;\n-\tif (!opts->no_commit) {\n+\tdrop_commit = 0;\n+\tif (opts->drop_redundant_commits && is_index_unchanged(r)) {\n+\t\tdrop_commit = 1;\n+\t\tfprintf(stderr, _(\"No changes -- Patch already applied.\"));\n+\t} else {\n+\t\tallow = allow_empty(r, opts, commit);\n+\t\tif (allow < 0) {\n+\t\t\tres = allow;\n+\t\t\tgoto leave;\n+\t\t} else if (allow) {\n+\t\t\tflags |= ALLOW_EMPTY;\n+\t\t}\n+\t}\n+\tif (!opts->no_commit && !drop_commit) {\n \t\tif (author || command == TODO_REVERT || (flags & AMEND_MSG))\n \t\t\tres = do_commit(r, msg_file, author, opts, flags);\n \t\telse\n@@ -2501,9 +2511,15 @@ static int populate_opts_cb(const char *key, const char *value, void *data)\n \telse if (!strcmp(key, \"options.allow-empty-message\"))\n \t\topts->allow_empty_message =\n \t\t\tgit_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.drop-redundant-commits\"))\n+\t\topts->drop_redundant_commits =\n+\t\t\tgit_config_bool_or_int(key, value, &error_flag);\n \telse if (!strcmp(key, \"options.keep-redundant-commits\"))\n \t\topts->keep_redundant_commits =\n \t\t\tgit_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.ask_on_initially_empty\"))\n+\t\topts->ask_on_initially_empty =\n+\t\t\tgit_config_bool_or_int(key, value, &error_flag);\n \telse if (!strcmp(key, \"options.signoff\"))\n \t\topts->signoff = git_config_bool_or_int(key, value, &error_flag);\n \telse if (!strcmp(key, \"options.record-origin\"))\n@@ -2612,6 +2628,15 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\tif (file_exists(rebase_path_reschedule_failed_exec()))\n \t\t\topts->reschedule_failed_exec = 1;\n \n+\t\tif (file_exists(rebase_path_drop_redundant_commits()))\n+\t\t\topts->drop_redundant_commits = 1;\n+\n+\t\tif (file_exists(rebase_path_keep_redundant_commits()))\n+\t\t\topts->keep_redundant_commits = 1;\n+\n+\t\tif (file_exists(rebase_path_ask_on_initially_empty()))\n+\t\t\topts->ask_on_initially_empty = 1;\n+\n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_release(&buf);\n \n@@ -2695,6 +2720,12 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,\n \t\twrite_file(rebase_path_cdate_is_adate(), \"%s\", \"\");\n \tif (opts->ignore_date)\n \t\twrite_file(rebase_path_ignore_date(), \"%s\", \"\");\n+\tif (opts->drop_redundant_commits)\n+\t\twrite_file(rebase_path_drop_redundant_commits(), \"%s\", \"\");\n+\tif (opts->keep_redundant_commits)\n+\t\twrite_file(rebase_path_keep_redundant_commits(), \"%s\", \"\");\n+\tif (opts->ask_on_initially_empty)\n+\t\twrite_file(rebase_path_ask_on_initially_empty(), \"%s\", \"\");\n \tif (opts->reschedule_failed_exec)\n \t\twrite_file(rebase_path_reschedule_failed_exec(), \"%s\", \"\");\n \n@@ -3033,9 +3064,15 @@ static int save_opts(struct replay_opts *opts)\n \tif (opts->allow_empty_message)\n \t\tres |= git_config_set_in_file_gently(opts_file,\n \t\t\t\t\"options.allow-empty-message\", \"true\");\n+\tif (opts->drop_redundant_commits)\n+\t\tres |= git_config_set_in_file_gently(opts_file,\n+\t\t\t\t\"options.drop-redundant-commits\", \"true\");\n \tif (opts->keep_redundant_commits)\n \t\tres |= git_config_set_in_file_gently(opts_file,\n \t\t\t\t\"options.keep-redundant-commits\", \"true\");\n+\tif (opts->ask_on_initially_empty)\n+\t\tres |= git_config_set_in_file_gently(opts_file,\n+\t\t\t\t\"options.ask_on_initially_empty\", \"true\");\n \tif (opts->signoff)\n \t\tres |= git_config_set_in_file_gently(opts_file,\n \t\t\t\t\t\"options.signoff\", \"true\");\n@@ -4691,7 +4728,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\t\t struct rev_info *revs, struct strbuf *out,\n \t\t\t\t unsigned flags)\n {\n-\tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n+\tint drop_empty = flags & TODO_LIST_DROP_EMPTY;\n+\tint ask_empty = flags & TODO_LIST_ASK_EMPTY;\n \tint rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;\n \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n@@ -4746,6 +4784,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\tis_empty = is_original_commit_empty(commit);\n \t\tif (!is_empty && (commit->object.flags & PATCHSAME))\n \t\t\tcontinue;\n+\t\tif (is_empty && drop_empty)\n+\t\t\tcontinue;\n \n \t\tstrbuf_reset(&oneline);\n \t\tpretty_print_commit(pp, commit, &oneline);\n@@ -4754,7 +4794,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\tif (!to_merge) {\n \t\t\t/* non-merge commit: easy case */\n \t\t\tstrbuf_reset(&buf);\n-\t\t\tif (!keep_empty && is_empty)\n+\t\t\tif (is_empty && ask_empty)\n \t\t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n \t\t\tstrbuf_addf(&buf, \"%s %s %s\", cmd_pick,\n \t\t\t\t oid_to_hex(&commit->object.oid),\n@@ -4922,7 +4962,8 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,\n \tstruct pretty_print_context pp = {0};\n \tstruct rev_info revs;\n \tstruct commit *commit;\n-\tint keep_empty = flags & TODO_LIST_KEEP_EMPTY;\n+\tint drop_empty = flags & TODO_LIST_DROP_EMPTY;\n+\tint ask_empty = flags & TODO_LIST_ASK_EMPTY;\n \tconst char *insn = flags & TODO_LIST_ABBREVIATE_CMDS ? \"p\" : \"pick\";\n \tint rebase_merges = flags & TODO_LIST_REBASE_MERGES;\n \n@@ -4958,11 +4999,13 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,\n \t\treturn make_script_with_merges(&pp, &revs, out, flags);\n \n \twhile ((commit = get_revision(&revs))) {\n-\t\tint is_empty = is_original_commit_empty(commit);\n+\t\tint is_empty = is_original_commit_empty(commit);\n \n \t\tif (!is_empty && (commit->object.flags & PATCHSAME))\n \t\t\tcontinue;\n-\t\tif (!keep_empty && is_empty)\n+\t\tif (is_empty && drop_empty)\n+\t\t\tcontinue;\n+\t\tif (is_empty && ask_empty)\n \t\t\tstrbuf_addf(out, \"%c \", comment_line_char);\n \t\tstrbuf_addf(out, \"%s %s \", insn,\n \t\t\t oid_to_hex(&commit->object.oid));\n@@ -5100,7 +5143,8 @@ int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,\n \n \ttodo_list_to_strbuf(r, todo_list, &buf, num, flags);\n \tif (flags & TODO_LIST_APPEND_TODO_HELP)\n-\t\tappend_todo_help(flags & TODO_LIST_KEEP_EMPTY, count_commands(todo_list),\n+\t\tappend_todo_help(!(flags & TODO_LIST_ASK_EMPTY),\n+\t\t\t\t count_commands(todo_list),\n \t\t\t\t shortrevisions, shortonto, &buf);\n \n \tres = write_message(buf.buf, buf.len, file, 0);\ndiff --git a/sequencer.h b/sequencer.h\nindex e9a0e03ea2..1c3abb661c 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -39,7 +39,9 @@ struct replay_opts {\n \tint allow_rerere_auto;\n \tint allow_empty;\n \tint allow_empty_message;\n+\tint drop_redundant_commits;\n \tint keep_redundant_commits;\n+\tint ask_on_initially_empty;\n \tint verbose;\n \tint quiet;\n \tint reschedule_failed_exec;\n@@ -134,7 +136,7 @@ int sequencer_rollback(struct repository *repo, struct replay_opts *opts);\n int sequencer_skip(struct repository *repo, struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n-#define TODO_LIST_KEEP_EMPTY (1U << 0)\n+/* #define TODO_LIST_KEEP_EMPTY (1U << 0) */ /* No longer used */\n #define TODO_LIST_SHORTEN_IDS (1U << 1)\n #define TODO_LIST_ABBREVIATE_CMDS (1U << 2)\n #define TODO_LIST_REBASE_MERGES (1U << 3)\n@@ -150,6 +152,8 @@ int sequencer_remove_state(struct replay_opts *opts);\n * `--onto`, we do not want to re-generate the root commits.\n */\n #define TODO_LIST_ROOT_WITH_ONTO (1U << 6)\n+#define TODO_LIST_DROP_EMPTY (1U << 7)\n+#define TODO_LIST_ASK_EMPTY (1U << 8)\n \n \n int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex 325072b0a3..d23e0bf778 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -230,7 +230,7 @@ test_run_rebase () {\n test_run_rebase success ''\n test_run_rebase success -m\n test_run_rebase success -i\n-test_have_prereq !REBASE_P || test_run_rebase failure -p\n+test_have_prereq !REBASE_P || test_run_rebase success -p\n \n test_run_rebase () {\n \tresult=$1\n@@ -245,7 +245,7 @@ test_run_rebase () {\n test_run_rebase success ''\n test_run_rebase success -m\n test_run_rebase success -i\n-test_have_prereq !REBASE_P || test_run_rebase failure -p\n+test_have_prereq !REBASE_P || test_run_rebase success -p\n test_run_rebase success --rebase-merges\n \n # m\ndiff --git a/t/t3424-rebase-empty.sh b/t/t3424-rebase-empty.sh\nnew file mode 100755\nindex 0000000000..9d52e1417f\n--- /dev/null\n+++ b/t/t3424-rebase-empty.sh\n@@ -0,0 +1,89 @@\n+#!/bin/sh\n+\n+test_description='git rebase of commits that start or become empty'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup test repository' '\n+\ttest_write_lines 1 2 3 4 5 6 7 8 9 10 >numbers &&\n+\ttest_write_lines A B C D E F G H I J >letters &&\n+\tgit add numbers letters &&\n+\tgit commit -m A &&\n+\n+\tgit branch upstream &&\n+\tgit branch localmods &&\n+\n+\tgit checkout upstream &&\n+\ttest_write_lines A B C D E >letters &&\n+\tgit add letters &&\n+\tgit commit -m B &&\n+\n+\ttest_write_lines 1 2 3 4 five 6 7 8 9 ten >numbers &&\n+\tgit add numbers &&\n+\tgit commit -m C &&\n+\n+\tgit checkout localmods &&\n+\ttest_write_lines 1 2 3 4 five 6 7 8 9 10 >numbers &&\n+\tgit add numbers &&\n+\tgit commit -m C2 &&\n+\n+\tgit commit --allow-empty -m D &&\n+\n+\ttest_write_lines A B C D E >letters &&\n+\tgit add letters &&\n+\tgit commit -m \"Five letters ought to be enough for anybody\"\n+'\n+\n+test_expect_success 'rebase --merge --empty=drop' '\n+\tgit checkout -B testing localmods &&\n+\tgit rebase --merge --empty=drop upstream &&\n+\n+\ttest_write_lines C B A >expect &&\n+\tgit log --format=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rebase --merge --empty=keep' '\n+\tgit checkout -B testing localmods &&\n+\tgit rebase --merge --empty=keep upstream &&\n+\n+\ttest_write_lines D C2 C B A >expect &&\n+\tgit log --format=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rebase --merge --empty=ask' '\n+\tgit checkout -B testing localmods &&\n+\ttest_must_fail git rebase --merge --empty=ask upstream &&\n+\n+\ttest_must_fail git rebase --skip &&\n+\tgit commit --allow-empty &&\n+\tgit rebase --continue &&\n+\n+\ttest_write_lines D C B A >expect &&\n+\tgit log --format=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+GIT_SEQUENCE_EDITOR=: && export GIT_SEQUENCE_EDITOR\n+\n+test_expect_success 'rebase --interactive --empty=drop' '\n+\tgit checkout -B testing localmods &&\n+\tgit rebase --interactive --empty=drop upstream &&\n+\n+\ttest_write_lines C B A >expect &&\n+\tgit log --format=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'rebase --interactive --empty=keep' '\n+\tgit checkout -B testing localmods &&\n+\tgit rebase --interactive --empty=keep upstream &&\n+\n+\ttest_write_lines D C2 C B A >expect &&\n+\tgit log --format=%s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+\n+test_done\ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nindex bec48e6a1f..468ebc1bef 100755\n--- a/t/t3427-rebase-subtree.sh\n+++ b/t/t3427-rebase-subtree.sh\n@@ -85,23 +85,27 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --\n \tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n '\n \n-test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '\n+test_expect_success 'Rebase -Xsubtree --empty=ask --onto commit' '\n \treset_rebase &&\n \tgit checkout -b rebase-onto to-rebase &&\n-\ttest_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&\n+\ttest_must_fail git rebase -Xsubtree=files_subtree --empty=ask --onto files-master master &&\n \t: first pick results in no changes &&\n-\tgit rebase --continue &&\n+\ttest_must_fail git rebase --skip &&\n+\t: last pick was an empty commit that has no changes, but we want to keep it &&\n+\tgit commit --allow-empty &&\n \tverbose test \"$(commit_message HEAD~2)\" = \"master4\" &&\n \tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/master5\" &&\n \tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n '\n \n-test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' '\n+test_expect_success 'Rebase -Xsubtree --empty=ask --rebase-merges --onto commit' '\n \treset_rebase &&\n \tgit checkout -b rebase-merges-onto to-rebase &&\n-\ttest_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root &&\n+\ttest_must_fail git rebase -Xsubtree=files_subtree --empty=ask --rebase-merges --onto files-master --root &&\n \t: first pick results in no changes &&\n-\tgit rebase --continue &&\n+\ttest_must_fail git rebase --skip &&\n+\t: last pick was an empty commit that has no changes, but we want to keep it &&\n+\tgit commit --allow-empty &&\n \tverbose test \"$(commit_message HEAD~2)\" = \"master4\" &&\n \tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/master5\" &&\n \tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n-- \ngitgitgadget\n\n"},{"id":"388621","messageId":"9b1ad46c58b7f8397acbf2a8339e150dfb04c956.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 06/15] t3432: make these tests work with either am or merge backends","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:39Z","receivedAt":"2019-12-20T17:09:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nt3432 had several stress tests for can_fast_forward(), whose intent was\nto ensure we were using the optimization of just fast forwarding when\npossible. However, these tests verified that fast forwards had happened\nbased on the output that rebase printed to the terminal. We can instead\ntest more directly that we actually fast-forwarded by checking the\nreflog, which also has the side effect of making the tests applicable\nfor the merge/interactive backend.\n\nThis change does lose the distinction between \"noop\" and \"noop-force\",\nbut as stated in commit c9efc216830f (\"t3432: test for --no-ff's\ninteraction with fast-forward\", 2019-08-27) which introduced that\ndistinction: \"These tests aren't supposed to endorse the status quo,\njust test for what we're currently doing.\".\n\nThis change does not actually run these tests with the merge/interactive\nbackend; instead this is just a preparatory commit. A subsequent commit\nwhich fixes can_fast_forward() to work with that backend will then also\nchange t3432 to add tests of that backend as well.\n\nSigned-off-by: Elijah Newren \n---\n t/t3432-rebase-fast-forward.sh | 53 ++++++++++++++++------------------\n 1 file changed, 25 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh\nindex 92f95b57da..3879a43fa0 100755\n--- a/t/t3432-rebase-fast-forward.sh\n+++ b/t/t3432-rebase-fast-forward.sh\n@@ -44,19 +44,16 @@ test_rebase_same_head_ () {\n \ttest_expect_$status \"git rebase$flag $* with $changes is $what with $cmp HEAD\" \"\n \t\toldhead=\\$(git rev-parse HEAD) &&\n \t\ttest_when_finished 'git reset --hard \\$oldhead' &&\n+\t\tcp .git/logs/HEAD expect &&\n \t\tgit rebase$flag $* >stdout &&\n \t\tif test $what = work\n \t\tthen\n-\t\t\t# Must check this case first, for 'is up to\n-\t\t\t# date, rebase forced[...]rewinding head' cases\n-\t\t\ttest_i18ngrep 'rewinding head' stdout\n+\t\t\twc -l .git/logs/HEAD >old &&\n+\t\t\twc -l .git/logs/HEAD >new &&\n+\t\t\ttest_line_count '-gt' $(($old + 2)) .git/logs/HEAD\n \t\telif test $what = noop\n \t\tthen\n-\t\t\ttest_i18ngrep 'is up to date' stdout &&\n-\t\t\ttest_i18ngrep ! 'rebase forced' stdout\n-\t\telif test $what = noop-force\n-\t\tthen\n-\t\t\ttest_i18ngrep 'is up to date, rebase forced' stdout\n+\t\t\ttest_cmp expect .git/logs/HEAD\n \t\tfi &&\n \t\tnewhead=\\$(git rev-parse HEAD) &&\n \t\tif test $cmp = same\n@@ -71,14 +68,14 @@ test_rebase_same_head_ () {\n \n changes='no changes'\n test_rebase_same_head success noop same success work same\n-test_rebase_same_head success noop same success noop-force same master\n-test_rebase_same_head success noop same success noop-force diff --onto B B\n-test_rebase_same_head success noop same success noop-force diff --onto B... B\n-test_rebase_same_head success noop same success noop-force same --onto master... master\n-test_rebase_same_head success noop same success noop-force same --keep-base master\n-test_rebase_same_head success noop same success noop-force same --keep-base\n-test_rebase_same_head success noop same success noop-force same --no-fork-point\n-test_rebase_same_head success noop same success noop-force same --keep-base --no-fork-point\n+test_rebase_same_head success noop same success work same master\n+test_rebase_same_head success noop same success work diff --onto B B\n+test_rebase_same_head success noop same success work diff --onto B... B\n+test_rebase_same_head success noop same success work same --onto master... master\n+test_rebase_same_head success noop same success work same --keep-base master\n+test_rebase_same_head success noop same success work same --keep-base\n+test_rebase_same_head success noop same success work same --no-fork-point\n+test_rebase_same_head success noop same success work same --keep-base --no-fork-point\n test_rebase_same_head success noop same success work same --fork-point master\n test_rebase_same_head success noop same success work diff --fork-point --onto B B\n test_rebase_same_head success noop same success work diff --fork-point --onto B... B\n@@ -91,14 +88,14 @@ test_expect_success 'add work same to side' '\n \n changes='our changes'\n test_rebase_same_head success noop same success work same\n-test_rebase_same_head success noop same success noop-force same master\n-test_rebase_same_head success noop same success noop-force diff --onto B B\n-test_rebase_same_head success noop same success noop-force diff --onto B... B\n-test_rebase_same_head success noop same success noop-force same --onto master... master\n-test_rebase_same_head success noop same success noop-force same --keep-base master\n-test_rebase_same_head success noop same success noop-force same --keep-base\n-test_rebase_same_head success noop same success noop-force same --no-fork-point\n-test_rebase_same_head success noop same success noop-force same --keep-base --no-fork-point\n+test_rebase_same_head success noop same success work same master\n+test_rebase_same_head success noop same success work diff --onto B B\n+test_rebase_same_head success noop same success work diff --onto B... B\n+test_rebase_same_head success noop same success work same --onto master... master\n+test_rebase_same_head success noop same success work same --keep-base master\n+test_rebase_same_head success noop same success work same --keep-base\n+test_rebase_same_head success noop same success work same --no-fork-point\n+test_rebase_same_head success noop same success work same --keep-base --no-fork-point\n test_rebase_same_head success noop same success work same --fork-point master\n test_rebase_same_head success noop same success work diff --fork-point --onto B B\n test_rebase_same_head success noop same success work diff --fork-point --onto B... B\n@@ -112,13 +109,13 @@ test_expect_success 'add work same to upstream' '\n '\n \n changes='our and their changes'\n-test_rebase_same_head success noop same success noop-force diff --onto B B\n-test_rebase_same_head success noop same success noop-force diff --onto B... B\n+test_rebase_same_head success noop same success work diff --onto B B\n+test_rebase_same_head success noop same success work diff --onto B... B\n test_rebase_same_head success noop same success work diff --onto master... master\n test_rebase_same_head success noop same success work diff --keep-base master\n test_rebase_same_head success noop same success work diff --keep-base\n-test_rebase_same_head failure work same success work diff --fork-point --onto B B\n-test_rebase_same_head failure work same success work diff --fork-point --onto B... B\n+test_rebase_same_head success work same success work diff --fork-point --onto B B\n+test_rebase_same_head success work same success work diff --fork-point --onto B... B\n test_rebase_same_head success noop same success work diff --fork-point --onto master... master\n test_rebase_same_head success noop same success work diff --fork-point --keep-base master\n \n-- \ngitgitgadget\n\n"},{"id":"388623","messageId":"66bee10d1ca45548cf9b5a5bc2e17703ed54a82f.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 04/15] rebase: make sure to pass along the quiet flag to the sequencer","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:37Z","receivedAt":"2019-12-20T17:09:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nSigned-off-by: Elijah Newren \n---\n builtin/rebase.c | 3 ++-\n t/t3400-rebase.sh | 8 +++++++-\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex c71d169688..cc8f3f008f 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -126,6 +126,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)\n \treplay.keep_redundant_commits = (opts->empty == EMPTY_KEEP);\n \treplay.ask_on_initially_empty = (opts->empty == EMPTY_ASK &&\n \t\t\t\t\t !(opts->flags & REBASE_INTERACTIVE_EXPLICIT));\n+\treplay.quiet = !(opts->flags & REBASE_NO_QUIET);\n \treplay.verbose = opts->flags & REBASE_VERBOSE;\n \treplay.reschedule_failed_exec = opts->reschedule_failed_exec;\n \treplay.committer_date_is_author_date =\n@@ -1506,7 +1507,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"allow pre-rebase hook to run\")),\n \t\tOPT_NEGBIT('q', \"quiet\", &options.flags,\n \t\t\t N_(\"be quiet. implies --no-stat\"),\n-\t\t\t REBASE_NO_QUIET| REBASE_VERBOSE | REBASE_DIFFSTAT),\n+\t\t\t REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT),\n \t\tOPT_BIT('v', \"verbose\", &options.flags,\n \t\t\tN_(\"display a diffstat of what changed upstream\"),\n \t\t\tREBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT),\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 221b35f2df..79762b989a 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -206,12 +206,18 @@ test_expect_success 'cherry-picked commits and fork-point work together' '\n \ttest_cmp expect D\n '\n \n-test_expect_success 'rebase -q is quiet' '\n+test_expect_success 'rebase --am -q is quiet' '\n \tgit checkout -b quiet topic &&\n \tgit rebase -q master >output.out 2>&1 &&\n \ttest_must_be_empty output.out\n '\n \n+test_expect_success 'rebase --merge -q is quiet' '\n+\tgit checkout -B quiet topic &&\n+\tgit rebase --merge -q master >output.out 2>&1 &&\n+\ttest_must_be_empty output.out\n+'\n+\n test_expect_success 'Rebase a commit that sprinkles CRs in' '\n \t(\n \t\techo \"One\" &&\n-- \ngitgitgadget\n\n"},{"id":"388625","messageId":"5c31d664f3b410cc165ec7757041a0a82ba2b8c0.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 09/15] rebase: move incompatibility checks between backend options a bit earlier","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:42Z","receivedAt":"2019-12-20T17:10:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nSigned-off-by: Elijah Newren \n---\n builtin/rebase.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 67bccd876f..9e7e88b147 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1910,6 +1910,17 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (isatty(2) && options.flags & REBASE_NO_QUIET)\n \t\tstrbuf_addstr(&options.git_format_patch_opt, \" --progress\");\n \n+\tif (options.git_am_opts.argc) {\n+\t\t/* all am options except -q are compatible only with --am */\n+\t\tfor (i = options.git_am_opts.argc - 1; i >= 0; i--)\n+\t\t\tif (strcmp(options.git_am_opts.argv[i], \"-q\"))\n+\t\t\t\tbreak;\n+\n+\t\tif (is_interactive(&options) && i >= 0)\n+\t\t\tdie(_(\"cannot combine am options with either \"\n+\t\t\t \"interactive or merge options\"));\n+\t}\n+\n \tswitch (options.type) {\n \tcase REBASE_MERGE:\n \tcase REBASE_INTERACTIVE:\n@@ -1940,17 +1951,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (reschedule_failed_exec >= 0)\n \t\toptions.reschedule_failed_exec = reschedule_failed_exec;\n \n-\tif (options.git_am_opts.argc) {\n-\t\t/* all am options except -q are compatible only with --am */\n-\t\tfor (i = options.git_am_opts.argc - 1; i >= 0; i--)\n-\t\t\tif (strcmp(options.git_am_opts.argv[i], \"-q\"))\n-\t\t\t\tbreak;\n-\n-\t\tif (is_interactive(&options) && i >= 0)\n-\t\t\tdie(_(\"cannot combine am options with either \"\n-\t\t\t \"interactive or merge options\"));\n-\t}\n-\n \tif (options.signoff) {\n \t\tif (options.type == REBASE_PRESERVE_MERGES)\n \t\t\tdie(\"cannot combine '--signoff' with \"\n-- \ngitgitgadget\n\n"},{"id":"388622","messageId":"a4073df9108b7e9a825371f5925d1841463aedb4.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 07/15] rebase: allow more types of rebases to fast-forward","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:40Z","receivedAt":"2019-12-20T17:10:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn the past, we dis-allowed rebases using the interactive backend from\nperforming a fast-forward to short-circuit the rebase operation. This\nmade sense for explicitly interactive rebases and some implicitly\ninteractive rebases, but certainly became overly stringent when the\nmerge backend was re-implemented via the interactive backend.\n\nJust as the am-based rebase has always had to disable the fast-forward\nbased on a variety of conditions or flags (e.g. --signoff, --whitespace,\netc.), we need to do the same but now with a few more options. However,\ncontinuing to use REBASE_FORCE for tracking this is problematic because\nthe interactive backend used it for a different purpose. (When\nREBASE_FORCE wasn't set, the interactive backend would not fast-forward\nthe whole series but would fast-forward individual \"pick\" commits at the\nbeginning of the todo list, and then a squash or something would cause\nit to start generating new commits.) So, introduce a new\nallow_preemptive_ff flag contained within cmd_rebase() and use it to\ntrack whether we are going to allow a pre-emptive fast-forward that\nshort-circuits the whole rebase.\n\nSigned-off-by: Elijah Newren \n---\n builtin/rebase.c | 18 ++++++++++++++----\n t/t3432-rebase-fast-forward.sh | 2 ++\n 2 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex b320bb3a30..67bccd876f 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1497,6 +1497,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tstruct object_id squash_onto;\n \tchar *squash_onto_name = NULL;\n \tint reschedule_failed_exec = -1;\n+\tint allow_preemptive_ff = 1;\n \tstruct option builtin_rebase_options[] = {\n \t\tOPT_STRING(0, \"onto\", &options.onto_name,\n \t\t\t N_(\"revision\"),\n@@ -1808,11 +1809,18 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t options.ignore_date)\n \t\toptions.flags |= REBASE_FORCE;\n \n+\tif ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||\n+\t (action != ACTION_NONE) ||\n+\t (exec.nr > 0) ||\n+\t options.autosquash) {\n+\t\tallow_preemptive_ff = 0;\n+\t}\n+\n \tfor (i = 0; i < options.git_am_opts.argc; i++) {\n \t\tconst char *option = options.git_am_opts.argv[i], *p;\n \t\tif (!strcmp(option, \"--whitespace=fix\") ||\n \t\t !strcmp(option, \"--whitespace=strip\"))\n-\t\t\toptions.flags |= REBASE_FORCE;\n+\t\t\tallow_preemptive_ff = 0;\n \t\telse if (skip_prefix(option, \"-C\", &p)) {\n \t\t\twhile (*p)\n \t\t\t\tif (!isdigit(*(p++)))\n@@ -2148,12 +2156,14 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t/*\n \t * Check if we are already based on onto with linear history,\n \t * in which case we could fast-forward without replacing the commits\n-\t * with new commits recreated by replaying their changes. This\n-\t * optimization must not be done if this is an interactive rebase.\n+\t * with new commits recreated by replaying their changes.\n+\t *\n+\t * Note that can_fast_forward() initializes merge_base, so we have to\n+\t * call it before checking allow_preemptive_ff.\n \t */\n \tif (can_fast_forward(options.onto, options.upstream, options.restrict_revision,\n \t\t &options.orig_head, &merge_base) &&\n-\t !is_interactive(&options)) {\n+\t allow_preemptive_ff) {\n \t\tint flag;\n \n \t\tif (!(options.flags & REBASE_FORCE)) {\ndiff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh\nindex 3879a43fa0..58c91c6899 100755\n--- a/t/t3432-rebase-fast-forward.sh\n+++ b/t/t3432-rebase-fast-forward.sh\n@@ -30,6 +30,8 @@ test_rebase_same_head () {\n \tshift &&\n \ttest_rebase_same_head_ $status_n $what_n $cmp_n \"\" \"$*\" &&\n \ttest_rebase_same_head_ $status_f $what_f $cmp_f \" --no-ff\" \"$*\"\n+\ttest_rebase_same_head_ $status_n $what_n $cmp_n \" --merge\" \"$*\" &&\n+\ttest_rebase_same_head_ $status_f $what_f $cmp_f \" --merge --no-ff\" \"$*\"\n }\n \n test_rebase_same_head_ () {\n-- \ngitgitgadget\n\n"},{"id":"388624","messageId":"90e8927ea0533a078fe7fade9264f798bcdaa6ad.1576861788.git.gitgitgadget@gmail.com","threadId":"52496","inReplyTo":"pull.679.git.git.1576861788.gitgitgadget@gmail.com","subject":"[PATCH 08/15] git-rebase.txt: add more details about behavioral differences of backends","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-20T17:09:41Z","receivedAt":"2019-12-20T17:10:03Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nSigned-off-by: Elijah Newren \n---\n Documentation/git-rebase.txt | 102 +++++++++++++++++++++---\n t/t3433-rebase-options-compatibility.sh | 5 +-\n 2 files changed, 94 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex ff32ca1080..f1ace07c38 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -409,13 +409,10 @@ your branch contains commits which were dropped, this option can be used\n with `--keep-base` in order to drop those commits from your branch.\n \n --ignore-whitespace::\n-\tBehaves differently depending on which backend is selected.\n-+\n-'am' backend: When applying a patch, ignore changes in whitespace in\n-context lines if necessary.\n-+\n-'interactive' backend: Treat lines with only whitespace changes as\n-unchanged for the sake of a three-way merge.\n+\tIgnore whitespace-only changes in the commits being rebased,\n+\twhich may avoid \"unnecessary\" conflicts. (Both backends\n+\tcurrently have differing edgecase bugs with this option; see\n+\tBEHAVIORAL DIFFERENCES.)\n \n --whitespace=