{"thread":{"id":"41157","subject":"[PATCH 1/5] Teach cherry-pick to skip redundant commits if asked","startedAt":"2016-01-11T05:00:16Z","lastAt":"2016-01-12T03:10:45Z","messageCount":9,"participants":["David Greene","Junio C Hamano","Eric Sunshine","David A. Greene"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"275629","messageId":"1452488421-26823-1-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":null,"subject":"[PATCH] Teach cherry-pick and rebase to ignore redundant commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:16Z","receivedAt":"2016-01-11T05:00:16Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"This patch set adds a --skip-redundant-commits option to both\ncherry-pick and rebase.  Currently, if cherry-pick applies a\ncommit that happens to become empty after conflict resolution,\nit will abort and ask the user what to do.  This behavior\npropagates to rebase when rebase is forced to fall back to\ncherry-pick in certain situations.\n\nThis abort failure mode makes it difficult to script certain\ntypes of operations that users might expect to work: for example\ncherry-picking a commit on top of itself or rebasing a set of\ncommits back onto its own history.\n\nWith --skip-redundant commits users and/or scripts can choose\nto have cherry-pick/rebase simply ignore the redundant commit\nand move on.  There is already a --keep-redundant-commits flag\nso this is really just supplying the natural counter-behavior\nusers might way.\n\n                     -Davod\n"},{"id":"275625","messageId":"1452488421-26823-2-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":"1452488421-26823-1-git-send-email-greened@obbligato.org","subject":"[PATCH 1/5] Teach cherry-pick to skip redundant commits if asked","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:17Z","receivedAt":"2016-01-11T05:00:17Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nAdd a \"--skip-redundant-commits\" option to cherry-pick to avoid\naborting if the cherry-picked commit becomes empty due to\nconflict resolution.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n builtin/revert.c |  7 +++++++\n sequencer.c      | 23 +++++++++++++++++++++++\n sequencer.h      |  1 +\n 3 files changed, 31 insertions(+)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 56a2c36..befd3ce 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -91,6 +91,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tN_(\"option for merge strategy\"), option_parse_x),\n \t\t{ OPTION_STRING, 'S', \"gpg-sign\", &opts->gpg_sign, N_(\"key-id\"),\n \t\t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n+\t\t/*\n+\t\t * There must be enough OPT_END() here to match the\n+\t\t * size of cp_extra below so that parse_options_concat\n+\t\t * will work.\n+\t\t */\n+\t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n@@ -106,6 +112,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tOPT_BOOL(0, \"allow-empty\", &opts->allow_empty, N_(\"preserve initially empty commits\")),\n \t\t\tOPT_BOOL(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n \t\t\tOPT_BOOL(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n+\t\t\tOPT_BOOL(0, \"skip-redundant-commits\", &opts->skip_redundant_commits, N_(\"skip redundant, empty commits\")),\n \t\t\tOPT_END(),\n \t\t};\n \t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\ndiff --git a/sequencer.c b/sequencer.c\nindex 8c58fa2..12361e7 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -185,6 +185,7 @@ static void print_advice(int show_hint, struct replay_opts *opts)\n \t\telse\n \t\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n \t\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n+\n \t\t\t\t \"and commit the result with 'git commit'\"));\n \t}\n }\n@@ -614,6 +615,28 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\tres = allow;\n \t\tgoto leave;\n \t}\n+\n+\t// If told, do not try to commit things that don't make any\n+\t// changes.\n+\tif (opts->skip_redundant_commits) {\n+\t\tint index_unchanged = is_index_unchanged();\n+\t\tif (index_unchanged < 0) {\n+\t\t\t// Something bad happened readhing HEAD or the\n+\t\t\t// index.  Abort.\n+\t\t\tres = index_unchanged;\n+\t\t\tgoto leave;\n+\t\t}\n+\t\tif (index_unchanged) {\n+\t\t\tfputs(_(\"Skipping redundant commit \"), stderr);\n+\t\t\tfputs(find_unique_abbrev(commit->object.oid.hash,\n+\t\t\t\t\t\t GIT_SHA1_HEXSZ),\n+\t\t\t      stderr);\n+\t\t\tfputs(\"\\n\", stderr);\n+\t\t\tres = 0;\n+\t\t\tgoto leave;\n+\t\t}\n+\t}\n+\n \tif (!opts->no_commit)\n \t\tres = run_git_commit(git_path_merge_msg(), opts, allow);\n \ndiff --git a/sequencer.h b/sequencer.h\nindex 5ed5cb1..ad6145d 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -34,6 +34,7 @@ struct replay_opts {\n \tint allow_empty;\n \tint allow_empty_message;\n \tint keep_redundant_commits;\n+\tint skip_redundant_commits;\n \n \tint mainline;\n \n-- \n2.6.1\n"},{"id":"275626","messageId":"1452488421-26823-3-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":"1452488421-26823-1-git-send-email-greened@obbligato.org","subject":"[PATCH 2/5] Add test for cherry-pick --skip-redundant-commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:18Z","receivedAt":"2016-01-11T05:00:18Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nTest that --skip-redundant commits suppresses an abort when\ncherry-picking a redundant commit.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n t/t3514-cherry-pick-redundant.sh | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n create mode 100755 t/t3514-cherry-pick-redundant.sh\n\ndiff --git a/t/t3514-cherry-pick-redundant.sh b/t/t3514-cherry-pick-redundant.sh\nnew file mode 100755\nindex 0000000..c433344\n--- /dev/null\n+++ b/t/t3514-cherry-pick-redundant.sh\n@@ -0,0 +1,26 @@\n+#!/bin/sh\n+\n+test_description='git cherry-pick tests for redundant commits\n+\n+This test runs git cherry-pick and tests handling of redundant commits.\n+'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit one &&\n+\ttest_commit two &&\n+\ttest_commit three\n+'\n+\n+test_expect_success 'cherry-pick redundant commit fails' '\n+\tgit branch test-start &&\n+\tgit reset --hard HEAD~2 &&\n+\tgit cherry-pick test-start &&\n+\ttest_must_fail git cherry-pick test-start\n+'\n+\n+test_expect_success 'cherry-pick with --skip-redundant-commits' '\n+\tgit cherry-pick --skip-redundant-commits test-start\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"275628","messageId":"1452488421-26823-4-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":"1452488421-26823-1-git-send-email-greened@obbligato.org","subject":"[PATCH 3/5] Add --skip-redundant-commits option to rebase","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:19Z","receivedAt":"2016-01-11T05:00:19Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nTeach rebase to ignore redundant commits if told.  Since rebase\nnormally automatically skips redundant commits, this only applies\nwhen it has to use cherry-pick to do its work.  In that case,\npass the --skip-redundant-commits flag to cherry-pick.\n\nThis allows scripted use of rebase with options like\n--preserve-merges that tend to invoke cherry-pick.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n git-rebase--interactive.sh | 14 ++++++++++++--\n git-rebase.sh              |  5 +++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex c0cfe88..5891ff5 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -269,9 +269,14 @@ pick_one () {\n \n \ttest -d \"$rewritten\" &&\n \t\tpick_one_preserving_merges \"$@\" && return\n+\tredundant_args=\n+\tif test -n \"$skip_redundant_commits\"\n+\tthen\n+\t\tredundant_args=\"--skip-redundant-commits\"\n+\tfi\n \toutput eval git cherry-pick \\\n \t\t\t${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")} \\\n-\t\t\t\"$strategy_args\" $empty_args $ff \"$@\"\n+\t\t\t\"$strategy_args\" $empty_args $redundant_args $ff \"$@\"\n \n \t# If cherry-pick dies it leaves the to-be-picked commit unrecorded. Reschedule\n \t# previous task so this commit is not lost.\n@@ -389,9 +394,14 @@ pick_one_preserving_merges () {\n \t\t\techo \"$sha1 $(git rev-parse HEAD^0)\" >> \"$rewritten_list\"\n \t\t\t;;\n \t\t*)\n+\t\t\tredundant_args=\n+\t\t\tif test -n \"$skip_redundant_commits\"\n+\t\t\tthen\n+\t\t\t\tredundant_args=\"--skip-redundant-commits\"\n+\t\t\tfi\n \t\t\toutput eval git cherry-pick \\\n \t\t\t\t${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")} \\\n-\t\t\t\t\"$strategy_args\" \"$@\" ||\n+\t\t\t\t\"$strategy_args\" $redundant_args \"$@\" ||\n \t\t\t\tdie_with_patch $sha1 \"Could not pick $sha1\"\n \t\t\t;;\n \t\tesac\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex af7ba5f..420a54f 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -24,6 +24,7 @@ m,merge!           use merging strategies to rebase\n i,interactive!     let the user edit the list of commits to rebase\n x,exec=!           add exec lines after each commit of the editable list\n k,keep-empty\t   preserve empty commits during rebase\n+skip-redundant-commits ignore redundant commits during rebase\n f,force-rebase!    force rebase even if branch is up to date\n X,strategy-option=! pass the argument through to the merge strategy\n stat!              display a diffstat of what changed upstream\n@@ -86,6 +87,7 @@ action=\n preserve_merges=\n autosquash=\n keep_empty=\n+skip_redundant_commits=\n test \"$(git config --bool rebase.autosquash)\" = \"true\" && autosquash=t\n gpg_sign_opt=\n \n@@ -255,6 +257,9 @@ do\n \t--keep-empty)\n \t\tkeep_empty=yes\n \t\t;;\n+\t--skip-redundant-commits)\n+\t\tskip_redundant_commits=yes\n+\t\t;;\n \t--preserve-merges)\n \t\tpreserve_merges=t\n \t\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n-- \n2.6.1\n"},{"id":"275630","messageId":"1452488421-26823-5-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":"1452488421-26823-1-git-send-email-greened@obbligato.org","subject":"[PATCH 4/5] Add test for redundant rebase","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:20Z","receivedAt":"2016-01-11T05:00:20Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nTest that rebase --skip-redundant-commits works.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n t/t3428-rebase-redundant.sh | 100 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 100 insertions(+)\n create mode 100755 t/t3428-rebase-redundant.sh\n\ndiff --git a/t/t3428-rebase-redundant.sh b/t/t3428-rebase-redundant.sh\nnew file mode 100755\nindex 0000000..eea63e9\n--- /dev/null\n+++ b/t/t3428-rebase-redundant.sh\n@@ -0,0 +1,100 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for redundant commits\n+\n+This test runs git rebase and tests handling of redundant commits.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tgit checkout -b empty &&\n+\ttest_commit empty1 &&\n+\ttest_commit empty2 &&\n+\tgit commit -m \"Empty commit 1\" --allow-empty &&\n+\ttest_commit empty3 &&\n+\tgit checkout -b onto master &&\n+\ttest_commit onto1 &&\n+\ttest_commit onto2 &&\n+\ttest_commit onto3 &&\n+\tgit checkout -b dev master &&\n+\ttest_commit dev1 &&\n+\ttest_commit dev2 &&\n+\ttest_commit dev3 &&\n+\tgit checkout -b redundant master &&\n+\ttest_commit onto1 onto1.t onto1 onto1-2 &&\n+\ttest_commit onto2 onto2.t onto2 onto2-2 &&\n+\ttest_commit onto3 onto3.t onto3 onto3-2 &&\n+\tgit checkout -b redundant-empty redundant &&\n+\tgit commit -m \"Empty commit 2\" --allow-empty &&\n+\ttest_commit empty4\n+'\n+\n+test_expect_success 'default rebase without empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b default-non-empty dev &&\n+\tgit rebase --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'default rebase with empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b default-empty empty &&\n+\tgit rebase --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'default rebase with redundant commits' '\n+\treset_rebase &&\n+\tgit checkout -b default-redundant redundant &&\n+\tgit rebase --preserve-merges --onto onto --root\n+'\n+\n+test_expect_success 'rebase --skip-redundant without empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b skip-redundant-non-empty dev &&\n+\tgit rebase --skip-redundant-commits --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'rebase --skip-redundant with empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b skip-redundant-empty empty &&\n+\tgit rebase --skip-redundant-commits --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'rebase --skip-redundant with redundant commits' '\n+\treset_rebase &&\n+\tgit checkout -b skip-redundant-redundant redundant &&\n+\tgit rebase --skip-redundant-commits --preserve-merges --onto onto --root\n+'\n+\n+test_expect_success 'rebase --skip-redundant with empty and redundant commits' '\n+\treset_rebase &&\n+\tgit checkout -b skip-redundant-empty-redundant redundant-empty &&\n+\tgit rebase --skip-redundant-commits --preserve-merges --onto onto --root\n+'\n+\n+test_expect_success 'rebase --keep-empty --skip-redundant without empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b keep-empty-skip-redundant-non-empty dev &&\n+\tgit rebase --keep-empty --skip-redundant-commits --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'rebase --keep-empty --skip-redundant with empty commits' '\n+\treset_rebase &&\n+\tgit checkout -b keep-empty-skip-redundant-empty empty &&\n+\tgit rebase --keep-empty --skip-redundant-commits --preserve-merges --onto onto master\n+'\n+\n+test_expect_success 'rebase --keep-empty --skip-redundant with redundant commits' '\n+\treset_rebase &&\n+\tgit checkout -b keep-empty-skip-redundant-redundant redundant &&\n+\tgit rebase --keep-empty --skip-redundant-commits --preserve-merges --onto onto --root\n+'\n+\n+test_expect_success 'rebase --keep-empty --skip-redundant with empty and redundant commits' '\n+\treset_rebase &&\n+\tgit checkout -b keep-empty-skip-redundant-empty-redundant redundant-empty &&\n+\tgit rebase --skip-redundant-commits --preserve-merges --onto onto --root\n+'\n+\n+test_done\n-- \n2.6.1\n"},{"id":"275627","messageId":"1452488421-26823-6-git-send-email-greened@obbligato.org","threadId":"41157","inReplyTo":"1452488421-26823-1-git-send-email-greened@obbligato.org","subject":"[PATCH 5/5] Add test for rebase with merges amd redundant commits","fromName":"David Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-11T05:00:21Z","receivedAt":"2016-01-11T05:00:21Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"From: \"David A. Greene\" <greened@obbligato.org>\n\nThis tests rebase --preserve-merges in the presence of redundant\ncommits when there are actual erges being rebased.  It primarily\nexercises the --skip-redundant-commits option.\n\nSigned-off-by: David A. Greene <greened@obbligato.org>\n---\n t/t3429-rebase-redundant-merge.sh | 73 +++++++++++++++++++++++++++++++++++++++\n 1 file changed, 73 insertions(+)\n create mode 100755 t/t3429-rebase-redundant-merge.sh\n\ndiff --git a/t/t3429-rebase-redundant-merge.sh b/t/t3429-rebase-redundant-merge.sh\nnew file mode 100755\nindex 0000000..f677bb1\n--- /dev/null\n+++ b/t/t3429-rebase-redundant-merge.sh\n@@ -0,0 +1,73 @@\n+#!/bin/sh\n+\n+test_description='git rebase tests for redundant commits\n+\n+This test runs git rebase and tests handling of redundant commits.\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n+# Setup this graph:\n+#\n+# O\n+# o-o-o-o-o R/rebaseOR/rebaseSR/rebasemergeOR/rebasemergeSR\n+#  \\   /\n+#   o-o S\n+#\n+test_expect_success 'setup' '\n+\ttest_commit README &&\n+\tgit branch O &&\n+\tgit checkout -b branch O &&\n+\ttest_commit branch1 &&\n+\ttest_commit branch2 &&\n+\tgit branch S &&\n+\tgit checkout master &&\n+\ttest_commit master1 &&\n+\ttest_commit master2 &&\n+\tgit merge branch -m \"Merge branch to master\" &&\n+\ttest_commit merged1 &&\n+\tgit branch R &&\n+\tgit branch rebaseOR &&\n+\tgit branch rebaseSR &&\n+\tgit branch rebasemergeOR &&\n+\tgit branch rebasemergeskipOR &&\n+\tgit branch rebasemergeSR &&\n+\tgit branch rebasemergeskipSR\n+'\n+\n+test_expect_success 'rebase O..R --onto S' '\n+\treset_rebase &&\n+\tgit checkout rebaseOR &&\n+\tgit rebase O --onto S\n+'\n+\n+test_expect_success 'rebase S..R --onto S' '\n+\treset_rebase &&\n+\tgit checkout rebaseSR &&\n+\tgit rebase S --onto S\n+'\n+\n+test_expect_success 'rebase O..R --onto S preserving merges fails' '\n+\treset_rebase &&\n+\tgit checkout rebasemergeOR &&\n+\ttest_must_fail git rebase --preserve-merges O --onto S\n+'\n+\n+test_expect_success 'rebase O..R --onto S preserving merges --skip-redundant' '\n+\treset_rebase &&\n+\tgit checkout rebasemergeskipOR &&\n+\tgit rebase --skip-redundant-commits --preserve-merges O --onto S\n+'\n+\n+test_expect_success 'rebase S..R --onto S preserving merges' '\n+\treset_rebase &&\n+\tgit checkout rebasemergeSR &&\n+\tgit rebase --preserve-merges S --onto S\n+'\n+\n+test_expect_success 'rebase S..R --onto S preserving merges --skip-redundant' '\n+\treset_rebase &&\n+\tgit checkout rebasemergeskipSR &&\n+\tgit rebase --skip-redundant-commits --preserve-merges S --onto S\n+'\n+test_done\n-- \n2.6.1\n"},{"id":"275690","messageId":"xmqqr3hnx6e2.fsf@gitster.mtv.corp.google.com","threadId":"41157","inReplyTo":"1452488421-26823-2-git-send-email-greened@obbligato.org","subject":"Re: [PATCH 1/5] Teach cherry-pick to skip redundant commits if asked","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-11T19:28:37Z","receivedAt":"2016-01-11T19:28:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Greene <greened@obbligato.org> writes:\n\n> From: \"David A. Greene\" <greened@obbligato.org>\n>\n> Add a \"--skip-redundant-commits\" option to cherry-pick to avoid\n> aborting if the cherry-picked commit becomes empty due to\n> conflict resolution.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n> ---\n>  builtin/revert.c |  7 +++++++\n>  sequencer.c      | 23 +++++++++++++++++++++++\n>  sequencer.h      |  1 +\n>  3 files changed, 31 insertions(+)\n>\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 56a2c36..befd3ce 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -91,6 +91,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>  \t\t\tN_(\"option for merge strategy\"), option_parse_x),\n>  \t\t{ OPTION_STRING, 'S', \"gpg-sign\", &opts->gpg_sign, N_(\"key-id\"),\n>  \t\t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n> +\t\t/*\n> +\t\t * There must be enough OPT_END() here to match the\n> +\t\t * size of cp_extra below so that parse_options_concat\n> +\t\t * will work.\n> +\t\t */\n\nGood ;-)\n\n> +\t\tOPT_END(),\n>  \t\tOPT_END(),\n>  \t\tOPT_END(),\n>  \t\tOPT_END(),\n> @@ -106,6 +112,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>  \t\t\tOPT_BOOL(0, \"allow-empty\", &opts->allow_empty, N_(\"preserve initially empty commits\")),\n>  \t\t\tOPT_BOOL(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n>  \t\t\tOPT_BOOL(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n> +\t\t\tOPT_BOOL(0, \"skip-redundant-commits\", &opts->skip_redundant_commits, N_(\"skip redundant, empty commits\")),\n>  \t\t\tOPT_END(),\n>  \t\t};\n\nThis however makes me wonder what should happen when both are\nspecified.  Shouldn't this patch change the keep_redundant_commits\nfield from a bool to a tristate that tells us what to do with\nredundant ones?  int/enum opts.redundant_commit can take 0 (fail,\nwhich would be the default), 1 (keep) or 2 (skip), or something\nlike that.\n\n\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 8c58fa2..12361e7 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -185,6 +185,7 @@ static void print_advice(int show_hint, struct replay_opts *opts)\n>  \t\telse\n>  \t\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n>  \t\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n> +\n\n???\n\n>  \t\t\t\t \"and commit the result with 'git commit'\"));\n>  \t}\n>  }\n> @@ -614,6 +615,28 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>  \t\tres = allow;\n>  \t\tgoto leave;\n>  \t}\n> +\n> +\t// If told, do not try to commit things that don't make any\n> +\t// changes.\n\nNo C++/C99 comments, please.\n\n> +\tif (opts->skip_redundant_commits) {\n> +\t\tint index_unchanged = is_index_unchanged();\n> +\t\tif (index_unchanged < 0) {\n> +\t\t\t// Something bad happened readhing HEAD or the\n> +\t\t\t// index.  Abort.\n> +\t\t\tres = index_unchanged;\n> +\t\t\tgoto leave;\n> +\t\t}\n> +\t\tif (index_unchanged) {\n> +\t\t\tfputs(_(\"Skipping redundant commit \"), stderr);\n> +\t\t\tfputs(find_unique_abbrev(commit->object.oid.hash,\n> +\t\t\t\t\t\t GIT_SHA1_HEXSZ),\n> +\t\t\t      stderr);\n> +\t\t\tfputs(\"\\n\", stderr);\n\nThis is a bad i18n; we do not know the sentence \"Skipping commit X\"\nis translated to have X at the end of the sentence in all languages.\n\n\tfprintf(stderr, _(\"Skipping ... %s\\n\"), find_unique_abbrev(...));\n\nwould allow it to be tranlated to \"Commit X is getting skipped\", for\nexample.\n\n> +\t\t\tres = 0;\n> +\t\t\tgoto leave;\n> +\t\t}\n> +\t}\n> +\n>  \tif (!opts->no_commit)\n>  \t\tres = run_git_commit(git_path_merge_msg(), opts, allow);\n>  \n> diff --git a/sequencer.h b/sequencer.h\n> index 5ed5cb1..ad6145d 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -34,6 +34,7 @@ struct replay_opts {\n>  \tint allow_empty;\n>  \tint allow_empty_message;\n>  \tint keep_redundant_commits;\n> +\tint skip_redundant_commits;\n\nContinuing from the top-part of the comments, this may be better to\nbe:\n\n\tenum {\n            REPLAY_REDUNDANT_FAIL = 0,\n            REPLAY_REDUNDANT_KEEP,\n            REPLAY_REDUNDANT_SKIP\n\t} redundant_commits;\n\nor something like that.\n\n>  \n>  \tint mainline;\n"},{"id":"275726","messageId":"CAPig+cQEF7w5rDQK3X9dRUEq_yewEEGA36tcOVMKjsN5hAT12g@mail.gmail.com","threadId":"41157","inReplyTo":"1452488421-26823-6-git-send-email-greened@obbligato.org","subject":"Re: [PATCH 5/5] Add test for rebase with merges amd redundant commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-01-11T23:50:06Z","receivedAt":"2016-01-11T23:50:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 11, 2016 at 12:00 AM, David Greene <greened@obbligato.org> wrote:\n> From: \"David A. Greene\" <greened@obbligato.org>\n>\n> This tests rebase --preserve-merges in the presence of redundant\n> commits when there are actual erges being rebased.  It primarily\n\ns/erges/merges/\n\n> exercises the --skip-redundant-commits option.\n>\n> Signed-off-by: David A. Greene <greened@obbligato.org>\n"},{"id":"275752","messageId":"87oacra3wq.fsf@waller.obbligato.org","threadId":"41157","inReplyTo":"xmqqr3hnx6e2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/5] Teach cherry-pick to skip redundant commits if asked","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2016-01-12T03:10:45Z","receivedAt":"2016-01-12T03:10:45Z","isPatch":true,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +\t\tOPT_END(),\n>>  \t\tOPT_END(),\n>>  \t\tOPT_END(),\n>>  \t\tOPT_END(),\n>> @@ -106,6 +112,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>>  \t\t\tOPT_BOOL(0, \"allow-empty\", &opts->allow_empty, N_(\"preserve initially empty commits\")),\n>>  \t\t\tOPT_BOOL(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n>>  \t\t\tOPT_BOOL(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n>> +\t\t\tOPT_BOOL(0, \"skip-redundant-commits\", &opts->skip_redundant_commits, N_(\"skip redundant, empty commits\")),\n>>  \t\t\tOPT_END(),\n>>  \t\t};\n>\n> This however makes me wonder what should happen when both are\n> specified.  Shouldn't this patch change the keep_redundant_commits\n> field from a bool to a tristate that tells us what to do with\n> redundant ones?  int/enum opts.redundant_commit can take 0 (fail,\n> which would be the default), 1 (keep) or 2 (skip), or something\n> like that.\n\nThis makes good sense.\n\n>> diff --git a/sequencer.c b/sequencer.c\n>> index 8c58fa2..12361e7 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -185,6 +185,7 @@ static void print_advice(int show_hint, struct replay_opts *opts)\n>>  \t\telse\n>>  \t\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n>>  \t\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n>> +\n>\n> ???\n\nOops.  :)\n\n>> @@ -614,6 +615,28 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>>  \t\tres = allow;\n>>  \t\tgoto leave;\n>>  \t}\n>> +\n>> +\t// If told, do not try to commit things that don't make any\n>> +\t// changes.\n>\n> No C++/C99 comments, please.\n\nWill fix.\n\n>> +\tif (opts->skip_redundant_commits) {\n>> +\t\tint index_unchanged = is_index_unchanged();\n>> +\t\tif (index_unchanged < 0) {\n>> +\t\t\t// Something bad happened readhing HEAD or the\n>> +\t\t\t// index.  Abort.\n>> +\t\t\tres = index_unchanged;\n>> +\t\t\tgoto leave;\n>> +\t\t}\n>> +\t\tif (index_unchanged) {\n>> +\t\t\tfputs(_(\"Skipping redundant commit \"), stderr);\n>> +\t\t\tfputs(find_unique_abbrev(commit->object.oid.hash,\n>> +\t\t\t\t\t\t GIT_SHA1_HEXSZ),\n>> +\t\t\t      stderr);\n>> +\t\t\tfputs(\"\\n\", stderr);\n>\n> This is a bad i18n; we do not know the sentence \"Skipping commit X\"\n> is translated to have X at the end of the sentence in all languages.\n>\n> \tfprintf(stderr, _(\"Skipping ... %s\\n\"), find_unique_abbrev(...));\n>\n> would allow it to be tranlated to \"Commit X is getting skipped\", for\n> example.\n\nOk, thank you for the guidance.\n\n>> diff --git a/sequencer.h b/sequencer.h\n>> index 5ed5cb1..ad6145d 100644\n>> --- a/sequencer.h\n>> +++ b/sequencer.h\n>> @@ -34,6 +34,7 @@ struct replay_opts {\n>>  \tint allow_empty;\n>>  \tint allow_empty_message;\n>>  \tint keep_redundant_commits;\n>> +\tint skip_redundant_commits;\n>\n> Continuing from the top-part of the comments, this may be better to\n> be:\n>\n> \tenum {\n>             REPLAY_REDUNDANT_FAIL = 0,\n>             REPLAY_REDUNDANT_KEEP,\n>             REPLAY_REDUNDANT_SKIP\n> \t} redundant_commits;\n>\n> or something like that.\n\nAgreed.\n\nI've also resumed work on my earlier rebase --keep-redundant-commits\nchange.  I think I'm going to reorganize things and send the cherry-pick\nchanges separate from the rebase changes since the latter depends on the\nformer.  Then all of the redundant commit work on rebase can be in one\nseries for review.\n\n                        -David\n"}]}