{"thread":{"id":"59448","subject":"[RFC PATCH] rebase: implement --rewind","startedAt":"2023-03-23T16:47:21Z","lastAt":"2023-04-11T10:07:11Z","messageCount":11,"participants":["Oswald Buddenhagen","Johannes Schindelin","Phillip Wood","Ævar Arnfjörð Bjarmason","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"474000","messageId":"20230323162235.995645-1-oswald.buddenhagen@gmx.de","threadId":"59448","inReplyTo":null,"subject":"[RFC PATCH] rebase: implement --rewind","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-03-23T16:22:35Z","receivedAt":"2023-03-23T16:47:21Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This is fundamentally --edit-todo, except that we first prepend the\nalready applied commits and reset back to `onto`. This is useful when\none finds that a prior change needs (further) modifications.\n\nThis patch implements \"flat\" rewind, that is, once the todo edit has\nbeen committed, one can abort only the complete rebase. The pre-rewind\nposition is marked with a `break` command (these pile up when rewinding\nmultiple times; the user is expected to clean them up as necessary).\n\nAn alternative to that would be \"nested\" rewind, where one can return to\nthe pre-rewind state even after committing the todo edit. However, this:\n- would add somewhat significant complexity due to having to maintain a\n  stack of todos and HEADs\n- would be mildly confusing to use due to needing to track the state of\n  the stack. One could simplify this somewhat by hiding the rest of the\n  previous todo before nesting, but this would be somewhat limiting in\n  turn (one might want to defer a factored out hunk, and stashing it is\n  not necessarily the most elegant way to do it).\n- would be of somewhat limited usefulness, speaking from experience\n\nThis patch leaves transitive resolution of rewritten-list to the\nconsumer. This is probably a bad idea.\nSomewhat related to that, --update-refs isn't properly handled yet.\n\nReference: <YhPiqlM81XCjNWpk@ugly>\nSigned-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>\n---\n Documentation/git-rebase.txt  |  14 ++++-\n builtin/rebase.c              |  98 ++++++++++++++++++++++++++++--\n rebase-interactive.c          |  34 ++++++++++-\n rebase-interactive.h          |   2 +\n sequencer.c                   |  37 +++++++++---\n sequencer.h                   |   3 +\n t/t3404-rebase-interactive.sh | 111 ++++++++++++++++++++++++++++++++++\n 7 files changed, 281 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 9a295bcee4..f736131a6c 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -12,7 +12,8 @@ SYNOPSIS\n \t[--onto <newbase> | --keep-base] [<upstream> [<branch>]]\n 'git rebase' [-i | --interactive] [<options>] [--exec <cmd>] [--onto <newbase>]\n \t--root [<branch>]\n-'git rebase' (--continue | --skip | --abort | --quit | --edit-todo | --show-current-patch)\n+'git rebase' (--continue | --skip | --abort | --quit | --edit-todo | --rewind |\n+\t--show-current-patch)\n \n DESCRIPTION\n -----------\n@@ -215,7 +216,8 @@ The options in this section cannot be used with any other option,\n including not with each other:\n \n --continue::\n-\tRestart the rebasing process after having resolved a merge conflict.\n+\tRestart the rebasing process after an interruption, e.g. having\n+\tresolved a merge conflict.\n \n --skip::\n \tRestart the rebasing process by skipping the current patch.\n@@ -236,6 +238,10 @@ including not with each other:\n --edit-todo::\n \tEdit the todo list during an interactive rebase.\n \n+--rewind::\n+\tEdit the todo list during an interactive rebase, but first\n+\tprepend the commits on top of the new base and reset to it.\n+\n --show-current-patch::\n \tShow the current patch in an interactive rebase or when rebase\n \tis stopped because of conflicts. This is the equivalent of\n@@ -975,6 +981,10 @@ pick f4593f9 four\n exec make test\n --------------------\n \n+If during editing a commit you notice that an ancestor commit should be\n+actually edited first, you may use `git rebase --rewind` to restart the\n+interactive rebase without starting from scratch.\n+\n SPLITTING COMMITS\n -----------------\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 61e5363ac7..3a14ac1a4f 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -36,7 +36,8 @@ static char const * const builtin_rebase_usage[] = {\n \t\t\"[--onto <newbase> | --keep-base] [<upstream> [<branch>]]\"),\n \tN_(\"git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] \"\n \t\t\"--root [<branch>]\"),\n-\t\"git rebase --continue | --abort | --skip | --edit-todo\",\n+\t\"git rebase --continue | --abort | --quit | --skip | --edit-todo | \"\n+\t\t\"--rewind\",\n \tNULL\n };\n \n@@ -65,6 +66,8 @@ static const char *action_names[] = {\n \t\"abort\",\n \t\"quit\",\n \t\"edit_todo\",\n+\t\"rewind\",\n+\t\"resume_rewind\",\n \t\"show_current_patch\"\n };\n \n@@ -183,17 +186,21 @@ static int edit_todo_file(unsigned flags)\n \tconst char *todo_file = rebase_path_todo();\n \tstruct todo_list todo_list = TODO_LIST_INIT,\n \t\tnew_todo = TODO_LIST_INIT;\n+\tenum rebase_action action = file_exists(rebase_path_todo_orig()) ?\n+\t\t\t\tACTION_RESUME_REWIND : ACTION_EDIT_TODO;\n \tint res = 0;\n \n \tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0)\n \t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n \n \tstrbuf_stripspace(&todo_list.buf, 1);\n \tres = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags,\n-\t\t\t     ACTION_EDIT_TODO);\n-\tif (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file,\n-\t\t\t\t\t    NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS),\n-\t\t\t\t\t    ACTION_EDIT_TODO))\n+\t\t\t     action);\n+\tif (res == EDIT_TODO_ABORT)\n+\t\tres = error(_(\"rewind aborted; state restored\"));\n+\telse if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file,\n+\t\t\t\t\t\t NULL, NULL, -1,\n+\t\t\t\t\t\t flags & ~(TODO_LIST_SHORTEN_IDS), action))\n \t\tres = error_errno(_(\"could not write '%s'\"), todo_file);\n \n \ttodo_list_release(&todo_list);\n@@ -301,6 +308,64 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)\n \treturn ret;\n }\n \n+static int rewind_todo_file(struct rebase_options *opts,\n+\t\t\t    unsigned flags)\n+{\n+\tint ret;\n+\tchar *revisions;\n+\tconst char *todo_file = rebase_path_todo();\n+\tstruct strvec make_script_args = STRVEC_INIT;\n+\tstruct todo_list todo_list = TODO_LIST_INIT;\n+\tstruct replay_opts replay = get_replay_opts(opts);\n+\tstruct string_list commands = STRING_LIST_INIT_DUP;\n+\n+\trequire_clean_work_tree(the_repository,\n+\t\tN_(\"rewind rebase\"),\n+\t\t_(\"Please commit or stash them.\"), 1, 0);\n+\n+\tif (file_exists(rebase_path_todo_orig()))\n+\t\treturn error(_(\"you are already rewinding a rebase.\\n\"\n+\t\t\t       \"Use rebase --edit-todo to continue.\"));\n+\n+\trevisions = xstrfmt(\"%s..HEAD\", oid_to_hex(&opts->onto->object.oid));\n+\tstrvec_pushl(&make_script_args, \"\", revisions, NULL);\n+\tfree(revisions);\n+\n+\tret = sequencer_make_script(the_repository, &todo_list.buf,\n+\t\t\t\t    make_script_args.nr, make_script_args.v,\n+\t\t\t\t    flags);\n+\tstrvec_clear(&make_script_args);\n+\n+\tif (ret)\n+\t\terror(_(\"could not generate todo list\"));\n+\telse {\n+\t\tif (flags & TODO_LIST_ABBREVIATE_CMDS)\n+\t\t\tstrbuf_addstr(&todo_list.buf, \"b\\n\\n\");\n+\t\telse\n+\t\t\tstrbuf_addstr(&todo_list.buf, \"break\\n\\n\");\n+\n+\t\tif (strbuf_read_file(&todo_list.buf, todo_file, 0) < 0) {\n+\t\t\tstrbuf_release(&todo_list.buf);\n+\t\t\treturn error_errno(_(\"could not read '%s'.\"), todo_file);\n+\t\t}\n+\n+\t\tdiscard_index(&the_index);\n+\t\tif (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf,\n+\t\t\t\t\t\t&todo_list))\n+\t\t\tBUG(\"unusable todo list\");\n+\n+\t\tret = complete_action(the_repository, &replay, flags,\n+\t\t\tNULL, opts->onto_name, &opts->onto->object.oid,\n+\t\t\t&opts->orig_head->object.oid, &opts->exec,\n+\t\t\topts->autosquash, opts->update_refs, &todo_list,\n+\t\t\topts->action);\n+\t}\n+\n+\ttodo_list_release(&todo_list);\n+\n+\treturn ret;\n+}\n+\n static int run_sequencer_rebase(struct rebase_options *opts)\n {\n \tunsigned flags = 0;\n@@ -342,6 +407,9 @@ static int run_sequencer_rebase(struct rebase_options *opts)\n \tcase ACTION_EDIT_TODO:\n \t\tret = edit_todo_file(flags);\n \t\tbreak;\n+\tcase ACTION_REWIND:\n+\t\tret = rewind_todo_file(opts, flags);\n+\t\tbreak;\n \tcase ACTION_SHOW_CURRENT_PATCH: {\n \t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n \n@@ -1088,6 +1156,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t    N_(\"abort but keep HEAD where it is\"), ACTION_QUIT),\n \t\tOPT_CMDMODE(0, \"edit-todo\", &options.action, N_(\"edit the todo list \"\n \t\t\t    \"during an interactive rebase\"), ACTION_EDIT_TODO),\n+\t\tOPT_CMDMODE(0, \"rewind\", &options.action, N_(\"rewind an interactive \"\n+\t\t\t    \"rebase\"), ACTION_REWIND),\n \t\tOPT_CMDMODE(0, \"show-current-patch\", &options.action,\n \t\t\t    N_(\"show the patch file being applied or merged\"),\n \t\t\t    ACTION_SHOW_CURRENT_PATCH),\n@@ -1235,6 +1305,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif (options.action == ACTION_EDIT_TODO && !is_merge(&options))\n \t\tdie(_(\"The --edit-todo action can only be used during \"\n \t\t      \"interactive rebase.\"));\n+\telse if (options.action == ACTION_REWIND && !is_merge(&options))\n+\t\tdie(_(\"The --rewind action can only be used during \"\n+\t\t      \"interactive rebase.\"));\n \n \tif (trace2_is_enabled()) {\n \t\tif (is_merge(&options))\n@@ -1339,17 +1412,21 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tcase ACTION_EDIT_TODO:\n \t\toptions.dont_finish_rebase = 1;\n \t\tgoto run_rebase;\n+\tcase ACTION_REWIND:\n+\t\tif (read_basic_state(&options))\n+\t\t\texit(1);\n+\t\tbreak;\n \tcase ACTION_SHOW_CURRENT_PATCH:\n \t\toptions.dont_finish_rebase = 1;\n \t\tgoto run_rebase;\n \tcase ACTION_NONE:\n \t\tbreak;\n \tdefault:\n \t\tBUG(\"action: %d\", options.action);\n \t}\n \n \t/* Make sure no rebase is in progress */\n-\tif (in_progress) {\n+\tif (in_progress && options.action != ACTION_REWIND) {\n \t\tconst char *last_slash = strrchr(options.state_dir, '/');\n \t\tconst char *state_dir_base =\n \t\t\tlast_slash ? last_slash + 1 : options.state_dir;\n@@ -1570,6 +1647,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\toptions.flags |= REBASE_FORCE;\n \t}\n \n+\t// We branch off after handling any option that could usefully\n+\t// affect the re-creation of the todo list.\n+\t// The omission of --onto from that is debatable.\n+\t// Options that will be overwritten by read_basic_state() are\n+\t// meaningless, so we can branch out before processing these;\n+\t// though arguably, it should be possible to change some of them.\n+\tif (options.action == ACTION_REWIND)\n+\t\tgoto run_rebase;\n+\n \tif (!options.root) {\n \t\tif (argc < 1) {\n \t\t\tstruct branch *branch;\ndiff --git a/rebase-interactive.c b/rebase-interactive.c\nindex a3d8925b06..d72ac7b8d1 100644\n--- a/rebase-interactive.c\n+++ b/rebase-interactive.c\n@@ -87,6 +87,16 @@ void append_todo_help(int command_count, enum rebase_action action,\n \t\t\t\"of an ongoing interactive rebase.\\n\"\n \t\t\t\"To continue rebase after editing, run:\\n\"\n \t\t\t\"    git rebase --continue\\n\\n\");\n+\telse if (action == ACTION_REWIND)\n+\t\tmsg = _(\"\\nYou are rewinding \"\n+\t\t\t\"an ongoing interactive rebase.\\n\"\n+\t\t\t\"If you remove everything, \"\n+\t\t\t\"the todo file will be left unchanged.\\n\\n\");\n+\telse if (action == ACTION_RESUME_REWIND)\n+\t\tmsg = _(\"\\nYou are correcting the rewind of \"\n+\t\t\t\"an ongoing interactive rebase.\\n\"\n+\t\t\t\"If you remove everything, \"\n+\t\t\t\"the todo file will be restored.\\n\\n\");\n \telse\n \t\tmsg = _(\"\\nHowever, if you remove everything, \"\n \t\t\t\"the rebase will be aborted.\\n\\n\");\n@@ -101,9 +111,18 @@ enum edit_todo_result edit_todo_list(\n \t\t   enum rebase_action action)\n {\n \tconst char *todo_file = rebase_path_todo(),\n-\t\t*todo_backup = rebase_path_todo_backup();\n+\t\t*todo_backup = rebase_path_todo_backup(),\n+\t\t*todo_file_orig = rebase_path_todo_orig(),\n+\t\t*done_file = rebase_path_done(),\n+\t\t*done_file_orig = rebase_path_done_orig();\n \tint incorrect = 0;\n \n+\tif (action == ACTION_REWIND) {\n+\t\tif (rename(todo_file, todo_file_orig) ||\n+\t\t    rename(done_file, done_file_orig))\n+\t\t\treturn error_errno(_(\"cannot displace todo file\"));\n+\t}\n+\n \t/* If the user is editing the todo list, we first try to parse\n \t * it.  If there is an error, we do not return, because the user\n \t * might want to fix it in the first place. */\n@@ -127,8 +146,14 @@ enum edit_todo_result edit_todo_list(\n \t\treturn EDIT_TODO_FAILED;\n \n \tstrbuf_stripspace(&new_todo->buf, 1);\n-\tif (action != ACTION_EDIT_TODO && new_todo->buf.len == 0)\n+\tif (action != ACTION_EDIT_TODO && new_todo->buf.len == 0) {\n+\t\tif (action == ACTION_REWIND || action == ACTION_RESUME_REWIND) {\n+\t\t\tif (rename(todo_file_orig, todo_file) ||\n+\t\t\t    rename(done_file_orig, done_file))\n+\t\t\t\treturn error_errno(_(\"cannot restore todo file\"));\n+\t\t}\n \t\treturn EDIT_TODO_ABORT;\n+\t}\n \n \tif (todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo)) {\n \t\tfprintf(stderr, _(edit_todo_list_advice));\n@@ -148,6 +173,11 @@ enum edit_todo_result edit_todo_list(\n \t\treturn EDIT_TODO_INCORRECT;\n \t}\n \n+\tif (action == ACTION_REWIND) {\n+\t\tunlink(todo_file_orig);\n+\t\tunlink(done_file_orig);\n+\t}\n+\n \t/*\n \t * See if branches need to be added or removed from the update-refs\n \t * file based on the new todo list.\ndiff --git a/rebase-interactive.h b/rebase-interactive.h\nindex 5aa4111b4f..260dc7c53f 100644\n--- a/rebase-interactive.h\n+++ b/rebase-interactive.h\n@@ -12,6 +12,8 @@ enum rebase_action {\n \tACTION_ABORT,\n \tACTION_QUIT,\n \tACTION_EDIT_TODO,\n+\tACTION_REWIND,\n+\tACTION_RESUME_REWIND,\n \tACTION_SHOW_CURRENT_PATCH,\n \tACTION_LAST\n };\ndiff --git a/sequencer.c b/sequencer.c\nindex 0b4d16b8e8..0e1d92b238 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -62,15 +62,17 @@ static GIT_PATH_FUNC(rebase_path, \"rebase-merge\")\n  */\n GIT_PATH_FUNC(rebase_path_todo, \"rebase-merge/git-rebase-todo\")\n GIT_PATH_FUNC(rebase_path_todo_backup, \"rebase-merge/git-rebase-todo.backup\")\n+GIT_PATH_FUNC(rebase_path_todo_orig, \"rebase-merge/git-rebase-todo.orig\")\n \n GIT_PATH_FUNC(rebase_path_dropped, \"rebase-merge/dropped\")\n \n /*\n  * The rebase command lines that have already been processed. A line\n  * is moved here when it is first handled, before any associated user\n  * actions.\n  */\n-static GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n+GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n+GIT_PATH_FUNC(rebase_path_done_orig, \"rebase-merge/done.orig\")\n /*\n  * The file to keep track of how many commands were already processed (e.g.\n  * for the prompt).\n@@ -6113,7 +6115,10 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla\n \tstruct strbuf *buf = &todo_list->buf;\n \tint res;\n \n-\tfind_unique_abbrev_r(shortonto, onto, DEFAULT_ABBREV);\n+\tif (action == ACTION_NONE)\n+\t\tfind_unique_abbrev_r(shortonto, onto, DEFAULT_ABBREV);\n+\telse if (read_populate_opts(opts))\n+\t\treturn -1;\n \n \tif (buf->len == 0) {\n \t\tstruct todo_item *item = append_new_todo(todo_list);\n@@ -6143,11 +6148,20 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla\n \tif (res == EDIT_TODO_IOERROR)\n \t\treturn -1;\n \telse if (res == EDIT_TODO_FAILED) {\n+\t\tif (action == ACTION_REWIND)\n+\t\t\treturn -1;\n+\n \t\tapply_autostash(rebase_path_autostash());\n \t\tsequencer_remove_state(opts);\n \n \t\treturn -1;\n \t} else if (res == EDIT_TODO_ABORT) {\n+\t\tif (action == ACTION_REWIND) {\n+\t\t\ttodo_list_release(&new_todo);\n+\n+\t\t\treturn error(_(\"rewind aborted; state unchanged\"));\n+\t\t}\n+\n \t\tapply_autostash(rebase_path_autostash());\n \t\tsequencer_remove_state(opts);\n \t\ttodo_list_release(&new_todo);\n@@ -6239,7 +6253,7 @@ static int skip_fixupish(const char *subject, const char **p) {\n int todo_list_rearrange_squash(struct todo_list *todo_list)\n {\n \tstruct hashmap subject2item;\n-\tint rearranged = 0, *next, *tail, i, nr = 0;\n+\tint rearranged = 0, *next, *tail, i, j, nr = 0;\n \tchar **subjects;\n \tstruct commit_todo_item commit_todo;\n \tstruct todo_item *items = NULL;\n@@ -6266,6 +6280,10 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)\n \t\tint i2 = -1;\n \t\tstruct subject2item_entry *entry;\n \n+\t\t// When rewinding, process only up to the marker.\n+\t\tif (item->command == TODO_BREAK)\n+\t\t\tbreak;\n+\n \t\tnext[i] = tail[i] = -1;\n \t\tif (!item->commit || item->command == TODO_DROP) {\n \t\t\tsubjects[i] = NULL;\n@@ -6350,9 +6368,9 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)\n \tif (rearranged) {\n \t\titems = ALLOC_ARRAY(items, todo_list->nr);\n \n-\t\tfor (i = 0; i < todo_list->nr; i++) {\n-\t\t\tenum todo_command command = todo_list->items[i].command;\n-\t\t\tint cur = i;\n+\t\tfor (j = 0; j < i; j++) {\n+\t\t\tenum todo_command command = todo_list->items[j].command;\n+\t\t\tint cur = j;\n \n \t\t\t/*\n \t\t\t * Initially, all commands are 'pick's. If it is a\n@@ -6367,16 +6385,19 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)\n \t\t\t}\n \t\t}\n \n+\t\tfor (; j < todo_list->nr; j++)\n+\t\t\titems[nr++] = todo_list->items[j];\n+\n \t\tassert(nr == todo_list->nr);\n \t\ttodo_list->alloc = nr;\n \t\tFREE_AND_NULL(todo_list->items);\n \t\ttodo_list->items = items;\n \t}\n \n \tfree(next);\n \tfree(tail);\n-\tfor (i = 0; i < todo_list->nr; i++)\n-\t\tfree(subjects[i]);\n+\tfor (j = 0; j < i; j++)\n+\t\tfree(subjects[j]);\n \tfree(subjects);\n \thashmap_clear_and_free(&subject2item, struct subject2item_entry, entry);\n \ndiff --git a/sequencer.h b/sequencer.h\nindex 33bcff89e0..84d7c076c0 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -12,7 +12,10 @@ struct repository;\n const char *git_path_commit_editmsg(void);\n const char *rebase_path_todo(void);\n const char *rebase_path_todo_backup(void);\n+const char *rebase_path_todo_orig(void);\n const char *rebase_path_dropped(void);\n+const char *rebase_path_done(void);\n+const char *rebase_path_done_orig(void);\n \n #define APPEND_SIGNOFF_DEDUP (1u << 0)\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex dd47f0bbce..ef68f4470a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -2180,6 +2180,117 @@ test_expect_success 'bad labels and refs rejected when parsing todo list' '\n \ttest_path_is_missing execed\n '\n \n+test_expect_success 'rebase --rewind' '\n+\tgit checkout primary^0 &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 reword 2 3 break 4\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i HEAD~4 &&\n+\t\tFAKE_LINES=\"reword 1 2 3 6\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"B_reworded\" \\\n+\t\t\tgit rebase --rewind >output 2>&1\n+\t) &&\n+\tgrep -q \"Rebasing (4/4)\" output &&\n+\ttest \"$(git log -1 --format=%B HEAD~3)\" = \"B_reworded\" &&\n+\ttest \"$(git log -1 --format=%B HEAD~2)\" = \"C_reworded\"\n+'\n+\n+test_expect_success 'rebase --rewind with initially botched todo' '\n+\tgit checkout primary^0 &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 reword 2 3 break 4\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i HEAD~4 &&\n+\t\ttest_must_fail env FAKE_LINES=\"reword 1 bad 2 3 6\" \\\n+\t\t\tgit rebase --rewind &&\n+\t\tgrep -q \"rewinding\" < .git/rebase-merge/git-rebase-todo.backup &&\n+\t\tFAKE_LINES=\"reword 1 pick 2 3 4\" \\\n+\t\t\tgit rebase --edit-todo &&\n+\t\tFAKE_COMMIT_MESSAGE=\"B_reworded\" \\\n+\t\t\tgit rebase --continue\n+\t) &&\n+\ttest \"$(git log -1 --format=%B HEAD~3)\" = \"B_reworded\" &&\n+\ttest \"$(git log -1 --format=%B HEAD~2)\" = \"C_reworded\"\n+'\n+\n+test_expect_success 'recursing rebase --rewind with initially botched todo' '\n+\tgit checkout primary^0 &&\n+\tcat >expect <<-\\EOF &&\n+\terror: you are already rewinding a rebase.\n+\tUse rebase --edit-todo to continue.\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 reword 2 3 break 4\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i HEAD~4 &&\n+\t\ttest_must_fail env FAKE_LINES=\"reword 1 bad 2 3 6\" \\\n+\t\t\tgit rebase --rewind &&\n+\t\ttest_must_fail env FAKE_LINES=\"reword 1 pick 2 3 4\" \\\n+\t\t\tgit rebase --rewind >actual 2>&1  &&\n+\t\ttest_cmp expect actual &&\n+\t\tgit rebase --abort\n+\t)\n+'\n+\n+test_expect_success 'rebase --rewind being aborted' '\n+\tgit checkout primary^0 &&\n+\tcat >expect <<-\\EOF &&\n+\terror: rewind aborted; state unchanged\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 reword 2 3 break 4\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i HEAD~4 &&\n+\t\ttest_must_fail env FAKE_LINES=\"#\" \\\n+\t\t\tgit rebase --rewind >output 2>&1 &&\n+\t\ttail -n 1 output >actual &&  # Ignore output about changing todo list\n+\t\ttest_cmp expect actual &&\n+\t\tgit rebase --continue\n+\t) &&\n+\ttest \"$(git log -1 --format=%B HEAD~2)\" = \"C_reworded\"\n+'\n+\n+test_expect_success 'rebase --rewind being aborted after initially botched todo' '\n+\tgit checkout primary^0 &&\n+\tcat >expect <<-\\EOF &&\n+\terror: rewind aborted; state restored\n+\tEOF\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 reword 2 3 break 4\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i HEAD~4 &&\n+\t\ttest_must_fail env FAKE_LINES=\"reword 1 bad 2 3 6\" \\\n+\t\t\tgit rebase --rewind &&\n+\t\ttest_must_fail env FAKE_LINES=\"#\" \\\n+\t\t\tgit rebase --edit-todo >output 2>&1 &&\n+\t\ttail -n 1 output >actual &&  # Ignore output about changing todo list\n+\t\ttest_cmp expect actual &&\n+\t\tgit rebase --continue\n+\t) &&\n+\ttest \"$(git log -1 --format=%B HEAD~2)\" = \"C_reworded\"\n+'\n+\n+test_expect_failure 'rebase --rewind vs. --update-refs' '\n+\tgit checkout primary^0 &&\n+\tgit branch -f first HEAD~3 &&\n+\t(\n+\t\tset_fake_editor &&\n+\t\tFAKE_LINES=\"1 2 reword 4 5 break 6 7\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"C_reworded\" \\\n+\t\t\tgit rebase -i --update-refs HEAD~4 &&\n+\t\tFAKE_LINES=\"reword 1 2 3 4 7 8\" \\\n+\t\t\tFAKE_COMMIT_MESSAGE=\"B_reworded\" \\\n+\t\t\tgit rebase --rewind\n+\t) &&\n+\ttest_cmp_rev HEAD~3 refs/heads/first &&\n+\ttest_cmp_rev HEAD refs/heads/primary\n+'\n+\n # This must be the last test in this file\n test_expect_success '$EDITOR and friends are unchanged' '\n \ttest_editor_unchanged\n-- \n2.40.0.152.g15d061e6df\n\n"},{"id":"474318","messageId":"7bd63d7e-ad13-d5b8-54ea-ba5f81da0c17@gmx.de","threadId":"59448","inReplyTo":"20230323162235.995645-1-oswald.buddenhagen@gmx.de","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-03-28T14:53:52Z","receivedAt":"2023-03-28T14:54:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Oswald,\n\nOn Thu, 23 Mar 2023, Oswald Buddenhagen wrote:\n\n> This is fundamentally --edit-todo, except that we first prepend the\n> already applied commits and reset back to `onto`. This is useful when\n> one finds that a prior change needs (further) modifications.\n>\n> This patch implements \"flat\" rewind, that is, once the todo edit has\n> been committed, one can abort only the complete rebase. The pre-rewind\n> position is marked with a `break` command (these pile up when rewinding\n> multiple times; the user is expected to clean them up as necessary).\n>\n> An alternative to that would be \"nested\" rewind, where one can return to\n> the pre-rewind state even after committing the todo edit. However, this:\n> - would add somewhat significant complexity due to having to maintain a\n>   stack of todos and HEADs\n> - would be mildly confusing to use due to needing to track the state of\n>   the stack. One could simplify this somewhat by hiding the rest of the\n>   previous todo before nesting, but this would be somewhat limiting in\n>   turn (one might want to defer a factored out hunk, and stashing it is\n>   not necessarily the most elegant way to do it).\n> - would be of somewhat limited usefulness, speaking from experience\n>\n> This patch leaves transitive resolution of rewritten-list to the\n> consumer. This is probably a bad idea.\n> Somewhat related to that, --update-refs isn't properly handled yet.\n\nThis is an interesting idea but I do not think that the concept in its\ncurrent form mixes well with being in the middle of a `--rebase-merges`\nrun. Which is what I would want to see supported because I find myself\nwishing for _something_ like this [*1*].\n\nOn the other hand, it might often be good enough to redo only the commits\nbetween `onto` and `HEAD`, not the complete rebase script that's now in\n`done`. But then it does not strike me so much as \"rewinding\" but as\n\"nesting.\n\nIn other words, I would then prefer to see support for `git rebase -i\n--nested` be added. I wrote up my thoughts about this in\nhttps://github.com/gitgitgadget/git/issues/211 and was tempted several\ntimes to start implementing it already, only for other, more pressing\nthings to come up and take my attention.\n\nWhat do you think, would you be amenable to combine efforts?\n\nCiao,\nJohannes\n\nFootnote *1*: I usually find myself adding a temporary worktree with a\ndetached `HEAD`, performing the nested rebase, and then resetting the\noriginal worktree to the output of `rev-parse HEAD` in the temporary\nworktree. Cumbersome, yes, and it works, but yes, I dearly want something\nthat works without requiring _me_ to keep tabs on the state.\n"},{"id":"474322","messageId":"ZCMRpnS9gzN1Rlbh@ugly","threadId":"59448","inReplyTo":"7bd63d7e-ad13-d5b8-54ea-ba5f81da0c17@gmx.de","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-03-28T16:11:18Z","receivedAt":"2023-03-28T16:11:24Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Tue, Mar 28, 2023 at 04:53:52PM +0200, Johannes Schindelin wrote:\n>I do not think that the concept in its\n>current form mixes well with being in the middle of a `--rebase-merges`\n>run.\n>\nfundamentally, it shouldn't pose a problem: the already done part leads \nup to a single commit, from which a complete todo with merges up to that \npoint can be built again, while the remainder of the pre-existing todo \nshould be unfazed by the fact that you're repeatedly messing with \nwhatever branch you stopped in.\ni *think* i even tried a few simple cases and found the result adequate, \nbut i don't remember for sure (it's been a few months since i authored \nthe patch).\njust give it a shot.\n\n>On the other hand, it might often be good enough to redo only the \n>commits\n>between `onto` and `HEAD`, not the complete rebase script that's now in\n>`done`. But then it does not strike me so much as \"rewinding\" but as\n>\"nesting.\n>\naccording to the terminology i'm using, this still qualifies as a flat \nrewind, only that it limits itself to --first-parent. we'll see whether \nthis turns out to be a necessary simplification, but i don't think so.\n\ntrue nesting would mean that the rewind itself can be aborted, in case \nyou change your mind back. adding that as an option on top of what i'm \ndoing isn't a hard problem _per se_. you would need to figure out the \nchallenges from my OP, though.\nalso note the reference in the OP; we discussed this here a while ago \nalready.\n\n"},{"id":"474838","messageId":"4fa6d2da-4885-09d9-dddb-6f19efda6398@gmx.de","threadId":"59448","inReplyTo":"ZCMRpnS9gzN1Rlbh@ugly","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-04-05T12:07:29Z","receivedAt":"2023-04-05T12:07:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Oswald,\n\nplease do reply-to-all on this list.\n\nOn Tue, 28 Mar 2023, Oswald Buddenhagen wrote:\n\n> On Tue, Mar 28, 2023 at 04:53:52PM +0200, Johannes Schindelin wrote:\n> >I do not think that the concept in its\n> >current form mixes well with being in the middle of a `--rebase-merges`\n> >run.\n>\n> fundamentally, it shouldn't pose a problem: the already done part leads up to\n> a single commit, from which a complete todo with merges up to that point can\n> be built again, while the remainder of the pre-existing todo should be unfazed\n> by the fact that you're repeatedly messing with whatever branch you stopped\n> in.\n\nI guess the most important question is: What problem is the proposed\n`--rewind` option supposed to solve?\n\nIf the idea is to let the user re-start the rebase (for whatever reason),\nthrowing away the current state, then the proposed code really does not\nhandle the `--rebase-merges` case at all. Instead, it would implicitly\nrestart the rebase with `--no-rebase-merges`, i.e. the opposite of what\nthe user asked for.\n\nBut a more important concern is: Is this `--rewind` idea even a good one?\nThis question brings me back to the initial question: What problem do we\ntry to solve here? (This is a question that try as I might, I cannot see\nanswered in the proposed commit message.)\n\nSince I do not want to speculate about your motivation, let me explain the\nchallenges I would like to see addressed with those rewound-or-nested\nrebases.\n\nI frequently find myself in _large_ rebases (we're talking about several\nthousand commits, with some 100-200 merge commits), where I notice in the\nmiddle that I should have resolved a previous merge conflict in a\ndifferent way.\n\nDo I want to start over and redo the whole rebase? Sometimes I do, and\n`git rebase --abort` and the Bash history (Ctrl+R -i will get me back to\nthe start of the interactive rebase) are my friend. No `--rewind`\nrequired. (Which makes me wonder why that same strategy is not good enough\nfor your scenarios, too.)\n\nHowever, that's what I need only in a few, rare instances.\n\nWhat I need much, much, much more often is a way to redo only _part_ of\nthe rebase. Like, the last 3 commits. And not from scratch, oh no! I do\nnot want the original commits to be cherry-picked, but the ones that were\nalready rebased.\n\nIn other words, I need a nested rebase.\n\nNow, why do I keep bringing up this idea of a nested rebase, when such a\nnested rebase would not be able to perform a rewind as you asked?\n\nThe reason is that I am still very much unconvinced that `--rewind` can do\nanything that `git rebase --abort` and starting over cannot do. So: no\npatches required, right?\n\nHowever, the use case that _immediately_ comes to mind when you talk about\nthese rewinds is when a part of a rebase needs to be redone, in the middle\nof said rebase. And that _does_ require a nested rebase, and the\n`--rewind` would in most cases only throw away too much work.\n\nCiao,\nJohannes\n\nP.S.: Yes, yes, I know, a nested rebase can be simulated via\n\n\tgit worktree add --detach /tmp/throw-away &&\n\tgit -C /tmp/throw-away rebase -i HEAD~3 &&\n\tgit reset --hard $(git -C /tmp/throw-away rev-parse HEAD) &&\n\tgit worktree remove /tmp/throw-away\n\nbut that is of course not only inconvenient, but leaves too much\nbook-keeping and safe-guarding up to the human user, e.g. to make sure\nthat the `git reset --hard` does not overwrite uncommitted changes/files.\n\nFWIW I simulate nested rebases in the illustrated way _a lot_.\n"},{"id":"474851","messageId":"ZC2Qhi73YKSOJrM2@ugly","threadId":"59448","inReplyTo":"4fa6d2da-4885-09d9-dddb-6f19efda6398@gmx.de","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-05T15:15:18Z","receivedAt":"2023-04-05T15:17:09Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, Apr 05, 2023 at 02:07:29PM +0200, Johannes Schindelin wrote:\n>This question brings me back to the initial question: What problem do \n>we try to solve here? (This is a question that try as I might, I cannot \n>see answered in the proposed commit message.)\n>\nand i, try as i might, don't understand what you're not understanding \n...\n\n>[...] In other words, I need a nested rebase.\n>\nthat's just *your* private terminology. i don't apply the term \"nested\" \nhere, because for me that implies the possibility to \"unnest\", which my \npatch doesn't implement. instead, it just continues past the point where \nthe rewind was initiated. it's the difference between a loop and \nrecursion.\nbut outside this difference in terminology, for all i can tell, my patch \nimplements *exactly* what you're asking for, and i don't understand why \nthat's not obvious to you, given how well you understand the problem \nspace yourself.\nplease describe what you want with _a few_ words and without introducing \nany new terminology first, i.e., something you'd actually want to see in \nthe feature's summary documentation. that should illuminate what \nkeywords you find critical.\n\ni just gave rewinding rebasing merges a shot, and it didn't work for the \nsimple reason that --rebase-merges is not saved in the state \n(understandably, because that was unnecessary so far) and the \ncombination of --rewind --rebase-merges is being rejected. i'll need to \nfix that.\n\nthen there is the problem that --rebase-merges only redoes merges rather \nthan replaying them. but it seems that the simple case with unmodified \nparents actually does get replayed (or rather, skipped over, just \nincredibly slowly), so rewinding to just the last merge would work fine.  \nother than that, i'm declaring the matter out of scope and deferring to \nyour \"replaying evil merges\" sub-thread.\n\n"},{"id":"474927","messageId":"cfb0d0f2-dc82-885d-99d6-fa641b5a2923@gmail.com","threadId":"59448","inReplyTo":"4fa6d2da-4885-09d9-dddb-6f19efda6398@gmx.de","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-04-06T10:01:27Z","receivedAt":"2023-04-06T10:02:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/04/2023 13:07, Johannes Schindelin wrote:\n> Hi Oswald,\n> \n> please do reply-to-all on this list.\n> \n> On Tue, 28 Mar 2023, Oswald Buddenhagen wrote:\n> \n>> On Tue, Mar 28, 2023 at 04:53:52PM +0200, Johannes Schindelin wrote:\n>>> I do not think that the concept in its\n>>> current form mixes well with being in the middle of a `--rebase-merges`\n>>> run.\n\nThat definitely needs to be addressed, I'd be happy to start with an \nimplementation that only rewinds linear history but it must error out if \nit encounters a merge and --rebase-merges was given. I'd also be very \nhappy if we could rewind across merges by updating existing labels in \nthe new todo list.\n\n> [...] \n> What I need much, much, much more often is a way to redo only _part_ of\n> the rebase. Like, the last 3 commits. And not from scratch, oh no! I do\n> not want the original commits to be cherry-picked, but the ones that were\n> already rebased.\n\nThat's what I want most often as well. Oswald's --rewind always rewinds \nto $onto but I think it does use the rebased commits in the new todo \nlist. It looks like the new todo list will contain the commits \n$onto..HEAD plus the old todo list\n\n> In other words, I need a nested rebase.\n\nI can see the benefit in being able to checkpoint while rebasing but I'm \nnot sure that needs to be tied to rewinding. For example if I'm making a \nchange I'm not sure about I'd like to be able to set a checkpoint before \nthat change so I can rewind to the previous state. I'd be happy to see \ncheckpointing and rewinding added separately.\n\nBest Wishes\n\nPhillip\n\n> Now, why do I keep bringing up this idea of a nested rebase, when such a\n> nested rebase would not be able to perform a rewind as you asked?\n> \n> The reason is that I am still very much unconvinced that `--rewind` can do\n> anything that `git rebase --abort` and starting over cannot do. So: no\n> patches required, right?\n> \n> However, the use case that _immediately_ comes to mind when you talk about\n> these rewinds is when a part of a rebase needs to be redone, in the middle\n> of said rebase. And that _does_ require a nested rebase, and the\n> `--rewind` would in most cases only throw away too much work.\n> \n> Ciao,\n> Johannes\n> \n> P.S.: Yes, yes, I know, a nested rebase can be simulated via\n> \n> \tgit worktree add --detach /tmp/throw-away &&\n> \tgit -C /tmp/throw-away rebase -i HEAD~3 &&\n> \tgit reset --hard $(git -C /tmp/throw-away rev-parse HEAD) &&\n> \tgit worktree remove /tmp/throw-away\n> \n> but that is of course not only inconvenient, but leaves too much\n> book-keeping and safe-guarding up to the human user, e.g. to make sure\n> that the `git reset --hard` does not overwrite uncommitted changes/files.\n> \n> FWIW I simulate nested rebases in the illustrated way _a lot_.\n\n"},{"id":"474929","messageId":"230406.86zg7ls2jx.gmgdl@evledraar.gmail.com","threadId":"59448","inReplyTo":"ZC2Qhi73YKSOJrM2@ugly","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-04-06T10:45:02Z","receivedAt":"2023-04-06T10:55:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Apr 05 2023, Oswald Buddenhagen wrote:\n\n> On Wed, Apr 05, 2023 at 02:07:29PM +0200, Johannes Schindelin wrote:\n>> This question brings me back to the initial question: What problem\n>> do we try to solve here? (This is a question that try as I might, I\n>> cannot see answered in the proposed commit message.)\n>>\n> and i, try as i might, don't understand what you're not understanding\n> ...\n>\n>>[...] In other words, I need a nested rebase.\n>>\n> that's just *your* private terminology. i don't apply the term\n> \"nested\" here, because for me that implies the possibility to\n> \"unnest\", which my patch doesn't implement. instead, it just continues\n> past the point where the rewind was initiated. it's the difference\n> between a loop and recursion.\n> but outside this difference in terminology, for all i can tell, my\n> patch implements *exactly* what you're asking for, and i don't\n> understand why that's not obvious to you, given how well you\n> understand the problem space yourself.\n> please describe what you want with _a few_ words and without\n> introducing any new terminology first, i.e., something you'd actually\n> want to see in the feature's summary documentation. that should\n> illuminate what keywords you find critical.\n>\n> i just gave rewinding rebasing merges a shot, and it didn't work for\n> the simple reason that --rebase-merges is not saved in the state\n> (understandably, because that was unnecessary so far) and the\n> combination of --rewind --rebase-merges is being rejected. i'll need\n> to fix that.\n>\n> then there is the problem that --rebase-merges only redoes merges\n> rather than replaying them. but it seems that the simple case with\n> unmodified parents actually does get replayed (or rather, skipped\n> over, just incredibly slowly), so rewinding to just the last merge\n> would work fine.  other than that, i'm declaring the matter out of\n> scope and deferring to your \"replaying evil merges\" sub-thread.\n\nNot Johannes, but I'd also like to have \"nested\", but maybe your feature\nwould also provide that. I haven't had time to test it, sorry.\n\nBut isn't the difference noted in this aspect of your commit message:\n\"where one can return to the pre-rewind state even after committing the\ntodo edit\".\n\nMy most common use-case for \"nested\" is certainly less complex that\nJohannes's, and is the following:\n\n * I've got e.g. a 10 patch series\n\n * I start rebasing that on \"master\", solve conflicts with \"1..4\", and\n   am now on a conflict on 5/10.\n\n * It now becomes obvious to me that the even larger conflict I'm about\n   to have on 6/10 would be better handled if I went back to 2/10 or\n   whatever, did a change I could do here in 5/10 differently, and then\n   proceeded.\n\nI.e. when I'm at 5/10 I'd conceptually like to do another \"git rebase -i\nHEAD~5\" or whatever, use the *already rewritten* commits (otherwise I'd\njust abort and restast), re-arrange/rewrite them, and when I'm done\nreturn to 5/10.\n\nThen do another \"continue\".\n\nFrom a UX perspective I think just as our $PS1 integration can be made\nto show \"5/10\" it would be ideal if in this case we could show\ne.g. \"5/10 -> 1/5\" or whatever. I.e. I'm in a nested rebase of 1/5,\nwhich started from that 5/10\".\n\nRight now I do this sort of thing manually, i.e. note the SHA-1's I've\ngot so far, --abort at 5/10, then start a rebase for all 10 again, but\nmanually replace the SHA-1's for 1-5 with the ones I had already.\n\nWhich, I suppose I could also do the other way around, i.e. at 5/10 I'd\n--edit-todo, wipe away 6/10, \"finish\" my rebase, then use \"git rebase\n--onto\" later when I'm done to transplant the remaining 6-10/10 on the\n1-5/5 I'm now happy with.\n\nBut here's the important bit: Sometimes I'm just wrong about my re-edit\nto 2/10 being the right thing, and it would actually just make things\nworse, as I might discover in my \"nested\" rebase once I'm at 4/5 or\nwhatever.\n\nSo being able to do an \"--abort\" ot that point to go back to the\n\"un-nested\" 5/10 (*not* \"original\" 5/10) and proceed from there would be\nnice.\n\nBut I think what you've implemented doesn't do that at all, or am I\nmisunderstanding you?\n\nI think a relatively simple hack to \"restart\" might still be very nice,\njust clarifying.\n\n"},{"id":"474935","messageId":"ZC7b3QjRTQ2k7bhf@ugly","threadId":"59448","inReplyTo":"230406.86zg7ls2jx.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-06T14:49:01Z","receivedAt":"2023-04-06T14:50:30Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Apr 06, 2023 at 12:45:02PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>My most common use-case for \"nested\" is certainly less complex that\n>Johannes's, and is the following:\n>\n> * I've got e.g. a 10 patch series\n>\n> * I start rebasing that on \"master\", solve conflicts with \"1..4\", and\n>   am now on a conflict on 5/10.\n>\n> * It now becomes obvious to me that the even larger conflict I'm about\n>   to have on 6/10 would be better handled if I went back to 2/10 or\n>   whatever, did a change I could do here in 5/10 differently, and then\n>   proceeded.\n>\n>I.e. when I'm at 5/10 I'd conceptually like to do another \"git rebase \n>-i\n>HEAD~5\" or whatever, use the *already rewritten* commits (otherwise I'd\n>just abort and restast), re-arrange/rewrite them, and when I'm done\n>return to 5/10.\n>\nyes, this patch addresses this use case - mostly.\n\ni'm generally dealing with an even more benign case, because i'm \n\"rebasing\" with --keep-base most of the time (and i have the thing \naliased to 'reshape' - maybe something for upstream?).\n\nthe case of rewinding from a conflicted state currently needs manual \nhandling. i suppose i should detect the state, re-insert the pick, and \nreset hard out of it, as if --skip was used. the implicit \ndestructiveness feels wrong, though. maybe require --force?\n\n>But here's the important bit: Sometimes I'm just wrong about my re-edit\n>to 2/10 being the right thing, and it would actually just make things\n>worse, as I might discover in my \"nested\" rebase once I'm at 4/5 or\n>whatever.\n>\n>So being able to do an \"--abort\" ot that point to go back to the\n>\"un-nested\" 5/10 (*not* \"original\" 5/10) and proceed from there would be\n>nice.\n>\nyeah, i'm experiencing that sometimes, but not often enough to bother \nautomating it. manual recovery by hand-editing the todo after rewinding \nagain did the trick so far.\n\n>From a UX perspective I think just as our $PS1 integration can be made\n>to show \"5/10\" it would be ideal if in this case we could show\n>e.g. \"5/10 -> 1/5\" or whatever. I.e. I'm in a nested rebase of 1/5,\n>which started from that 5/10\".\n>\nhmm, i think you just pointed out johannes' hangup to me. ^^\n\nyou both are assuming a limited rewind, where you explicitly specify the \naffected range, and the todo list editor presents only that. you're \nderiving the term \"nested\" from the fact that it's an isolated subset of \nthe rewritten commits.\n\nhowever, i see these problems with that aproach:\n- as mentioned in the OP, i might want to move hunks out of the nested \n   range. i could stash them, but then i'm dealing with two methods of \n   organizing the history, which gets really messy\n- it gets even trickier if i want to move commits *into* the nested \n   range - i'd have to manually insert a pick, and then deal with the \n   possible conflict after unnesting\n- who says that the nesting point should be the last chance to change my \n   mind? suppose i stop at 10, get the idea to re-edit 5, but after \n   reaching 15 i notice that re-editing 5 (and thus probably also 10)  \n   was a terrible idea, so i want to go back to pre-nest 10\n\nnow suppose my approach, where the rebase is rewound right to `onto`, \nand the whole remaining todo is left in place. the nested base is \nimplicitly determined by the first modified line of the rewound todo, so \nthere is no harm in rewinding the whole rebase (*). and the rebase can \njust continue past the rewind point without anything special happening.  \n\nif we want to be able to undo the rewind, we push HEAD and the todo list \nonto a stack. as phillip said, that's basically just a checkpoint, which \nhappens to be automatically created when we are rewinding. that could be \npresented at the prompt as \"REBASE 5/10 [1]\" to signify the number of \navailable checkpoints (and you'd access them with 'git rebase --restore \n[<id>]', quite similarly to stashes).\n\nof course it gets really \"interesting\" when you want to go back to a \ncheckpoint, but also want to salvage some of the rewritten commits. then \nyou'll have to manually pick commits from the reflog, etc., but i don't \nsee how one could possibly get around the complexity (we could present a \ncombined todo file where alternative versions of commits are shown in \ncomments, but that's quite some effort for only a slight improvement).\n\n(*) actually, there is:\n- firstly, having the entire todo in front of you can be rather annoying \n   when it's more than two dozen commits long and the part you want to \n   edit isn't near the beginning.\n- secondly, skipping over merges doesn't appear to be a thing, so \n   johannes' use case would be *insanely* slow. but that's \"only\" an \n   implementation issue.\n\ngiven these problems, i can see that it would make sense to accept an \noptional argument that limits the depth of the rewind (without impacting \nthe overall approach).\n\nthanks!\n"},{"id":"474962","messageId":"CAMP44s13z=hZHzU+EB7qBZnqQcmRGe4aknF=wocOK9uh6NHbcA@mail.gmail.com","threadId":"59448","inReplyTo":"230406.86zg7ls2jx.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-04-07T00:21:39Z","receivedAt":"2023-04-07T00:21:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Apr 6, 2023 at 7:03 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Wed, Apr 05 2023, Oswald Buddenhagen wrote:\n\n> Right now I do this sort of thing manually, i.e. note the SHA-1's I've\n> got so far, --abort at 5/10, then start a rebase for all 10 again, but\n> manually replace the SHA-1's for 1-5 with the ones I had already.\n\nCouldn't this be considered a rerebase?\n\nThis is what I do most of the time, except I don't even bother saving\nand replacing the SHA-1s, rerere reapplies my fixes so it's\nstraightforward to reach the desired state.\n\n> Which, I suppose I could also do the other way around, i.e. at 5/10 I'd\n> --edit-todo, wipe away 6/10, \"finish\" my rebase, then use \"git rebase\n> --onto\" later when I'm done to transplant the remaining 6-10/10 on the\n> 1-5/5 I'm now happy with.\n\nWith this approach the reflog wouldn't be an accurate representation\nof what happened.\n\nMost of the times I do a rebase I want to see the difference with the\nprevious state of the branch, so I do `git diff @{1}`, but this won't\nwork with this frankensteinian rebase which in true is comprised of\nmultiple subrebases.\n\n> But here's the important bit: Sometimes I'm just wrong about my re-edit\n> to 2/10 being the right thing, and it would actually just make things\n> worse, as I might discover in my \"nested\" rebase once I'm at 4/5 or\n> whatever.\n>\n> So being able to do an \"--abort\" ot that point to go back to the\n> \"un-nested\" 5/10 (*not* \"original\" 5/10) and proceed from there would be\n> nice.\n\nYes, this is something I often desire.\n\n\nBut I feel you guys are overcomplicating the problem.\n\nImagine there was a rebase log for each branch, then `git rebase`\ncould use that information to redo a previous rebase, even if that\nrebase was aborted. To restart your current rebase you could do `git\nrebase -i --redo 1` (1 being the previous one). If in the middle of\nthat you decide actually your original approach was better, you just\nfreely abort, and do `git rebase -i --redo 2`.\n\nWouldn't that solve all the problems?\n\nThe complication comes in trying to do that without the concept of\nrebase history.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"474974","messageId":"ZC+/nYp2RRF9Gjrd@ugly","threadId":"59448","inReplyTo":"CAMP44s13z=hZHzU+EB7qBZnqQcmRGe4aknF=wocOK9uh6NHbcA@mail.gmail.com","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2023-04-07T07:00:45Z","receivedAt":"2023-04-07T07:00:54Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Thu, Apr 06, 2023 at 07:21:39PM -0500, Felipe Contreras wrote:\n>Imagine there was a rebase log for each branch, then `git rebase`\n>could use that information to redo a previous rebase, even if that\n>rebase was aborted. To restart your current rebase you could do `git\n>rebase -i --redo 1` (1 being the previous one). If in the middle of\n>that you decide actually your original approach was better, you just\n>freely abort, and do `git rebase -i --redo 2`.\n>\nwhat exactly would you save to that log?\nwhat comes to my mind is the todo file produced by my --rewind before \nthe user edits it: the already rewritten commits (which can of course be \nsaved as a single ref), and the remaining todo.\nthat would make it very much the same thing as the checkpoints phillip \npostulated and i expanded upon.\n\none difference to what i envisaged would be that one could easily resume \na rebase one erroneously discarded entirely.\n\n>Wouldn't that solve all the problems?\n>\nit would, but not necessarily optimally.\n\nconsider that after the initial implementation phase, my working branch \nis most of the time inside a 'reshape' (rebase -i --keep-base), and \nsince i wrote the initial version of rewind, i initiate new reshapes \nmuch less often. i basically move freely between the commits in the \nbranch.\ninserting an additional step of aborting prior to redoing feels just \nclumsy.\nat this point i'm actually thinking in the opposite direction: introduce \ncommands that let me move by a few commits without even opening the todo \neditor (where have i seen that already? jj?).\n\nthe second aspect is performance/resource usage.\nthe intermediate abort would potentially touch a lot of files each time.  \nthat costs ssd life and often unneeded recompiles.\nand given johannes' use case with *many* merges, rebasing from scratch \nwould waste *quite* some time. as i pointed out in the other mail, my \napproach currently suffers from that as well, but it would be rather \neasy to sidestep it. your approach otoh would definitely need a \nfundamental improvement to the skipping algo (*).\n\n(*) this of course sounds like a good idea regardless, but it's not \nnecessarily wise to bet on it. i think the problem here is that redoing \nmerges is *expected* to be \"lossy\". if they were marked for replay as \nproposed in https://github.com/gitgitgadget/git/pull/1491 , one could \nalso just skip over them.\n\n"},{"id":"475139","messageId":"390e6a25-72db-8a9e-97af-7b9d803cfb2d@gmail.com","threadId":"59448","inReplyTo":"ZC+/nYp2RRF9Gjrd@ugly","subject":"Re: [RFC PATCH] rebase: implement --rewind","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-04-11T10:06:04Z","receivedAt":"2023-04-11T10:07:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/04/2023 08:00, Oswald Buddenhagen wrote:\n\n> at this point i'm actually thinking in the opposite direction: introduce \n> commands that let me move by a few commits without even opening the todo \n> editor (where have i seen that already? jj?).\n\nWhen I'm working on a patch series I use this approach a lot with a \"git \nrewrite\" script I have. To amend a commit I run \"git rewrite amend \n<commit>\" and it will start a rebase or rewind the current one. It will \nalso take a file, line number pair and use \"git diff\" to map that line \nonto HEAD and then \"git blame\" to work out which commit to amend so you \ncan run it from your editor and say \"amend the commit that added this \nline\" which is a great time saver. I'd love to a command like that \nupstream in git but I don't think it covers all the cases that \"rebase \n--rewind\" does such as dscho rebasing git-for-windows.\n\nBest Wishes\n\nPhillip\n\n\n"}]}