{"thread":{"id":"46387","subject":"[PATCH v6 00/10] The final building block for a faster rebase -i","startedAt":"2017-07-14T14:44:55Z","lastAt":"2017-07-22T11:45:03Z","messageCount":17,"participants":["Johannes Schindelin","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":6,"patchTotal":10},"messages":[{"id":"324513","messageId":"cover.1500043436.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":null,"subject":"[PATCH v6 00/10] The final building block for a faster rebase -i","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:44:32Z","receivedAt":"2017-07-14T14:44:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch series reimplements the expensive pre- and post-processing of\nthe todo script in C.\n\nAnd it concludes the work I did to accelerate rebase -i so far.\n\nI am still unwilling to replace a compile-time safe way to pass the\noptions to the revision machinery by the alternative (which I am still\nflabbergasted about) proposed by Junio. This will not change.\n\nI know that we want to concentrate on bug fixes on `master`, but this\npatch series will most likely take a couple of months to get there, anyway.\nSo I may just as well send this iteration now. It's also not like I haven't\ncontributed any bug fixes lately...\n\nChanges since v5:\n\n- replaced a get_sha1() call by a get_oid() call already.\n\n- adjusted to hashmap API changes\n\n\nJohannes Schindelin (10):\n t3415: verify that an empty instructionFormat is handled as before\n rebase -i: generate the script via rebase--helper\n rebase -i: remove useless indentation\n rebase -i: do not invent onelines when expanding/collapsing SHA-1s\n rebase -i: also expand/collapse the SHA-1s via the rebase--helper\n t3404: relax rebase.missingCommitsCheck tests\n rebase -i: check for missing commits in the rebase--helper\n rebase -i: skip unnecessary picks using the rebase--helper\n t3415: test fixup with wrapped oneline\n rebase -i: rearrange fixup/squash lines using the rebase--helper\n\n Documentation/git-rebase.txt | 16 +-\n builtin/rebase--helper.c | 29 ++-\n git-rebase--interactive.sh | 373 ++++-------------------------\n sequencer.c | 531 ++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 8 +\n t/t3404-rebase-interactive.sh | 22 +-\n t/t3415-rebase-autosquash.sh | 28 ++-\n 7 files changed, 647 insertions(+), 360 deletions(-)\n\n\nbase-commit: f3da2b79be9565779e4f76dc5812c68e156afdf0\nBased-On: rebase--helper at https://github.com/dscho/git\nFetch-Base-Via: git fetch https://github.com/dscho/git rebase--helper\nPublished-As: https://github.com/dscho/git/releases/tag/rebase-i-extra-v6\nFetch-It-Via: git fetch https://github.com/dscho/git rebase-i-extra-v6\n\nInterdiff vs v5:\n diff --git a/sequencer.c b/sequencer.c\n index 8713cc8d1d5..c54596f9699 100644\n --- a/sequencer.c\n +++ b/sequencer.c\n @@ -2654,7 +2654,7 @@ int skip_unnecessary_picks(void)\n \n \tif (!read_oneliner(&buf, rebase_path_onto(), 0))\n \t\treturn error(_(\"could not read 'onto'\"));\n -\tif (get_sha1(buf.buf, onto_oid.hash)) {\n +\tif (get_oid(buf.buf, &onto_oid)) {\n \t\tstrbuf_release(&buf);\n \t\treturn error(_(\"need a HEAD to fixup\"));\n \t}\n @@ -2756,8 +2756,9 @@ struct subject2item_entry {\n \tchar subject[FLEX_ARRAY];\n };\n \n -static int subject2item_cmp(const struct subject2item_entry *a,\n -\tconst struct subject2item_entry *b, const void *key)\n +static int subject2item_cmp(const void *fndata,\n +\t\t\t const struct subject2item_entry *a,\n +\t\t\t const struct subject2item_entry *b, const void *key)\n {\n \treturn key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);\n }\n @@ -2802,7 +2803,7 @@ int rearrange_squash(void)\n \t * be moved to appear after the i'th.\n \t */\n \thashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,\n -\t\t todo_list.nr);\n +\t\t NULL, todo_list.nr);\n \tALLOC_ARRAY(next, todo_list.nr);\n \tALLOC_ARRAY(tail, todo_list.nr);\n \tALLOC_ARRAY(subjects, todo_list.nr);\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n"},{"id":"324514","messageId":"52e711346eba2fca30225da1d4f42cb17f7ee08b.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 01/10] t3415: verify that an empty instructionFormat is handled as before","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:44:38Z","receivedAt":"2017-07-14T14:45:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"An upcoming patch will move the todo list generation into the\nrebase--helper. An early version of that patch regressed on an empty\nrebase.instructionFormat value (the shell version could not discern\nbetween an empty one and a non-existing one, but the C version used the\nempty one as if that was intended to skip the oneline from the `pick\n` lines).\n\nLet's verify that this still works as before.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3415-rebase-autosquash.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 5848949ec37..6d99f624b62 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -271,6 +271,18 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' '\n \ttest 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)\n '\n \n+test_expect_success 'autosquash with empty custom instructionFormat' '\n+\tgit reset --hard base &&\n+\ttest_commit empty-instructionFormat-test &&\n+\t(\n+\t\tset_cat_todo_editor &&\n+\t\ttest_must_fail git -c rebase.instructionFormat= \\\n+\t\t\trebase --autosquash --force -i HEAD^ >actual &&\n+\t\tgit log -1 --format=\"pick %h %s\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n set_backup_editor () {\n \twrite_script backup-editor.sh <<-\\EOF\n \tcp \"$1\" .git/backup-\"$(basename \"$1\")\"\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324515","messageId":"b3c71a9d577956dc0aae0763d4a5683bc1eb6304.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 03/10] rebase -i: remove useless indentation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:03Z","receivedAt":"2017-07-14T14:45:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The commands used to be indented, and it is nice to look at, but when we\ntransform the SHA-1s, the indentation is removed. So let's do away with it.\n\nFor the moment, at least: when we will use the upcoming rebase--helper\nto transform the SHA-1s, we *will* keep the indentation and can\nreintroduce it. Yet, to be able to validate the rebase--helper against\nthe output of the current shell script version, we need to remove the\nextra indentation.\n\nSigned-off-by: Johannes Schindelin \n---\n git-rebase--interactive.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 05766835de1..93372c62b2e 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -155,13 +155,13 @@ reschedule_last_action () {\n append_todo_help () {\n \tgettext \"\n Commands:\n- p, pick = use commit\n- r, reword = use commit, but edit the commit message\n- e, edit = use commit, but stop for amending\n- s, squash = use commit, but meld into previous commit\n- f, fixup = like \\\"squash\\\", but discard this commit's log message\n- x, exec = run command (the rest of the line) using shell\n- d, drop = remove commit\n+p, pick = use commit\n+r, reword = use commit, but edit the commit message\n+e, edit = use commit, but stop for amending\n+s, squash = use commit, but meld into previous commit\n+f, fixup = like \\\"squash\\\", but discard this commit's log message\n+x, exec = run command (the rest of the line) using shell\n+d, drop = remove commit\n \n These lines can be re-ordered; they are executed from top to bottom.\n \" | git stripspace --comment-lines >>\"$todo\"\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324516","messageId":"814284075336e715dd52859e69aeb41226624af7.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 02/10] rebase -i: generate the script via rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:44:58Z","receivedAt":"2017-07-14T14:45:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The first step of an interactive rebase is to generate the so-called \"todo\nscript\", to be stored in the state directory as \"git-rebase-todo\" and to\nbe edited by the user.\n\nOriginally, we adjusted the output of `git log ` using a simple\nsed script. Over the course of the years, the code became more\ncomplicated. We now use shell scripting to edit the output of `git log`\nconditionally, depending whether to keep \"empty\" commits (i.e. commits\nthat do not change any files).\n\nOn platforms where shell scripting is not native, this can be a serious\ndrag. And it opens the door for incompatibilities between platforms when\nit comes to shell scripting or to Unix-y commands.\n\nLet's just re-implement the todo script generation in plain C, using the\nrevision machinery directly.\n\nThis is substantially faster, improving the speed relative to the\nshell script version of the interactive rebase from 2x to 3x on Windows.\n\nNote that the rearrange_squash() function in git-rebase--interactive\nrelied on the fact that we set the \"format\" variable to the config setting\nrebase.instructionFormat. Relying on a side effect like this is no good,\nhence we explicitly perform that assignment (possibly again) in\nrearrange_squash().\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase--helper.c | 8 +++++++-\n git-rebase--interactive.sh | 44 +++++++++++++++++++++--------------------\n sequencer.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 3 +++\n 4 files changed, 82 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex c82b4dce683..613053276e6 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -12,15 +12,19 @@ static const char * const builtin_rebase_helper_usage[] = {\n int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n {\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n+\tint keep_empty = 0;\n \tenum {\n-\t\tCONTINUE = 1, ABORT\n+\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n+\t\tOPT_BOOL(0, \"keep-empty\", &keep_empty, N_(\"keep empty commits\")),\n \t\tOPT_CMDMODE(0, \"continue\", &command, N_(\"continue rebase\"),\n \t\t\t\tCONTINUE),\n \t\tOPT_CMDMODE(0, \"abort\", &command, N_(\"abort rebase\"),\n \t\t\t\tABORT),\n+\t\tOPT_CMDMODE(0, \"make-script\", &command,\n+\t\t\tN_(\"make rebase script\"), MAKE_SCRIPT),\n \t\tOPT_END()\n \t};\n \n@@ -37,5 +41,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!sequencer_continue(&opts);\n \tif (command == ABORT && argc == 1)\n \t\treturn !!sequencer_remove_state(&opts);\n+\tif (command == MAKE_SCRIPT && argc > 1)\n+\t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 90b1fbe9cf6..05766835de1 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -785,6 +785,7 @@ collapse_todo_ids() {\n # each log message will be re-retrieved in order to normalize the\n # autosquash arrangement\n rearrange_squash () {\n+\tformat=$(git config --get rebase.instructionFormat)\n \t# extract fixup!/squash! lines and resolve any referenced sha1's\n \twhile read -r pick sha1 message\n \tdo\n@@ -1210,26 +1211,27 @@ else\n \trevisions=$onto...$orig_head\n \tshortrevisions=$shorthead\n fi\n-format=$(git config --get rebase.instructionFormat)\n-# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-git rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n-\t--reverse --left-right --topo-order \\\n-\t$revisions ${restrict_revision+^$restrict_revision} | \\\n-\tsed -n \"s/^>//p\" |\n-while read -r sha1 rest\n-do\n-\n-\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n-\tthen\n-\t\tcomment_out=\"$comment_char \"\n-\telse\n-\t\tcomment_out=\n-\tfi\n+if test t != \"$preserve_merges\"\n+then\n+\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n+\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\"\n+else\n+\tformat=$(git config --get rebase.instructionFormat)\n+\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n+\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n+\t\t--reverse --left-right --topo-order \\\n+\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n+\t\tsed -n \"s/^>//p\" |\n+\twhile read -r sha1 rest\n+\tdo\n+\n+\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n+\t\tthen\n+\t\t\tcomment_out=\"$comment_char \"\n+\t\telse\n+\t\t\tcomment_out=\n+\t\tfi\n \n-\tif test t != \"$preserve_merges\"\n-\tthen\n-\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n-\telse\n \t\tif test -z \"$rebase_root\"\n \t\tthen\n \t\t\tpreserve=t\n@@ -1248,8 +1250,8 @@ do\n \t\t\ttouch \"$rewritten\"/$sha1\n \t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n \t\tfi\n-\tfi\n-done\n+\tdone\n+fi\n \n # Watch for commits that been dropped by --cherry-pick\n if test t = \"$preserve_merges\"\ndiff --git a/sequencer.c b/sequencer.c\nindex 3010faf8639..afcb3d00a32 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2413,3 +2413,52 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)\n \n \tstrbuf_release(&sob);\n }\n+\n+int sequencer_make_script(int keep_empty, FILE *out,\n+\t\tint argc, const char **argv)\n+{\n+\tchar *format = NULL;\n+\tstruct pretty_print_context pp = {0};\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct rev_info revs;\n+\tstruct commit *commit;\n+\n+\tinit_revisions(&revs, NULL);\n+\trevs.verbose_header = 1;\n+\trevs.max_parents = 1;\n+\trevs.cherry_pick = 1;\n+\trevs.limited = 1;\n+\trevs.reverse = 1;\n+\trevs.right_only = 1;\n+\trevs.sort_order = REV_SORT_IN_GRAPH_ORDER;\n+\trevs.topo_order = 1;\n+\n+\trevs.pretty_given = 1;\n+\tgit_config_get_string(\"rebase.instructionFormat\", &format);\n+\tif (!format || !*format) {\n+\t\tfree(format);\n+\t\tformat = xstrdup(\"%s\");\n+\t}\n+\tget_commit_format(format, &revs);\n+\tfree(format);\n+\tpp.fmt = revs.commit_format;\n+\tpp.output_encoding = get_log_output_encoding();\n+\n+\tif (setup_revisions(argc, argv, &revs, NULL) > 1)\n+\t\treturn error(_(\"make_script: unhandled options\"));\n+\n+\tif (prepare_revision_walk(&revs) < 0)\n+\t\treturn error(_(\"make_script: error preparing revisions\"));\n+\n+\twhile ((commit = get_revision(&revs))) {\n+\t\tstrbuf_reset(&buf);\n+\t\tif (!keep_empty && is_original_commit_empty(commit))\n+\t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n+\t\tstrbuf_addf(&buf, \"pick %s \", oid_to_hex(&commit->object.oid));\n+\t\tpretty_print_commit(&pp, commit, &buf);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t\tfputs(buf.buf, out);\n+\t}\n+\tstrbuf_release(&buf);\n+\treturn 0;\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex f885b68395f..83f2943b7a9 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -45,6 +45,9 @@ int sequencer_continue(struct replay_opts *opts);\n int sequencer_rollback(struct replay_opts *opts);\n int sequencer_remove_state(struct replay_opts *opts);\n \n+int sequencer_make_script(int keep_empty, FILE *out,\n+\t\tint argc, const char **argv);\n+\n extern const char sign_off_header[];\n \n void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324517","messageId":"4a5f540d4dcde47a7ba5e08b990de164cc0c52b2.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 04/10] rebase -i: do not invent onelines when expanding/collapsing SHA-1s","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:06Z","receivedAt":"2017-07-14T14:45:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"To avoid problems with short SHA-1s that become non-unique during the\nrebase, we rewrite the todo script with short/long SHA-1s before and\nafter letting the user edit the script. Since SHA-1s are not intuitive\nfor humans, rebase -i also provides the onelines (commit message\nsubjects) in the script, purely for the user's convenience.\n\nIt is very possible to generate a todo script via different means than\nrebase -i and then to let rebase -i run with it; In this case, these\nonelines are not required.\n\nAnd this is where the expand/collapse machinery has a bug: it *expects*\nthat oneline, and failing to find one reuses the previous SHA-1 as\n\"oneline\".\n\nIt was most likely an oversight, and made implementation in the (quite\nlimiting) shell script language less convoluted. However, we are about\nto reimplement performance-critical parts in C (and due to spawning a\ngit.exe process for every single line of the todo script, the\nexpansion/collapsing of the SHA-1s *is* performance-hampering on\nWindows), therefore let's fix this bug to make cross-validation with the\nC version of that functionality possible.\n\nSigned-off-by: Johannes Schindelin \n---\n git-rebase--interactive.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 93372c62b2e..9d65212b7f1 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -760,7 +760,12 @@ transform_todo_ids () {\n \t\t\t;;\n \t\t*)\n \t\t\tsha1=$(git rev-parse --verify --quiet \"$@\" ${rest%%[\t ]*}) &&\n-\t\t\trest=\"$sha1 ${rest#*[\t ]}\"\n+\t\t\tif test \"a$rest\" = \"a${rest#*[\t ]}\"\n+\t\t\tthen\n+\t\t\t\trest=$sha1\n+\t\t\telse\n+\t\t\t\trest=\"$sha1 ${rest#*[\t ]}\"\n+\t\t\tfi\n \t\t\t;;\n \t\tesac\n \t\tprintf '%s\\n' \"$command${rest:+ }$rest\"\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324518","messageId":"d0bdac394f19291f614defdd7f22c0c6d7f3be0a.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 05/10] rebase -i: also expand/collapse the SHA-1s via the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:11Z","receivedAt":"2017-07-14T14:45:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is crucial to improve performance on Windows, as the speed is now\nmostly dominated by the SHA-1 transformation (because it spawns a new\nrev-parse process for *every* line, and spawning processes is pretty\nslow from Git for Windows' MSYS2 Bash).\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase--helper.c | 10 +++++++-\n git-rebase--interactive.sh | 27 ++--------------------\n sequencer.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 2 ++\n 4 files changed, 70 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 613053276e6..791b62901c3 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -14,7 +14,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tint keep_empty = 0;\n \tenum {\n-\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT\n+\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -25,6 +25,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\t\tABORT),\n \t\tOPT_CMDMODE(0, \"make-script\", &command,\n \t\t\tN_(\"make rebase script\"), MAKE_SCRIPT),\n+\t\tOPT_CMDMODE(0, \"shorten-ids\", &command,\n+\t\t\tN_(\"shorten SHA-1s in the todo list\"), SHORTEN_SHA1S),\n+\t\tOPT_CMDMODE(0, \"expand-ids\", &command,\n+\t\t\tN_(\"expand SHA-1s in the todo list\"), EXPAND_SHA1S),\n \t\tOPT_END()\n \t};\n \n@@ -43,5 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!sequencer_remove_state(&opts);\n \tif (command == MAKE_SCRIPT && argc > 1)\n \t\treturn !!sequencer_make_script(keep_empty, stdout, argc, argv);\n+\tif (command == SHORTEN_SHA1S && argc == 1)\n+\t\treturn !!transform_todo_ids(1);\n+\tif (command == EXPAND_SHA1S && argc == 1)\n+\t\treturn !!transform_todo_ids(0);\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 9d65212b7f1..d5df02435ae 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -750,35 +750,12 @@ skip_unnecessary_picks () {\n \t\tdie \"$(gettext \"Could not skip unnecessary pick commands\")\"\n }\n \n-transform_todo_ids () {\n-\twhile read -r command rest\n-\tdo\n-\t\tcase \"$command\" in\n-\t\t\"$comment_char\"* | exec)\n-\t\t\t# Be careful for oddball commands like 'exec'\n-\t\t\t# that do not have a SHA-1 at the beginning of $rest.\n-\t\t\t;;\n-\t\t*)\n-\t\t\tsha1=$(git rev-parse --verify --quiet \"$@\" ${rest%%[\t ]*}) &&\n-\t\t\tif test \"a$rest\" = \"a${rest#*[\t ]}\"\n-\t\t\tthen\n-\t\t\t\trest=$sha1\n-\t\t\telse\n-\t\t\t\trest=\"$sha1 ${rest#*[\t ]}\"\n-\t\t\tfi\n-\t\t\t;;\n-\t\tesac\n-\t\tprintf '%s\\n' \"$command${rest:+ }$rest\"\n-\tdone <\"$todo\" >\"$todo.new\" &&\n-\tmv -f \"$todo.new\" \"$todo\"\n-}\n-\n expand_todo_ids() {\n-\ttransform_todo_ids\n+\tgit rebase--helper --expand-ids\n }\n \n collapse_todo_ids() {\n-\ttransform_todo_ids --short\n+\tgit rebase--helper --shorten-ids\n }\n \n # Rearrange the todo list that has both \"pick sha1 msg\" and\ndiff --git a/sequencer.c b/sequencer.c\nindex afcb3d00a32..36ed45d9050 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2462,3 +2462,60 @@ int sequencer_make_script(int keep_empty, FILE *out,\n \tstrbuf_release(&buf);\n \treturn 0;\n }\n+\n+\n+int transform_todo_ids(int shorten_ids)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tint fd, res, i;\n+\tFILE *out;\n+\n+\tstrbuf_reset(&todo_list.buf);\n+\tfd = open(todo_file, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\t}\n+\tclose(fd);\n+\n+\tres = parse_insn_buffer(todo_list.buf.buf, &todo_list);\n+\tif (res) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unusable todo list: '%s'\"), todo_file);\n+\t}\n+\n+\tout = fopen(todo_file, \"w\");\n+\tif (!out) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn error(_(\"unable to open '%s' for writing\"), todo_file);\n+\t}\n+\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\tstruct todo_item *item = todo_list.items + i;\n+\t\tint bol = item->offset_in_buf;\n+\t\tconst char *p = todo_list.buf.buf + bol;\n+\t\tint eol = i + 1 < todo_list.nr ?\n+\t\t\ttodo_list.items[i + 1].offset_in_buf :\n+\t\t\ttodo_list.buf.len;\n+\n+\t\tif (item->command >= TODO_EXEC && item->command != TODO_DROP)\n+\t\t\tfwrite(p, eol - bol, 1, out);\n+\t\telse {\n+\t\t\tconst char *id = shorten_ids ?\n+\t\t\t\tshort_commit_name(item->commit) :\n+\t\t\t\toid_to_hex(&item->commit->object.oid);\n+\t\t\tint len;\n+\n+\t\t\tp += strspn(p, \" \\t\"); /* left-trim command */\n+\t\t\tlen = strcspn(p, \" \\t\"); /* length of command */\n+\n+\t\t\tfprintf(out, \"%.*s %s %.*s\\n\",\n+\t\t\t\tlen, p, id, item->arg_len, item->arg);\n+\t\t}\n+\t}\n+\tfclose(out);\n+\ttodo_list_release(&todo_list);\n+\treturn 0;\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex 83f2943b7a9..71d25374afe 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -48,6 +48,8 @@ int sequencer_remove_state(struct replay_opts *opts);\n int sequencer_make_script(int keep_empty, FILE *out,\n \t\tint argc, const char **argv);\n \n+int transform_todo_ids(int shorten_ids);\n+\n extern const char sign_off_header[];\n \n void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324519","messageId":"47d54a53ad8b5efdb74e0e8624f2740d6ae84c2c.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 06/10] t3404: relax rebase.missingCommitsCheck tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:14Z","receivedAt":"2017-07-14T14:45:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"These tests were a bit anal about the *exact* warning/error message\nprinted by git rebase. But those messages are intended for the *end\nuser*, therefore it does not make sense to test so rigidly for the\n*exact* wording.\n\nIn the following, we will reimplement the missing commits check in\nthe sequencer, with slightly different words.\n\nSo let's just test for the parts in the warning/error message that\nwe *really* care about, nothing more, nothing less.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3404-rebase-interactive.sh | 22 ++++------------------\n 1 file changed, 4 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 37821d24543..3704dbb2ecf 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1249,20 +1249,13 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' '\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n '\n \n-cat >expect <actual &&\n-\ttest_i18ncmp expect actual &&\n+\ttest_i18ngrep \"badcmd $(git rev-list --oneline -1 master~1)\" actual &&\n+\ttest_i18ngrep \"You can fix this with .git rebase --edit-todo..\" actual &&\n \tFAKE_LINES=\"1 2 3 drop 4 5\" git rebase --edit-todo &&\n \tgit rebase --continue &&\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p) &&\n@@ -1284,20 +1277,13 @@ test_expect_success 'tabs and spaces are accepted in the todolist' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n-cat >expect <actual &&\n-\ttest_i18ncmp expect actual &&\n+\ttest_i18ngrep \"edit XXXXXXX False commit\" actual &&\n+\ttest_i18ngrep \"You can fix this with .git rebase --edit-todo..\" actual &&\n \tFAKE_LINES=\"1 2 4 5 6\" git rebase --edit-todo &&\n \tgit rebase --continue &&\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324520","messageId":"0d1408155589314b87307e956b2cf19e97c5155d.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 07/10] rebase -i: check for missing commits in the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:21Z","receivedAt":"2017-07-14T14:45:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In particular on Windows, where shell scripts are even more expensive\nthan on MacOSX or Linux, it makes sense to move a loop that forks\nGit at least once for every line in the todo list into a builtin.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase--helper.c | 7 +-\n git-rebase--interactive.sh | 164 ++-------------------------------------------\n sequencer.c | 122 +++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n 4 files changed, 134 insertions(+), 160 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 791b62901c3..305adb5b707 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -14,7 +14,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tstruct replay_opts opts = REPLAY_OPTS_INIT;\n \tint keep_empty = 0;\n \tenum {\n-\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S\n+\t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n+\t\tCHECK_TODO_LIST\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -29,6 +30,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"shorten SHA-1s in the todo list\"), SHORTEN_SHA1S),\n \t\tOPT_CMDMODE(0, \"expand-ids\", &command,\n \t\t\tN_(\"expand SHA-1s in the todo list\"), EXPAND_SHA1S),\n+\t\tOPT_CMDMODE(0, \"check-todo-list\", &command,\n+\t\t\tN_(\"check the todo list\"), CHECK_TODO_LIST),\n \t\tOPT_END()\n \t};\n \n@@ -51,5 +54,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!transform_todo_ids(1);\n \tif (command == EXPAND_SHA1S && argc == 1)\n \t\treturn !!transform_todo_ids(0);\n+\tif (command == CHECK_TODO_LIST && argc == 1)\n+\t\treturn !!check_todo_list();\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d5df02435ae..c8cad318fa4 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -867,96 +867,6 @@ add_exec_commands () {\n \tmv \"$1.new\" \"$1\"\n }\n \n-# Check if the SHA-1 passed as an argument is a\n-# correct one, if not then print $2 in \"$todo\".badsha\n-# $1: the SHA-1 to test\n-# $2: the line number of the input\n-# $3: the input filename\n-check_commit_sha () {\n-\tbadsha=0\n-\tif test -z \"$1\"\n-\tthen\n-\t\tbadsha=1\n-\telse\n-\t\tsha1_verif=\"$(git rev-parse --verify --quiet $1^{commit})\"\n-\t\tif test -z \"$sha1_verif\"\n-\t\tthen\n-\t\t\tbadsha=1\n-\t\tfi\n-\tfi\n-\n-\tif test $badsha -ne 0\n-\tthen\n-\t\tline=\"$(sed -n -e \"${2}p\" \"$3\")\"\n-\t\twarn \"$(eval_gettext \"\\\n-Warning: the SHA-1 is missing or isn't a commit in the following line:\n- - \\$line\")\"\n-\t\twarn\n-\tfi\n-\n-\treturn $badsha\n-}\n-\n-# prints the bad commits and bad commands\n-# from the todolist in stdin\n-check_bad_cmd_and_sha () {\n-\tretval=0\n-\tlineno=0\n-\twhile read -r command rest\n-\tdo\n-\t\tlineno=$(( $lineno + 1 ))\n-\t\tcase $command in\n-\t\t\"$comment_char\"*|''|noop|x|exec)\n-\t\t\t# Doesn't expect a SHA-1\n-\t\t\t;;\n-\t\t\"$cr\")\n-\t\t\t# Work around CR left by \"read\" (e.g. with Git for\n-\t\t\t# Windows' Bash).\n-\t\t\t;;\n-\t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n-\t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n-\t\t\tthen\n-\t\t\t\tretval=1\n-\t\t\tfi\n-\t\t\t;;\n-\t\t*)\n-\t\t\tline=\"$(sed -n -e \"${lineno}p\" \"$1\")\"\n-\t\t\twarn \"$(eval_gettext \"\\\n-Warning: the command isn't recognized in the following line:\n- - \\$line\")\"\n-\t\t\twarn\n-\t\t\tretval=1\n-\t\t\t;;\n-\t\tesac\n-\tdone <\"$1\"\n-\treturn $retval\n-}\n-\n-# Print the list of the SHA-1 of the commits\n-# from stdin to stdout\n-todo_list_to_sha_list () {\n-\tgit stripspace --strip-comments |\n-\twhile read -r command sha1 rest\n-\tdo\n-\t\tcase $command in\n-\t\t\"$comment_char\"*|''|noop|x|\"exec\")\n-\t\t\t;;\n-\t\t*)\n-\t\t\tlong_sha=$(git rev-list --no-walk \"$sha1\" 2>/dev/null)\n-\t\t\tprintf \"%s\\n\" \"$long_sha\"\n-\t\t\t;;\n-\t\tesac\n-\tdone\n-}\n-\n-# Use warn for each line in stdin\n-warn_lines () {\n-\twhile read -r line\n-\tdo\n-\t\twarn \" - $line\"\n-\tdone\n-}\n-\n # Switch to the branch in $into and notify it in the reflog\n checkout_onto () {\n \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $onto_name\"\n@@ -971,74 +881,6 @@ get_missing_commit_check_level () {\n \tprintf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n }\n \n-# Check if the user dropped some commits by mistake\n-# Behaviour determined by rebase.missingCommitsCheck.\n-# Check if there is an unrecognized command or a\n-# bad SHA-1 in a command.\n-check_todo_list () {\n-\traise_error=f\n-\n-\tcheck_level=$(get_missing_commit_check_level)\n-\n-\tcase \"$check_level\" in\n-\twarn|error)\n-\t\t# Get the SHA-1 of the commits\n-\t\ttodo_list_to_sha_list <\"$todo\".backup >\"$todo\".oldsha1\n-\t\ttodo_list_to_sha_list <\"$todo\" >\"$todo\".newsha1\n-\n-\t\t# Sort the SHA-1 and compare them\n-\t\tsort -u \"$todo\".oldsha1 >\"$todo\".oldsha1+\n-\t\tmv \"$todo\".oldsha1+ \"$todo\".oldsha1\n-\t\tsort -u \"$todo\".newsha1 >\"$todo\".newsha1+\n-\t\tmv \"$todo\".newsha1+ \"$todo\".newsha1\n-\t\tcomm -2 -3 \"$todo\".oldsha1 \"$todo\".newsha1 >\"$todo\".miss\n-\n-\t\t# Warn about missing commits\n-\t\tif test -s \"$todo\".miss\n-\t\tthen\n-\t\t\ttest \"$check_level\" = error && raise_error=t\n-\n-\t\t\twarn \"$(gettext \"\\\n-Warning: some commits may have been dropped accidentally.\n-Dropped commits (newer to older):\")\"\n-\n-\t\t\t# Make the list user-friendly and display\n-\t\t\topt=\"--no-walk=sorted --format=oneline --abbrev-commit --stdin\"\n-\t\t\tgit rev-list $opt <\"$todo\".miss | warn_lines\n-\n-\t\t\twarn \"$(gettext \"\\\n-To avoid this message, use \\\"drop\\\" to explicitly remove a commit.\n-\n-Use 'git config rebase.missingCommitsCheck' to change the level of warnings.\n-The possible behaviours are: ignore, warn, error.\")\"\n-\t\t\twarn\n-\t\tfi\n-\t\t;;\n-\tignore)\n-\t\t;;\n-\t*)\n-\t\twarn \"$(eval_gettext \"Unrecognized setting \\$check_level for option rebase.missingCommitsCheck. Ignoring.\")\"\n-\t\t;;\n-\tesac\n-\n-\tif ! check_bad_cmd_and_sha \"$todo\"\n-\tthen\n-\t\traise_error=t\n-\tfi\n-\n-\tif test $raise_error = t\n-\tthen\n-\t\t# Checkout before the first commit of the\n-\t\t# rebase: this way git rebase --continue\n-\t\t# will work correctly as it expects HEAD to be\n-\t\t# placed before the commit of the next action\n-\t\tcheckout_onto\n-\n-\t\twarn \"$(gettext \"You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.\")\"\n-\t\tdie \"$(gettext \"Or you can abort the rebase with 'git rebase --abort'.\")\"\n-\tfi\n-}\n-\n # The whole contents of this file is run by dot-sourcing it from\n # inside a shell function. It used to be that \"return\"s we see\n # below were not inside any function, and expected to return\n@@ -1299,7 +1141,11 @@ git_sequence_editor \"$todo\" ||\n has_action \"$todo\" ||\n \treturn 2\n \n-check_todo_list\n+git rebase--helper --check-todo-list || {\n+\tret=$?\n+\tcheckout_onto\n+\texit $ret\n+}\n \n expand_todo_ids\n \ndiff --git a/sequencer.c b/sequencer.c\nindex 36ed45d9050..15107de1e1c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2519,3 +2519,125 @@ int transform_todo_ids(int shorten_ids)\n \ttodo_list_release(&todo_list);\n \treturn 0;\n }\n+\n+enum check_level {\n+\tCHECK_IGNORE = 0, CHECK_WARN, CHECK_ERROR\n+};\n+\n+static enum check_level get_missing_commit_check_level(void)\n+{\n+\tconst char *value;\n+\n+\tif (git_config_get_value(\"rebase.missingcommitscheck\", &value) ||\n+\t\t\t!strcasecmp(\"ignore\", value))\n+\t\treturn CHECK_IGNORE;\n+\tif (!strcasecmp(\"warn\", value))\n+\t\treturn CHECK_WARN;\n+\tif (!strcasecmp(\"error\", value))\n+\t\treturn CHECK_ERROR;\n+\twarning(_(\"unrecognized setting %s for option\"\n+\t\t \"rebase.missingCommitsCheck. Ignoring.\"), value);\n+\treturn CHECK_IGNORE;\n+}\n+\n+/*\n+ * Check if the user dropped some commits by mistake\n+ * Behaviour determined by rebase.missingCommitsCheck.\n+ * Check if there is an unrecognized command or a\n+ * bad SHA-1 in a command.\n+ */\n+int check_todo_list(void)\n+{\n+\tenum check_level check_level = get_missing_commit_check_level();\n+\tstruct strbuf todo_file = STRBUF_INIT;\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct strbuf missing = STRBUF_INIT;\n+\tint advise_to_edit_todo = 0, res = 0, fd, i;\n+\n+\tstrbuf_addstr(&todo_file, rebase_path_todo());\n+\tfd = open(todo_file.buf, O_RDONLY);\n+\tif (fd < 0) {\n+\t\tres = error_errno(_(\"could not open '%s'\"), todo_file.buf);\n+\t\tgoto leave_check;\n+\t}\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\tres = error(_(\"could not read '%s'.\"), todo_file.buf);\n+\t\tgoto leave_check;\n+\t}\n+\tclose(fd);\n+\tadvise_to_edit_todo = res =\n+\t\tparse_insn_buffer(todo_list.buf.buf, &todo_list);\n+\n+\tif (res || check_level == CHECK_IGNORE)\n+\t\tgoto leave_check;\n+\n+\t/* Mark the commits in git-rebase-todo as seen */\n+\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\tstruct commit *commit = todo_list.items[i].commit;\n+\t\tif (commit)\n+\t\t\tcommit->util = (void *)1;\n+\t}\n+\n+\ttodo_list_release(&todo_list);\n+\tstrbuf_addstr(&todo_file, \".backup\");\n+\tfd = open(todo_file.buf, O_RDONLY);\n+\tif (fd < 0) {\n+\t\tres = error_errno(_(\"could not open '%s'\"), todo_file.buf);\n+\t\tgoto leave_check;\n+\t}\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\tres = error(_(\"could not read '%s'.\"), todo_file.buf);\n+\t\tgoto leave_check;\n+\t}\n+\tclose(fd);\n+\tstrbuf_release(&todo_file);\n+\tres = !!parse_insn_buffer(todo_list.buf.buf, &todo_list);\n+\n+\t/* Find commits in git-rebase-todo.backup yet unseen */\n+\tfor (i = todo_list.nr - 1; i >= 0; i--) {\n+\t\tstruct todo_item *item = todo_list.items + i;\n+\t\tstruct commit *commit = item->commit;\n+\t\tif (commit && !commit->util) {\n+\t\t\tstrbuf_addf(&missing, \" - %s %.*s\\n\",\n+\t\t\t\t short_commit_name(commit),\n+\t\t\t\t item->arg_len, item->arg);\n+\t\t\tcommit->util = (void *)1;\n+\t\t}\n+\t}\n+\n+\t/* Warn about missing commits */\n+\tif (!missing.len)\n+\t\tgoto leave_check;\n+\n+\tif (check_level == CHECK_ERROR)\n+\t\tadvise_to_edit_todo = res = 1;\n+\n+\tfprintf(stderr,\n+\t\t_(\"Warning: some commits may have been dropped accidentally.\\n\"\n+\t\t\"Dropped commits (newer to older):\\n\"));\n+\n+\t/* Make the list user-friendly and display */\n+\tfputs(missing.buf, stderr);\n+\tstrbuf_release(&missing);\n+\n+\tfprintf(stderr, _(\"To avoid this message, use \\\"drop\\\" to \"\n+\t\t\"explicitly remove a commit.\\n\\n\"\n+\t\t\"Use 'git config rebase.missingCommitsCheck' to change \"\n+\t\t\"the level of warnings.\\n\"\n+\t\t\"The possible behaviours are: ignore, warn, error.\\n\\n\"));\n+\n+leave_check:\n+\tstrbuf_release(&todo_file);\n+\ttodo_list_release(&todo_list);\n+\n+\tif (advise_to_edit_todo)\n+\t\tfprintf(stderr,\n+\t\t\t_(\"You can fix this with 'git rebase --edit-todo' \"\n+\t\t\t \"and then run 'git rebase --continue'.\\n\"\n+\t\t\t \"Or you can abort the rebase with 'git rebase\"\n+\t\t\t \" --abort'.\\n\"));\n+\n+\treturn res;\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex 71d25374afe..878dd296f8c 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,6 +49,7 @@ int sequencer_make_script(int keep_empty, FILE *out,\n \t\tint argc, const char **argv);\n \n int transform_todo_ids(int shorten_ids);\n+int check_todo_list(void);\n \n extern const char sign_off_header[];\n \n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324521","messageId":"3883eb7fcc4efb414aab51f586474d846241e8a1.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 09/10] t3415: test fixup with wrapped oneline","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:28Z","receivedAt":"2017-07-14T14:45:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The `git commit --fixup` command unwraps wrapped onelines when\nconstructing the commit message, without wrapping the result.\n\nWe need to make sure that `git rebase --autosquash` keeps handling such\ncases correctly, in particular since we are about to move the autosquash\nhandling into the rebase--helper.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t3415-rebase-autosquash.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 6d99f624b62..62cb977e4ec 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -316,4 +316,18 @@ test_expect_success 'extra spaces after fixup!' '\n \ttest $base = $parent\n '\n \n+test_expect_success 'wrapped original subject' '\n+\tif test -d .git/rebase-merge; then git rebase --abort; fi &&\n+\tbase=$(git rev-parse HEAD) &&\n+\techo \"wrapped subject\" >wrapped &&\n+\tgit add wrapped &&\n+\ttest_tick &&\n+\tgit commit --allow-empty -m \"$(printf \"To\\nfixup\")\" &&\n+\ttest_tick &&\n+\tgit commit --allow-empty -m \"fixup! To fixup\" &&\n+\tgit rebase -i --autosquash --keep-empty HEAD~2 &&\n+\tparent=$(git rev-parse HEAD^) &&\n+\ttest $base = $parent\n+'\n+\n test_done\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324522","messageId":"7f234693bdce5a4539ef6890569185c6a0ddda5a.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 08/10] rebase -i: skip unnecessary picks using the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:25Z","receivedAt":"2017-07-14T14:45:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In particular on Windows, where shell scripts are even more expensive\nthan on MacOSX or Linux, it makes sense to move a loop that forks\nGit at least once for every line in the todo list into a builtin.\n\nNote: The original code did not try to skip unnecessary picks of root\ncommits but punts instead (probably --root was not considered common\nenough of a use case to bother optimizing). We do the same, for now.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/rebase--helper.c | 6 ++-\n git-rebase--interactive.sh | 41 ++---------------\n sequencer.c | 107 +++++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n 4 files changed, 116 insertions(+), 39 deletions(-)\n\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 305adb5b707..057ae7102ff 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -15,7 +15,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tint keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n-\t\tCHECK_TODO_LIST\n+\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -32,6 +32,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"expand SHA-1s in the todo list\"), EXPAND_SHA1S),\n \t\tOPT_CMDMODE(0, \"check-todo-list\", &command,\n \t\t\tN_(\"check the todo list\"), CHECK_TODO_LIST),\n+\t\tOPT_CMDMODE(0, \"skip-unnecessary-picks\", &command,\n+\t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n \t\tOPT_END()\n \t};\n \n@@ -56,5 +58,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!transform_todo_ids(0);\n \tif (command == CHECK_TODO_LIST && argc == 1)\n \t\treturn !!check_todo_list();\n+\tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\n+\t\treturn !!skip_unnecessary_picks();\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex c8cad318fa4..af8d7bd77fb 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -713,43 +713,6 @@ do_rest () {\n \tdone\n }\n \n-# skip picking commits whose parents are unchanged\n-skip_unnecessary_picks () {\n-\tfd=3\n-\twhile read -r command rest\n-\tdo\n-\t\t# fd=3 means we skip the command\n-\t\tcase \"$fd,$command\" in\n-\t\t3,pick|3,p)\n-\t\t\t# pick a commit whose parent is current $onto -> skip\n-\t\t\tsha1=${rest%% *}\n-\t\t\tcase \"$(git rev-parse --verify --quiet \"$sha1\"^)\" in\n-\t\t\t\"$onto\"*)\n-\t\t\t\tonto=$sha1\n-\t\t\t\t;;\n-\t\t\t*)\n-\t\t\t\tfd=1\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\t;;\n-\t\t3,\"$comment_char\"*|3,)\n-\t\t\t# copy comments\n-\t\t\t;;\n-\t\t*)\n-\t\t\tfd=1\n-\t\t\t;;\n-\t\tesac\n-\t\tprintf '%s\\n' \"$command${rest:+ }$rest\" >&$fd\n-\tdone <\"$todo\" >\"$todo.new\" 3>>\"$done\" &&\n-\tmv -f \"$todo\".new \"$todo\" &&\n-\tcase \"$(peek_next_command)\" in\n-\tsquash|s|fixup|f)\n-\t\trecord_in_rewritten \"$onto\"\n-\t\t;;\n-\tesac ||\n-\t\tdie \"$(gettext \"Could not skip unnecessary pick commands\")\"\n-}\n-\n expand_todo_ids() {\n \tgit rebase--helper --expand-ids\n }\n@@ -1149,7 +1112,9 @@ git rebase--helper --check-todo-list || {\n \n expand_todo_ids\n \n-test -d \"$rewritten\" || test -n \"$force_rebase\" || skip_unnecessary_picks\n+test -d \"$rewritten\" || test -n \"$force_rebase\" ||\n+onto=\"$(git rebase--helper --skip-unnecessary-picks)\" ||\n+die \"Could not skip unnecessary pick commands\"\n \n checkout_onto\n if test -z \"$rebase_root\" && test ! -d \"$rewritten\"\ndiff --git a/sequencer.c b/sequencer.c\nindex 15107de1e1c..96d43aec764 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2641,3 +2641,110 @@ int check_todo_list(void)\n \n \treturn res;\n }\n+\n+/* skip picking commits whose parents are unchanged */\n+int skip_unnecessary_picks(void)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct object_id onto_oid, *oid = &onto_oid, *parent_oid;\n+\tint fd, i;\n+\n+\tif (!read_oneliner(&buf, rebase_path_onto(), 0))\n+\t\treturn error(_(\"could not read 'onto'\"));\n+\tif (get_oid(buf.buf, &onto_oid)) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn error(_(\"need a HEAD to fixup\"));\n+\t}\n+\tstrbuf_release(&buf);\n+\n+\tfd = open(todo_file, O_RDONLY);\n+\tif (fd < 0) {\n+\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n+\t}\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\t}\n+\tclose(fd);\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn -1;\n+\t}\n+\n+\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\tstruct todo_item *item = todo_list.items + i;\n+\n+\t\tif (item->command >= TODO_NOOP)\n+\t\t\tcontinue;\n+\t\tif (item->command != TODO_PICK)\n+\t\t\tbreak;\n+\t\tif (parse_commit(item->commit)) {\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\treturn error(_(\"could not parse commit '%s'\"),\n+\t\t\t\toid_to_hex(&item->commit->object.oid));\n+\t\t}\n+\t\tif (!item->commit->parents)\n+\t\t\tbreak; /* root commit */\n+\t\tif (item->commit->parents->next)\n+\t\t\tbreak; /* merge commit */\n+\t\tparent_oid = &item->commit->parents->item->object.oid;\n+\t\tif (hashcmp(parent_oid->hash, oid->hash))\n+\t\t\tbreak;\n+\t\toid = &item->commit->object.oid;\n+\t}\n+\tif (i > 0) {\n+\t\tint offset = i < todo_list.nr ?\n+\t\t\ttodo_list.items[i].offset_in_buf : todo_list.buf.len;\n+\t\tconst char *done_path = rebase_path_done();\n+\n+\t\tfd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);\n+\t\tif (fd < 0) {\n+\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n+\t\t\t\t done_path);\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (write_in_full(fd, todo_list.buf.buf, offset) < 0) {\n+\t\t\terror_errno(_(\"could not write to '%s'\"), done_path);\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\tclose(fd);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tclose(fd);\n+\n+\t\tfd = open(rebase_path_todo(), O_WRONLY, 0666);\n+\t\tif (fd < 0) {\n+\t\t\terror_errno(_(\"could not open '%s' for writing\"),\n+\t\t\t\t rebase_path_todo());\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (write_in_full(fd, todo_list.buf.buf + offset,\n+\t\t\t\ttodo_list.buf.len - offset) < 0) {\n+\t\t\terror_errno(_(\"could not write to '%s'\"),\n+\t\t\t\t rebase_path_todo());\n+\t\t\tclose(fd);\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (ftruncate(fd, todo_list.buf.len - offset) < 0) {\n+\t\t\terror_errno(_(\"could not truncate '%s'\"),\n+\t\t\t\t rebase_path_todo());\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\tclose(fd);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tclose(fd);\n+\n+\t\ttodo_list.current = i;\n+\t\tif (is_fixup(peek_command(&todo_list, 0)))\n+\t\t\trecord_in_rewritten(oid, peek_command(&todo_list, 0));\n+\t}\n+\n+\ttodo_list_release(&todo_list);\n+\tprintf(\"%s\\n\", oid_to_hex(oid));\n+\n+\treturn 0;\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex 878dd296f8c..04a57e09a1d 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -50,6 +50,7 @@ int sequencer_make_script(int keep_empty, FILE *out,\n \n int transform_todo_ids(int shorten_ids);\n int check_todo_list(void);\n+int skip_unnecessary_picks(void);\n \n extern const char sign_off_header[];\n \n-- \n2.13.3.windows.1.13.gaf0c2223da0\n\n\n"},{"id":"324523","messageId":"79a6c70b25e2461e8f2a058932f6c2481a8411f6.1500043437.git.johannes.schindelin@gmx.de","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"[PATCH v6 10/10] rebase -i: rearrange fixup/squash lines using the rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T14:45:31Z","receivedAt":"2017-07-14T14:45:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This operation has quadratic complexity, which is especially painful\non Windows, where shell scripts are *already* slow (mainly due to the\noverhead of the POSIX emulation layer).\n\nLet's reimplement this with linear complexity (using a hash map to\nmatch the commits' subject lines) for the common case; Sadly, the\nfixup/squash feature's design neglected performance considerations,\nallowing arbitrary prefixes (read: `fixup! hell` will match the\ncommit subject `hello world`), which means that we are stuck with\nquadratic performance in the worst case.\n\nThe reimplemented logic also happens to fix a bug where commented-out\nlines (representing empty patches) were dropped by the previous code.\n\nWhile at it, clarify how the fixup/squash feature works in `git rebase\n-i`'s man page.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-rebase.txt | 16 ++--\n builtin/rebase--helper.c | 6 +-\n git-rebase--interactive.sh | 90 +-------------------\n sequencer.c | 196 +++++++++++++++++++++++++++++++++++++++++++\n sequencer.h | 1 +\n t/t3415-rebase-autosquash.sh | 2 +-\n 6 files changed, 213 insertions(+), 98 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 4f6bed61a92..a3c01dfdc8a 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -430,13 +430,15 @@ without an explicit `--interactive`.\n --autosquash::\n --no-autosquash::\n \tWhen the commit log message begins with \"squash! ...\" (or\n-\t\"fixup! ...\"), and there is a commit whose title begins with\n-\tthe same ..., automatically modify the todo list of rebase -i\n-\tso that the commit marked for squashing comes right after the\n-\tcommit to be modified, and change the action of the moved\n-\tcommit from `pick` to `squash` (or `fixup`). Ignores subsequent\n-\t\"fixup! \" or \"squash! \" after the first, in case you referred to an\n-\tearlier fixup/squash with `git commit --fixup/--squash`.\n+\t\"fixup! ...\"), and there is already a commit in the todo list that\n+\tmatches the same `...`, automatically modify the todo list of rebase\n+\t-i so that the commit marked for squashing comes right after the\n+\tcommit to be modified, and change the action of the moved commit\n+\tfrom `pick` to `squash` (or `fixup`). A commit matches the `...` if\n+\tthe commit subject matches, or if the `...` refers to the commit's\n+\thash. As a fall-back, partial matches of the commit subject work,\n+\ttoo. The recommended way to create fixup/squash commits is by using\n+\tthe `--fixup`/`--squash` options of linkgit:git-commit[1].\n +\n This option is only valid when the `--interactive` option is used.\n +\ndiff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c\nindex 057ae7102ff..f8519363a39 100644\n--- a/builtin/rebase--helper.c\n+++ b/builtin/rebase--helper.c\n@@ -15,7 +15,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \tint keep_empty = 0;\n \tenum {\n \t\tCONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_SHA1S, EXPAND_SHA1S,\n-\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS\n+\t\tCHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH\n \t} command = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOL(0, \"ff\", &opts.allow_ff, N_(\"allow fast-forward\")),\n@@ -34,6 +34,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"check the todo list\"), CHECK_TODO_LIST),\n \t\tOPT_CMDMODE(0, \"skip-unnecessary-picks\", &command,\n \t\t\tN_(\"skip unnecessary picks\"), SKIP_UNNECESSARY_PICKS),\n+\t\tOPT_CMDMODE(0, \"rearrange-squash\", &command,\n+\t\t\tN_(\"rearrange fixup/squash lines\"), REARRANGE_SQUASH),\n \t\tOPT_END()\n \t};\n \n@@ -60,5 +62,7 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)\n \t\treturn !!check_todo_list();\n \tif (command == SKIP_UNNECESSARY_PICKS && argc == 1)\n \t\treturn !!skip_unnecessary_picks();\n+\tif (command == REARRANGE_SQUASH && argc == 1)\n+\t\treturn !!rearrange_squash();\n \tusage_with_options(builtin_rebase_helper_usage, options);\n }\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex af8d7bd77fb..3b0340e7cc9 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -721,94 +721,6 @@ collapse_todo_ids() {\n \tgit rebase--helper --shorten-ids\n }\n \n-# Rearrange the todo list that has both \"pick sha1 msg\" and\n-# \"pick sha1 fixup!/squash! msg\" appears in it so that the latter\n-# comes immediately after the former, and change \"pick\" to\n-# \"fixup\"/\"squash\".\n-#\n-# Note that if the config has specified a custom instruction format\n-# each log message will be re-retrieved in order to normalize the\n-# autosquash arrangement\n-rearrange_squash () {\n-\tformat=$(git config --get rebase.instructionFormat)\n-\t# extract fixup!/squash! lines and resolve any referenced sha1's\n-\twhile read -r pick sha1 message\n-\tdo\n-\t\ttest -z \"${format}\" || message=$(git log -n 1 --format=\"%s\" ${sha1})\n-\t\tcase \"$message\" in\n-\t\t\"squash! \"*|\"fixup! \"*)\n-\t\t\taction=\"${message%%!*}\"\n-\t\t\trest=$message\n-\t\t\tprefix=\n-\t\t\t# skip all squash! or fixup! (but save for later)\n-\t\t\twhile :\n-\t\t\tdo\n-\t\t\t\tcase \"$rest\" in\n-\t\t\t\t\"squash! \"*|\"fixup! \"*)\n-\t\t\t\t\tprefix=\"$prefix${rest%%!*},\"\n-\t\t\t\t\trest=\"${rest#*! }\"\n-\t\t\t\t\t;;\n-\t\t\t\t*)\n-\t\t\t\t\tbreak\n-\t\t\t\t\t;;\n-\t\t\t\tesac\n-\t\t\tdone\n-\t\t\tprintf '%s %s %s %s\\n' \"$sha1\" \"$action\" \"$prefix\" \"$rest\"\n-\t\t\t# if it's a single word, try to resolve to a full sha1 and\n-\t\t\t# emit a second copy. This allows us to match on both message\n-\t\t\t# and on sha1 prefix\n-\t\t\tif test \"${rest#* }\" = \"$rest\"; then\n-\t\t\t\tfullsha=\"$(git rev-parse -q --verify \"$rest\" 2>/dev/null)\"\n-\t\t\t\tif test -n \"$fullsha\"; then\n-\t\t\t\t\t# prefix the action to uniquely identify this line as\n-\t\t\t\t\t# intended for full sha1 match\n-\t\t\t\t\techo \"$sha1 +$action $prefix $fullsha\"\n-\t\t\t\tfi\n-\t\t\tfi\n-\t\tesac\n-\tdone >\"$1.sq\" <\"$1\"\n-\ttest -s \"$1.sq\" || return\n-\n-\tused=\n-\twhile read -r pick sha1 message\n-\tdo\n-\t\tcase \" $used\" in\n-\t\t*\" $sha1 \"*) continue ;;\n-\t\tesac\n-\t\tprintf '%s\\n' \"$pick $sha1 $message\"\n-\t\ttest -z \"${format}\" || message=$(git log -n 1 --format=\"%s\" ${sha1})\n-\t\tused=\"$used$sha1 \"\n-\t\twhile read -r squash action msg_prefix msg_content\n-\t\tdo\n-\t\t\tcase \" $used\" in\n-\t\t\t*\" $squash \"*) continue ;;\n-\t\t\tesac\n-\t\t\temit=0\n-\t\t\tcase \"$action\" in\n-\t\t\t+*)\n-\t\t\t\taction=\"${action#+}\"\n-\t\t\t\t# full sha1 prefix test\n-\t\t\t\tcase \"$msg_content\" in \"$sha1\"*) emit=1;; esac ;;\n-\t\t\t*)\n-\t\t\t\t# message prefix test\n-\t\t\t\tcase \"$message\" in \"$msg_content\"*) emit=1;; esac ;;\n-\t\t\tesac\n-\t\t\tif test $emit = 1; then\n-\t\t\t\tif test -n \"${format}\"\n-\t\t\t\tthen\n-\t\t\t\t\tmsg_content=$(git log -n 1 --format=\"${format}\" ${squash})\n-\t\t\t\telse\n-\t\t\t\t\tmsg_content=\"$(echo \"$msg_prefix\" | sed \"s/,/! /g\")$msg_content\"\n-\t\t\t\tfi\n-\t\t\t\tprintf '%s\\n' \"$action $squash $msg_content\"\n-\t\t\t\tused=\"$used$squash \"\n-\t\t\tfi\n-\t\tdone <\"$1.sq\"\n-\tdone >\"$1.rearranged\" <\"$1\"\n-\tcat \"$1.rearranged\" >\"$1\"\n-\trm -f \"$1.sq\" \"$1.rearranged\"\n-}\n-\n # Add commands after a pick or after a squash/fixup serie\n # in the todo list.\n add_exec_commands () {\n@@ -1068,7 +980,7 @@ then\n fi\n \n test -s \"$todo\" || echo noop >> \"$todo\"\n-test -n \"$autosquash\" && rearrange_squash \"$todo\"\n+test -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n test -n \"$cmd\" && add_exec_commands \"$todo\"\n \n todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\ndiff --git a/sequencer.c b/sequencer.c\nindex 96d43aec764..c54596f9699 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -20,6 +20,7 @@\n #include \"trailer.h\"\n #include \"log-tree.h\"\n #include \"wt-status.h\"\n+#include \"hashmap.h\"\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n@@ -2748,3 +2749,198 @@ int skip_unnecessary_picks(void)\n \n \treturn 0;\n }\n+\n+struct subject2item_entry {\n+\tstruct hashmap_entry entry;\n+\tint i;\n+\tchar subject[FLEX_ARRAY];\n+};\n+\n+static int subject2item_cmp(const void *fndata,\n+\t\t\t const struct subject2item_entry *a,\n+\t\t\t const struct subject2item_entry *b, const void *key)\n+{\n+\treturn key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);\n+}\n+\n+/*\n+ * Rearrange the todo list that has both \"pick commit-id msg\" and \"pick\n+ * commit-id fixup!/squash! msg\" in it so that the latter is put immediately\n+ * after the former, and change \"pick\" to \"fixup\"/\"squash\".\n+ *\n+ * Note that if the config has specified a custom instruction format, each log\n+ * message will have to be retrieved from the commit (as the oneline in the\n+ * script cannot be trusted) in order to normalize the autosquash arrangement.\n+ */\n+int rearrange_squash(void)\n+{\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct hashmap subject2item;\n+\tint res = 0, rearranged = 0, *next, *tail, fd, i;\n+\tchar **subjects;\n+\n+\tfd = open(todo_file, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn error_errno(_(\"could not open '%s'\"), todo_file);\n+\tif (strbuf_read(&todo_list.buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\treturn error(_(\"could not read '%s'.\"), todo_file);\n+\t}\n+\tclose(fd);\n+\tif (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {\n+\t\ttodo_list_release(&todo_list);\n+\t\treturn -1;\n+\t}\n+\n+\t/*\n+\t * The hashmap maps onelines to the respective todo list index.\n+\t *\n+\t * If any items need to be rearranged, the next[i] value will indicate\n+\t * which item was moved directly after the i'th.\n+\t *\n+\t * In that case, last[i] will indicate the index of the latest item to\n+\t * be moved to appear after the i'th.\n+\t */\n+\thashmap_init(&subject2item, (hashmap_cmp_fn) subject2item_cmp,\n+\t\t NULL, todo_list.nr);\n+\tALLOC_ARRAY(next, todo_list.nr);\n+\tALLOC_ARRAY(tail, todo_list.nr);\n+\tALLOC_ARRAY(subjects, todo_list.nr);\n+\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct todo_item *item = todo_list.items + i;\n+\t\tconst char *commit_buffer, *subject, *p;\n+\t\tsize_t subject_len;\n+\t\tint i2 = -1;\n+\t\tstruct subject2item_entry *entry;\n+\n+\t\tnext[i] = tail[i] = -1;\n+\t\tif (item->command >= TODO_EXEC) {\n+\t\t\tsubjects[i] = NULL;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (is_fixup(item->command)) {\n+\t\t\ttodo_list_release(&todo_list);\n+\t\t\treturn error(_(\"the script was already rearranged.\"));\n+\t\t}\n+\n+\t\titem->commit->util = item;\n+\n+\t\tparse_commit(item->commit);\n+\t\tcommit_buffer = get_commit_buffer(item->commit, NULL);\n+\t\tfind_commit_subject(commit_buffer, &subject);\n+\t\tformat_subject(&buf, subject, \" \");\n+\t\tsubject = subjects[i] = strbuf_detach(&buf, &subject_len);\n+\t\tunuse_commit_buffer(item->commit, commit_buffer);\n+\t\tif ((skip_prefix(subject, \"fixup! \", &p) ||\n+\t\t skip_prefix(subject, \"squash! \", &p))) {\n+\t\t\tstruct commit *commit2;\n+\n+\t\t\tfor (;;) {\n+\t\t\t\twhile (isspace(*p))\n+\t\t\t\t\tp++;\n+\t\t\t\tif (!skip_prefix(p, \"fixup! \", &p) &&\n+\t\t\t\t !skip_prefix(p, \"squash! \", &p))\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\tif ((entry = hashmap_get_from_hash(&subject2item,\n+\t\t\t\t\t\t\t strhash(p), p)))\n+\t\t\t\t/* found by title */\n+\t\t\t\ti2 = entry->i;\n+\t\t\telse if (!strchr(p, ' ') &&\n+\t\t\t\t (commit2 =\n+\t\t\t\t lookup_commit_reference_by_name(p)) &&\n+\t\t\t\t commit2->util)\n+\t\t\t\t/* found by commit name */\n+\t\t\t\ti2 = (struct todo_item *)commit2->util\n+\t\t\t\t\t- todo_list.items;\n+\t\t\telse {\n+\t\t\t\t/* copy can be a prefix of the commit subject */\n+\t\t\t\tfor (i2 = 0; i2 < i; i2++)\n+\t\t\t\t\tif (subjects[i2] &&\n+\t\t\t\t\t starts_with(subjects[i2], p))\n+\t\t\t\t\t\tbreak;\n+\t\t\t\tif (i2 == i)\n+\t\t\t\t\ti2 = -1;\n+\t\t\t}\n+\t\t}\n+\t\tif (i2 >= 0) {\n+\t\t\trearranged = 1;\n+\t\t\ttodo_list.items[i].command =\n+\t\t\t\tstarts_with(subject, \"fixup!\") ?\n+\t\t\t\tTODO_FIXUP : TODO_SQUASH;\n+\t\t\tif (next[i2] < 0)\n+\t\t\t\tnext[i2] = i;\n+\t\t\telse\n+\t\t\t\tnext[tail[i2]] = i;\n+\t\t\ttail[i2] = i;\n+\t\t} else if (!hashmap_get_from_hash(&subject2item,\n+\t\t\t\t\t\tstrhash(subject), subject)) {\n+\t\t\tFLEX_ALLOC_MEM(entry, subject, subject, subject_len);\n+\t\t\tentry->i = i;\n+\t\t\thashmap_entry_init(entry, strhash(entry->subject));\n+\t\t\thashmap_put(&subject2item, entry);\n+\t\t}\n+\t}\n+\n+\tif (rearranged) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\n+\t\tfor (i = 0; i < todo_list.nr; i++) {\n+\t\t\tenum todo_command command = todo_list.items[i].command;\n+\t\t\tint cur = i;\n+\n+\t\t\t/*\n+\t\t\t * Initially, all commands are 'pick's. If it is a\n+\t\t\t * fixup or a squash now, we have rearranged it.\n+\t\t\t */\n+\t\t\tif (is_fixup(command))\n+\t\t\t\tcontinue;\n+\n+\t\t\twhile (cur >= 0) {\n+\t\t\t\tint offset = todo_list.items[cur].offset_in_buf;\n+\t\t\t\tint end_offset = cur + 1 < todo_list.nr ?\n+\t\t\t\t\ttodo_list.items[cur + 1].offset_in_buf :\n+\t\t\t\t\ttodo_list.buf.len;\n+\t\t\t\tchar *bol = todo_list.buf.buf + offset;\n+\t\t\t\tchar *eol = todo_list.buf.buf + end_offset;\n+\n+\t\t\t\t/* replace 'pick', by 'fixup' or 'squash' */\n+\t\t\t\tcommand = todo_list.items[cur].command;\n+\t\t\t\tif (is_fixup(command)) {\n+\t\t\t\t\tstrbuf_addstr(&buf,\n+\t\t\t\t\t\ttodo_command_info[command].str);\n+\t\t\t\t\tbol += strcspn(bol, \" \\t\");\n+\t\t\t\t}\n+\n+\t\t\t\tstrbuf_add(&buf, bol, eol - bol);\n+\n+\t\t\t\tcur = next[cur];\n+\t\t\t}\n+\t\t}\n+\n+\t\tfd = open(todo_file, O_WRONLY);\n+\t\tif (fd < 0)\n+\t\t\tres = error_errno(_(\"could not open '%s'\"), todo_file);\n+\t\telse if (write(fd, buf.buf, buf.len) < 0)\n+\t\t\tres = error_errno(_(\"could not read '%s'.\"), todo_file);\n+\t\telse if (ftruncate(fd, buf.len) < 0)\n+\t\t\tres = error_errno(_(\"could not finish '%s'\"),\n+\t\t\t\t\t todo_file);\n+\t\tclose(fd);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\n+\tfree(next);\n+\tfree(tail);\n+\tfor (i = 0; i < todo_list.nr; i++)\n+\t\tfree(subjects[i]);\n+\tfree(subjects);\n+\thashmap_free(&subject2item, 1);\n+\ttodo_list_release(&todo_list);\n+\n+\treturn res;\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex 04a57e09a1d..6f3d3df82c0 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -51,6 +51,7 @@ int sequencer_make_script(int keep_empty, FILE *out,\n int transform_todo_ids(int shorten_ids);\n int check_todo_list(void);\n int skip_unnecessary_picks(void);\n+int rearrange_squash(void);\n \n extern const char sign_off_header[];\n \ndiff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh\nindex 62cb977e4ec..e364c12622f 100755\n--- a/t/t3415-rebase-autosquash.sh\n+++ b/t/t3415-rebase-autosquash.sh\n@@ -290,7 +290,7 @@ set_backup_editor () {\n \ttest_set_editor \"$PWD/backup-editor.sh\"\n }\n \n-test_expect_failure 'autosquash with multiple empty patches' '\n+test_expect_success 'autosquash with multiple empty patches' '\n \ttest_tick &&\n \tgit commit --allow-empty -m \"empty\" &&\n \ttest_tick &&\n-- \n2.13.3.windows.1.13.gaf0c2223da0\n"},{"id":"324539","messageId":"CAGZ79kbGqSadRDSQbg3N6pj04igFQG1x15S_vDUD4A6sXRSzXA@mail.gmail.com","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v6 00/10] The final building block for a faster rebase -i","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-14T17:27:04Z","receivedAt":"2017-07-14T17:27:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin\n wrote:\n> This patch series reimplements the expensive pre- and post-processing of\n> the todo script in C.\n>\n> And it concludes the work I did to accelerate rebase -i so far.\n>\n> I am still unwilling to replace a compile-time safe way to pass the\n> options to the revision machinery by the alternative (which I am still\n> flabbergasted about) proposed by Junio. This will not change.\n\nThere is new input for this discussion, though. :)\n\nhttps://public-inbox.org/git/20170706202739.6056-1-sbeller@google.com/\nwas sent out to gauge interest if we want to pull through with removing\nall the submodule hack, such as 'add_submodule_odb' in submodule.c\nwhich just adds the submodule objects as an alternate object store, such\nthat we can do some things in-process (check if a submodule has certain\ncommits, merge_submodule).\n\nFor one of these ('merge_submodule') I would have to add the\n'struct repository' as another parameter to pass around to the\ndiff and revision walking machinery. But the API for these subsystems\nis traditionally only operated via an array of strings instead of passing\nassigning a member field to a value.\n\nSo If I were to follow the \"use arrays of strings only to operate the\nrevision machinery\" I would:\n* pass a string indicating which repo to use\n (probably the path to git dir?)\n* the repository objects are cached, so we can lookup e.g.\n \"the_repository\" via the correct string.\n* use that repo object inside the revision machinery.\n\nThat sounds cumbersome and I would prefer to pass in\nthe repository object directory. So maybe we want to have some\nother way opposed to \"an array of strings\" to operate the revision\nmachinery.\n\n> -static int subject2item_cmp(const struct subject2item_entry *a,\n> - const struct subject2item_entry *b, const void *key)\n> +static int subject2item_cmp(const void *fndata,\n\nThis could also be named unused_fndata.\nPlease see origin/sb/hashmap-cleanup, if that makes sense as well\n(have all arguments const void and cast them inside the function, such\nthat we can avoid the cast to hashmap_cmp_fn in hashmap_init)\n\nThanks,\nStefan\n"},{"id":"324553","messageId":"alpine.DEB.2.21.1.1707142237310.4193@virtualbox","threadId":"46387","inReplyTo":"CAGZ79kbGqSadRDSQbg3N6pj04igFQG1x15S_vDUD4A6sXRSzXA@mail.gmail.com","subject":"Re: [PATCH v6 00/10] The final building block for a faster rebase -i","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-14T20:39:19Z","receivedAt":"2017-07-14T20:39:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 14 Jul 2017, Stefan Beller wrote:\n\n> On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin\n> wrote:\n>\n> > -static int subject2item_cmp(const struct subject2item_entry *a,\n> > - const struct subject2item_entry *b, const void *key)\n> > +static int subject2item_cmp(const void *fndata,\n> \n> This could also be named unused_fndata.\n\nSure. I simply took the version Junio used when he merged the previous\npatch series iteration into `pu`.\n\nThe function is short enough, though, that I feel it obvious that the\nparameter is unused.\n\nCiao,\nDscho\n"},{"id":"324563","messageId":"CAGZ79kYFgR_Lo-hWq6_UanvidG6a9H54qop=sg_33FCsA9Z0pQ@mail.gmail.com","threadId":"46387","inReplyTo":"814284075336e715dd52859e69aeb41226624af7.1500043437.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v6 02/10] rebase -i: generate the script via rebase--helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-14T22:50:39Z","receivedAt":"2017-07-14T22:50:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin\n wrote:\n> The first step of an interactive rebase is to generate the so-called \"todo\n> script\", to be stored in the state directory as \"git-rebase-todo\" and to\n> be edited by the user.\n>\n> Originally, we adjusted the output of `git log ` using a simple\n> sed script. Over the course of the years, the code became more\n> complicated. We now use shell scripting to edit the output of `git log`\n> conditionally, depending whether to keep \"empty\" commits (i.e. commits\n> that do not change any files).\n>\n> On platforms where shell scripting is not native, this can be a serious\n> drag. And it opens the door for incompatibilities between platforms when\n> it comes to shell scripting or to Unix-y commands.\n>\n> Let's just re-implement the todo script generation in plain C, using the\n> revision machinery directly.\n>\n> This is substantially faster, improving the speed relative to the\n> shell script version of the interactive rebase from 2x to 3x on Windows.\n\nThanks for working on this\n\n\n> +int sequencer_make_script(int keep_empty, FILE *out,\n> + int argc, const char **argv)\n> +{\n\n> + init_revisions(&revs, NULL);\n> + revs.verbose_header = 1;\n> + revs.max_parents = 1;\n> + revs.cherry_pick = 1;\n> + revs.limited = 1;\n> + revs.reverse = 1;\n> + revs.right_only = 1;\n> + revs.sort_order = REV_SORT_IN_GRAPH_ORDER;\n> + revs.topo_order = 1;\n> +\n> + revs.pretty_given = 1;\n> + git_config_get_string(\"rebase.instructionFormat\", &format);\n> + if (!format || !*format) {\n> + free(format);\n> + format = xstrdup(\"%s\");\n> + }\n\nhttps://public-inbox.org/git/xmqqvapqo4i8.fsf@gitster.mtv.corp.google.com/\n\nSo this is the core part that you and Junio have differing opinions on.\n\n> All of the above feels like inviting unnecessary future breakages by\n> knowing too much about the implementation the current version of\n> revision.c happens to use. A more careful implementation would be\n> to allocate our own av[] and prepare \"--reverse\", \"--left-right\",\n> \"--cherry-pick\", etc. to be parsed by setup_revisions() call we see\n> below. The parsing is not an expensive part of the operation\n> anyway, and that way we do not have to worry about one less thing.\n\nAllow me go through each of the options which may help\nus finding a consensus (at least it helps me having a more\ninformed opinion).\nList of options used outside of revision.c, which in the ideal\nworld of Git are parsed in e.g. handle_revision_opt called\nfrom setup_revisions:\n\n.verbose_header\n bisect.c: opt.verbose_header = 1;\n builtin/commit.c: rev.verbose_header = 1;\n builtin/log.c: rev->verbose_header = 1;\n builtin/log.c: rev.verbose_header = 1;\n builtin/log.c: rev.verbose_header = 1;\n\n.max_parents\n builtin/log.c: check_rev.max_parents = 1;\n builtin/log.c: revs.max_parents = 1;\n builtin/log.c: rev.max_parents = 1;\n builtin/log.c: revs.max_parents = 1;\n\n.cherry_pick\n is clean!\n\n.limited\n ref-filter.c: revs.limited = 1;\n\n.reverse\n seems clean.\n\n.right_only:\n.sort_order:\n is clean!\n\n.topo_order:\n builtin/fast-export.c: revs.topo_order = 1;\n builtin/log.c: revs.topo_order = 1;\n\n.pretty_given\n builtin/log.c: if (!rev->show_notes_given && (!rev->pretty_given || w.notes))\n builtin/log.c: if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {\n\nThere are two conflicting messages I get:\n* only a few fields seem to be polluted (verbose_header,\n max_parents), much fewer than I thought\n* we do use these undocumented ways already,\n but not at the scale that DScho is trying to here.\n\nIn the reply to the cover letter I outlined that we may have\na problem with integrating the repository struct when using\nstring arrays only.\n\nThanks,\nStefan\n"},{"id":"324573","messageId":"alpine.DEB.2.21.1.1707151454230.4193@virtualbox","threadId":"46387","inReplyTo":"CAGZ79kYFgR_Lo-hWq6_UanvidG6a9H54qop=sg_33FCsA9Z0pQ@mail.gmail.com","subject":"Re: [PATCH v6 02/10] rebase -i: generate the script via rebase--helper","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-15T12:56:44Z","receivedAt":"2017-07-15T12:57:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 14 Jul 2017, Stefan Beller wrote:\n\n> There are two conflicting messages I get:\n> * only a few fields seem to be polluted (verbose_header,\n> max_parents), much fewer than I thought\n> * we do use these undocumented ways already,\n> but not at the scale that DScho is trying to here.\n\nWe use mostly undocumented ways, because there is simply no API. It was\ndecided early on that libgit was never intended to be a public API. So\nthat's that.\n\nTBH a much better way to handle this would have been to say: this is\nfragile, as the pretty-printing is not properly abstracted, and hence we\nmay need to break the contract at some stage (which would not cause\ncompile errors, though), maybe you, Dscho, can fix that after this patch\nseries was applied by abstracting it properly?\n\nCiao,\nDscho\n"},{"id":"324844","messageId":"xmqqfudqkcux.fsf@gitster.mtv.corp.google.com","threadId":"46387","inReplyTo":"cover.1500043436.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v6 00/10] The final building block for a faster rebase -i","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-20T21:38:46Z","receivedAt":"2017-07-20T21:39:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Changes since v5:\n>\n> - replaced a get_sha1() call by a get_oid() call already.\n>\n> - adjusted to hashmap API changes\n\nApplying this to the tip of 'master' yields exactly the same result\nas merging the previous round js/rebase-i-final to the tip of\n'master' and then applying merge-fix/js/rebase-i-final to adjust to\nthe codebase, so the net effect of this reroll is none. Which is a\ngood sign, as it means there wasn't any rebase mistake and the evil\nmerge we've been carrying was a good one.\n\nBut at the same time, I prefer to avoid rebasing to newer 'master'\nuntil the codebase starts drifting too far apart, or until a new\nfeature release is made out of newer 'master'. This is primarily\nbecause I want dates on commits to mean something---namely, \"this\nchange hasn't seen a need to be updated for 'oops, that was wrong'\nsince this date\". This use of commit dates as 'priority date'\nmatters much less for a topic not in 'next', but as a general\nprinciple, my workflow tries to preserve commit dates for all\ntopics.\n\nFor the above reason, I may hold onto this patch series in my inbox\nwithout actually updating js/rebase-i-final topic until the current\ncycle is over; please do not mistake it as this new reroll being\nignored.\n\nThanks.\n"},{"id":"324898","messageId":"alpine.DEB.2.21.1.1707221335150.4271@virtualbox","threadId":"46387","inReplyTo":"xmqqfudqkcux.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v6 00/10] The final building block for a faster rebase -i","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-07-22T11:44:30Z","receivedAt":"2017-07-22T11:45:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 20 Jul 2017, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Changes since v5:\n> >\n> > - replaced a get_sha1() call by a get_oid() call already.\n> >\n> > - adjusted to hashmap API changes\n> \n> Applying this to the tip of 'master' yields exactly the same result\n> as merging the previous round js/rebase-i-final to the tip of\n> 'master' and then applying merge-fix/js/rebase-i-final to adjust to\n> the codebase, so the net effect of this reroll is none. Which is a\n> good sign, as it means there wasn't any rebase mistake and the evil\n> merge we've been carrying was a good one.\n\nGood.\n\n> But at the same time, I prefer to avoid rebasing to newer 'master'\n> until the codebase starts drifting too far apart, or until a new\n> feature release is made out of newer 'master'. This is primarily\n> because I want dates on commits to mean something---namely, \"this\n> change hasn't seen a need to be updated for 'oops, that was wrong'\n> since this date\". This use of commit dates as 'priority date'\n> matters much less for a topic not in 'next', but as a general\n> principle, my workflow tries to preserve commit dates for all\n> topics.\n\nBy that token, commit message updates would also be inappropriate, in\nparticular when they came from somebody else than the patch author ;-P\n\nAs to avoiding a rebase: we can add that to the growing list of things on\nwhich we disagree.\n\nIf the author dates really meant anything, we would also have to avoid v2,\nv3, v4, ... v226 of patch series. So that flies in the face of trying to\nkeep the meaning of author dates.\n\nIn addition, the development flow I prefer is one that is in harmony with\nthe modern Continuous Integration style, where topic branches are merged\ninto a single, always-ready-to-release integration branch.\n\nThat means that I always work off of `master`, unless there is a good\nreason to base off of `next` or even `pu`. That's to avoid merge\nconflicts, to see what really gets applied.\n\nI am *especially* adamant about rebasing to a newer upstream commit when\nthere are merge conflicts.\n\nSuch as is the case here.\n\n> For the above reason, I may hold onto this patch series in my inbox\n> without actually updating js/rebase-i-final topic until the current\n> cycle is over; please do not mistake it as this new reroll being\n> ignored.\n\nYou do as you want, of course. But please note that I will not rebase my\ntopic branches to an ancient revision, especially if that would cause merge\nconflicts with the current `master`.\n\nAnd if there should be another iteration of this wallflower patch series,\nI will rebase it to the then-current `master` again [*1*].\n\nCiao,\nDscho\n\nFootnote *1*: in general, I try to abide by the wishes of maintainers when\ncontributing code, unless those wishes are contrary to what I consider\ncorrect software development. Like, when in Rome, I will do as the Romans\ndo. Except when I see them looting a parking meter.\n"}]}