{"thread":{"id":"48372","subject":"[PATCH 0/6] Let the sequencer handle `git rebase -i --root`","startedAt":"2018-04-27T22:30:15Z","lastAt":"2018-05-05T19:24:49Z","messageCount":22,"participants":["Johannes Schindelin","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"346010","messageId":"cover.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":null,"subject":"[PATCH 0/6] Let the sequencer handle `git rebase -i --root`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:29:26Z","receivedAt":"2018-04-27T22:30:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When I reimplemented the most performance-critical bits of the\ninteractive rebase in the sequencer, to speed up `git rebase -i`\nparticularly on Windows (even if the benefits are still quite notable on\nLinux or macOS), I punted on the --root part.\n\nI had always hoped that some other contributor (or I myself) would come\nback later to address the --root part in the sequencer, too, with the\nidea that this would move the last remaining complicated code from\ngit-rebase--interactive.sh into sequencer.c, to facilitate converting\nthe rest of git-rebase--interactive.sh.\n\nWhen I say \"the last remaining complicated code\", of course I neglect\nthe --preserve-merges code, but as I worked hard on the --rebase-merges\npatch series with the intention to eventually deprecate and maybe even\nremove the --preserve-merges mode, I always implicitly assume that the\n--preserve-merges code will be moved into its own shell script\n(git-rebase--preserve-merges.sh, maybe?) and never be converted.\n\nSo here goes: the patches to move the handling of --root into the\nsequencer. After two preparatory patches, the real conversion takes\nplace in the third patch. After that, we take care of the --root related\nconcerns that arise in conjunction with the --rebase-merges mode.\n\nAs the --rebase-merges/--root patches overlap quite a bit (not so much\nin the code itself as in philosophical considerations such as \"what\nshould happen if you try to merge a branch into a new root\", or the\nfact that the label/reset/merge commands make it desirable to be able to\ncreate a new root commit in the middle of a todo list), I had to\nconsider in which order to contribute them. In the end, I decided to go\nwith --rebase-merges first, so the --root patches are based on the\n--rebase-merges patch series.\n\nI consider this patch series a critical prerequisite for Alban's Google\nSummer of Code project to convert rebase -i into a builtin.\n\n\nJohannes Schindelin (6):\n sequencer: extract helper to update active_cache_tree\n sequencer: learn about the special \"fake root commit\" handling\n rebase -i --root: let the sequencer handle even the initial part\n sequencer: allow introducing new root commits\n rebase --rebase-merges: a \"merge\" into a new root is a fast-forward\n rebase --rebase-merges: root commits can be cousins, too\n\n git-rebase--interactive.sh | 4 +-\n sequencer.c | 186 ++++++++++++++++++++++++++----\n sequencer.h | 4 +\n t/t3404-rebase-interactive.sh | 19 ++-\n t/t3421-rebase-topology-linear.sh | 6 +-\n t/t3430-rebase-merges.sh | 72 ++++++++++++\n 6 files changed, 256 insertions(+), 35 deletions(-)\n\n\nbase-commit: 673fb9cb8b5c7d57cb560b6ade45e419c8dd09fc\nBased-On: recreate-merges at https://github.com/dscho/git\nFetch-Base-Via: git fetch https://github.com/dscho/git recreate-merges\nPublished-As: https://github.com/dscho/git/releases/tag/sequencer-and-root-commits-v1\nFetch-It-Via: git fetch https://github.com/dscho/git sequencer-and-root-commits-v1\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n"},{"id":"346011","messageId":"ad31aab8e735f7de813860cacb32abbc406f43d1.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/6] sequencer: extract helper to update active_cache_tree","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:30:05Z","receivedAt":"2018-04-27T22:31:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch extracts the code from is_index_unchanged() to initialize or\nupdate the index' cache tree (i.e. a tree object reflecting the current\nindex' top-level tree).\n\nThe new helper will be used in the upcoming code to support `git rebase\n-i --root` via the sequencer.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e2f83942843..90c8218aa9a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -562,9 +562,23 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \treturn !clean;\n }\n \n+static struct object_id *get_cache_tree_oid(void)\n+{\n+\tif (!active_cache_tree)\n+\t\tactive_cache_tree = cache_tree();\n+\n+\tif (!cache_tree_fully_valid(active_cache_tree))\n+\t\tif (cache_tree_update(&the_index, 0)) {\n+\t\t\terror(_(\"unable to update cache tree\"));\n+\t\t\treturn NULL;\n+\t\t}\n+\n+\treturn &active_cache_tree->oid;\n+}\n+\n static int is_index_unchanged(void)\n {\n-\tstruct object_id head_oid;\n+\tstruct object_id head_oid, *cache_tree_oid;\n \tstruct commit *head_commit;\n \n \tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &head_oid, NULL))\n@@ -583,15 +597,10 @@ static int is_index_unchanged(void)\n \tif (parse_commit(head_commit))\n \t\treturn -1;\n \n-\tif (!active_cache_tree)\n-\t\tactive_cache_tree = cache_tree();\n-\n-\tif (!cache_tree_fully_valid(active_cache_tree))\n-\t\tif (cache_tree_update(&the_index, 0))\n-\t\t\treturn error(_(\"unable to update cache tree\"));\n+\tif (!(cache_tree_oid = get_cache_tree_oid()))\n+\t\treturn -1;\n \n-\treturn !oidcmp(&active_cache_tree->oid,\n-\t\t &head_commit->tree->object.oid);\n+\treturn !oidcmp(cache_tree_oid, &head_commit->tree->object.oid);\n }\n \n static int write_author_script(const char *message)\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n\n"},{"id":"346012","messageId":"42db734a98059fcfd67627aecc93cc8f0439fd37.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/6] sequencer: learn about the special \"fake root commit\" handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:31:13Z","receivedAt":"2018-04-27T22:31:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When an interactive rebase wants to recreate a root commit, it\n- first creates a new, empty root commit,\n- checks it out,\n- converts the next `pick` command so that it amends the empty root\n commit\n\nIntroduce support in the sequencer to handle such an empty root commit,\nby looking for the file /rebase-merge/squash-onto; if it exists\nand contains a commit name, the sequencer will compare the HEAD to said\nroot commit, and if identical, a new root commit will be created.\n\nWhile converting scripted code into proper, portable C, we also do away\nwith the old \"amend with an empty commit message, then cherry-pick\nwithout committing, then amend again\" dance and replace it with code\nthat uses the internal API properly to do exactly what we want: create a\nnew root commit.\n\nTo keep the implementation simple, we always spawn `git commit` to create\nnew root commits.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n sequencer.h | 4 ++\n 2 files changed, 105 insertions(+), 3 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 90c8218aa9a..fc124596b53 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -125,6 +125,12 @@ static GIT_PATH_FUNC(rebase_path_rewritten_list, \"rebase-merge/rewritten-list\")\n static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n \t\"rebase-merge/rewritten-pending\")\n \n+/*\n+ * The path of the file containig the OID of the \"squash onto\" commit, i.e.\n+ * the dummy commit used for `reset [new root]`.\n+ */\n+static GIT_PATH_FUNC(rebase_path_squash_onto, \"rebase-merge/squash-onto\")\n+\n /*\n * The path of the file listing refs that need to be deleted after the rebase\n * finishes. This is used by the `label` command to record the need for cleanup.\n@@ -470,7 +476,8 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_update(transaction, \"HEAD\",\n-\t\t\t\t to, unborn ? &null_oid : from,\n+\t\t\t\t to, unborn && !is_rebase_i(opts) ?\n+\t\t\t\t &null_oid : from,\n \t\t\t\t 0, sb.buf, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_free(transaction);\n@@ -692,6 +699,42 @@ static char *get_author(const char *message)\n \treturn NULL;\n }\n \n+static const char *read_author_ident(struct strbuf *buf)\n+{\n+\tchar *p, *p2;\n+\n+\tif (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)\n+\t\treturn NULL;\n+\n+\tfor (p = buf->buf; *p; p++)\n+\t\tif (skip_prefix(p, \"'\\\\\\\\''\", (const char **)&p2))\n+\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p, \"'\", 1);\n+\t\telse if (*p == '\\'')\n+\t\t\tstrbuf_splice(buf, p-- - buf->buf, 1, \"\", 0);\n+\n+\tif (skip_prefix(buf->buf, \"GIT_AUTHOR_NAME=\", (const char **)&p)) {\n+\t\tstrbuf_splice(buf, 0, p - buf->buf, \"\", 0);\n+\t\tp = strchr(buf->buf, '\\n');\n+\t\tif (skip_prefix(p, \"\\nGIT_AUTHOR_EMAIL=\", (const char **)&p2)) {\n+\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p, \" <\", 2);\n+\t\t\tp = strchr(p, '\\n');\n+\t\t\tif (skip_prefix(p, \"\\nGIT_AUTHOR_DATE=@\",\n+\t\t\t\t\t(const char **)&p2)) {\n+\t\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p,\n+\t\t\t\t\t \"> \", 2);\n+\t\t\t\tp = strchr(p, '\\n');\n+\t\t\t\tif (p) {\n+\t\t\t\t\tstrbuf_setlen(buf, p - buf->buf);\n+\t\t\t\t\treturn buf->buf;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\twarning(_(\"could not parse '%s'\"), rebase_path_author_script());\n+\treturn NULL;\n+}\n+\n static const char staged_changes_advice[] =\n N_(\"you have staged changes in your working tree\\n\"\n \"If these changes are meant to be squashed into the previous commit, run:\\n\"\n@@ -711,6 +754,7 @@ N_(\"you have staged changes in your working tree\\n\"\n #define AMEND_MSG (1<<2)\n #define CLEANUP_MSG (1<<3)\n #define VERIFY_MSG (1<<4)\n+#define CREATE_ROOT_COMMIT (1<<5)\n \n /*\n * If we are cherry-pick, and if the merge did not result in\n@@ -730,6 +774,40 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tconst char *value;\n \n+\tif (flags & CREATE_ROOT_COMMIT) {\n+\t\tstruct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;\n+\t\tconst char *author = is_rebase_i(opts) ?\n+\t\t\tread_author_ident(&script) : NULL;\n+\t\tstruct object_id root_commit, *cache_tree_oid;\n+\t\tint res = 0;\n+\n+\t\tif (!defmsg)\n+\t\t\tBUG(\"root commit without message\");\n+\n+\t\tif (!(cache_tree_oid = get_cache_tree_oid()))\n+\t\t\tres = -1;\n+\n+\t\tif (!res)\n+\t\t\tres = strbuf_read_file(&msg, defmsg, 0);\n+\n+\t\tif (res <= 0)\n+\t\t\tres = error_errno(_(\"could not read '%s'\"), defmsg);\n+\t\telse\n+\t\t\tres = commit_tree(msg.buf, msg.len, cache_tree_oid,\n+\t\t\t\t\t NULL, &root_commit, author,\n+\t\t\t\t\t opts->gpg_sign);\n+\n+\t\tstrbuf_release(&msg);\n+\t\tstrbuf_release(&script);\n+\t\tif (!res) {\n+\t\t\tupdate_ref(NULL, \"CHERRY_PICK_HEAD\", &root_commit, NULL,\n+\t\t\t\t REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR);\n+\t\t\tres = update_ref(NULL, \"HEAD\", &root_commit, NULL, 0,\n+\t\t\t\t\t UPDATE_REFS_MSG_ON_ERR);\n+\t\t}\n+\t\treturn res < 0 ? error(_(\"writing root commit\")) : 0;\n+\t}\n+\n \tcmd.git_cmd = 1;\n \n \tif (is_rebase_i(opts)) {\n@@ -1216,7 +1294,8 @@ static int do_commit(const char *msg_file, const char *author,\n {\n \tint res = 1;\n \n-\tif (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG)) {\n+\tif (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG) &&\n+\t !(flags & CREATE_ROOT_COMMIT)) {\n \t\tstruct object_id oid;\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n@@ -1369,6 +1448,12 @@ static int is_fixup(enum todo_command command)\n \treturn command == TODO_FIXUP || command == TODO_SQUASH;\n }\n \n+/* Does this command create a (non-merge) commit? */\n+static int is_pick_or_similar(enum todo_command command)\n+{\n+\treturn command <= TODO_SQUASH;\n+}\n+\n static int update_squash_messages(enum todo_command command,\n \t\tstruct commit *commit, struct replay_opts *opts)\n {\n@@ -1523,7 +1608,14 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t\t\treturn error(_(\"your index file is unmerged.\"));\n \t} else {\n \t\tunborn = get_oid(\"HEAD\", &head);\n-\t\tif (unborn)\n+\t\t/* Do we want to generate a root commit? */\n+\t\tif (is_pick_or_similar(command) && opts->have_squash_onto &&\n+\t\t !oidcmp(&head, &opts->squash_onto)) {\n+\t\t\tif (is_fixup(command))\n+\t\t\t\treturn error(_(\"cannot fixup root commit\"));\n+\t\t\tflags |= CREATE_ROOT_COMMIT;\n+\t\t\tunborn = 1;\n+\t\t} else if (unborn)\n \t\t\toidcpy(&head, the_hash_algo->empty_tree);\n \t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\",\n \t\t\t\t NULL, 0))\n@@ -2136,6 +2228,12 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_release(&buf);\n \n+\t\tif (read_oneliner(&buf, rebase_path_squash_onto(), 0)) {\n+\t\t\tif (get_oid_hex(buf.buf, &opts->squash_onto) < 0)\n+\t\t\t\treturn error(_(\"unusable squash-onto\"));\n+\t\t\topts->have_squash_onto = 1;\n+\t\t}\n+\n \t\treturn 0;\n \t}\n \ndiff --git a/sequencer.h b/sequencer.h\nindex d9570d92b11..4b2717881fa 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -44,6 +44,10 @@ struct replay_opts {\n \tchar **xopts;\n \tsize_t xopts_nr, xopts_alloc;\n \n+\t/* placeholder commit for -i --root */\n+\tstruct object_id squash_onto;\n+\tint have_squash_onto;\n+\n \t/* Only used by REPLAY_NONE */\n \tstruct rev_info *revs;\n };\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n\n"},{"id":"346013","messageId":"1c8740eaa916351832cbea4cf75b77e5dc965db8.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:31:22Z","receivedAt":"2018-04-27T22:31:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In this developer's earlier attempt to accelerate interactive rebases by\nconverting large parts from Unix shell script into portable, performant\nC, the --root handling was specifically excluded (to simplify the task a\nlittle bit; it still took over a year to get that reduced set of patches\ninto Git proper).\n\nThis patch ties up that loose end: now only --preserve-merges uses the\nslow Unix shell script implementation to perform the interactive rebase.\n\nAs the rebase--helper reports progress to stderr (unlike the scripted\ninteractive rebase, which reports it to stdout, of all places), we have\nto adjust a couple of tests that did not expect that for `git rebase -i\n--root`.\n\nThis patch fixes -- at long last! -- the really old bug reported in\n6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with\n--root *always* rewrote the root commit, even if there were no changes.\n\nThe bug still persists in --preserve-merges mode, of course, but that\nmode will be deprecated as soon as the new --rebase-merges mode\nstabilizes, anyway.\n\nSigned-off-by: Johannes Schindelin \n---\n git-rebase--interactive.sh | 4 +++-\n t/t3404-rebase-interactive.sh | 19 +++++++++++++------\n t/t3421-rebase-topology-linear.sh | 6 +++---\n 3 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex cbf44f86482..2f4941d0fc9 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () {\n \telse\n \t\trevisions=$onto...$orig_head\n \t\tshortrevisions=$shorthead\n+\t\ttest -z \"$squash_onto\" ||\n+\t\techo \"$squash_onto\" >\"$state_dir\"/squash-onto\n \tfi\n }\n \n@@ -948,7 +950,7 @@ EOF\n \tdie \"Could not skip unnecessary pick commands\"\n \n \tcheckout_onto\n-\tif test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n+\tif test ! -d \"$rewritten\"\n \tthen\n \t\trequire_clean_work_tree \"rebase\"\n \t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 59c766540e5..c65826ddace 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1204,10 +1204,6 @@ test_expect_success 'drop' '\n \ttest A = $(git cat-file commit HEAD^^ | sed -ne \\$p)\n '\n \n-cat >expect <actual &&\n \ttest D = $(git cat-file commit HEAD | sed -ne \\$p) &&\n-\ttest_i18ncmp expect actual\n+\ttest_i18ngrep \\\n+\t\t\"Successfully rebased and updated refs/heads/missing-commit\" \\\n+\t\tactual\n '\n \n cat >expect <actual &&\n+\t\tgit rebase -i --root 2>actual.2 &&\n+\tcr_to_nl actual &&\n \ttest_i18ncmp expect actual &&\n \ttest D = $(git cat-file commit HEAD | sed -ne \\$p)\n '\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex e7438ad06ac..99b2aac9219 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -328,9 +328,9 @@ test_run_rebase () {\n \t\ttest_cmp_rev c HEAD\n \t\"\n }\n-test_run_rebase failure ''\n-test_run_rebase failure -m\n-test_run_rebase failure -i\n+test_run_rebase success ''\n+test_run_rebase success -m\n+test_run_rebase success -i\n test_run_rebase failure -p\n \n test_run_rebase () {\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n\n"},{"id":"346014","messageId":"8a7f575141218c8780b94d5df948906695294a2f.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 4/6] sequencer: allow introducing new root commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:31:30Z","receivedAt":"2018-04-27T22:31:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In the context of the new --rebase-merges mode, which was designed\nspecifically to allow for changing the existing branch topology\nliberally, a user may want to extract commits into a completely fresh\nbranch that starts with a newly-created root commit.\n\nThis is now possible by inserting the command `reset [new root]` before\n`pick`ing the commit that wants to become a root commit. Example:\n\n\treset [new root]\n\tpick 012345 a commit that is about to become a root commit\n\tpick 234567 this commit will have the previous one as parent\n\nThis does not conflict with other uses of the `reset` command because\n`[new root]` is not (part of) a valid ref name: both the opening bracket\nas well as the space are illegal in ref names.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 40 ++++++++++++++++++++++++++++------------\n t/t3430-rebase-merges.sh | 34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 12 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex fc124596b53..d10ebd62520 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2727,18 +2727,34 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)\n \tif (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)\n \t\treturn -1;\n \n-\t/* Determine the length of the label */\n-\tfor (i = 0; i < len; i++)\n-\t\tif (isspace(name[i]))\n-\t\t\tlen = i;\n-\n-\tstrbuf_addf(&ref_name, \"refs/rewritten/%.*s\", len, name);\n-\tif (get_oid(ref_name.buf, &oid) &&\n-\t get_oid(ref_name.buf + strlen(\"refs/rewritten/\"), &oid)) {\n-\t\terror(_(\"could not read '%s'\"), ref_name.buf);\n-\t\trollback_lock_file(&lock);\n-\t\tstrbuf_release(&ref_name);\n-\t\treturn -1;\n+\tif (len == 10 && !strncmp(\"[new root]\", name, len)) {\n+\t\tif (!opts->have_squash_onto) {\n+\t\t\tconst char *hex;\n+\t\t\tif (commit_tree(\"\", 0, the_hash_algo->empty_tree,\n+\t\t\t\t\tNULL, &opts->squash_onto,\n+\t\t\t\t\tNULL, NULL))\n+\t\t\t\treturn error(_(\"writing fake root commit\"));\n+\t\t\topts->have_squash_onto = 1;\n+\t\t\thex = oid_to_hex(&opts->squash_onto);\n+\t\t\tif (write_message(hex, strlen(hex),\n+\t\t\t\t\t rebase_path_squash_onto(), 0))\n+\t\t\t\treturn error(_(\"writing squash-onto\"));\n+\t\t}\n+\t\toidcpy(&oid, &opts->squash_onto);\n+\t} else {\n+\t\t/* Determine the length of the label */\n+\t\tfor (i = 0; i < len; i++)\n+\t\t\tif (isspace(name[i]))\n+\t\t\t\tlen = i;\n+\n+\t\tstrbuf_addf(&ref_name, \"refs/rewritten/%.*s\", len, name);\n+\t\tif (get_oid(ref_name.buf, &oid) &&\n+\t\t get_oid(ref_name.buf + strlen(\"refs/rewritten/\"), &oid)) {\n+\t\t\terror(_(\"could not read '%s'\"), ref_name.buf);\n+\t\t\trollback_lock_file(&lock);\n+\t\t\tstrbuf_release(&ref_name);\n+\t\t\treturn -1;\n+\t\t}\n \t}\n \n \tmemset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 3d4dfdf7bec..35260862fcb 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -241,4 +241,38 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n \ttest_cmp_rev HEAD $before\n '\n \n+test_expect_success 'root commits' '\n+\tgit checkout --orphan unrelated &&\n+\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n+\t test_commit second-root) &&\n+\ttest_commit third-root &&\n+\tcat >script-from-scratch <<-\\EOF &&\n+\tpick third-root\n+\tlabel first-branch\n+\treset [new root]\n+\tpick second-root\n+\tmerge first-branch # Merge the 3rd root\n+\tEOF\n+\ttest_config sequence.editor \\\"\"$PWD\"/replace-editor.sh\\\" &&\n+\ttest_tick &&\n+\tgit rebase -i --force --root -r &&\n+\ttest \"Parsnip\" = \"$(git show -s --format=%an HEAD^)\" &&\n+\ttest $(git rev-parse second-root^0) != $(git rev-parse HEAD^) &&\n+\ttest $(git rev-parse second-root:second-root.t) = \\\n+\t\t$(git rev-parse HEAD^:second-root.t) &&\n+\ttest_cmp_graph HEAD <<-\\EOF &&\n+\t* Merge the 3rd root\n+\t|\\\n+\t| * third-root\n+\t* second-root\n+\tEOF\n+\n+\t: fast forward if possible &&\n+\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n+\ttest_might_fail git config --unset sequence.editor &&\n+\ttest_tick &&\n+\tgit rebase -i --root -r &&\n+\ttest_cmp_rev HEAD $before\n+'\n+\n test_done\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n\n"},{"id":"346015","messageId":"0b7379b576be2cee807b8237dc52e5412663a2fd.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 5/6] rebase --rebase-merges: a \"merge\" into a new root is a fast-forward","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:31:32Z","receivedAt":"2018-04-27T22:32:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When a user provides a todo list containing something like\n\n\treset [new root]\n\tmerge my-branch\n\nlet's do the same as if pulling into an orphan branch: simply\nfast-forward.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 12 ++++++++++++\n t/t3430-rebase-merges.sh | 13 +++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex d10ebd62520..ad5ff2709a6 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2850,6 +2850,18 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\tgoto leave_merge;\n \t}\n \n+\tif (opts->have_squash_onto &&\n+\t !oidcmp(&head_commit->object.oid, &opts->squash_onto)) {\n+\t\t/*\n+\t\t * When the user tells us to \"merge\" something into a\n+\t\t * \"[new root]\", let's simply fast-forward to the merge head.\n+\t\t */\n+\t\trollback_lock_file(&lock);\n+\t\tret = fast_forward_to(&merge_commit->object.oid,\n+\t\t\t\t &head_commit->object.oid, 0, opts);\n+\t\tgoto leave_merge;\n+\t}\n+\n \tif (commit) {\n \t\tconst char *message = get_commit_buffer(commit, NULL);\n \t\tconst char *body;\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 35260862fcb..5543f1d5a34 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -275,4 +275,17 @@ test_expect_success 'root commits' '\n \ttest_cmp_rev HEAD $before\n '\n \n+test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n+\thead=$(git rev-parse HEAD) &&\n+\tcat >script-from-scratch <<-EOF &&\n+\treset [new root]\n+\tmerge $head\n+\tEOF\n+\ttest_config sequence.editor \\\"\"$PWD\"/replace-editor.sh\\\" &&\n+\ttest_tick &&\n+\tgit rebase -i -r HEAD^ &&\n+\ttest_cmp_rev HEAD $head\n+'\n+\n+\n test_done\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n\n\n"},{"id":"346016","messageId":"270b8fdf477b6c876e203ae6a6b461b010d25f76.1524868165.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH 6/6] rebase --rebase-merges: root commits can be cousins, too","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-27T22:31:58Z","receivedAt":"2018-04-27T22:32:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Reported by Wink Saville: when rebasing with no-rebase-cousins, we\nwill want to refrain from rebasing all of them, even when they are\nroot commits.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 3 ++-\n t/t3430-rebase-merges.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ad5ff2709a6..2bcd13e1fc6 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3883,7 +3883,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t}\n \n \t\tif (!commit)\n-\t\t\tfprintf(out, \"%s onto\\n\", cmd_reset);\n+\t\t\tfprintf(out, \"%s %s\\n\", cmd_reset,\n+\t\t\t\trebase_cousins ? \"onto\" : \"[new root]\");\n \t\telse {\n \t\t\tconst char *to = NULL;\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 5543f1d5a34..ce6de6f491e 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -287,5 +287,30 @@ test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n \ttest_cmp_rev HEAD $head\n '\n \n+test_expect_success 'A root commit can be a cousin, treat it that way' '\n+\tgit checkout --orphan khnum &&\n+\ttest_commit yama &&\n+\tgit checkout -b asherah master &&\n+\ttest_commit shamkat &&\n+\tgit merge --allow-unrelated-histories khnum &&\n+\ttest_tick &&\n+\tgit rebase -f -r HEAD^ &&\n+\t! test_cmp_rev HEAD^2 khnum &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF &&\n+\t* Merge branch '\\''khnum'\\'' into asherah\n+\t|\\\n+\t| * yama\n+\to shamkat\n+\tEOF\n+\ttest_tick &&\n+\tgit rebase --rebase-merges=rebase-cousins HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge branch '\\''khnum'\\'' into asherah\n+\t|\\\n+\t| * yama\n+\t|/\n+\to shamkat\n+\tEOF\n+'\n \n test_done\n-- \n2.17.0.windows.1.33.gfcbb1fa0445\n"},{"id":"346042","messageId":"CAGZ79kZbdkNPngcuQe-VWMRCkaHpcGFBa+DGkhU-91FqyhPbrg@mail.gmail.com","threadId":"48372","inReplyTo":"ad31aab8e735f7de813860cacb32abbc406f43d1.1524868165.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/6] sequencer: extract helper to update active_cache_tree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-28T15:28:14Z","receivedAt":"2018-04-28T15:28:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Johannes,\n\nOn Fri, Apr 27, 2018 at 3:30 PM, Johannes Schindelin\n wrote:\n> This patch extracts the code from is_index_unchanged() to initialize or\n> update the index' cache tree (i.e. a tree object reflecting the current\n> index' top-level tree).\n>\n> The new helper will be used in the upcoming code to support `git rebase\n> -i --root` via the sequencer.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> sequencer.c | 27 ++++++++++++++++++---------\n> 1 file changed, 18 insertions(+), 9 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index e2f83942843..90c8218aa9a 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -562,9 +562,23 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n> return !clean;\n> }\n>\n> +static struct object_id *get_cache_tree_oid(void)\n> +{\n> + if (!active_cache_tree)\n> + active_cache_tree = cache_tree();\n> +\n> + if (!cache_tree_fully_valid(active_cache_tree))\n> + if (cache_tree_update(&the_index, 0)) {\n> + error(_(\"unable to update cache tree\"));\n> + return NULL;\n> + }\n\nThis move is a verbatim move of the code below, except that\nwe need to add braces. For some reason I fantasize of using\nthe comma operator in C eventually (which we do not use in\nour code base AFAICT), then we could leave out the braces\nand have a\n\n return error(...), NULL;\n\nAnyway, this patch is\nReviewed-by: Stefan Beller \n"},{"id":"346044","messageId":"CAGZ79kZJJadXVhcMfxqq2kH=F-6nKVuhOc_s-vgj_9w5YcOxkg@mail.gmail.com","threadId":"48372","inReplyTo":"42db734a98059fcfd67627aecc93cc8f0439fd37.1524868165.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/6] sequencer: learn about the special \"fake root commit\" handling","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-28T16:11:54Z","receivedAt":"2018-04-28T16:11:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin\n wrote:\n> When an interactive rebase wants to recreate a root commit, it\n> - first creates a new, empty root commit,\n> - checks it out,\n> - converts the next `pick` command so that it amends the empty root\n> commit\n>\n> Introduce support in the sequencer to handle such an empty root commit,\n> by looking for the file /rebase-merge/squash-onto; if it exists\n> and contains a commit name, the sequencer will compare the HEAD to said\n> root commit, and if identical, a new root commit will be created.\n>\n> While converting scripted code into proper, portable C, we also do away\n> with the old \"amend with an empty commit message, then cherry-pick\n> without committing, then amend again\" dance and replace it with code\n> that uses the internal API properly to do exactly what we want: create a\n> new root commit.\n>\n> To keep the implementation simple, we always spawn `git commit` to create\n> new root commits.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> sequencer.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n> sequencer.h | 4 ++\n> 2 files changed, 105 insertions(+), 3 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 90c8218aa9a..fc124596b53 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -125,6 +125,12 @@ static GIT_PATH_FUNC(rebase_path_rewritten_list, \"rebase-merge/rewritten-list\")\n> static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n> \"rebase-merge/rewritten-pending\")\n>\n> +/*\n> + * The path of the file containig the OID of the \"squash onto\" commit, i.e.\n> + * the dummy commit used for `reset [new root]`.\n> + */\n> +static GIT_PATH_FUNC(rebase_path_squash_onto, \"rebase-merge/squash-onto\")\n> +\n> /*\n> * The path of the file listing refs that need to be deleted after the rebase\n> * finishes. This is used by the `label` command to record the need for cleanup.\n> @@ -470,7 +476,8 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n> transaction = ref_transaction_begin(&err);\n> if (!transaction ||\n> ref_transaction_update(transaction, \"HEAD\",\n> - to, unborn ? &null_oid : from,\n> + to, unborn && !is_rebase_i(opts) ?\n> + &null_oid : from,\n> 0, sb.buf, &err) ||\n> ref_transaction_commit(transaction, &err)) {\n> ref_transaction_free(transaction);\n> @@ -692,6 +699,42 @@ static char *get_author(const char *message)\n> return NULL;\n> }\n>\n> +static const char *read_author_ident(struct strbuf *buf)\n\nThis seems to be the counter part of write_author_script(*msg),\nwould it make sense to either rename this to read_author_script\nor rename the counter part to write_author_ident ?\n\n> +{\n> + char *p, *p2;\n> +\n> + if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)\n\nThe 256 is a hint for read_file how to size the buffer initially.\nIf not given it defaults to 8k, which presumably is too much for\nan author identity.\n\n\n\n> + for (p = buf->buf; *p; p++)\n> + if (skip_prefix(p, \"'\\\\\\\\''\", (const char **)&p2))\n> + strbuf_splice(buf, p - buf->buf, p2 - p, \"'\", 1);\n> + else if (*p == '\\'')\n> + strbuf_splice(buf, p-- - buf->buf, 1, \"\", 0);\n\nThis part could be prefixed with\n /* un-escape text: turn \\\\ into ' and remove single quotes. */\n\n> + if (skip_prefix(buf->buf, \"GIT_AUTHOR_NAME=\", (const char **)&p)) {\n> + strbuf_splice(buf, 0, p - buf->buf, \"\", 0);\n> + p = strchr(buf->buf, '\\n');\n> + if (skip_prefix(p, \"\\nGIT_AUTHOR_EMAIL=\", (const char **)&p2)) {\n> + strbuf_splice(buf, p - buf->buf, p2 - p, \" <\", 2);\n> + p = strchr(p, '\\n');\n> + if (skip_prefix(p, \"\\nGIT_AUTHOR_DATE=@\",\n> + (const char **)&p2)) {\n> + strbuf_splice(buf, p - buf->buf, p2 - p,\n> + \"> \", 2);\n> + p = strchr(p, '\\n');\n> + if (p) {\n> + strbuf_setlen(buf, p - buf->buf);\n> + return buf->buf;\n\nSo here we have read GIT_AUTHOR_NAME, _EMAIL\nand _DATE in that order and converted it to its form\n\"name date\" in a single line.\n\nIt would be better to invert the conditions and keep\nthe indentation level lower by:\n\n if (!skip_prefix(...))\n goto warning_and_return;\n strbuf_splice(...);\n ...\n\nI wondered if we want to factor out the conversion of\n\"author string in commit form\" to \"author information\nin script form\" into their own functions, and keep the reading\nwriting out of them. But then again we only need them in\nthese use cases for now, and such a refactoring can happen\nlater if needed.\n\n> + warning(_(\"could not parse '%s'\"), rebase_path_author_script());\n\nThis function needs all three environment variables in its correct order,\nwhich sounds a little brittle, but then again we do not expect manual\nediting of that file, but expect it to be written by Git.\n\n> @@ -1369,6 +1448,12 @@ static int is_fixup(enum todo_command command)\n> return command == TODO_FIXUP || command == TODO_SQUASH;\n> }\n>\n> +/* Does this command create a (non-merge) commit? */\n> +static int is_pick_or_similar(enum todo_command command)\n> +{\n> + return command <= TODO_SQUASH;\n> +}\n\nThis code looks scary.\nFortunately the enum todo_command hints that the order matters,\nsuch that we're probably protected from wild reordering in the future,\nhowever this implies that the section /* commands that handle commits */\ncomes first and that TODO_SQUASH is the last entry of that section.\nSo maybe we'd want to add a /* must be last in section */ to\nTODO_squash and also document that the section must be first?\n\nDo we have other code that needs a very specific ordering\nwith similar further assumptions (section being first/last, a\ncommand being first/last in their section)?\n\nI wondered what the _or_similar means and by looking up\nthat enum, I would think a name like\n\nstatic int handles_single_commit(enum todo_command)\n\nmight be better?\n\n> @@ -1523,7 +1608,14 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n> return error(_(\"your index file is unmerged.\"));\n> } else {\n> unborn = get_oid(\"HEAD\", &head);\n> - if (unborn)\n> + /* Do we want to generate a root commit? */\n> + if (is_pick_or_similar(command) && opts->have_squash_onto &&\n> + !oidcmp(&head, &opts->squash_onto)) {\n> + if (is_fixup(command))\n> + return error(_(\"cannot fixup root commit\"));\n\nI would expect you also cannot squash into root commit?\n\nThanks,\nStefan\n"},{"id":"346045","messageId":"CAGZ79kYi5W+m9TQ_iEwPmEaXQ_tvpRjcycGioY5jThtfGM6mYw@mail.gmail.com","threadId":"48372","inReplyTo":"1c8740eaa916351832cbea4cf75b77e5dc965db8.1524868165.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-28T16:19:09Z","receivedAt":"2018-04-28T16:19:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin\n wrote:\n> In this developer's earlier attempt to accelerate interactive rebases by\n> converting large parts from Unix shell script into portable, performant\n> C, the --root handling was specifically excluded (to simplify the task a\n> little bit; it still took over a year to get that reduced set of patches\n> into Git proper).\n>\n> This patch ties up that loose end: now only --preserve-merges uses the\n> slow Unix shell script implementation to perform the interactive rebase.\n>\n> As the rebase--helper reports progress to stderr (unlike the scripted\n> interactive rebase, which reports it to stdout, of all places), we have\n> to adjust a couple of tests that did not expect that for `git rebase -i\n> --root`.\n>\n> This patch fixes -- at long last! -- the really old bug reported in\n> 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with\n> --root *always* rewrote the root commit, even if there were no changes.\n>\n> The bug still persists in --preserve-merges mode, of course, but that\n> mode will be deprecated as soon as the new --rebase-merges mode\n> stabilizes, anyway.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> git-rebase--interactive.sh | 4 +++-\n> t/t3404-rebase-interactive.sh | 19 +++++++++++++------\n> t/t3421-rebase-topology-linear.sh | 6 +++---\n> 3 files changed, 19 insertions(+), 10 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index cbf44f86482..2f4941d0fc9 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () {\n> else\n> revisions=$onto...$orig_head\n> shortrevisions=$shorthead\n> + test -z \"$squash_onto\" ||\n> + echo \"$squash_onto\" >\"$state_dir\"/squash-onto\n> fi\n> }\n>\n> @@ -948,7 +950,7 @@ EOF\n> die \"Could not skip unnecessary pick commands\"\n>\n> checkout_onto\n> - if test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n> + if test ! -d \"$rewritten\"\n\nI have the impression this is the line that is really well\nexplained in the commit message (\"migrate to rebase\nhelper even when there is $rebase_root set\")\n\nThe rest of the patch is covered as \"a couple of places\nwhere we adjust stdout to stderr\"?\n\nMakes sense,\nStefan\n"},{"id":"346072","messageId":"nycvar.QRO.7.76.6.1804291255010.79@tvgsbejvaqbjf.bet","threadId":"48372","inReplyTo":"CAGZ79kZJJadXVhcMfxqq2kH=F-6nKVuhOc_s-vgj_9w5YcOxkg@mail.gmail.com","subject":"Re: [PATCH 2/6] sequencer: learn about the special \"fake root commit\" handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-29T12:33:01Z","receivedAt":"2018-04-29T12:33:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Sat, 28 Apr 2018, Stefan Beller wrote:\n\n> On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin\n> wrote:\n> > When an interactive rebase wants to recreate a root commit, it\n> > - first creates a new, empty root commit,\n> > - checks it out,\n> > - converts the next `pick` command so that it amends the empty root\n> > commit\n> >\n> > Introduce support in the sequencer to handle such an empty root commit,\n> > by looking for the file /rebase-merge/squash-onto; if it exists\n> > and contains a commit name, the sequencer will compare the HEAD to said\n> > root commit, and if identical, a new root commit will be created.\n> >\n> > While converting scripted code into proper, portable C, we also do away\n> > with the old \"amend with an empty commit message, then cherry-pick\n> > without committing, then amend again\" dance and replace it with code\n> > that uses the internal API properly to do exactly what we want: create a\n> > new root commit.\n> >\n> > To keep the implementation simple, we always spawn `git commit` to create\n> > new root commits.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > sequencer.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n> > sequencer.h | 4 ++\n> > 2 files changed, 105 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index 90c8218aa9a..fc124596b53 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -125,6 +125,12 @@ static GIT_PATH_FUNC(rebase_path_rewritten_list, \"rebase-merge/rewritten-list\")\n> > static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n> > \"rebase-merge/rewritten-pending\")\n> >\n> > +/*\n> > + * The path of the file containig the OID of the \"squash onto\" commit, i.e.\n> > + * the dummy commit used for `reset [new root]`.\n> > + */\n> > +static GIT_PATH_FUNC(rebase_path_squash_onto, \"rebase-merge/squash-onto\")\n> > +\n> > /*\n> > * The path of the file listing refs that need to be deleted after the rebase\n> > * finishes. This is used by the `label` command to record the need for cleanup.\n> > @@ -470,7 +476,8 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n> > transaction = ref_transaction_begin(&err);\n> > if (!transaction ||\n> > ref_transaction_update(transaction, \"HEAD\",\n> > - to, unborn ? &null_oid : from,\n> > + to, unborn && !is_rebase_i(opts) ?\n> > + &null_oid : from,\n> > 0, sb.buf, &err) ||\n> > ref_transaction_commit(transaction, &err)) {\n> > ref_transaction_free(transaction);\n> > @@ -692,6 +699,42 @@ static char *get_author(const char *message)\n> > return NULL;\n> > }\n> >\n> > +static const char *read_author_ident(struct strbuf *buf)\n> \n> This seems to be the counter part of write_author_script(*msg),\n> would it make sense to either rename this to read_author_script\n> or rename the counter part to write_author_ident ?\n\nIt is not really *the* counterpart of write_author_script(). There are\nmany conceivable counterparts, one of them already exists and is called\nread_env_script(). They serve different purposes, even if both read the\nauthor-script file, and they parse things in a fundamentally different\nway. I had already pointed out something along those lines in the review\nof the patch introducing the read_env_script() and Junio did not believe\nme. To make sure, it was my fault that I failed to make a compelling\nenough argument. I am glad that I now have proof that I was right: just\nbecause you read the same file, for a similar purpose, does not\nnecessarily imply that you can share code for those purposes. All you can\nshare is the name of the input file.\n\nHaving said that, *this* time round, what we need to do is actually very\nsimilar to what builtin/am.c's read_author_script() does (even if we\ncannot use it as-is: it populates part of a `struct am_state`). I'll have\nto look into this more closely.\n\n> > +{\n> > + char *p, *p2;\n> > +\n> > + if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)\n> \n> The 256 is a hint for read_file how to size the buffer initially.\n> If not given it defaults to 8k, which presumably is too much for\n> an author identity.\n\nThat is a correct reading of the code's intent.\n\n> > + for (p = buf->buf; *p; p++)\n> > + if (skip_prefix(p, \"'\\\\\\\\''\", (const char **)&p2))\n> > + strbuf_splice(buf, p - buf->buf, p2 - p, \"'\", 1);\n> > + else if (*p == '\\'')\n> > + strbuf_splice(buf, p-- - buf->buf, 1, \"\", 0);\n> \n> This part could be prefixed with\n> /* un-escape text: turn \\\\ into ' and remove single quotes. */\n\nIf could be prefixed that way, but it would be incorrect. We never turn \\\\\ninto '. What we do here (and I do not want to repeat in a comment what the\ncode does): we dequote what we previously quoted using single quotes. So\nwe use the fact that we know that the value is of the form 'abc', or if it\ncontains single quotes: 'this has '\\''single'\\'' quotes'.\n\n> \n> > + if (skip_prefix(buf->buf, \"GIT_AUTHOR_NAME=\", (const char **)&p)) {\n> > + strbuf_splice(buf, 0, p - buf->buf, \"\", 0);\n> > + p = strchr(buf->buf, '\\n');\n> > + if (skip_prefix(p, \"\\nGIT_AUTHOR_EMAIL=\", (const char **)&p2)) {\n> > + strbuf_splice(buf, p - buf->buf, p2 - p, \" <\", 2);\n> > + p = strchr(p, '\\n');\n> > + if (skip_prefix(p, \"\\nGIT_AUTHOR_DATE=@\",\n> > + (const char **)&p2)) {\n> > + strbuf_splice(buf, p - buf->buf, p2 - p,\n> > + \"> \", 2);\n> > + p = strchr(p, '\\n');\n> > + if (p) {\n> > + strbuf_setlen(buf, p - buf->buf);\n> > + return buf->buf;\n> \n> So here we have read GIT_AUTHOR_NAME, _EMAIL\n> and _DATE in that order and converted it to its form\n> \"name date\" in a single line.\n> \n> It would be better to invert the conditions and keep\n> the indentation level lower by:\n> \n> if (!skip_prefix(...))\n> goto warning_and_return;\n> strbuf_splice(...);\n> ...\n> \n> I wondered if we want to factor out the conversion of\n> \"author string in commit form\" to \"author information\n> in script form\" into their own functions, and keep the reading\n> writing out of them. But then again we only need them in\n> these use cases for now, and such a refactoring can happen\n> later if needed.\n\nRight, but there *is* an opportunity now to share code with builtin/am.c.\n\n> > + warning(_(\"could not parse '%s'\"), rebase_path_author_script());\n> \n> This function needs all three environment variables in its correct order,\n> which sounds a little brittle, but then again we do not expect manual\n> editing of that file, but expect it to be written by Git.\n\nIt would not hurt to make it less brittle, I agree. Maybe I can do that\neasily. If not, in my opinion it is not a big deal to expect that order,\nfor the reason you stated: we expect to have written the file ourselves.\n\n> > @@ -1369,6 +1448,12 @@ static int is_fixup(enum todo_command command)\n> > return command == TODO_FIXUP || command == TODO_SQUASH;\n> > }\n> >\n> > +/* Does this command create a (non-merge) commit? */\n> > +static int is_pick_or_similar(enum todo_command command)\n> > +{\n> > + return command <= TODO_SQUASH;\n> > +}\n> \n> This code looks scary.\n> Fortunately the enum todo_command hints that the order matters,\n> such that we're probably protected from wild reordering in the future,\n> however this implies that the section /* commands that handle commits */\n> comes first and that TODO_SQUASH is the last entry of that section.\n> So maybe we'd want to add a /* must be last in section */ to\n> TODO_squash and also document that the section must be first?\n> \n> Do we have other code that needs a very specific ordering\n> with similar further assumptions (section being first/last, a\n> command being first/last in their section)?\n\nThe code already does things like that, by testing `command <\nTODO_COMMENT`.\n\nBut I guess your concerns would go away if I made this a big honking\nswitch() statement that lists *explicitly* what should be considered \"pick\nor similar\"?\n\n> I wondered what the _or_similar means and by looking up\n> that enum, I would think a name like\n> \n> static int handles_single_commit(enum todo_command)\n> \n> might be better?\n\nNo, that would be incorrect. TODO_MERGE also *potentially* handles a\nsingle commit (and one or more labels).\n\nI really mean \"is it a pick or similar\", i.e. does it cherry-pick a\nsingle, non-merge commit, possibly doing funny stuff such as `reword` on\ntop of it.\n\n> > @@ -1523,7 +1608,14 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n> > return error(_(\"your index file is unmerged.\"));\n> > } else {\n> > unborn = get_oid(\"HEAD\", &head);\n> > - if (unborn)\n> > + /* Do we want to generate a root commit? */\n> > + if (is_pick_or_similar(command) && opts->have_squash_onto &&\n> > + !oidcmp(&head, &opts->squash_onto)) {\n> > + if (is_fixup(command))\n> > + return error(_(\"cannot fixup root commit\"));\n> \n> I would expect you also cannot squash into root commit?\n\nIn sequencer.c, `is_fixup()` really means \"is it a fixup or a squash?\". So\nyes, you are correct that we also cannot squash into a root commit.\n\nHowever, a squash is the same as a fixup with the only difference being\nthat the squash lets you edit the final commit message (and does not\ncomment out the squash commit's message, in contrast to fixup).\n\nIs it really worth adding an ugly line break in the middle of the error()\nstatement just to say \"cannot fixup/squash into root commit\"? I'd rather\nnot.\n\nCiao,\nDscho\n"},{"id":"346073","messageId":"nycvar.QRO.7.76.6.1804291433520.79@tvgsbejvaqbjf.bet","threadId":"48372","inReplyTo":"CAGZ79kYi5W+m9TQ_iEwPmEaXQ_tvpRjcycGioY5jThtfGM6mYw@mail.gmail.com","subject":"Re: [PATCH 3/6] rebase -i --root: let the sequencer handle even the initial part","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-29T12:34:11Z","receivedAt":"2018-04-29T12:34:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Sat, 28 Apr 2018, Stefan Beller wrote:\n\n> On Fri, Apr 27, 2018 at 3:31 PM, Johannes Schindelin\n> wrote:\n> > In this developer's earlier attempt to accelerate interactive rebases by\n> > converting large parts from Unix shell script into portable, performant\n> > C, the --root handling was specifically excluded (to simplify the task a\n> > little bit; it still took over a year to get that reduced set of patches\n> > into Git proper).\n> >\n> > This patch ties up that loose end: now only --preserve-merges uses the\n> > slow Unix shell script implementation to perform the interactive rebase.\n> >\n> > As the rebase--helper reports progress to stderr (unlike the scripted\n> > interactive rebase, which reports it to stdout, of all places), we have\n> > to adjust a couple of tests that did not expect that for `git rebase -i\n> > --root`.\n> >\n> > This patch fixes -- at long last! -- the really old bug reported in\n> > 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with\n> > --root *always* rewrote the root commit, even if there were no changes.\n> >\n> > The bug still persists in --preserve-merges mode, of course, but that\n> > mode will be deprecated as soon as the new --rebase-merges mode\n> > stabilizes, anyway.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > git-rebase--interactive.sh | 4 +++-\n> > t/t3404-rebase-interactive.sh | 19 +++++++++++++------\n> > t/t3421-rebase-topology-linear.sh | 6 +++---\n> > 3 files changed, 19 insertions(+), 10 deletions(-)\n> >\n> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> > index cbf44f86482..2f4941d0fc9 100644\n> > --- a/git-rebase--interactive.sh\n> > +++ b/git-rebase--interactive.sh\n> > @@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () {\n> > else\n> > revisions=$onto...$orig_head\n> > shortrevisions=$shorthead\n> > + test -z \"$squash_onto\" ||\n> > + echo \"$squash_onto\" >\"$state_dir\"/squash-onto\n> > fi\n> > }\n> >\n> > @@ -948,7 +950,7 @@ EOF\n> > die \"Could not skip unnecessary pick commands\"\n> >\n> > checkout_onto\n> > - if test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n> > + if test ! -d \"$rewritten\"\n> \n> I have the impression this is the line that is really well\n> explained in the commit message (\"migrate to rebase\n> helper even when there is $rebase_root set\")\n> \n> The rest of the patch is covered as \"a couple of places\n> where we adjust stdout to stderr\"?\n\nCorrect.\n\nThanks for reviewing!\nDscho\n"},{"id":"346104","messageId":"CAGZ79kbfOd9r9JGE4qP_=CVnYDbn2A2aY2i+UrttC_0x2xOFuQ@mail.gmail.com","threadId":"48372","inReplyTo":"nycvar.QRO.7.76.6.1804291255010.79@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/6] sequencer: learn about the special \"fake root commit\" handling","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-29T21:44:56Z","receivedAt":"2018-04-29T21:45:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Johannes,\n\nthanks for taking your time to explain things. It shows I am not\nfamiliar with the rebase code, yet.\n\n>\n> Having said that, *this* time round, what we need to do is actually very\n> similar to what builtin/am.c's read_author_script() does (even if we\n> cannot use it as-is: it populates part of a `struct am_state`). I'll have\n> to look into this more closely.\n\nHeh, so my rambling was worth it. Thanks for looking into that.\n\n\n>> This part could be prefixed with\n>> /* un-escape text: turn \\\\ into ' and remove single quotes. */\n>\n> If could be prefixed that way, but it would be incorrect. We never turn \\\\\n> into '. What we do here (and I do not want to repeat in a comment what the\n> code does): we dequote what we previously quoted using single quotes. So\n> we use the fact that we know that the value is of the form 'abc', or if it\n> contains single quotes: 'this has '\\''single'\\'' quotes'.\n\nIs there a helper 'dequote' somewhere?\n\n>> > +/* Does this command create a (non-merge) commit? */\n>> > +static int is_pick_or_similar(enum todo_command command)\n>> > +{\n>> > + return command <= TODO_SQUASH;\n>> > +}\n>>\n>> This code looks scary.\n[...]\n\n> The code already does things like that, by testing `command <\n> TODO_COMMENT`.\n\nok great. So that is a widely used pattern for enum todo_command,\nsuch that rearranging their order would break a lot. (Hence people will\nfind out quickly when doing so).\n\n> But I guess your concerns would go away if I made this a big honking\n> switch() statement that lists *explicitly* what should be considered \"pick\n> or similar\"?\n\nI did not spell that out as producing lots of lines of code is not the\nprimary goal, but understandability is.\n\nAnd it looked like it would just pick the first section, so I thought about\nsome different approaches, either by making the enum todo_command\na lot more complex than an enum (an array of structs?) or adding\na new parallel array, that contains additional information for each\nvalue at the given index, e.g.\n\nstatic int is_pick_or_similar(enum todo_command command)\n{\n return todo_command_flags[value] & TODO_CMDS_IS_PICKING;\n}\n\nbut all approaches I had were more complicated, such that the additional\nverbosity would not be enough of a trade off.\n\nI think this function was only scary as I was not familiar with the rebase\ncode as that employs similar patterns already.\n\n>> > + if (is_fixup(command))\n>> > + return error(_(\"cannot fixup root commit\"));\n>>\n>> I would expect you also cannot squash into root commit?\n>\n> In sequencer.c, `is_fixup()` really means \"is it a fixup or a squash?\". So\n> yes, you are correct that we also cannot squash into a root commit.\n>\n> However, a squash is the same as a fixup with the only difference being\n> that the squash lets you edit the final commit message (and does not\n> comment out the squash commit's message, in contrast to fixup).\n>\n> Is it really worth adding an ugly line break in the middle of the error()\n> statement just to say \"cannot fixup/squash into root commit\"? I'd rather\n> not.\n\nMakes sense,\n\nThanks for your patience,\nStefan\n"},{"id":"346594","messageId":"cover.1525388472.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1524868165.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/6] Let the sequencer handle `git rebase -i --root`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:13Z","receivedAt":"2018-05-03T23:01:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When I reimplemented the most performance-critical bits of the\ninteractive rebase in the sequencer, to speed up `git rebase -i`\nparticularly on Windows (even if the benefits are still quite notable on\nLinux or macOS), I punted on the --root part.\n\nI had always hoped that some other contributor (or I myself) would come\nback later to address the --root part in the sequencer, too, with the\nidea that this would move the last remaining complicated code from\ngit-rebase--interactive.sh into sequencer.c, to facilitate converting\nthe rest of git-rebase--interactive.sh.\n\nWhen I say \"the last remaining complicated code\", of course I neglect\nthe --preserve-merges code, but as I worked hard on the --rebase-merges\npatch series with the intention to eventually deprecate and maybe even\nremove the --preserve-merges mode, I always implicitly assume that the\n--preserve-merges code will be moved into its own shell script\n(git-rebase--preserve-merges.sh, maybe?) and never be converted.\n\nSo here goes: the patches to move the handling of --root into the\nsequencer. After two preparatory patches, the real conversion takes\nplace in the third patch. After that, we take care of the --root related\nconcerns that arise in conjunction with the --rebase-merges mode.\n\nAs the --rebase-merges/--root patches overlap quite a bit (not so much\nin the code itself as in philosophical considerations such as \"what\nshould happen if you try to merge a branch into a new root\", or the\nfact that the label/reset/merge commands make it desirable to be able to\ncreate a new root commit in the middle of a todo list), I had to\nconsider in which order to contribute them. In the end, I decided to go\nwith --rebase-merges first, so the --root patches are based on the\n--rebase-merges patch series.\n\nI consider this patch series a critical prerequisite for Alban's Google\nSummer of Code project to convert rebase -i into a builtin.\n\nChanges since v1:\n\n- Expanded is_pick_or_similar() to use a clear and verbose switch()\n statement, replacing the magic \"command <= TODO_SQUASH\".\n\n- Completely revamped read_author_ident(), using sq_dequote(); Sadly,\n trying to reuse builtin/am.c's read_author_script() would result in\n substantialy more lines of code, and I also failed to find an easy way\n to allow for arbitrary order of the environment variables in\n author-script.\n\n\nJohannes Schindelin (6):\n sequencer: extract helper to update active_cache_tree\n sequencer: learn about the special \"fake root commit\" handling\n rebase -i --root: let the sequencer handle even the initial part\n sequencer: allow introducing new root commits\n rebase --rebase-merges: a \"merge\" into a new root is a fast-forward\n rebase --rebase-merges: root commits can be cousins, too\n\n git-rebase--interactive.sh | 4 +-\n sequencer.c | 206 ++++++++++++++++++++++++++----\n sequencer.h | 4 +\n t/t3404-rebase-interactive.sh | 19 ++-\n t/t3421-rebase-topology-linear.sh | 6 +-\n t/t3430-rebase-merges.sh | 72 +++++++++++\n 6 files changed, 276 insertions(+), 35 deletions(-)\n\n\nbase-commit: 673fb9cb8b5c7d57cb560b6ade45e419c8dd09fc\nBased-On: recreate-merges at https://github.com/dscho/git\nFetch-Base-Via: git fetch https://github.com/dscho/git recreate-merges\nPublished-As: https://github.com/dscho/git/releases/tag/sequencer-and-root-commits-v2\nFetch-It-Via: git fetch https://github.com/dscho/git sequencer-and-root-commits-v2\n\nBranch-diff vs v1:\n 1: 42db734a980 ! 1: 73398da7119 sequencer: learn about the special \"fake root commit\" handling\n @@ -54,40 +54,50 @@\n \treturn NULL;\n }\n \n ++/* Read author-script and return an ident line (author timestamp) */\n +static const char *read_author_ident(struct strbuf *buf)\n +{\n -+\tchar *p, *p2;\n ++\tconst char *keys[] = {\n ++\t\t\"GIT_AUTHOR_NAME=\", \"GIT_AUTHOR_EMAIL=\", \"GIT_AUTHOR_DATE=\"\n ++\t};\n ++\tchar *in, *out, *eol;\n ++\tint i = 0, len;\n +\n +\tif (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)\n +\t\treturn NULL;\n +\n -+\tfor (p = buf->buf; *p; p++)\n -+\t\tif (skip_prefix(p, \"'\\\\\\\\''\", (const char **)&p2))\n -+\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p, \"'\", 1);\n -+\t\telse if (*p == '\\'')\n -+\t\t\tstrbuf_splice(buf, p-- - buf->buf, 1, \"\", 0);\n -+\n -+\tif (skip_prefix(buf->buf, \"GIT_AUTHOR_NAME=\", (const char **)&p)) {\n -+\t\tstrbuf_splice(buf, 0, p - buf->buf, \"\", 0);\n -+\t\tp = strchr(buf->buf, '\\n');\n -+\t\tif (skip_prefix(p, \"\\nGIT_AUTHOR_EMAIL=\", (const char **)&p2)) {\n -+\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p, \" <\", 2);\n -+\t\t\tp = strchr(p, '\\n');\n -+\t\t\tif (skip_prefix(p, \"\\nGIT_AUTHOR_DATE=@\",\n -+\t\t\t\t\t(const char **)&p2)) {\n -+\t\t\t\tstrbuf_splice(buf, p - buf->buf, p2 - p,\n -+\t\t\t\t\t \"> \", 2);\n -+\t\t\t\tp = strchr(p, '\\n');\n -+\t\t\t\tif (p) {\n -+\t\t\t\t\tstrbuf_setlen(buf, p - buf->buf);\n -+\t\t\t\t\treturn buf->buf;\n -+\t\t\t\t}\n -+\t\t\t}\n ++\t/* dequote values and construct ident line in-place */\n ++\tfor (in = out = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {\n ++\t\tif (!skip_prefix(in, keys[i], (const char **)&in)) {\n ++\t\t\twarning(\"could not parse '%s' (looking for '%s'\",\n ++\t\t\t\trebase_path_author_script(), keys[i]);\n ++\t\t\treturn NULL;\n +\t\t}\n -+\t}\n -+\n -+\twarning(_(\"could not parse '%s'\"), rebase_path_author_script());\n -+\treturn NULL;\n ++\n ++\t\teol = strchrnul(in, '\\n');\n ++\t\t*eol = '\\0';\n ++\t\tsq_dequote(in);\n ++\t\tlen = strlen(in);\n ++\n ++\t\tif (i > 0) /* separate values by spaces */\n ++\t\t\t*(out++) = ' ';\n ++\t\tif (i == 1) /* email needs to be surrounded by <...> */\n ++\t\t\t*(out++) = '<';\n ++\t\tmemmove(out, in, len);\n ++\t\tout += len;\n ++\t\tif (i == 1) /* email needs to be surrounded by <...> */\n ++\t\t\t*(out++) = '>';\n ++\t\tin = eol + 1;\n ++\t}\n ++\n ++\tif (i < 3) {\n ++\t\twarning(\"could not parse '%s' (looking for '%s')\",\n ++\t\t\trebase_path_author_script(), keys[i]);\n ++\t\treturn NULL;\n ++\t}\n ++\n ++\tbuf->len = out - buf->buf;\n ++\treturn buf->buf;\n +}\n +\n static const char staged_changes_advice[] =\n @@ -159,7 +169,17 @@\n +/* Does this command create a (non-merge) commit? */\n +static int is_pick_or_similar(enum todo_command command)\n +{\n -+\treturn command <= TODO_SQUASH;\n ++\tswitch (command) {\n ++\tcase TODO_PICK:\n ++\tcase TODO_REVERT:\n ++\tcase TODO_EDIT:\n ++\tcase TODO_REWORD:\n ++\tcase TODO_FIXUP:\n ++\tcase TODO_SQUASH:\n ++\t\treturn 1;\n ++\tdefault:\n ++\t\treturn 0;\n ++\t}\n +}\n +\n static int update_squash_messages(enum todo_command command,\n 2: 1c8740eaa91 = 2: 2dfe8315993 rebase -i --root: let the sequencer handle even the initial part\n 3: 8a7f5751412 = 3: 0ee765bbb36 sequencer: allow introducing new root commits\n 4: 0b7379b576b = 4: 29d7a9265e3 rebase --rebase-merges: a \"merge\" into a new root is a fast-forward\n 5: 270b8fdf477 = 5: f7ee1b3de12 rebase --rebase-merges: root commits can be cousins, too\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n"},{"id":"346595","messageId":"ad31aab8e735f7de813860cacb32abbc406f43d1.1525388472.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/6] sequencer: extract helper to update active_cache_tree","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:15Z","receivedAt":"2018-05-03T23:01:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This patch extracts the code from is_index_unchanged() to initialize or\nupdate the index' cache tree (i.e. a tree object reflecting the current\nindex' top-level tree).\n\nThe new helper will be used in the upcoming code to support `git rebase\n-i --root` via the sequencer.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 27 ++++++++++++++++++---------\n 1 file changed, 18 insertions(+), 9 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e2f83942843..90c8218aa9a 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -562,9 +562,23 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \treturn !clean;\n }\n \n+static struct object_id *get_cache_tree_oid(void)\n+{\n+\tif (!active_cache_tree)\n+\t\tactive_cache_tree = cache_tree();\n+\n+\tif (!cache_tree_fully_valid(active_cache_tree))\n+\t\tif (cache_tree_update(&the_index, 0)) {\n+\t\t\terror(_(\"unable to update cache tree\"));\n+\t\t\treturn NULL;\n+\t\t}\n+\n+\treturn &active_cache_tree->oid;\n+}\n+\n static int is_index_unchanged(void)\n {\n-\tstruct object_id head_oid;\n+\tstruct object_id head_oid, *cache_tree_oid;\n \tstruct commit *head_commit;\n \n \tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, &head_oid, NULL))\n@@ -583,15 +597,10 @@ static int is_index_unchanged(void)\n \tif (parse_commit(head_commit))\n \t\treturn -1;\n \n-\tif (!active_cache_tree)\n-\t\tactive_cache_tree = cache_tree();\n-\n-\tif (!cache_tree_fully_valid(active_cache_tree))\n-\t\tif (cache_tree_update(&the_index, 0))\n-\t\t\treturn error(_(\"unable to update cache tree\"));\n+\tif (!(cache_tree_oid = get_cache_tree_oid()))\n+\t\treturn -1;\n \n-\treturn !oidcmp(&active_cache_tree->oid,\n-\t\t &head_commit->tree->object.oid);\n+\treturn !oidcmp(cache_tree_oid, &head_commit->tree->object.oid);\n }\n \n static int write_author_script(const char *message)\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n\n"},{"id":"346596","messageId":"2dfe8315993abb882c8b63ff54076218d4a911d3.1525388472.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 3/6] rebase -i --root: let the sequencer handle even the initial part","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:18Z","receivedAt":"2018-05-03T23:01:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In this developer's earlier attempt to accelerate interactive rebases by\nconverting large parts from Unix shell script into portable, performant\nC, the --root handling was specifically excluded (to simplify the task a\nlittle bit; it still took over a year to get that reduced set of patches\ninto Git proper).\n\nThis patch ties up that loose end: now only --preserve-merges uses the\nslow Unix shell script implementation to perform the interactive rebase.\n\nAs the rebase--helper reports progress to stderr (unlike the scripted\ninteractive rebase, which reports it to stdout, of all places), we have\nto adjust a couple of tests that did not expect that for `git rebase -i\n--root`.\n\nThis patch fixes -- at long last! -- the really old bug reported in\n6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with\n--root *always* rewrote the root commit, even if there were no changes.\n\nThe bug still persists in --preserve-merges mode, of course, but that\nmode will be deprecated as soon as the new --rebase-merges mode\nstabilizes, anyway.\n\nSigned-off-by: Johannes Schindelin \n---\n git-rebase--interactive.sh | 4 +++-\n t/t3404-rebase-interactive.sh | 19 +++++++++++++------\n t/t3421-rebase-topology-linear.sh | 6 +++---\n 3 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex cbf44f86482..2f4941d0fc9 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -894,6 +894,8 @@ init_revisions_and_shortrevisions () {\n \telse\n \t\trevisions=$onto...$orig_head\n \t\tshortrevisions=$shorthead\n+\t\ttest -z \"$squash_onto\" ||\n+\t\techo \"$squash_onto\" >\"$state_dir\"/squash-onto\n \tfi\n }\n \n@@ -948,7 +950,7 @@ EOF\n \tdie \"Could not skip unnecessary pick commands\"\n \n \tcheckout_onto\n-\tif test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n+\tif test ! -d \"$rewritten\"\n \tthen\n \t\trequire_clean_work_tree \"rebase\"\n \t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 59c766540e5..c65826ddace 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1204,10 +1204,6 @@ test_expect_success 'drop' '\n \ttest A = $(git cat-file commit HEAD^^ | sed -ne \\$p)\n '\n \n-cat >expect <actual &&\n \ttest D = $(git cat-file commit HEAD | sed -ne \\$p) &&\n-\ttest_i18ncmp expect actual\n+\ttest_i18ngrep \\\n+\t\t\"Successfully rebased and updated refs/heads/missing-commit\" \\\n+\t\tactual\n '\n \n cat >expect <actual &&\n+\t\tgit rebase -i --root 2>actual.2 &&\n+\tcr_to_nl actual &&\n \ttest_i18ncmp expect actual &&\n \ttest D = $(git cat-file commit HEAD | sed -ne \\$p)\n '\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex e7438ad06ac..99b2aac9219 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -328,9 +328,9 @@ test_run_rebase () {\n \t\ttest_cmp_rev c HEAD\n \t\"\n }\n-test_run_rebase failure ''\n-test_run_rebase failure -m\n-test_run_rebase failure -i\n+test_run_rebase success ''\n+test_run_rebase success -m\n+test_run_rebase success -i\n test_run_rebase failure -p\n \n test_run_rebase () {\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n\n"},{"id":"346597","messageId":"73398da7119311c10fedfcfcbce7cc50203377ac.1525388472.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 2/6] sequencer: learn about the special \"fake root commit\" handling","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:17Z","receivedAt":"2018-05-03T23:01:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When an interactive rebase wants to recreate a root commit, it\n- first creates a new, empty root commit,\n- checks it out,\n- converts the next `pick` command so that it amends the empty root\n commit\n\nIntroduce support in the sequencer to handle such an empty root commit,\nby looking for the file /rebase-merge/squash-onto; if it exists\nand contains a commit name, the sequencer will compare the HEAD to said\nroot commit, and if identical, a new root commit will be created.\n\nWhile converting scripted code into proper, portable C, we also do away\nwith the old \"amend with an empty commit message, then cherry-pick\nwithout committing, then amend again\" dance and replace it with code\nthat uses the internal API properly to do exactly what we want: create a\nnew root commit.\n\nTo keep the implementation simple, we always spawn `git commit` to create\nnew root commits.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n sequencer.h | 4 ++\n 2 files changed, 125 insertions(+), 3 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 90c8218aa9a..8eddda681d1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -125,6 +125,12 @@ static GIT_PATH_FUNC(rebase_path_rewritten_list, \"rebase-merge/rewritten-list\")\n static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n \t\"rebase-merge/rewritten-pending\")\n \n+/*\n+ * The path of the file containig the OID of the \"squash onto\" commit, i.e.\n+ * the dummy commit used for `reset [new root]`.\n+ */\n+static GIT_PATH_FUNC(rebase_path_squash_onto, \"rebase-merge/squash-onto\")\n+\n /*\n * The path of the file listing refs that need to be deleted after the rebase\n * finishes. This is used by the `label` command to record the need for cleanup.\n@@ -470,7 +476,8 @@ static int fast_forward_to(const struct object_id *to, const struct object_id *f\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_update(transaction, \"HEAD\",\n-\t\t\t\t to, unborn ? &null_oid : from,\n+\t\t\t\t to, unborn && !is_rebase_i(opts) ?\n+\t\t\t\t &null_oid : from,\n \t\t\t\t 0, sb.buf, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_free(transaction);\n@@ -692,6 +699,52 @@ static char *get_author(const char *message)\n \treturn NULL;\n }\n \n+/* Read author-script and return an ident line (author timestamp) */\n+static const char *read_author_ident(struct strbuf *buf)\n+{\n+\tconst char *keys[] = {\n+\t\t\"GIT_AUTHOR_NAME=\", \"GIT_AUTHOR_EMAIL=\", \"GIT_AUTHOR_DATE=\"\n+\t};\n+\tchar *in, *out, *eol;\n+\tint i = 0, len;\n+\n+\tif (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)\n+\t\treturn NULL;\n+\n+\t/* dequote values and construct ident line in-place */\n+\tfor (in = out = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {\n+\t\tif (!skip_prefix(in, keys[i], (const char **)&in)) {\n+\t\t\twarning(\"could not parse '%s' (looking for '%s'\",\n+\t\t\t\trebase_path_author_script(), keys[i]);\n+\t\t\treturn NULL;\n+\t\t}\n+\n+\t\teol = strchrnul(in, '\\n');\n+\t\t*eol = '\\0';\n+\t\tsq_dequote(in);\n+\t\tlen = strlen(in);\n+\n+\t\tif (i > 0) /* separate values by spaces */\n+\t\t\t*(out++) = ' ';\n+\t\tif (i == 1) /* email needs to be surrounded by <...> */\n+\t\t\t*(out++) = '<';\n+\t\tmemmove(out, in, len);\n+\t\tout += len;\n+\t\tif (i == 1) /* email needs to be surrounded by <...> */\n+\t\t\t*(out++) = '>';\n+\t\tin = eol + 1;\n+\t}\n+\n+\tif (i < 3) {\n+\t\twarning(\"could not parse '%s' (looking for '%s')\",\n+\t\t\trebase_path_author_script(), keys[i]);\n+\t\treturn NULL;\n+\t}\n+\n+\tbuf->len = out - buf->buf;\n+\treturn buf->buf;\n+}\n+\n static const char staged_changes_advice[] =\n N_(\"you have staged changes in your working tree\\n\"\n \"If these changes are meant to be squashed into the previous commit, run:\\n\"\n@@ -711,6 +764,7 @@ N_(\"you have staged changes in your working tree\\n\"\n #define AMEND_MSG (1<<2)\n #define CLEANUP_MSG (1<<3)\n #define VERIFY_MSG (1<<4)\n+#define CREATE_ROOT_COMMIT (1<<5)\n \n /*\n * If we are cherry-pick, and if the merge did not result in\n@@ -730,6 +784,40 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tconst char *value;\n \n+\tif (flags & CREATE_ROOT_COMMIT) {\n+\t\tstruct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;\n+\t\tconst char *author = is_rebase_i(opts) ?\n+\t\t\tread_author_ident(&script) : NULL;\n+\t\tstruct object_id root_commit, *cache_tree_oid;\n+\t\tint res = 0;\n+\n+\t\tif (!defmsg)\n+\t\t\tBUG(\"root commit without message\");\n+\n+\t\tif (!(cache_tree_oid = get_cache_tree_oid()))\n+\t\t\tres = -1;\n+\n+\t\tif (!res)\n+\t\t\tres = strbuf_read_file(&msg, defmsg, 0);\n+\n+\t\tif (res <= 0)\n+\t\t\tres = error_errno(_(\"could not read '%s'\"), defmsg);\n+\t\telse\n+\t\t\tres = commit_tree(msg.buf, msg.len, cache_tree_oid,\n+\t\t\t\t\t NULL, &root_commit, author,\n+\t\t\t\t\t opts->gpg_sign);\n+\n+\t\tstrbuf_release(&msg);\n+\t\tstrbuf_release(&script);\n+\t\tif (!res) {\n+\t\t\tupdate_ref(NULL, \"CHERRY_PICK_HEAD\", &root_commit, NULL,\n+\t\t\t\t REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR);\n+\t\t\tres = update_ref(NULL, \"HEAD\", &root_commit, NULL, 0,\n+\t\t\t\t\t UPDATE_REFS_MSG_ON_ERR);\n+\t\t}\n+\t\treturn res < 0 ? error(_(\"writing root commit\")) : 0;\n+\t}\n+\n \tcmd.git_cmd = 1;\n \n \tif (is_rebase_i(opts)) {\n@@ -1216,7 +1304,8 @@ static int do_commit(const char *msg_file, const char *author,\n {\n \tint res = 1;\n \n-\tif (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG)) {\n+\tif (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG) &&\n+\t !(flags & CREATE_ROOT_COMMIT)) {\n \t\tstruct object_id oid;\n \t\tstruct strbuf sb = STRBUF_INIT;\n \n@@ -1369,6 +1458,22 @@ static int is_fixup(enum todo_command command)\n \treturn command == TODO_FIXUP || command == TODO_SQUASH;\n }\n \n+/* Does this command create a (non-merge) commit? */\n+static int is_pick_or_similar(enum todo_command command)\n+{\n+\tswitch (command) {\n+\tcase TODO_PICK:\n+\tcase TODO_REVERT:\n+\tcase TODO_EDIT:\n+\tcase TODO_REWORD:\n+\tcase TODO_FIXUP:\n+\tcase TODO_SQUASH:\n+\t\treturn 1;\n+\tdefault:\n+\t\treturn 0;\n+\t}\n+}\n+\n static int update_squash_messages(enum todo_command command,\n \t\tstruct commit *commit, struct replay_opts *opts)\n {\n@@ -1523,7 +1628,14 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t\t\treturn error(_(\"your index file is unmerged.\"));\n \t} else {\n \t\tunborn = get_oid(\"HEAD\", &head);\n-\t\tif (unborn)\n+\t\t/* Do we want to generate a root commit? */\n+\t\tif (is_pick_or_similar(command) && opts->have_squash_onto &&\n+\t\t !oidcmp(&head, &opts->squash_onto)) {\n+\t\t\tif (is_fixup(command))\n+\t\t\t\treturn error(_(\"cannot fixup root commit\"));\n+\t\t\tflags |= CREATE_ROOT_COMMIT;\n+\t\t\tunborn = 1;\n+\t\t} else if (unborn)\n \t\t\toidcpy(&head, the_hash_algo->empty_tree);\n \t\tif (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : \"HEAD\",\n \t\t\t\t NULL, 0))\n@@ -2136,6 +2248,12 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_release(&buf);\n \n+\t\tif (read_oneliner(&buf, rebase_path_squash_onto(), 0)) {\n+\t\t\tif (get_oid_hex(buf.buf, &opts->squash_onto) < 0)\n+\t\t\t\treturn error(_(\"unusable squash-onto\"));\n+\t\t\topts->have_squash_onto = 1;\n+\t\t}\n+\n \t\treturn 0;\n \t}\n \ndiff --git a/sequencer.h b/sequencer.h\nindex d9570d92b11..4b2717881fa 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -44,6 +44,10 @@ struct replay_opts {\n \tchar **xopts;\n \tsize_t xopts_nr, xopts_alloc;\n \n+\t/* placeholder commit for -i --root */\n+\tstruct object_id squash_onto;\n+\tint have_squash_onto;\n+\n \t/* Only used by REPLAY_NONE */\n \tstruct rev_info *revs;\n };\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n\n"},{"id":"346598","messageId":"29d7a9265e30fb9b27b87aba1d6cd5efff93a596.1525388473.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 5/6] rebase --rebase-merges: a \"merge\" into a new root is a fast-forward","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:28Z","receivedAt":"2018-05-03T23:01:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When a user provides a todo list containing something like\n\n\treset [new root]\n\tmerge my-branch\n\nlet's do the same as if pulling into an orphan branch: simply\nfast-forward.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 12 ++++++++++++\n t/t3430-rebase-merges.sh | 13 +++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex a7832399b1f..65a8c493781 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2870,6 +2870,18 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\tgoto leave_merge;\n \t}\n \n+\tif (opts->have_squash_onto &&\n+\t !oidcmp(&head_commit->object.oid, &opts->squash_onto)) {\n+\t\t/*\n+\t\t * When the user tells us to \"merge\" something into a\n+\t\t * \"[new root]\", let's simply fast-forward to the merge head.\n+\t\t */\n+\t\trollback_lock_file(&lock);\n+\t\tret = fast_forward_to(&merge_commit->object.oid,\n+\t\t\t\t &head_commit->object.oid, 0, opts);\n+\t\tgoto leave_merge;\n+\t}\n+\n \tif (commit) {\n \t\tconst char *message = get_commit_buffer(commit, NULL);\n \t\tconst char *body;\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 35260862fcb..5543f1d5a34 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -275,4 +275,17 @@ test_expect_success 'root commits' '\n \ttest_cmp_rev HEAD $before\n '\n \n+test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n+\thead=$(git rev-parse HEAD) &&\n+\tcat >script-from-scratch <<-EOF &&\n+\treset [new root]\n+\tmerge $head\n+\tEOF\n+\ttest_config sequence.editor \\\"\"$PWD\"/replace-editor.sh\\\" &&\n+\ttest_tick &&\n+\tgit rebase -i -r HEAD^ &&\n+\ttest_cmp_rev HEAD $head\n+'\n+\n+\n test_done\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n\n"},{"id":"346599","messageId":"f7ee1b3de12e2163c6b370eac5fbbc62c718b973.1525388473.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 6/6] rebase --rebase-merges: root commits can be cousins, too","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:29Z","receivedAt":"2018-05-03T23:01:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Reported by Wink Saville: when rebasing with no-rebase-cousins, we\nwill want to refrain from rebasing all of them, even when they are\nroot commits.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 3 ++-\n t/t3430-rebase-merges.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 65a8c493781..01e561bc20e 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3903,7 +3903,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t}\n \n \t\tif (!commit)\n-\t\t\tfprintf(out, \"%s onto\\n\", cmd_reset);\n+\t\t\tfprintf(out, \"%s %s\\n\", cmd_reset,\n+\t\t\t\trebase_cousins ? \"onto\" : \"[new root]\");\n \t\telse {\n \t\t\tconst char *to = NULL;\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 5543f1d5a34..ce6de6f491e 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -287,5 +287,30 @@ test_expect_success 'a \"merge\" into a root commit is a fast-forward' '\n \ttest_cmp_rev HEAD $head\n '\n \n+test_expect_success 'A root commit can be a cousin, treat it that way' '\n+\tgit checkout --orphan khnum &&\n+\ttest_commit yama &&\n+\tgit checkout -b asherah master &&\n+\ttest_commit shamkat &&\n+\tgit merge --allow-unrelated-histories khnum &&\n+\ttest_tick &&\n+\tgit rebase -f -r HEAD^ &&\n+\t! test_cmp_rev HEAD^2 khnum &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF &&\n+\t* Merge branch '\\''khnum'\\'' into asherah\n+\t|\\\n+\t| * yama\n+\to shamkat\n+\tEOF\n+\ttest_tick &&\n+\tgit rebase --rebase-merges=rebase-cousins HEAD^ &&\n+\ttest_cmp_graph HEAD^.. <<-\\EOF\n+\t* Merge branch '\\''khnum'\\'' into asherah\n+\t|\\\n+\t| * yama\n+\t|/\n+\to shamkat\n+\tEOF\n+'\n \n test_done\n-- \n2.17.0.windows.1.38.g05ca542f78d\n"},{"id":"346600","messageId":"0ee765bbb3627a19329d34ae3631a145653f5468.1525388472.git.johannes.schindelin@gmx.de","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 4/6] sequencer: allow introducing new root commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-03T23:01:23Z","receivedAt":"2018-05-03T23:01:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In the context of the new --rebase-merges mode, which was designed\nspecifically to allow for changing the existing branch topology\nliberally, a user may want to extract commits into a completely fresh\nbranch that starts with a newly-created root commit.\n\nThis is now possible by inserting the command `reset [new root]` before\n`pick`ing the commit that wants to become a root commit. Example:\n\n\treset [new root]\n\tpick 012345 a commit that is about to become a root commit\n\tpick 234567 this commit will have the previous one as parent\n\nThis does not conflict with other uses of the `reset` command because\n`[new root]` is not (part of) a valid ref name: both the opening bracket\nas well as the space are illegal in ref names.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 40 ++++++++++++++++++++++++++++------------\n t/t3430-rebase-merges.sh | 34 ++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 12 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 8eddda681d1..a7832399b1f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2747,18 +2747,34 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)\n \tif (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)\n \t\treturn -1;\n \n-\t/* Determine the length of the label */\n-\tfor (i = 0; i < len; i++)\n-\t\tif (isspace(name[i]))\n-\t\t\tlen = i;\n-\n-\tstrbuf_addf(&ref_name, \"refs/rewritten/%.*s\", len, name);\n-\tif (get_oid(ref_name.buf, &oid) &&\n-\t get_oid(ref_name.buf + strlen(\"refs/rewritten/\"), &oid)) {\n-\t\terror(_(\"could not read '%s'\"), ref_name.buf);\n-\t\trollback_lock_file(&lock);\n-\t\tstrbuf_release(&ref_name);\n-\t\treturn -1;\n+\tif (len == 10 && !strncmp(\"[new root]\", name, len)) {\n+\t\tif (!opts->have_squash_onto) {\n+\t\t\tconst char *hex;\n+\t\t\tif (commit_tree(\"\", 0, the_hash_algo->empty_tree,\n+\t\t\t\t\tNULL, &opts->squash_onto,\n+\t\t\t\t\tNULL, NULL))\n+\t\t\t\treturn error(_(\"writing fake root commit\"));\n+\t\t\topts->have_squash_onto = 1;\n+\t\t\thex = oid_to_hex(&opts->squash_onto);\n+\t\t\tif (write_message(hex, strlen(hex),\n+\t\t\t\t\t rebase_path_squash_onto(), 0))\n+\t\t\t\treturn error(_(\"writing squash-onto\"));\n+\t\t}\n+\t\toidcpy(&oid, &opts->squash_onto);\n+\t} else {\n+\t\t/* Determine the length of the label */\n+\t\tfor (i = 0; i < len; i++)\n+\t\t\tif (isspace(name[i]))\n+\t\t\t\tlen = i;\n+\n+\t\tstrbuf_addf(&ref_name, \"refs/rewritten/%.*s\", len, name);\n+\t\tif (get_oid(ref_name.buf, &oid) &&\n+\t\t get_oid(ref_name.buf + strlen(\"refs/rewritten/\"), &oid)) {\n+\t\t\terror(_(\"could not read '%s'\"), ref_name.buf);\n+\t\t\trollback_lock_file(&lock);\n+\t\t\tstrbuf_release(&ref_name);\n+\t\t\treturn -1;\n+\t\t}\n \t}\n \n \tmemset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));\ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 3d4dfdf7bec..35260862fcb 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -241,4 +241,38 @@ test_expect_success 'refuse to merge ancestors of HEAD' '\n \ttest_cmp_rev HEAD $before\n '\n \n+test_expect_success 'root commits' '\n+\tgit checkout --orphan unrelated &&\n+\t(GIT_AUTHOR_NAME=\"Parsnip\" GIT_AUTHOR_EMAIL=\"root@example.com\" \\\n+\t test_commit second-root) &&\n+\ttest_commit third-root &&\n+\tcat >script-from-scratch <<-\\EOF &&\n+\tpick third-root\n+\tlabel first-branch\n+\treset [new root]\n+\tpick second-root\n+\tmerge first-branch # Merge the 3rd root\n+\tEOF\n+\ttest_config sequence.editor \\\"\"$PWD\"/replace-editor.sh\\\" &&\n+\ttest_tick &&\n+\tgit rebase -i --force --root -r &&\n+\ttest \"Parsnip\" = \"$(git show -s --format=%an HEAD^)\" &&\n+\ttest $(git rev-parse second-root^0) != $(git rev-parse HEAD^) &&\n+\ttest $(git rev-parse second-root:second-root.t) = \\\n+\t\t$(git rev-parse HEAD^:second-root.t) &&\n+\ttest_cmp_graph HEAD <<-\\EOF &&\n+\t* Merge the 3rd root\n+\t|\\\n+\t| * third-root\n+\t* second-root\n+\tEOF\n+\n+\t: fast forward if possible &&\n+\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n+\ttest_might_fail git config --unset sequence.editor &&\n+\ttest_tick &&\n+\tgit rebase -i --root -r &&\n+\ttest_cmp_rev HEAD $before\n+'\n+\n test_done\n-- \n2.17.0.windows.1.38.g05ca542f78d\n\n\n"},{"id":"346690","messageId":"CAGZ79ka2SmfA_ZUA2MjuqSwb=gUphbRxp=NQgj5AcqvMiePZFw@mail.gmail.com","threadId":"48372","inReplyTo":"cover.1525388472.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 0/6] Let the sequencer handle `git rebase -i --root`","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-04T19:55:51Z","receivedAt":"2018-05-04T19:55:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">\n> Branch-diff vs v1:\n> 1: 42db734a980 ! 1: 73398da7119 sequencer: learn about the special \"fake root commit\" handling\n> @@ -54,40 +54,50 @@\n> return NULL;\n> }\n>\n> ++/* Read author-script and return an ident line (author timestamp) */\n> +static const char *read_author_ident(struct strbuf *buf)\n\n\nI like the new way of read_author_ident. Thanks for writing it!\n\n\n> +\n> static const char staged_changes_advice[] =\n> @@ -159,7 +169,17 @@\n> +/* Does this command create a (non-merge) commit? */\n> +static int is_pick_or_similar(enum todo_command command)\n> +{\n> -+ return command <= TODO_SQUASH;\n> ++ switch (command) {\n> ++ case TODO_PICK:\n> ++ case TODO_REVERT:\n> ++ case TODO_EDIT:\n> ++ case TODO_REWORD:\n> ++ case TODO_FIXUP:\n> ++ case TODO_SQUASH:\n> ++ return 1;\n> ++ default:\n> ++ return 0;\n> ++ }\n\nThe switch case is not as bad as I thought following the discussion on of v1.\n\nThis series is\nReviewed-by: Stefan Beller \n\nThanks!\n\n\nDuring a lunch discussion I wondered if the branch diff format could lead to\nanother form of machine readable communication, i.e. if we want to add the\nability to read the branch diff format and apply the changes. Note how applying\nthis diff-diff would not create new commits, but rather amend existing commits.\n"},{"id":"346728","messageId":"nycvar.QRO.7.76.6.1805052121090.77@tvgsbejvaqbjf.bet","threadId":"48372","inReplyTo":"CAGZ79ka2SmfA_ZUA2MjuqSwb=gUphbRxp=NQgj5AcqvMiePZFw@mail.gmail.com","subject":"Re: [PATCH v2 0/6] Let the sequencer handle `git rebase -i --root`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-05-05T19:24:37Z","receivedAt":"2018-05-05T19:24:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 4 May 2018, Stefan Beller wrote:\n\n> > Branch-diff vs v1:\n> > 1: 42db734a980 ! 1: 73398da7119 sequencer: learn about the special \"fake root commit\" handling\n> > @@ -54,40 +54,50 @@\n> > return NULL;\n> > }\n> >\n> > ++/* Read author-script and return an ident line (author timestamp) */\n> > +static const char *read_author_ident(struct strbuf *buf)\n> \n> \n> I like the new way of read_author_ident. Thanks for writing it!\n\nYou're welcome. After sleeping a night over it, I think this function\nmight also benefit from a new name: extract_ident_from_author_script().\nWhat do you think?\n\n> > @@ -159,7 +169,17 @@\n> > +/* Does this command create a (non-merge) commit? */\n> > +static int is_pick_or_similar(enum todo_command command)\n> > +{\n> > -+ return command <= TODO_SQUASH;\n> > ++ switch (command) {\n> > ++ case TODO_PICK:\n> > ++ case TODO_REVERT:\n> > ++ case TODO_EDIT:\n> > ++ case TODO_REWORD:\n> > ++ case TODO_FIXUP:\n> > ++ case TODO_SQUASH:\n> > ++ return 1;\n> > ++ default:\n> > ++ return 0;\n> > ++ }\n> \n> The switch case is not as bad as I thought following the discussion on of v1.\n\nYes, it makes things explicit, and it is not too long a case-chain.\n\n> This series is\n> Reviewed-by: Stefan Beller \n\nThanks!\n\n> \n> During a lunch discussion I wondered if the branch diff format could lead to\n> another form of machine readable communication, i.e. if we want to add the\n> ability to read the branch diff format and apply the changes. Note how applying\n> this diff-diff would not create new commits, but rather amend existing commits.\n\n (which BTW is not valid XML)\n\nI do not think that it would be a wise idea to detour even further from\nGit when exchanging patch series iterations. We have a lovely way to\nexchange commits, after all: `git fetch` and `git push`, and for times you\ncannot agree on a central server, `git bundle`.\n\nCiao,\nDscho\n"}]}