{"thread":{"id":"51790","subject":"[PATCH] rebase: introduce --update-branches option","startedAt":"2019-09-02T23:41:27Z","lastAt":"2019-09-23T09:40:38Z","messageCount":16,"participants":["Warren He","brian m. carlson","Junio C Hamano","Taylor Blau","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"381697","messageId":"20190902234109.2922-1-wh109@yahoo.com","threadId":"51790","inReplyTo":null,"subject":"[PATCH] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-02T23:41:08Z","receivedAt":"2019-09-02T23:41:27Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Sometimes people have to rebase multiple related branches. One way to do that\nquickly, when there are branches pointing to ancestors of a later branch (which\nhappens a lot if you try hard to pad your PR count on GitHub--I mean if you try\nto make small, logically separate changes), is to rebase that later branch and\nthen reset ancestor branches to the rewritten commits. You just have to work\nout which branches correspond to which of the new commits.\n\nHere's an automated way to update those ancestor branches.\n\nIt's implemented as a function that processes a todo list, modeled after\n`todo_list_add_exec_commands`. Currently steps are added as `exec git branch -f\n<branchname>`, which comes with the caveat that they're not applied atomically\nwhen it finishes rebasing.\n\nIf you were to use this feature to rebase `my-dev` onto `master` in this\nsituation:\n\n```\n A - B          (master)\n  |\\\n  |  E          (feat-e)\n   \\   \\\n     F  |       (feat-f)\n       \\|\n         G      (interim)\n           \\\n             H  (my-dev)\n```\n\nyou'd get a todo list like this:\n\n```\nlabel onto\n\n# Branch G\nreset onto\npick 65945ab E\nexec git branch -f feat-e\nlabel G\n\nreset onto\npick 4f0b0ad F\nexec git branch -f feat-f\nmerge -C e50066c G # G\nexec git branch -f interim\npick 539e556 H\n```\n\nWarren He (1):\n  rebase: introduce --update-branches option\n\n Documentation/git-rebase.txt      |  8 +++++\n builtin/rebase.c                  | 13 ++++++--\n sequencer.c                       | 68 ++++++++++++++++++++++++++++++++++++++-\n sequencer.h                       |  6 ++--\n t/t3431-rebase-update-branches.sh | 64 ++++++++++++++++++++++++++++++++++++\n 5 files changed, 154 insertions(+), 5 deletions(-)\n create mode 100755 t/t3431-rebase-update-branches.sh\n\n-- \n2.7.4\n\n"},{"id":"381698","messageId":"20190902234109.2922-2-wh109@yahoo.com","threadId":"51790","inReplyTo":"20190902234109.2922-1-wh109@yahoo.com","subject":"[PATCH] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-02T23:41:09Z","receivedAt":"2019-09-02T23:41:30Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Rebasing normally updates the current branch to the rewritten version.\nIf any other branches point to commits rewritten along the way, those\nremain untouched. This commit adds an `--update-branches` option, which\ninstructs the command to update any such branches that it encounters to\npoint to the rewritten versions of those commits.\n\nSigned-off-by: Warren He <wh109@yahoo.com>\n---\n Documentation/git-rebase.txt      |  8 +++++\n builtin/rebase.c                  | 13 ++++++--\n sequencer.c                       | 68 ++++++++++++++++++++++++++++++++++++++-\n sequencer.h                       |  6 ++--\n t/t3431-rebase-update-branches.sh | 64 ++++++++++++++++++++++++++++++++++++\n 5 files changed, 154 insertions(+), 5 deletions(-)\n create mode 100755 t/t3431-rebase-update-branches.sh\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 6156609..c37a0db 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -246,6 +246,13 @@ leave out at most one of A and B, in which case it defaults to HEAD.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n+--update-branches::\n+\tIf there are branch refs that point to commits that will be\n+\treapplied, add shell commands to the todo list to update those\n+\trefs to point to the commits in the final history.\n++\n+See also INCOMPATIBLE OPTIONS below.\n+\n --allow-empty-message::\n \tBy default, rebasing commits with an empty message will fail.\n \tThis option overrides that behavior, allowing commits with empty\n@@ -535,6 +542,7 @@ are incompatible with the following options:\n  * --interactive\n  * --exec\n  * --keep-empty\n+ * --update-branches\n  * --edit-todo\n  * --root when used in combination with --onto\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 670096c..cf87c53 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -78,6 +78,7 @@ struct rebase_options {\n \tint signoff;\n \tint allow_rerere_autoupdate;\n \tint keep_empty;\n+\tint update_branches;\n \tint autosquash;\n \tchar *gpg_sign_opt;\n \tint autostash;\n@@ -349,8 +350,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)\n \n \t\tsplit_exec_commands(opts->cmd, &commands);\n \t\tret = complete_action(the_repository, &replay, flags,\n-\t\t\tshortrevisions, opts->onto_name, opts->onto, head_hash,\n-\t\t\t&commands, opts->autosquash, &todo_list);\n+\t\t\tshortrevisions, opts->onto_name, opts->onto, opts->head_name,\n+\t\t\thead_hash, &commands, opts->autosquash, &todo_list);\n \t}\n \n \tstring_list_clear(&commands, 0);\n@@ -375,6 +376,7 @@ static int run_rebase_interactive(struct rebase_options *opts,\n \tflags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;\n \tflags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;\n \tflags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;\n+\tflags |= opts->update_branches ? TODO_LIST_UPDATE_BRANCHES : 0;\n \n \tswitch (command) {\n \tcase ACTION_NONE: {\n@@ -447,6 +449,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)\n \t\tOPT_NEGBIT(0, \"ff\", &opts.flags, N_(\"allow fast-forward\"),\n \t\t\t   REBASE_FORCE),\n \t\tOPT_BOOL(0, \"keep-empty\", &opts.keep_empty, N_(\"keep empty commits\")),\n+\t\tOPT_BOOL(0, \"update-branches\", &opts.update_branches,\n+\t\t\t N_(\"update branches that point to reapplied commits\")),\n \t\tOPT_BOOL(0, \"allow-empty-message\", &opts.allow_empty_message,\n \t\t\t N_(\"allow commits with empty messages\")),\n \t\tOPT_BOOL(0, \"rebase-merges\", &opts.rebase_merges, N_(\"rebase merge commits\")),\n@@ -1453,6 +1457,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n \t\tOPT_BOOL('k', \"keep-empty\", &options.keep_empty,\n \t\t\t N_(\"preserve empty commits during rebase\")),\n+\t\tOPT_BOOL(0, \"update-branches\", &options.update_branches,\n+\t\t\t N_(\"update branches that point to reapplied commits\")),\n \t\tOPT_BOOL(0, \"autosquash\", &options.autosquash,\n \t\t\t N_(\"move commits that begin with \"\n \t\t\t    \"squash!/fixup! under -i\")),\n@@ -1710,6 +1716,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.keep_empty)\n \t\timply_interactive(&options, \"--keep-empty\");\n \n+\tif (options.update_branches)\n+\t\timply_interactive(&options, \"--update-branches\");\n+\n \tif (gpg_sign) {\n \t\tfree(options.gpg_sign_opt);\n \t\toptions.gpg_sign_opt = xstrfmt(\"-S%s\", gpg_sign);\ndiff --git a/sequencer.c b/sequencer.c\nindex 34ebf8e..c6749ff 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4901,6 +4901,69 @@ void todo_list_add_exec_commands(struct todo_list *todo_list,\n \ttodo_list->alloc = alloc;\n }\n \n+/*\n+ * Add commands to update branch refs after the todo list would pick a commit\n+ * that a branch ref points to.\n+ */\n+static void todo_list_add_branch_updates(struct todo_list *todo_list,\n+\t\t\t\t       const char *head_name)\n+{\n+\tstruct strbuf *buf = &todo_list->buf;\n+\tint i, nr = 0, alloc = 0;\n+\tstruct todo_item *items = NULL;\n+\n+\tload_ref_decorations(NULL, 0);\n+\n+\tfor (i = 0; i < todo_list->nr; i++) {\n+\t\tconst struct todo_item *item = &todo_list->items[i];\n+\t\tenum todo_command command = item->command;\n+\t\tconst struct name_decoration *decoration;\n+\n+\t\tALLOC_GROW(items, nr + 1, alloc);\n+\t\titems[nr++] = todo_list->items[i];\n+\n+\t\tswitch (command) {\n+\t\tcase TODO_PICK:\n+\t\tcase TODO_MERGE:\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tdecoration = get_name_decoration(&item->commit->object);\n+\t\tfor (; decoration; decoration = decoration->next) {\n+\t\t\tsize_t base_offset, pretty_name_len;\n+\t\t\tconst char *pretty_name;\n+\n+\t\t\tif (decoration->type != DECORATION_REF_LOCAL)\n+\t\t\t\tcontinue;\n+\t\t\tif (!strcmp(decoration->name, head_name))\n+\t\t\t\t// Rebase itself will update the current branch for us.\n+\t\t\t\tcontinue;\n+\n+\t\t\tbase_offset = buf->len;\n+\t\t\tpretty_name = prettify_refname(decoration->name);\n+\t\t\tpretty_name_len = strlen(pretty_name);\n+\t\t\tstrbuf_addstr(buf, \"exec git branch -f \");\n+\t\t\tstrbuf_addstr(buf, pretty_name);\n+\t\t\tstrbuf_addch(buf, '\\n');\n+\n+\t\t\tALLOC_GROW(items, nr + 1, alloc);\n+\t\t\titems[nr++] = (struct todo_item) {\n+\t\t\t\t.command = TODO_EXEC,\n+\t\t\t\t.offset_in_buf = base_offset,\n+\t\t\t\t.arg_offset = base_offset + strlen(\"exec \"),\n+\t\t\t\t.arg_len = strlen(\"git branch -f \") + pretty_name_len,\n+\t\t\t};\n+\t\t}\n+\t}\n+\n+\tFREE_AND_NULL(todo_list->items);\n+\ttodo_list->items = items;\n+\ttodo_list->nr = nr;\n+\ttodo_list->alloc = alloc;\n+}\n+\n static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,\n \t\t\t\tstruct strbuf *buf, int num, unsigned flags)\n {\n@@ -5051,7 +5114,7 @@ static int skip_unnecessary_picks(struct repository *r,\n \n int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,\n \t\t    const char *shortrevisions, const char *onto_name,\n-\t\t    struct commit *onto, const char *orig_head,\n+\t\t    struct commit *onto, const char *head_name, const char *orig_head,\n \t\t    struct string_list *commands, unsigned autosquash,\n \t\t    struct todo_list *todo_list)\n {\n@@ -5076,6 +5139,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla\n \tif (commands->nr)\n \t\ttodo_list_add_exec_commands(todo_list, commands);\n \n+\tif (flags & TODO_LIST_UPDATE_BRANCHES)\n+\t\ttodo_list_add_branch_updates(todo_list, head_name);\n+\n \tif (count_commands(todo_list) == 0) {\n \t\tapply_autostash(opts);\n \t\tsequencer_remove_state(opts);\ndiff --git a/sequencer.h b/sequencer.h\nindex 6704acb..69c6f71 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -143,6 +143,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n  */\n #define TODO_LIST_REBASE_COUSINS (1U << 4)\n #define TODO_LIST_APPEND_TODO_HELP (1U << 5)\n+#define TODO_LIST_UPDATE_BRANCHES (1U << 6)\n \n int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,\n \t\t\t  const char **argv, unsigned flags);\n@@ -152,8 +153,9 @@ void todo_list_add_exec_commands(struct todo_list *todo_list,\n int check_todo_list_from_file(struct repository *r);\n int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,\n \t\t    const char *shortrevisions, const char *onto_name,\n-\t\t    struct commit *onto, const char *orig_head, struct string_list *commands,\n-\t\t    unsigned autosquash, struct todo_list *todo_list);\n+\t\t    struct commit *onto, const char *head_name, const char *orig_head,\n+\t\t    struct string_list *commands, unsigned autosquash,\n+\t\t    struct todo_list *todo_list);\n int todo_list_rearrange_squash(struct todo_list *todo_list);\n \n /*\ndiff --git a/t/t3431-rebase-update-branches.sh b/t/t3431-rebase-update-branches.sh\nnew file mode 100755\nindex 0000000..221c25d\n--- /dev/null\n+++ b/t/t3431-rebase-update-branches.sh\n@@ -0,0 +1,64 @@\n+#!/bin/sh\n+\n+test_description='git rebase -i --update-branches\n+\n+This test runs git rebase, moving branch refs that point to commits\n+that are reapplied.\n+\n+Initial setup:\n+\n+ A - B          (master)\n+  |\\\n+  |  C          (linear-early)\n+  |    \\\n+  |      D      (linear-late)\n+  |\\\n+  |  E          (feat-e)\n+   \\   \\\n+     F  |       (feat-f)\n+       \\|\n+         G      (interim)\n+           \\\n+             H  (my-dev)\n+'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup linear' '\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tgit checkout -b linear-early A &&\n+\ttest_commit C &&\n+\tgit checkout -b linear-late &&\n+\ttest_commit D\n+'\n+\n+test_expect_success 'smoketest linear' '\n+\tgit rebase --update-branches master\n+'\n+\n+test_expect_success 'check linear' '\n+\tgit rev-parse linear-early:B.t\n+'\n+\n+test_expect_success 'setup merge' '\n+\tgit checkout -b feat-e A &&\n+\ttest_commit E &&\n+\tgit checkout -b feat-f A &&\n+\ttest_commit F &&\n+\tgit checkout -b interim &&\n+\ttest_merge G feat-e &&\n+\tgit checkout -b my-dev &&\n+\ttest_commit H\n+'\n+\n+test_expect_success 'smoketest merge' '\n+\tgit rebase -r --update-branches master\n+'\n+\n+test_expect_success 'check merge' '\n+\tgit rev-parse feat-e:B.t &&\n+\tgit rev-parse feat-f:B.t &&\n+\tgit rev-parse interim:B.t\n+'\n+\n+test_done\n-- \n2.7.4\n\n"},{"id":"381699","messageId":"20190903005018.GH11334@genre.crustytoothpaste.net","threadId":"51790","inReplyTo":"20190902234109.2922-1-wh109@yahoo.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-09-03T00:50:18Z","receivedAt":"2019-09-03T00:50:28Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-09-02 at 23:41:08, Warren He wrote:\n> Sometimes people have to rebase multiple related branches. One way to do that\n> quickly, when there are branches pointing to ancestors of a later branch (which\n> happens a lot if you try hard to pad your PR count on GitHub--I mean if you try\n> to make small, logically separate changes), is to rebase that later branch and\n> then reset ancestor branches to the rewritten commits. You just have to work\n> out which branches correspond to which of the new commits.\n> \n> Here's an automated way to update those ancestor branches.\n> \n> It's implemented as a function that processes a todo list, modeled after\n> `todo_list_add_exec_commands`. Currently steps are added as `exec git branch -f\n> <branchname>`, which comes with the caveat that they're not applied atomically\n> when it finishes rebasing.\n\nThis is an interesting idea, and I definitely would find myself using\nit.  I maintain multiple nested branches for the SHA-256 transition and\nrebasing tends to be a bit of a hassle.  The idea of reordering commits\nfurther down into earlier branches using this technique is also\nappealing.\n\nI like the idea of using existing tooling for this and not needing an\nadditional verb.\n\nMy gut tells me folks may want a bit more control over *which* branches\nare rebased, but I don't have a personal need for that, so I'm not going\nto request it or propose an interface for it.  If nobody else does, then\nI think we should adopt the simplest approach, which is what you've\nproposed.  Users can always edit the todo list if they find an\nunexpected branch, after all.\n\nThe other thought I had about this is a question about how it performs\nwith many refs.  I've worked with a repository with easily 80,000 refs,\nand I wonder if the current technique will perform adequately there.\n\nI'm interested to hear others' opinions on this series and am looking\nforward to seeing it progress.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"381701","messageId":"xmqqwoeqtbs2.fsf@gitster-ct.c.googlers.com","threadId":"51790","inReplyTo":"20190903005018.GH11334@genre.crustytoothpaste.net","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-03T01:21:33Z","receivedAt":"2019-09-03T01:21:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n\n> I like the idea of using existing tooling for this and not needing an\n> additional verb.\n>\n> My gut tells me folks may want a bit more control over *which* branches\n> are rebased, but I don't have a personal need for that, so I'm not going\n> to request it or propose an interface for it.\n\nFWIW, I am in favor of both of the above two points.  It is quite\nclear because \"exec git branch -f <that branch name>\" is spelled\nout, people can remove the ones they want to keep a copy of when\nthey need to.\n\nI didn't look at the code more deeply than just eyeballing and\nnoticing style violations etc., and will leave the reviewing to\nothers for now.\n\nThanks.\n"},{"id":"381702","messageId":"20190903013959.GA40029@syl.lan","threadId":"51790","inReplyTo":"xmqqwoeqtbs2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2019-09-03T01:39:59Z","receivedAt":"2019-09-03T01:40:03Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Junio,\n\nOn Mon, Sep 02, 2019 at 06:21:33PM -0700, Junio C Hamano wrote:\n> \"brian m. carlson\" <sandals@crustytoothpaste.net> writes:\n>\n> > I like the idea of using existing tooling for this and not needing an\n> > additional verb.\n> >\n> > My gut tells me folks may want a bit more control over *which* branches\n> > are rebased, but I don't have a personal need for that, so I'm not going\n> > to request it or propose an interface for it.\n>\n> FWIW, I am in favor of both of the above two points.  It is quite\n> clear because \"exec git branch -f <that branch name>\" is spelled\n> out, people can remove the ones they want to keep a copy of when\n> they need to.\n\nI agree with what you're saying here. I don't see myself having a need\nto modify the 'exec git branch -f ...' line, so I have no real need to\nmake an interface suggestion per brian's comment above, but I happened\nto think of one while reading this thread that seemed worth pointing\nout.\n\nPerhaps we could avoid inserting the 'exec git branch -f' step in the\ntodo list for branches that have a configuration section forbidding them\nfrom being updated?\n\nFor example, the configuration:\n\n  [branch \"dont-update-me\"]\n    updateAfterRebase = false\n\nwould cause the rebase of a branch upstream from 'dont-update-me' to\nomit the 'git branch -f dont-update-me'.\n\nTo be honest, this configuration option seems like a knob we don't need.\nEspecially since now three of us feel that removing the 'exec' line\nsuffices anyway. I suppose that I could see a user wanting some\nintegration branch to not get updated over many rebases, but it seems a\nlittle contrived.\n\nAnyway, perhaps others can chime in and share what they think about this\ninterface and whether or not we need it. If not, we can simply file this\naway.\n\n> I didn't look at the code more deeply than just eyeballing and\n> noticing style violations etc., and will leave the reviewing to\n> others for now.\n>\n> Thanks.\n\nThanks,\nTaylor\n"},{"id":"381716","messageId":"713b055b-aaf2-4294-12d3-2941d53eac16@gmail.com","threadId":"51790","inReplyTo":"20190902234109.2922-1-wh109@yahoo.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-09-03T12:19:04Z","receivedAt":"2019-09-03T12:19:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Warren\n\nOn 03/09/2019 00:41, Warren He wrote:\n> Sometimes people have to rebase multiple related branches. One way to do that\n> quickly, when there are branches pointing to ancestors of a later branch (which\n> happens a lot if you try hard to pad your PR count on GitHub--I mean if you try\n> to make small, logically separate changes), is to rebase that later branch and\n> then reset ancestor branches to the rewritten commits. You just have to work\n> out which branches correspond to which of the new commits.\n> \n> Here's an automated way to update those ancestor branches.\n\nI think this would be really useful, but as it is implemented here it \nonly updates branch heads that are in the todo list. This means that if \nI have a branch structure like\n\nA - B (master)\n|\nC - E (topic-2)\n|\nD (topic-1)\n\nand I do `git rebase --update-branches master topic1` Then topic-2 will \nnot be updated and if I do `git rebase --update-branches master topic2` \nthen topic-1 will not be updated even though C is updated in both cases \nand is a common ancestor of topic-1 and topic-2. Conceptually to update \nall the branches descended from a rewritten commit would require using \n`git for-each-ref --contains $(git merge-base <upstream> <branch>)` and \nthen using `git rev-list <upstream>..<branch-head>` on each of those \nrefs to get the commits to generate the todo list.\n\nAnother case is applying fixup commits. In the example above if I squash \na fixup for C from either branch I probably want to update both the \nbranches descended from it.\n\nOne other thing is that if the user aborts the rebase then ideally we \ndon't want to update all the branches so it would be better to store the \nupdated heads as we go along and then update them all at the end of the \nrebase. Worktrees add another complication as if one of the branches \nthat is to be updated is checked out in another worktree then we need \nsome way to deal with that. If there are no local changes in that \nworktree then we could just update the local HEAD and working copy.\n\nBest Wishes\n\nPhillip\n\n> \n> It's implemented as a function that processes a todo list, modeled after\n> `todo_list_add_exec_commands`. Currently steps are added as `exec git branch -f\n> <branchname>`, which comes with the caveat that they're not applied atomically\n> when it finishes rebasing.\n> \n> If you were to use this feature to rebase `my-dev` onto `master` in this\n> situation:\n> \n> ```\n>   A - B          (master)\n>    |\\\n>    |  E          (feat-e)\n>     \\   \\\n>       F  |       (feat-f)\n>         \\|\n>           G      (interim)\n>             \\\n>               H  (my-dev)\n> ```\n> \n> you'd get a todo list like this:\n> \n> ```\n> label onto\n> \n> # Branch G\n> reset onto\n> pick 65945ab E\n> exec git branch -f feat-e\n> label G\n> \n> reset onto\n> pick 4f0b0ad F\n> exec git branch -f feat-f\n> merge -C e50066c G # G\n> exec git branch -f interim\n> pick 539e556 H\n> ```\n> \n> Warren He (1):\n>    rebase: introduce --update-branches option\n> \n>   Documentation/git-rebase.txt      |  8 +++++\n>   builtin/rebase.c                  | 13 ++++++--\n>   sequencer.c                       | 68 ++++++++++++++++++++++++++++++++++++++-\n>   sequencer.h                       |  6 ++--\n>   t/t3431-rebase-update-branches.sh | 64 ++++++++++++++++++++++++++++++++++++\n>   5 files changed, 154 insertions(+), 5 deletions(-)\n>   create mode 100755 t/t3431-rebase-update-branches.sh\n> \n"},{"id":"381720","messageId":"nycvar.QRO.7.76.6.1909031345330.46@tvgsbejvaqbjf.bet","threadId":"51790","inReplyTo":"20190902234109.2922-2-wh109@yahoo.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-09-03T13:26:31Z","receivedAt":"2019-09-03T13:26:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Warren,\n\nOn Mon, 2 Sep 2019, Warren He wrote:\n\n> Rebasing normally updates the current branch to the rewritten version.\n> If any other branches point to commits rewritten along the way, those\n> remain untouched. This commit adds an `--update-branches` option, which\n> instructs the command to update any such branches that it encounters to\n> point to the rewritten versions of those commits.\n\nThis is definitely a feature I longed for, myself. I worked around it by\nusing some complicated vim command (I think it was something like\n`%s/^label (.*)/&^Mexec git update-ref refs/heads/\\1 HEAD` or some such,\nwhere the `^M` is actually typed via Ctrl+V Ctrl+M, and of course, this\ndoes not take existing branches into account, but updates refs based on\nthe merge commits' onelines).\n\nOnce `--update-branches` is available, I will want to use it e.g. for my\nwork on the built-in `git add -i`, where I have to update 6\ninter-dependent PRs in parallel.\n\nA couple of suggestions below. (If you are like me, trying to address\nall the suggestions while reading the code review, I would like to\nsuggest reading all the way through to the end first, it might save you\nsome work.)\n\n> [...]\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index 670096c..cf87c53 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -447,6 +449,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_NEGBIT(0, \"ff\", &opts.flags, N_(\"allow fast-forward\"),\n>  \t\t\t   REBASE_FORCE),\n>  \t\tOPT_BOOL(0, \"keep-empty\", &opts.keep_empty, N_(\"keep empty commits\")),\n> +\t\tOPT_BOOL(0, \"update-branches\", &opts.update_branches,\n> +\t\t\t N_(\"update branches that point to reapplied commits\")),\n\nMaybe `s/reapplied/rebased/`?\n\nAlso, seeing as the `cmd_rebase__interactive()` is only used by `git\nrebase --preserve-merges`, which cannot make use of `--update-branches`\nanyway, I would suggest to simply drop this hunk and only keep the next\none.\n\n>  \t\tOPT_BOOL(0, \"allow-empty-message\", &opts.allow_empty_message,\n>  \t\t\t N_(\"allow commits with empty messages\")),\n>  \t\tOPT_BOOL(0, \"rebase-merges\", &opts.rebase_merges, N_(\"rebase merge commits\")),\n> @@ -1453,6 +1457,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n>  \t\tOPT_BOOL('k', \"keep-empty\", &options.keep_empty,\n>  \t\t\t N_(\"preserve empty commits during rebase\")),\n> +\t\tOPT_BOOL(0, \"update-branches\", &options.update_branches,\n> +\t\t\t N_(\"update branches that point to reapplied commits\")),\n>  \t\tOPT_BOOL(0, \"autosquash\", &options.autosquash,\n>  \t\t\t N_(\"move commits that begin with \"\n>  \t\t\t    \"squash!/fixup! under -i\")),\n> @@ -1710,6 +1716,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  \tif (options.keep_empty)\n>  \t\timply_interactive(&options, \"--keep-empty\");\n>\n> +\tif (options.update_branches)\n> +\t\timply_interactive(&options, \"--update-branches\");\n\nIt should not really imply `--interactive`, but `--rebase-merges`.\nImagine for example this call:\n\n\tgit rebase --update-branches @{u}\n\nThis would not even be interactive (even if it uses the interactive\nrebase backend to execute), but it would have to rebase the merge\ncommits. So I think we need something like this instead:\n\n\tif (options.update_branches && !options.rebase_merges) {\n\t\tif (options.type == REBASE_UNSPECIFIED)\n\t\t\toptions.type = REBASE_INTERACTIVE;\n\t\telse if (options.type != REBASE_INTERACTIVE)\n\t\t\tdie(_(\"%s requires an interactive rebase\"),\n\t\t\t    \"--update-branches\");\n\t\toptions.rebase_merges = 1;\n\t}\n\nI don't think we can use `imply_interactive()` here because that would\nalso allow `--update-branches` to be combined with `--preserve-merges`\n(which does not work, and as the latter is deprecated, I would advise\nagainst trying to make it work, either).\n\n> +\n>  \tif (gpg_sign) {\n>  \t\tfree(options.gpg_sign_opt);\n>  \t\toptions.gpg_sign_opt = xstrfmt(\"-S%s\", gpg_sign);\n> [...]\n> diff --git a/sequencer.c b/sequencer.c\n> index 34ebf8e..c6749ff 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -4901,6 +4901,69 @@ void todo_list_add_exec_commands(struct todo_list *todo_list,\n>  \ttodo_list->alloc = alloc;\n>  }\n>\n> +/*\n> + * Add commands to update branch refs after the todo list would pick a commit\n> + * that a branch ref points to.\n> + */\n> +static void todo_list_add_branch_updates(struct todo_list *todo_list,\n> +\t\t\t\t       const char *head_name)\n> +{\n> +\tstruct strbuf *buf = &todo_list->buf;\n> +\tint i, nr = 0, alloc = 0;\n> +\tstruct todo_item *items = NULL;\n> +\n> +\tload_ref_decorations(NULL, 0);\n\nThis loads also tags, correct? I am fairly certain that we don't want to\nupdate tags here, but maybe the check for `DECORATION_REF_LOCAL` later\non already ensures that?\n\n> +\n> +\tfor (i = 0; i < todo_list->nr; i++) {\n> +\t\tconst struct todo_item *item = &todo_list->items[i];\n> +\t\tenum todo_command command = item->command;\n> +\t\tconst struct name_decoration *decoration;\n> +\n> +\t\tALLOC_GROW(items, nr + 1, alloc);\n> +\t\titems[nr++] = todo_list->items[i];\n> +\n> +\t\tswitch (command) {\n> +\t\tcase TODO_PICK:\n> +\t\tcase TODO_MERGE:\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tcontinue;\n> +\t\t}\n\nI think this misses the `edit` and `reword` commands. How about using\n`is_pick_or_similar()` instead?\n\nOf course, there is one slight problem with that: you would _also_ have\nto call `is_fixup()` and delay the addition of the corresponding branch\nupdates (you don't want to apply multiple `fixup` commits and possibly\npoint a local branch to an already fixed-up commit).\n\n> +\n> +\t\tdecoration = get_name_decoration(&item->commit->object);\n> +\t\tfor (; decoration; decoration = decoration->next) {\n> +\t\t\tsize_t base_offset, pretty_name_len;\n> +\t\t\tconst char *pretty_name;\n> +\n> +\t\t\tif (decoration->type != DECORATION_REF_LOCAL)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (!strcmp(decoration->name, head_name))\n> +\t\t\t\t// Rebase itself will update the current branch for us.\n\nPlease use C-style /* ... */ comments, Git insists on not using\nC++-style // comments.\n\n> +\t\t\t\tcontinue;\n\nMy instant reaction was: just compare `&item->commit->object.oid` to\n`head_hash`, but your code is careful to take care of the scenario where\nmultiple local branches point to the pre-rebase `HEAD`. Good. Maybe you\nwant to test for that in the regression test, too?\n\nHowever, you have two `if` conditions that both guard the same\noperation: `continue`. How about combining the combinations? It's like\nsaying: under these circumstances, we skip adding a command.\n\n> +\n> +\t\t\tbase_offset = buf->len;\n> +\t\t\tpretty_name = prettify_refname(decoration->name);\n> +\t\t\tpretty_name_len = strlen(pretty_name);\n> +\t\t\tstrbuf_addstr(buf, \"exec git branch -f \");\n> +\t\t\tstrbuf_addstr(buf, pretty_name);\n> +\t\t\tstrbuf_addch(buf, '\\n');\n> +\n> +\t\t\tALLOC_GROW(items, nr + 1, alloc);\n> +\t\t\titems[nr++] = (struct todo_item) {\n> +\t\t\t\t.command = TODO_EXEC,\n> +\t\t\t\t.offset_in_buf = base_offset,\n> +\t\t\t\t.arg_offset = base_offset + strlen(\"exec \"),\n> +\t\t\t\t.arg_len = strlen(\"git branch -f \") + pretty_name_len,\n> +\t\t\t};\n\nWow. This is the first time I see anybody trying to use this kind of\nassignment: `<item> = (struct <name>) { ... }`. I don't really know that\nwe can expect all C compilers that are currently used to compile Git to\nhandle this kind of construct. So I'd rather be a little bit more\nverbose and safe:\n\n\t\t\tmemset(items[nr], 0, sizeof(items[nr]));\n\t\t\titems[nr].command = TODO_EXEC;\n\t\t\t[...]\n\t\t\tnr++;\n\nTaking another step back, I think we should actually be super lazy here\n(it's not performance-critical code, so we can save us some time\nverifying correctness, at the cost of a little slower execution):\n\n\t\t\tsize_t len = buf->len;\n\t\t\tstrbuf_addf(buf, \"exec git branch -f %s\\n\",\n\t\t\t\t    prettify_refname(decoration->name));\n\t\t\tparse_insn_line(r, items[nr], buf.buf,\n\t\t\t\t\tbuf.buf + len, buf.buf + buf.len);\n\nCome to think of it, I wonder whether you could simply use a full\n`struct todo_list` instead of maintaining the `items` manually, and then\nuse `append_new_todo()`.\n\nAt the end, you would simply call\n\n\tSWAP(new_list.items, todo_list->items);\n\tSWAP(new_list.nr, todo_list->nr);\n\tSWAP(new_list.alloc, todo_list->alloc);\n\ttodo_list_release(&new_list);\n\nNow, let's talk about this command: `git branch -f <name>`. I _think_ it\nis the right command to use (instead of `git update-ref`, for example),\nyet I have a couple doubts about that:\n\n- with `git update-ref <name> HEAD <old-commit>`, we could safe-guard\n  against ref updates while the rebase is running. This is _not_\n  possible with `git branch -f`... We would overwrite the branch, even\n  if another Git process had updated it in the meantime, e.g. in a\n  manually-inserted `exec` call.\n\n- we definitely want to try our best to handle the case where the branch\n  in question is currently checked out in a separate worktree. This is a\n  use case that bit me in the past (with my complicated vim method of\n  emulating `--update-branches`).\n\n- If we use `git branch`, we _have_ to use the short branch name.\n  However, `prettify_refname()` does not guarantee that, I don't think:\n  it strips `refs/heads/` or `refs/tags/` or `refs/remotes/` _if found_.\n  So I would feel a lot more comfortable with\n\n  \tif (!skip_prefix(decoration->name, \"refs/heads/\", &name))\n\t\tcontinue;\n\n\t[...]\n\n\tstrbuf_addf(buf, \"exec git branch -f %s\\n\", name);\n\n- I actually do not know off-hand what the reflog message for the branch\n  updates would be! Would it be prefixed by `rebase -i`? I think we\n  should make sure that the code uses an informative reflog message, and\n  definitely verify that behavior in the regression test.\n\n- An `exec git branch -f <name>` will update that branch _immediately_.\n  Even if the user runs into a road-block with the current rebase and\n  decides to abandon it via `git rebase --abort`. A subsequent attempt\n  at rebasing would _no longer be able_ to pick up on the local\n  branches, as they would not have been rolled back.\n\nThis last point is worth dwelling on for a moment. The interactive\nrebase goes out of its way to first switch to an unnamed branch, and\nonly update the current branch at the very end, when the rebase\nsucceeded. If the rebase does not succeed, i.e. if it is aborted or\nquit, the current branch will not be updated. The interactive rebase is\ntherefore sort of \"transactional\": either it succeeds in its entirety,\nor all changes are rolled back.\n\nHowever, the way this patch is implemented, it would break that design,\nbecause the local branches would not be updated at the very end, but\nalready while the transaction is still being processed, so to say.\n\nSpontaneously, I can think of a few ways to address this:\n\n1. Add a new todo command, or an option for the `label` command (`label\n   --branch <name>`) where it would remember to update those refs upon\n   successful completion, much like the file\n   `<GIT-DIR>/rebase-merge/refs-to-delete` accumulates the\n   `refs/rewritten/<label>` refs to delete at the end of the interactive\n   rebase, and teach `todo_list_add_branch_updates()` to use it.\n\n2. Maybe even better would be to add a `label <branch-name>` (if there\n   is no such command already) instead of the `exec git branch -f`\n   lines, and then add a slur of `exec git branch -f` lines at the end\n   of the todo list that pick up those labels and run them through\n   `update-ref` to update them. That way, if the user aborts the\n   interactive rebase somewhere in the middle, the refs would not have\n   been updated yet.\n\n3. Another method would combine the approaches 1. & 2.: add an option to\n   the `label` command, say, `--update-branch=<name>`, which would append\n   the following line to `<GIT-DIR>/rebase-merge/branches-to-update`:\n\n   \tupdate <name> <old-hash>\n\n   And then a single line would be appended to the end of the todo\n   list:\n\n   \texec git update-ref --stdin <\"<GIT-DIR>/rebase-merge/branches-to-update\"\n\n   This would have the advantage of making things a bit more explicit,\n   but it would break expectations when a user would add the\n   `--update-branch=<name>` option manually and forgot to make sure that\n   that `update-ref` line was appended to the end.\n\nOf course, with all of those methods, you would still have to make sure\nthat the todo command would be inserted _after_ `fixup`/`squash` chains.\n\nHowever, now that I think of it, the `fixup`/`squash` thing is actually\na pretty tricky thing to deal with in this context. Imagine this commit\ngraph (newest commit first):\n\n\t*   Merge branch 'fix-names'\n\t| \\\n\t|  * fixup! Implement the `names` command\n\t|  * Document the `names` feature\n\t|  * Implement the `names` command\n\t* / Fiddle with the README.md\n\to\n\nThe `fixup!` will be re-ordered before `todo_list_add_branch_updates()`\nis called, yet we do not want to update the `fix-names` branch to point\nto the fixed-up commit, but still to the tip of the branch!\n\nThis consideration brings me to my favorite idea so far:\n\n4. If we were to extend the `label` command by the `--update-branch`\n   option, it would be nice if we could reuse the actual branch name as\n   label name. However, at this stage we no longer have access to the\n   `label_oid()` function, as the todo list was generated a long time\n   ago. And we would not be able to guarantee that the branch name was\n   not already used as a label.\n\n   So why not implement this feature as part of\n   `make_script_with_merges()`?\n\n   That way, we could have an initial run through the commits to see\n   whether they are decorated with any local branch, and if they are,\n   make sure that they get the label(s) they want.\n\n   (The only tricky situation with that would be that you could have a\n   local branch `refs/heads/<40-digit-hex>` that clashes with a `reset\n   <40-digit-hex>`, which situation can arise under\n   `no-rebase-cousins`.)\n\n   The current design actually refuses to have multiple labels per OID,\n   but that could be fixed easily, as there is nothing preventing us\n   from declaring the `state.commit2label` to allow multiple values for\n   the same key. We would simply have to record together with a label\n   our wish to update a local branch under that name, e.g. by adding a\n   field `unsigned update_branch:1;` to `struct label_entry`, and then\n   adding the appropriate `label` commands.\n\n   This method would also have the advantage of adding the `label`\n   commands in the right location even in the `fixup!` case I\n   illustrated above: the `label` command would be added before the\n   `fixup` command gets reordered.\n\n   And it would probably make more sense to _not_ require that `exec git\n   update-ref [...]` line at the end of the todo list, but to perform\n   the updates transparently similarly to how the `refs/rewritten/` refs\n   are removed at the end of an interactive rebase.\n\n> +\t\t}\n> +\t}\n> +\n> +\tFREE_AND_NULL(todo_list->items);\n> +\ttodo_list->items = items;\n> +\ttodo_list->nr = nr;\n> +\ttodo_list->alloc = alloc;\n> +}\n> +\n>  static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,\n>  \t\t\t\tstruct strbuf *buf, int num, unsigned flags)\n>  {\n> [...]\n> diff --git a/t/t3431-rebase-update-branches.sh b/t/t3431-rebase-update-branches.sh\n> new file mode 100755\n> index 0000000..221c25d\n> --- /dev/null\n> +++ b/t/t3431-rebase-update-branches.sh\n> @@ -0,0 +1,64 @@\n> +#!/bin/sh\n> +\n> +test_description='git rebase -i --update-branches\n\nHow about dropping that `-i`? I think this option should also work\nwithout an explicit `--interactive`.\n\n> +\n> +This test runs git rebase, moving branch refs that point to commits\n> +that are reapplied.\n> +\n> +Initial setup:\n> +\n> + A - B          (master)\n> +  |\\\n> +  |  C          (linear-early)\n> +  |    \\\n> +  |      D      (linear-late)\n> +  |\\\n> +  |  E          (feat-e)\n> +   \\   \\\n> +     F  |       (feat-f)\n> +       \\|\n> +         G      (interim)\n> +           \\\n> +             H  (my-dev)\n> +'\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup linear' '\n> +\ttest_commit A &&\n> +\ttest_commit B &&\n> +\tgit checkout -b linear-early A &&\n> +\ttest_commit C &&\n> +\tgit checkout -b linear-late &&\n> +\ttest_commit D\n> +'\n> +\n> +test_expect_success 'smoketest linear' '\n> +\tgit rebase --update-branches master\n> +'\n> +\n> +test_expect_success 'check linear' '\n> +\tgit rev-parse linear-early:B.t\n\nI'd like to suggest this command instead:\n\n\ttest_cmp_rev linear-early C\n\nThe `C` tag was created automatically by `test_commit C`, and the\n`test_cmp_rev` command has the advantage of helping diagnose regressions\nmuch better than `git rev-parse` would.\n\n> +'\n> +\n> +test_expect_success 'setup merge' '\n> +\tgit checkout -b feat-e A &&\n> +\ttest_commit E &&\n> +\tgit checkout -b feat-f A &&\n> +\ttest_commit F &&\n> +\tgit checkout -b interim &&\n> +\ttest_merge G feat-e &&\n> +\tgit checkout -b my-dev &&\n> +\ttest_commit H\n> +'\n> +\n> +test_expect_success 'smoketest merge' '\n> +\tgit rebase -r --update-branches master\n\nLet's try to make this work without `-r`, too.\n\n> +'\n> +\n> +test_expect_success 'check merge' '\n> +\tgit rev-parse feat-e:B.t &&\n> +\tgit rev-parse feat-f:B.t &&\n> +\tgit rev-parse interim:B.t\n\nLikewise, I would love to see `test_cmp_rev` calls here.\n\nThank you for working on this feature, this is exciting!\n\nCiao,\nJohannes\n\n> +'\n> +\n> +test_done\n> --\n> 2.7.4\n>\n>\n"},{"id":"382033","messageId":"20190907234146.1473-1-wh109@yahoo.com","threadId":"51790","inReplyTo":"20190903013959.GA40029@syl.lan","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-07T23:41:46Z","receivedAt":"2019-09-07T23:42:13Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Brian, thanks for looking. The only thing I can come up with to say about having\nlots of refs is that at least that part of this isn't brand new code. The part\nthat collects ref info uses the same routines as `git log --decorate`. Do you\nrecall how long that took in the repository with 80,000 refs?\n\nJunio, thanks for eyeballing. Let me know if some style violations remain.\n\nTaylor, thanks for coming up with a way to configure branches to be excluded. I\nhaven't worked on implementing that yet. Let's continue to watch for feedback if\npeople will desire this kind of control.\n\nUsing `exec git branch -f` is further discussed in the review, where there are\narguments for introducing a new todo command. I'll add some comments there when\nI post a new patch set. But if someone's in favor of accepting a version of this\nwithout that change, with the potential harm from this feature being low enough,\nI'm not opposed to that either.\n"},{"id":"382034","messageId":"20190907234305.1524-1-wh109@yahoo.com","threadId":"51790","inReplyTo":"713b055b-aaf2-4294-12d3-2941d53eac16@gmail.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-07T23:43:05Z","receivedAt":"2019-09-07T23:43:29Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Phillip, thanks for the comments. Indeed other branches that are not in the todo\nlist are still not updated.\n\nI'm collecting the arguments about deferring the branch updates and issue with\nworktrees for discussion along with Johannes's review.\n"},{"id":"382035","messageId":"20190907234413.1591-1-wh109@yahoo.com","threadId":"51790","inReplyTo":"nycvar.QRO.7.76.6.1909031345330.46@tvgsbejvaqbjf.bet","subject":"[PATCH v2] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-07T23:44:12Z","receivedAt":"2019-09-07T23:44:32Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Everyone in this thread, thanks for your support and encouragement.\n\nJohannes, thanks for reviewing.\n\n> Maybe `s/reapplied/rebased/`?\n\nOk. I've changed most occurrences, except in Documentation/git-rebase.txt, where\nthe term 'reapplied' is already in use.\n\n> drop this hunk and only keep the next one.\n\nI didn't know that. (Actually, assume this for most of these responses.)\nDropped, thanks.\n\n> It should not really imply `--interactive`, but `--rebase-merges`.\n\n`imply_interactive` doesn't fully switch on `--interactive`, i.e., causing the\neditor to open. It only selects the backend, which I think we're saying is the\nright thing. I've dropped the `-i` from the test description.\n\nAnd we don't really have to imply --rebase-merges, in case someone would prefer\nto linearize things, which who knows? Running that non-rebase-merges command in\nthe example scenario from my original post should give something like this:\n\n```\n A - B              (master)\n       \\\n         F          (feat-f)\n           \\ \n             E      (feat-e)\n               \\\n                 H  (my-dev)\n```\n\nSo for now I haven't moved the implementation into `make_script_with_merges`.\n\n> This loads also tags, correct? I am fairly certain that we don't want to\n> update tags here, but maybe the check for `DECORATION_REF_LOCAL` later\n> on already ensures that?\n\nRight on both points. This isn't as efficient as possible, since we're wasting\nthe work of loading tags and remote refs. Currently I don't know if the\nperformance is worth the maintainability cost of replicating most of the\n`load_ref_decorations` and `get_name_decoration` family of functions and global\nvariables though.\n\n> How about using `is_pick_or_similar()` instead?\n\nThat's the function I need. Although I'm not aware of anything that generates\n`edit` or `reword` commands before we'll call `todo_list_add_branch_updates`. I\nended up not needing further logic with `is_fixup`. See below for the new\nhandling of fixup chains.\n\n> Please use C-style /* ... */ comments, Git insists on not using\n> C++-style // comments.\n\nThanks for pointing that out. Changed.\n\n> your code is careful to take care of the scenario where\n> multiple local branches point to the pre-rebase `HEAD`. Good. Maybe you\n> want to test for that in the regression test, too?\n\nAh, yes I do. It is added, with `my-hotfixes` = `HEAD` -> `my-dev` in the test.\n\n> However, you have two `if` conditions that both guard the same\n> operation: `continue`. How about combining the combinations? It's like\n> saying: under these circumstances, we skip adding a command.\n\nOk. Combined.\n\n> [several ways to simplify how we build todo items]\n\nI had only looked at how `todo_list_add_exec_commands` works. Let's try doing it\nwith a full `struct todo_list` and `parse_insn_line`. Thanks for posting these\nsuggestions.\n\n> [handling `fixup`/`squash` chains]\n\nI've moved `todo_list_add_branch_updates` to run before\n`todo_list_rearrange_squash`. The rearranging pulls fixups out, causing the\nbranch update to \"fall\" onto the items before, and reinserts them between a\ncommit and its branch update, casing them to be included in the updated branch.\nwhich is my opinion of the right thing to do. I've added a test about this with\nthe following scenario:\n\n```\n A - B  (master)\n   \\\n     I - J - fixup! I                 (fixup-early)\n\t\t      \\\n\t\t\tK - fixup! J  (fixup-late)\n```\n\nwhich results in the following todo list with `--autosquash`:\n\n```\npick 9eadc32 I\nfixup 265fa32 fixup! I\npick a0754fc J\nfixup e7d1999 fixup! J\nexec git branch -f fixup-early\npick c8bc4af K\n```\n\n> I'd like to suggest [`test_cmp_rev`] instead\n\nI've updated the test to use `test_cmp_rev`. It's not with your suggested\ninvocation though. We don't update the `C` tag. I've referred to the rebased `C`\nwith `test_cmp_rev linear-early HEAD^` and similar for the other checks.\n\n* * *\n\nAnd then there's the discussion about using `exec git branch -f`. To summarize\nthe issues collected from the entire thread:\n\n1. the changes aren't atomically applied at the end of the rebase\n2. it fails when the branch is checked out in a worktree\n3. it clobbers the branch if anything else updates it during the rebase\n4. the way we prepare the unprefixed branch doesn't work right some exotic cases\n5. the reflog message it leaves is uninformative\n\nFor #4, I think we've lucked out actually. The `load_ref_decorations` routine we\nuse determines that a ref is `DECORATION_REF_LOCAL` under the condition\n`starts_with(refname, \"refs/heads/\")` (log-tree.c:114, add_ref_decoration), so\n`prettify_refname` will find the prefix and skip it. But that's an invariant\nmaintained by two pieces of code pretty far away from each other.\n\nFor #5, for the convenience of readers, the reflog entry it leaves looks like this:\n\n```\n00873f2 feat-e@{0}: branch: Reset to HEAD\n```\n\nNot great.\n\nI haven't made any changes to this yet, but I've thought about what I want. My\nfavorite so far is to add a new todo command that just does everything right. It\nwould make a temparary ref `refs/rewritten-heads/xxx` (or something), and update\n`refs/heads/xxx` at the end.\n\nI agree that requiring a separate update-ref step at the end of the todo list is\nunfriendly. Manually putting in some branch update commands and then realizing\nthat they weren't applied would be extremely frustrating. I don't see the option\nof using existing tools as the easiest-to-use solution.\n\nI'm reluctant to combine this with the existing `label` command. So far it\nsounds like we generally want to be more willing to skip branch updates while\nperforming the rebase, with aforementioned scenarios where something else\nupdates the branch before we do, or if the branch becomes checked out in a\nworktree. We don't want to mess up the structure of a `rebase -r` as a result of\nskipping some branch updates. I think it would be conceptually simpler and\nimplementation-wise less tricky if we didn't combine it with the `label` and\n`reset` system.\n\nWarren He (1):\n  rebase: introduce --update-branches option\n\n Documentation/git-rebase.txt      |  9 ++++\n builtin/rebase.c                  | 11 ++++-\n sequencer.c                       | 58 +++++++++++++++++++++++-\n sequencer.h                       |  6 ++-\n t/t3431-rebase-update-branches.sh | 94 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 173 insertions(+), 5 deletions(-)\n create mode 100755 t/t3431-rebase-update-branches.sh\n\n-- \n2.7.4\n\n"},{"id":"382036","messageId":"20190907234413.1591-2-wh109@yahoo.com","threadId":"51790","inReplyTo":"20190907234413.1591-1-wh109@yahoo.com","subject":"[PATCH v2] rebase: introduce --update-branches option","fromName":"Warren He","fromEmail":"pickydaemon@gmail.com","sentAt":"2019-09-07T23:44:13Z","receivedAt":"2019-09-07T23:44:38Z","isPatch":true,"sender":{"key":"pickydaemon@gmail.com","avatar":null},"body":"Rebasing normally updates the current branch to the rewritten version.\nIf any other branches point to commits rewritten along the way, those\nremain untouched. This commit adds an `--update-branches` option, which\ninstructs the command to update any such branches that it encounters to\npoint to the rewritten versions of those commits.\n\nSigned-off-by: Warren He <wh109@yahoo.com>\n---\n Documentation/git-rebase.txt      |  9 ++++\n builtin/rebase.c                  | 11 ++++-\n sequencer.c                       | 58 +++++++++++++++++++++++-\n sequencer.h                       |  6 ++-\n t/t3431-rebase-update-branches.sh | 94 +++++++++++++++++++++++++++++++++++++++\n 5 files changed, 173 insertions(+), 5 deletions(-)\n create mode 100755 t/t3431-rebase-update-branches.sh\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 6156609..b650a8f 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -246,6 +246,13 @@ leave out at most one of A and B, in which case it defaults to HEAD.\n +\n See also INCOMPATIBLE OPTIONS below.\n \n+--update-branches::\n+\tIf there are branch refs that point to commits that will be\n+\treapplied, add shell commands to the todo list to update those\n+\trefs to point to the commits in the final history.\n++\n+See also INCOMPATIBLE OPTIONS below.\n+\n --allow-empty-message::\n \tBy default, rebasing commits with an empty message will fail.\n \tThis option overrides that behavior, allowing commits with empty\n@@ -535,6 +542,7 @@ are incompatible with the following options:\n  * --interactive\n  * --exec\n  * --keep-empty\n+ * --update-branches\n  * --edit-todo\n  * --root when used in combination with --onto\n \n@@ -543,6 +551,7 @@ In addition, the following pairs of options are incompatible:\n  * --preserve-merges and --interactive\n  * --preserve-merges and --signoff\n  * --preserve-merges and --rebase-merges\n+ * --preserve-merges and --update-branches\n  * --rebase-merges and --strategy\n  * --rebase-merges and --strategy-option\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 670096c..ab2308c 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -78,6 +78,7 @@ struct rebase_options {\n \tint signoff;\n \tint allow_rerere_autoupdate;\n \tint keep_empty;\n+\tint update_branches;\n \tint autosquash;\n \tchar *gpg_sign_opt;\n \tint autostash;\n@@ -349,8 +350,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)\n \n \t\tsplit_exec_commands(opts->cmd, &commands);\n \t\tret = complete_action(the_repository, &replay, flags,\n-\t\t\tshortrevisions, opts->onto_name, opts->onto, head_hash,\n-\t\t\t&commands, opts->autosquash, &todo_list);\n+\t\t\tshortrevisions, opts->onto_name, opts->onto, opts->head_name,\n+\t\t\thead_hash, &commands, opts->autosquash, &todo_list);\n \t}\n \n \tstring_list_clear(&commands, 0);\n@@ -375,6 +376,7 @@ static int run_rebase_interactive(struct rebase_options *opts,\n \tflags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;\n \tflags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;\n \tflags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;\n+\tflags |= opts->update_branches ? TODO_LIST_UPDATE_BRANCHES : 0;\n \n \tswitch (command) {\n \tcase ACTION_NONE: {\n@@ -1453,6 +1455,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tOPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),\n \t\tOPT_BOOL('k', \"keep-empty\", &options.keep_empty,\n \t\t\t N_(\"preserve empty commits during rebase\")),\n+\t\tOPT_BOOL(0, \"update-branches\", &options.update_branches,\n+\t\t\t N_(\"update branches that point to rebased commits\")),\n \t\tOPT_BOOL(0, \"autosquash\", &options.autosquash,\n \t\t\t N_(\"move commits that begin with \"\n \t\t\t    \"squash!/fixup! under -i\")),\n@@ -1710,6 +1714,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.keep_empty)\n \t\timply_interactive(&options, \"--keep-empty\");\n \n+\tif (options.update_branches)\n+\t\timply_interactive(&options, \"--update-branches\");\n+\n \tif (gpg_sign) {\n \t\tfree(options.gpg_sign_opt);\n \t\toptions.gpg_sign_opt = xstrfmt(\"-S%s\", gpg_sign);\ndiff --git a/sequencer.c b/sequencer.c\nindex 34ebf8e..0fe8452 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4901,6 +4901,59 @@ void todo_list_add_exec_commands(struct todo_list *todo_list,\n \ttodo_list->alloc = alloc;\n }\n \n+/*\n+ * Add commands to update branch refs after the todo list would pick a commit\n+ * that a branch ref points to.\n+ */\n+static void todo_list_add_branch_updates(struct repository *r,\n+\t\t\t\t\t struct todo_list *todo_list,\n+\t\t\t\t\t const char *head_name)\n+{\n+\tstruct strbuf *buf = &todo_list->buf;\n+\t/* watch out: items in here point into todo_list's buf, not its own */\n+\tstruct todo_list new_list = TODO_LIST_INIT;\n+\tint i;\n+\n+\tload_ref_decorations(NULL, 0);\n+\n+\tfor (i = 0; i < todo_list->nr; i++) {\n+\t\tconst struct todo_item *item = todo_list->items + i;\n+\t\tenum todo_command command = item->command;\n+\t\tconst struct name_decoration *decoration;\n+\n+\t\t*append_new_todo(&new_list) = todo_list->items[i];\n+\n+\t\tif (!(is_pick_or_similar(command) || command == TODO_MERGE))\n+\t\t\tcontinue;\n+\n+\t\tdecoration = get_name_decoration(&item->commit->object);\n+\t\tfor (; decoration; decoration = decoration->next) {\n+\t\t\tsize_t base_offset;\n+\n+\t\t\t/*\n+\t\t\t * (i)  skip other refs like tags and remote refs\n+\t\t\t * (ii) rebase itself will update the current branch\n+\t\t\t *      for us\n+\t\t\t */\n+\t\t\tif (decoration->type != DECORATION_REF_LOCAL ||\n+\t\t\t    !strcmp(decoration->name, head_name))\n+\t\t\t\tcontinue;\n+\n+\t\t\tbase_offset = buf->len;\n+\t\t\tstrbuf_addf(buf, \"exec git branch -f %s\\n\",\n+\t\t\t\t    prettify_refname(decoration->name));\n+\t\t\tparse_insn_line(r, append_new_todo(&new_list), buf->buf,\n+\t\t\t\t\tbuf->buf + base_offset,\n+\t\t\t\t\tbuf->buf + buf->len - 1);\n+\t\t}\n+\t}\n+\n+\tSWAP(new_list.items, todo_list->items);\n+\tSWAP(new_list.nr, todo_list->nr);\n+\tSWAP(new_list.alloc, todo_list->alloc);\n+\ttodo_list_release(&new_list);\n+}\n+\n static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,\n \t\t\t\tstruct strbuf *buf, int num, unsigned flags)\n {\n@@ -5051,7 +5104,7 @@ static int skip_unnecessary_picks(struct repository *r,\n \n int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,\n \t\t    const char *shortrevisions, const char *onto_name,\n-\t\t    struct commit *onto, const char *orig_head,\n+\t\t    struct commit *onto, const char *head_name, const char *orig_head,\n \t\t    struct string_list *commands, unsigned autosquash,\n \t\t    struct todo_list *todo_list)\n {\n@@ -5070,6 +5123,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla\n \t\titem->arg_len = item->arg_offset = item->flags = item->offset_in_buf = 0;\n \t}\n \n+\tif (flags & TODO_LIST_UPDATE_BRANCHES)\n+\t\ttodo_list_add_branch_updates(r, todo_list, head_name);\n+\n \tif (autosquash && todo_list_rearrange_squash(todo_list))\n \t\treturn -1;\n \ndiff --git a/sequencer.h b/sequencer.h\nindex 6704acb..69c6f71 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -143,6 +143,7 @@ int sequencer_remove_state(struct replay_opts *opts);\n  */\n #define TODO_LIST_REBASE_COUSINS (1U << 4)\n #define TODO_LIST_APPEND_TODO_HELP (1U << 5)\n+#define TODO_LIST_UPDATE_BRANCHES (1U << 6)\n \n int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,\n \t\t\t  const char **argv, unsigned flags);\n@@ -152,8 +153,9 @@ void todo_list_add_exec_commands(struct todo_list *todo_list,\n int check_todo_list_from_file(struct repository *r);\n int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,\n \t\t    const char *shortrevisions, const char *onto_name,\n-\t\t    struct commit *onto, const char *orig_head, struct string_list *commands,\n-\t\t    unsigned autosquash, struct todo_list *todo_list);\n+\t\t    struct commit *onto, const char *head_name, const char *orig_head,\n+\t\t    struct string_list *commands, unsigned autosquash,\n+\t\t    struct todo_list *todo_list);\n int todo_list_rearrange_squash(struct todo_list *todo_list);\n \n /*\ndiff --git a/t/t3431-rebase-update-branches.sh b/t/t3431-rebase-update-branches.sh\nnew file mode 100755\nindex 0000000..b57b64a\n--- /dev/null\n+++ b/t/t3431-rebase-update-branches.sh\n@@ -0,0 +1,94 @@\n+#!/bin/sh\n+\n+test_description='git rebase --update-branches\n+\n+This test runs git rebase, updating branch refs that point to commits\n+that are rebased.\n+\n+Initial setup:\n+\n+ A - B  (master)\n+  |\\\n+  |  C      (linear-early)\n+  |    \\\n+  |      D  (linear-late)\n+  |\\\n+  |  E          (feat-e)\n+  |\\   \\\n+  |  F  |       (feat-f)\n+  |    \\|\n+  |      G      (interim)\n+  |        \\\n+  |          H  (my-dev, my-hotfixes)\n+   \\\n+     I - J - fixup! I                 (fixup-early)\n+\t\t      \\\n+\t\t\tK - fixup! J  (fixup-late)\n+'\n+. ./test-lib.sh\n+\n+test_expect_success 'set up common' '\n+\ttest_commit A &&\n+\ttest_commit B\n+'\n+\n+test_expect_success 'set up linear' '\n+\tgit checkout -b linear-early A &&\n+\ttest_commit C &&\n+\tgit checkout -b linear-late &&\n+\ttest_commit D\n+'\n+\n+test_expect_success 'smoketest linear' '\n+\tgit rebase --update-branches master\n+'\n+\n+test_expect_success 'check linear' '\n+\ttest_cmp_rev linear-early HEAD^\n+'\n+\n+test_expect_success 'set up merge' '\n+\tgit checkout -b feat-e A &&\n+\ttest_commit E &&\n+\tgit checkout -b feat-f A &&\n+\ttest_commit F &&\n+\tgit checkout -b interim &&\n+\ttest_merge G feat-e &&\n+\tgit checkout -b my-dev &&\n+\ttest_commit H &&\n+\tgit branch my-hotfixes\n+'\n+\n+test_expect_success 'smoketest merge' '\n+\tgit rebase -r --update-branches master\n+'\n+\n+test_expect_success 'check merge' '\n+\ttest_cmp_rev feat-e HEAD^^2 &&\n+\ttest_cmp_rev feat-f HEAD^^ &&\n+\ttest_cmp_rev interim HEAD^ &&\n+\ttest_cmp_rev my-hotfixes HEAD\n+'\n+\n+test_expect_success 'set up fixup' '\n+\tgit checkout -b fixup-early A &&\n+\ttest_commit I &&\n+\ttest_commit J &&\n+\ttest_commit \"fixup! I\" I.t II fixup-I &&\n+\tgit checkout -b fixup-late &&\n+\ttest_commit K &&\n+\ttest_commit \"fixup! J\" J.t JJ fixup-J\n+'\n+\n+test_expect_success 'smoketest fixup' '\n+\tgit rebase -i --autosquash --update-branches master\n+'\n+\n+test_expect_success 'check fixup' '\n+\ttest_cmp_rev fixup-early HEAD^ &&\n+\ttest_cmp_rev fixup-early^:I.t fixup-I:I.t &&\n+\ttest_cmp_rev fixup-early:J.t fixup-J:J.t &&\n+\ttest_cmp_rev HEAD:K.t K:K.t\n+'\n+\n+test_done\n-- \n2.7.4\n\n"},{"id":"382051","messageId":"20190908180400.GM11334@genre.crustytoothpaste.net","threadId":"51790","inReplyTo":"20190907234146.1473-1-wh109@yahoo.com","subject":"Re: [PATCH] rebase: introduce --update-branches option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-09-08T18:04:00Z","receivedAt":"2019-09-08T18:04:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-09-07 at 23:41:46, Warren He wrote:\n> Brian, thanks for looking. The only thing I can come up with to say about having\n> lots of refs is that at least that part of this isn't brand new code. The part\n> that collects ref info uses the same routines as `git log --decorate`. Do you\n> recall how long that took in the repository with 80,000 refs?\n\nIt worked, but some colleagues at the time turned it off because it was\nat the threshold of annoying.  Overall, it didn't perform so badly that\nI think it would be a huge problem here, considering this is usually an\ninteractive operation, and I'm fine with adopting this solution for now\nand then focusing on optimizing a bit more in the future.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"382061","messageId":"31a37eb1-8a75-40f7-7d1c-a8b7b9d75f92@gmail.com","threadId":"51790","inReplyTo":"20190907234413.1591-1-wh109@yahoo.com","subject":"Re: [PATCH v2] rebase: introduce --update-branches option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-09-09T10:44:47Z","receivedAt":"2019-09-09T10:44:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Warren\n\nOn 08/09/2019 00:44, Warren He wrote:\n> Everyone in this thread, thanks for your support and encouragement.\n> [...]\n>> It should not really imply `--interactive`, but `--rebase-merges`.\n> \n> `imply_interactive` doesn't fully switch on `--interactive`, i.e., causing the\n> editor to open. It only selects the backend, which I think we're saying is the\n> right thing. I've dropped the `-i` from the test description.\n> \n> And we don't really have to imply --rebase-merges, in case someone would prefer\n> to linearize things, which who knows? Running that non-rebase-merges command in\n> the example scenario from my original post should give something like this:\n\nI think it would probably be less confusing to default to preserving \nmerges and having an option to turn that off - people are going to be \nsurprised if their history is linearized.\n\n> ```\n>   A - B              (master)\n>         \\\n>           F          (feat-f)\n>             \\\n>               E      (feat-e)\n>                 \\\n>                   H  (my-dev)\n> ```\n> \n> So for now I haven't moved the implementation into `make_script_with_merges`.\n>\n > [...]>\n>> [handling `fixup`/`squash` chains]\n> \n> I've moved `todo_list_add_branch_updates` to run before\n> `todo_list_rearrange_squash`. The rearranging pulls fixups out, causing the\n> branch update to \"fall\" onto the items before, and reinserts them between a\n> commit and its branch update, casing them to be included in the updated branch.\n> which is my opinion of the right thing to do. I've added a test about this with\n> the following scenario:\n> \n> ```\n>   A - B  (master)\n>     \\\n>       I - J - fixup! I                 (fixup-early)\n> \t\t      \\\n> \t\t\tK - fixup! J  (fixup-late)\n> ```\n> \n> which results in the following todo list with `--autosquash`:\n> \n> ```\n> pick 9eadc32 I\n> fixup 265fa32 fixup! I\n> pick a0754fc J\n> fixup e7d1999 fixup! J\n> exec git branch -f fixup-early\n> pick c8bc4af K\n> ```\n\nThat makes sense I think\n\n>> I'd like to suggest [`test_cmp_rev`] instead\n> \n> I've updated the test to use `test_cmp_rev`. It's not with your suggested\n> invocation though. We don't update the `C` tag. I've referred to the rebased `C`\n> with `test_cmp_rev linear-early HEAD^` and similar for the other checks.\n\nThat sounds right\n\n> * * *\n> \n> And then there's the discussion about using `exec git branch -f`. To summarize\n> the issues collected from the entire thread:\n> \n> 1. the changes aren't atomically applied at the end of the rebase\n> 2. it fails when the branch is checked out in a worktree\n> 3. it clobbers the branch if anything else updates it during the rebase\n> 4. the way we prepare the unprefixed branch doesn't work right some exotic cases\n> 5. the reflog message it leaves is uninformative\n> \n> For #4, I think we've lucked out actually. The `load_ref_decorations` routine we\n> use determines that a ref is `DECORATION_REF_LOCAL` under the condition\n> `starts_with(refname, \"refs/heads/\")` (log-tree.c:114, add_ref_decoration), so\n> `prettify_refname` will find the prefix and skip it. But that's an invariant\n> maintained by two pieces of code pretty far away from each other.\n> \n> For #5, for the convenience of readers, the reflog entry it leaves looks like this:\n> \n> ```\n> 00873f2 feat-e@{0}: branch: Reset to HEAD\n> ```\n> \n> Not great.\n> \n> I haven't made any changes to this yet, but I've thought about what I want. My\n> favorite so far is to add a new todo command that just does everything right. It\n> would make a temparary ref `refs/rewritten-heads/xxx` (or something), and update\n> `refs/heads/xxx` at the end.\n\nI think that's the best way to do it. If we had a command like 'branch \n<branch-name>' that creates a ref to remember the current HEAD and saves \nthe current branch head. Then at the end rebase can update the branches \nto point to the saved commits if the branch is unchanged. If the rebase \nis aborted then we don't end up with some branches updated and others not.\n\nSide Note\n   I'd avoid creating another worktree local ref refs/rewritten-heads/.\n   Either store them under refs/rewritten/ or refs/worktree/\n\nBest Wishes\n\nPhillip\n\n> \n> I agree that requiring a separate update-ref step at the end of the todo list is\n> unfriendly. Manually putting in some branch update commands and then realizing\n> that they weren't applied would be extremely frustrating. I don't see the option\n> of using existing tools as the easiest-to-use solution.\n> \n> I'm reluctant to combine this with the existing `label` command. So far it\n> sounds like we generally want to be more willing to skip branch updates while\n> performing the rebase, with aforementioned scenarios where something else\n> updates the branch before we do, or if the branch becomes checked out in a\n> worktree. We don't want to mess up the structure of a `rebase -r` as a result of\n> skipping some branch updates. I think it would be conceptually simpler and\n> implementation-wise less tricky if we didn't combine it with the `label` and\n> `reset` system.\n> \n> Warren He (1):\n>    rebase: introduce --update-branches option\n> \n>   Documentation/git-rebase.txt      |  9 ++++\n>   builtin/rebase.c                  | 11 ++++-\n>   sequencer.c                       | 58 +++++++++++++++++++++++-\n>   sequencer.h                       |  6 ++-\n>   t/t3431-rebase-update-branches.sh | 94 +++++++++++++++++++++++++++++++++++++++\n>   5 files changed, 173 insertions(+), 5 deletions(-)\n>   create mode 100755 t/t3431-rebase-update-branches.sh\n> \n"},{"id":"382065","messageId":"nycvar.QRO.7.76.6.1909091605540.5377@tvgsbejvaqbjf.bet","threadId":"51790","inReplyTo":"31a37eb1-8a75-40f7-7d1c-a8b7b9d75f92@gmail.com","subject":"Re: [PATCH v2] rebase: introduce --update-branches option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-09-09T14:13:16Z","receivedAt":"2019-09-09T14:13:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 9 Sep 2019, Phillip Wood wrote:\n\n> On 08/09/2019 00:44, Warren He wrote:\n> > Everyone in this thread, thanks for your support and encouragement.\n> > [...]\n> > > It should not really imply `--interactive`, but `--rebase-merges`.\n> >\n> > `imply_interactive` doesn't fully switch on `--interactive`, i.e., causing\n> > the\n> > editor to open. It only selects the backend, which I think we're saying is\n> > the\n> > right thing. I've dropped the `-i` from the test description.\n> >\n> > And we don't really have to imply --rebase-merges, in case someone would\n> > prefer\n> > to linearize things, which who knows? Running that non-rebase-merges command\n> > in\n> > the example scenario from my original post should give something like this:\n>\n> I think it would probably be less confusing to default to preserving merges\n\ns/preserving/rebasing/?\n\n> and having an option to turn that off - people are going to be surprised if\n> their history is linearized.\n\nI don't think it makes any sense to linearize the history while updating\nbranches, as the commits will be all jumbled up. Imagine this history:\n\n\t- A - B - C - D -\n\t    \\       /\n\t      E - F\n\nTypically, it does not elicit any \"bug\" reports, but this can easily be\nlinearized to\n\n\t- A' - B' - E' - C' - F' -\n\nIn my mind, it makes no sense to update any local branches that pointed\nto C and F to point to C' and F', respectively.\n\n> [...]\n> > * * *\n> >\n> > And then there's the discussion about using `exec git branch -f`. To\n> > summarize\n> > the issues collected from the entire thread:\n> >\n> > 1. the changes aren't atomically applied at the end of the rebase\n> > 2. it fails when the branch is checked out in a worktree\n> > 3. it clobbers the branch if anything else updates it during the rebase\n> > 4. the way we prepare the unprefixed branch doesn't work right some exotic\n> > cases\n> > 5. the reflog message it leaves is uninformative\n> >\n> > For #4, I think we've lucked out actually. The `load_ref_decorations`\n> > routine we\n> > use determines that a ref is `DECORATION_REF_LOCAL` under the condition\n> > `starts_with(refname, \"refs/heads/\")` (log-tree.c:114, add_ref_decoration),\n> > so\n> > `prettify_refname` will find the prefix and skip it. But that's an invariant\n> > maintained by two pieces of code pretty far away from each other.\n> >\n> > For #5, for the convenience of readers, the reflog entry it leaves looks\n> > like this:\n> >\n> > ```\n> > 00873f2 feat-e@{0}: branch: Reset to HEAD\n> > ```\n> >\n> > Not great.\n> >\n> > I haven't made any changes to this yet, but I've thought about what I want.\n> > My\n> > favorite so far is to add a new todo command that just does everything\n> > right. It\n> > would make a temparary ref `refs/rewritten-heads/xxx` (or something), and\n> > update\n> > `refs/heads/xxx` at the end.\n>\n> I think that's the best way to do it. If we had a command like 'branch\n> <branch-name>' that creates a ref to remember the current HEAD and saves the\n> current branch head. Then at the end rebase can update the branches to point\n> to the saved commits if the branch is unchanged. If the rebase is aborted then\n> we don't end up with some branches updated and others not.\n\nI'd avoid cluttering the space with more commands. For `branch`, for\nexample, the natural short command would be `b`, but that already means\n`break`.\n\nIn contrast, I would think that\n\n\tlabel --update-branch my-side-track\n\nwould make for a nicer read than\n\n\tlabel my-side-track\n\tbranch my-side-track\n\nOf course, it would be a lot harder to bring back the safety of `git\nupdate-ref`'s `<old-revision>` safe-guard, in either forms.\n\nAnd of course, the first form would _require_ the logic to be moved to\n`make_script_with_merges()` because we could not otherwise guarantee\nthat the labels corresponding to local branch names aren't already in\nuse, for different commits.\n\n> Side Note\n>   I'd avoid creating another worktree local ref refs/rewritten-heads/.\n>   Either store them under refs/rewritten/ or refs/worktree/\n\nYep.\n\nCiao,\nDscho\n"},{"id":"382082","messageId":"xmqqwoehjq5h.fsf@gitster-ct.c.googlers.com","threadId":"51790","inReplyTo":"nycvar.QRO.7.76.6.1909091605540.5377@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2] rebase: introduce --update-branches option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-09T18:11:38Z","receivedAt":"2019-09-09T18:11:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> In contrast, I would think that\n>\n> \tlabel --update-branch my-side-track\n>\n> would make for a nicer read than\n>\n> \tlabel my-side-track\n> \tbranch my-side-track\n\nBecause labelling while recreating a mergey history is conceptually\nthe same as temporarily updating the branch head from end users'\npoint of view, so this sounds quite sensible.\n"},{"id":"382757","messageId":"6a440f3e-611e-c00d-9b4d-1f7e2fea853e@gmail.com","threadId":"51790","inReplyTo":"nycvar.QRO.7.76.6.1909091605540.5377@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2] rebase: introduce --update-branches option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-09-23T09:40:30Z","receivedAt":"2019-09-23T09:40:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi\n\nOn 09/09/2019 15:13, Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 9 Sep 2019, Phillip Wood wrote:\n> \n>> On 08/09/2019 00:44, Warren He wrote:\n>>> Everyone in this thread, thanks for your support and encouragement.\n>>> [...]\n>>>> It should not really imply `--interactive`, but `--rebase-merges`.\n>>>\n>>> `imply_interactive` doesn't fully switch on `--interactive`, i.e., causing\n>>> the\n>>> editor to open. It only selects the backend, which I think we're saying is\n>>> the\n>>> right thing. I've dropped the `-i` from the test description.\n>>>\n>>> And we don't really have to imply --rebase-merges, in case someone would\n>>> prefer\n>>> to linearize things, which who knows? Running that non-rebase-merges command\n>>> in\n>>> the example scenario from my original post should give something like this:\n>>\n>> I think it would probably be less confusing to default to preserving merges\n> \n> s/preserving/rebasing/?\n> \n>> and having an option to turn that off - people are going to be surprised if\n>> their history is linearized.\n> \n> I don't think it makes any sense to linearize the history while updating\n> branches, as the commits will be all jumbled up. Imagine this history:\n> \n> \t- A - B - C - D -\n> \t    \\       /\n> \t      E - F\n> \n> Typically, it does not elicit any \"bug\" reports, but this can easily be\n> linearized to\n> \n> \t- A' - B' - E' - C' - F' -\n> \n> In my mind, it makes no sense to update any local branches that pointed\n> to C and F to point to C' and F', respectively.\n> \n\nI agree\n\n>> [...]\n>>> * * *\n>>>\n>>> And then there's the discussion about using `exec git branch -f`. To\n>>> summarize\n>>> the issues collected from the entire thread:\n>>>\n>>> 1. the changes aren't atomically applied at the end of the rebase\n>>> 2. it fails when the branch is checked out in a worktree\n>>> 3. it clobbers the branch if anything else updates it during the rebase\n>>> 4. the way we prepare the unprefixed branch doesn't work right some exotic\n>>> cases\n>>> 5. the reflog message it leaves is uninformative\n>>>\n>>> For #4, I think we've lucked out actually. The `load_ref_decorations`\n>>> routine we\n>>> use determines that a ref is `DECORATION_REF_LOCAL` under the condition\n>>> `starts_with(refname, \"refs/heads/\")` (log-tree.c:114, add_ref_decoration),\n>>> so\n>>> `prettify_refname` will find the prefix and skip it. But that's an invariant\n>>> maintained by two pieces of code pretty far away from each other.\n>>>\n>>> For #5, for the convenience of readers, the reflog entry it leaves looks\n>>> like this:\n>>>\n>>> ```\n>>> 00873f2 feat-e@{0}: branch: Reset to HEAD\n>>> ```\n>>>\n>>> Not great.\n>>>\n>>> I haven't made any changes to this yet, but I've thought about what I want.\n>>> My\n>>> favorite so far is to add a new todo command that just does everything\n>>> right. It\n>>> would make a temparary ref `refs/rewritten-heads/xxx` (or something), and\n>>> update\n>>> `refs/heads/xxx` at the end.\n>>\n>> I think that's the best way to do it. If we had a command like 'branch\n>> <branch-name>' that creates a ref to remember the current HEAD and saves the\n>> current branch head. Then at the end rebase can update the branches to point\n>> to the saved commits if the branch is unchanged. If the rebase is aborted then\n>> we don't end up with some branches updated and others not.\n> \n> I'd avoid cluttering the space with more commands. For `branch`, for\n> example, the natural short command would be `b`, but that already means\n> `break`.\n\nWe could just not have a short name, after all --update-branch is hardly \na short alternative\n\n> \n> In contrast, I would think that\n> \n> \tlabel --update-branch my-side-track\n> \n> would make for a nicer read than\n> \n> \tlabel my-side-track\n> \tbranch my-side-track\n\nI agree it would be nice to do both on a single line, my argument was \nmainly against using 'exec branch ...' so that we can defer the branch \nupdates until the very end of the rebase. The branch command could set a \nlabel as well or we could add an option to label I'm not that bothered \neither way at the moment. Another possibility which we probably don't \nwant is to have labels starting refs/ imply --update-branch\n\n> Of course, it would be a lot harder to bring back the safety of `git\n> update-ref`'s `<old-revision>` safe-guard, in either forms.\n\nIs it that difficult to write the current branch HEAD to a file when we \nlabel it and then read those back at the end when we update the refs? or \nare you thinking of calling 'git branch' instead of \nref_transaction_update()? I'm not sure what the advantage of 'git \nbranch' is though.\n\nBest Wishes\n\nPhillip\n\n> \n> And of course, the first form would _require_ the logic to be moved to\n> `make_script_with_merges()` because we could not otherwise guarantee\n> that the labels corresponding to local branch names aren't already in\n> use, for different commits.\n> \n>> Side Note\n>>    I'd avoid creating another worktree local ref refs/rewritten-heads/.\n>>    Either store them under refs/rewritten/ or refs/worktree/\n> \n> Yep.\n> \n> Ciao,\n> Dscho\n> \n"}]}