{"thread":{"id":"57782","subject":"[PATCH v1] rebase - recycle","startedAt":"2022-04-22T19:32:10Z","lastAt":"2022-04-23T16:34:39Z","messageCount":6,"participants":["Edmundo Carmona Antoranz","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454274","messageId":"20220422183744.347327-1-eantoranz@gmail.com","threadId":"57782","inReplyTo":null,"subject":"[PATCH v1] rebase - recycle","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-22T18:37:44Z","receivedAt":"2022-04-22T19:32:10Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"When rebasing a branch, there is a very narrow workflow\nwhere a branch is asked to be placed on top of another\nbranch that has the same tree as the upstream branch.\n\nWhen this happens, the rebased revisions can be generated using\nthe trees of the revisions that will be rebased without any\nneed to involve work from the merge engine and they can also be\ncreated without moving the working tree until the tip of\nthe target rebased branch is created. This accelerates\nthe process of rebasing even straight lines and avoids completely\nthe need to have to go over merge revisions where there might\nhave been conflicts as is the case with current rebase.\n\nAs an example, let's create a sample repo:\n\n$ mkdir temp\n$ cd temp\n$ git init -b main .\n$ for i in $( seq 1 1000 ); do echo $i >> count.txt; git add count.txt; git commit -m $i; done\n$ git branch new-main\n$ git checkout -b new-base main~999\n$ git commit --amend --no-edit\n\nRebasing this straight line onto new-base:\n$ time git rebase --onto HEAD main~999 new-main\nSuccessfully rebased and updated refs/heads/new-main.\n\nreal    0m5,801s\nuser    0m0,844s\nsys     0m1,899s\n\nSame operation recycling:\n$ time git rebase --recycle --onto HEAD main~999 new-main\n$ time ../git rebase --recycle --onto HEAD main~999 new-main\nRecycled new-main onto HEAD.\n\nreal    0m0,263s\nuser    0m0,098s\nsys     0m0,135s\n\nIf we tried recycling a complex tree that has multiple\nmerges, it avoids having to go through conflict resolution. Using\nrather recent releases of git as an example:\n\n$ git checkout v2.34.0\n$ git commit --amend --no-edit\n$ time ../git rebase --recycle --onto HEAD v2.34.0 v2.36.0\nRecycled v2.36.0 onto HEAD.\n\nreal    0m5,137s\nuser    0m0,841s\nsys     0m0,530s\n\nTwo options are added to support this feature:\n--recycle\n  Try to recycle. If it's not possible to recycle, fail.\n--attempt-recycle\n  Try to recycle. If it's not possible to recycle, allow\n  the normal preexisting implementation of rebase to work.\n\nAvoided implementing it to be triggered based on just a check\nfor the trees to match and then recycle because recycling\nimplies that merges will be applied which differs from\ndefault rebase behavior.\n\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n builtin/rebase.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 229 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 27fde7bf28..aa764991e2 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -29,6 +29,9 @@\n #include \"rebase-interactive.h\"\n #include \"reset.h\"\n #include \"hook.h\"\n+#include \"oidmap.h\"\n+#include \"progress.h\"\n+#include \"wt-status.h\"\n \n #define DEFAULT_REFLOG_ACTION \"rebase\"\n \n@@ -49,7 +52,8 @@ static GIT_PATH_FUNC(merge_dir, \"rebase-merge\")\n enum rebase_type {\n \tREBASE_UNSPECIFIED = -1,\n \tREBASE_APPLY,\n-\tREBASE_MERGE\n+\tREBASE_MERGE,\n+\tREBASE_RECYCLE\n };\n \n enum empty_type {\n@@ -83,6 +87,8 @@ struct rebase_options {\n \t\tREBASE_DIFFSTAT = 1<<2,\n \t\tREBASE_FORCE = 1<<3,\n \t\tREBASE_INTERACTIVE_EXPLICIT = 1<<4,\n+\t\tREBASE_RECYCLE_OR_FAIL = 1 << 5,\n+\t\tREBASE_ATTEMPT_RECYCLE = 1<<6,\n \t} flags;\n \tstruct strvec git_am_opts;\n \tconst char *action;\n@@ -104,6 +110,16 @@ struct rebase_options {\n \tint fork_point;\n };\n \n+struct recycle_parent_mapping {\n+\tstruct oidmap_entry e;\n+\tstruct commit *new_parent;\n+};\n+\n+struct recycle_progress_info {\n+\tstruct progress *progress;\n+\tint commits;\n+};\n+\n #define REBASE_OPTIONS_INIT {\t\t\t  \t\\\n \t\t.type = REBASE_UNSPECIFIED,\t  \t\\\n \t\t.empty = EMPTY_UNSPECIFIED,\t  \t\\\n@@ -384,6 +400,12 @@ static int is_merge(struct rebase_options *opts)\n \treturn opts->type == REBASE_MERGE;\n }\n \n+static int can_recycle(struct rebase_options *opts)\n+{\n+\treturn oideq(get_commit_tree_oid(opts->onto),\n+\t\t     get_commit_tree_oid(opts->upstream));\n+}\n+\n static void imply_merge(struct rebase_options *opts, const char *option)\n {\n \tswitch (opts->type) {\n@@ -771,6 +793,173 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)\n \treturn status ? -1 : 0;\n }\n \n+static struct commit *recycle_commit(struct commit *orig_commit,\n+\t\t\t\t     struct oidmap *parents)\n+{\n+\tconst char *body;\n+\tsize_t body_length;\n+\tconst char *author_raw;\n+\tsize_t author_length;\n+\tstruct strbuf author = STRBUF_INIT;\n+\tconst char *message;\n+\tsize_t message_length;\n+\tint result;\n+\n+\tstruct commit *new_commit;\n+\tstruct object_id new_commit_oid;\n+\tstruct commit_list *parent = orig_commit->parents;\n+\tstruct commit_list *new_parents_head = NULL;\n+\tstruct commit_list **new_parents = &new_parents_head;\n+\n+\twhile (parent) {\n+\t\tstruct commit *parent_commit = parent->item;\n+\t\tstruct commit *new_parent;\n+\t\tstruct recycle_parent_mapping *parent_mapping;\n+\n+\t\tparent_mapping = oidmap_get(parents,\n+\t\t\t\t\t    &parent_commit->object.oid);\n+\n+\t\tnew_parent = parent_mapping ?\n+\t\t\t\tparent_mapping->new_parent :\n+\t\t\t\tparent_commit;\n+\n+\t\tnew_parents = commit_list_append(new_parent,\n+\t\t\t\t\t\t new_parents);\n+\t\tparent = parent->next;\n+\t}\n+\n+\tmessage = get_commit_buffer(orig_commit, &message_length);\n+\tauthor_raw = find_commit_header(message, \"author\", &author_length);\n+\tstrbuf_add(&author, author_raw, author_length);\n+\tfind_commit_subject(message, &body);\n+\tbody_length = message_length - (body - message);\n+\n+\tresult = commit_tree(body, body_length,\n+\t\t\t     get_commit_tree_oid(orig_commit),\n+\t\t\t     new_parents_head, &new_commit_oid,\n+\t\t\t     author.buf, NULL);\n+\n+\tif (result)\n+\t\tdie(\"Could not create a recycled revision for %s\\n\",\n+\t\t    oid_to_hex(&orig_commit->object.oid));\n+\n+\tnew_commit = lookup_commit_or_die(&new_commit_oid,\n+\t\t\t\t\t  \"new commit\");\n+\n+\tunuse_commit_buffer(orig_commit, message);\n+\tstrbuf_release(&author);\n+\n+\treturn new_commit;\n+}\n+\n+static void recycle_save_parent_mapping(struct oidmap *parents,\n+\t\t\t\t\tstruct commit *old_parent,\n+\t\t\t\t\tstruct commit *new_parent)\n+{\n+\tstruct recycle_parent_mapping *mapping;\n+\tmapping = xmalloc(sizeof(*mapping));\n+\tmapping->new_parent = new_parent;\n+\tmapping->e.oid = old_parent->object.oid;\n+\toidmap_put(parents, mapping);\n+}\n+\n+static struct commit *run_recycle(struct rebase_options *opts)\n+{\n+\tstruct rev_info revs;\n+\tstruct commit *orig_head;\n+\tstruct commit *new_head = NULL;\n+\tstruct commit *commit;\n+\tstruct commit_list *old_commits = NULL;\n+\tstruct commit_list *old_commit;\n+\tstruct oidmap parents;\n+\tstruct progress *progress = NULL;\n+\tint commit_counter = 0;\n+\n+\tinit_revisions(&revs, NULL);\n+\trevs.commit_format = CMIT_FMT_RAW;\n+\torig_head = lookup_commit_or_die(&opts->orig_head, \"head\");\n+\n+\topts->upstream->object.flags |= UNINTERESTING;\n+\tadd_pending_object(&revs, &opts->upstream->object, \"upstream\");\n+\tadd_pending_object(&revs, &orig_head->object, \"head\");\n+\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"Could not get commits to recycle\");\n+\n+\twhile ((commit = get_revision(&revs)) != NULL)\n+\t\tcommit_list_insert(commit, &old_commits);\n+\tsort_in_topological_order(&old_commits, REV_SORT_IN_GRAPH_ORDER);\n+\told_commits = reverse_commit_list(old_commits);\n+\n+\toidmap_init(&parents, commit_list_count(old_commits) + 1);\n+\trecycle_save_parent_mapping(&parents, opts->upstream, opts->onto);\n+\n+\tif (isatty(2)) {\n+\t\tstart_delayed_progress(_(\"Recycling commits\"),\n+\t\t\t\t       commit_list_count(old_commits));\n+\t}\n+\n+\told_commit = old_commits;\n+\twhile (old_commit) {\n+\t\tdisplay_progress(progress, ++commit_counter);\n+\t\tnew_head = recycle_commit(old_commit->item, &parents);\n+\t\trecycle_save_parent_mapping(&parents, old_commit->item,\n+\t\t\t\t\t    new_head);\n+\t\told_commit = old_commit->next;\n+\t}\n+\n+\tstop_progress(&progress);\n+\n+\treturn new_head;\n+}\n+\n+static void recycle_wrapup(struct rebase_options *opts,\n+\t\t\t   const char *branch_name, struct commit *new_head)\n+{\n+\tstruct strvec args = STRVEC_INIT;\n+\tif (opts->head_name) {\n+\t\tstruct wt_status s = { 0 };\n+\n+\t\ts.show_branch = 1;\n+\t\twt_status_prepare(the_repository, &s);\n+\t\twt_status_collect(&s);\n+\t\tif (!strcmp(s.branch, opts->head_name)) {\n+\t\t\tstruct reset_head_opts ropts = { 0 };\n+\t\t\tstruct strbuf msg = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&msg, \"rebase recycle: \"\n+\t\t\t\t    \"moving to %s\",\n+\t\t\t\t    oid_to_hex(&new_head->object.oid));\n+\t\t\tropts.oid = &new_head->object.oid;\n+\t\t\tropts.orig_head = &opts->orig_head,\n+\t\t\tropts.flags = RESET_HEAD_HARD |\n+\t\t\t\t      RESET_HEAD_RUN_POST_CHECKOUT_HOOK;\n+\t\t\tropts.head_msg = msg.buf;\n+\t\t\tropts.default_reflog_action = DEFAULT_REFLOG_ACTION;\n+\t\t\tif (reset_head(the_repository, &ropts))\n+\t\t\t\tdie(_(\"Could not reset\"));\n+\t\t\tstrbuf_release(&msg);\n+\t\t} else {\n+\t\t\tupdate_ref(NULL, opts->head_name,\n+\t\t\t\t   &new_head->object.oid, NULL,\n+\t\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n+\n+\t\t\tstrvec_pushf(&args, \"checkout\");\n+\t\t\tstrvec_pushf(&args, \"--quiet\");\n+\t\t\tstrvec_pushf(&args, \"%s\", branch_name);\n+\n+\t\t\trun_command_v_opt(args.v, RUN_GIT_CMD);\n+\t\t\tstrvec_clear(&args);\n+\t\t}\n+\t} else {\n+\t\tstrvec_pushf(&args, \"checkout\");\n+\t\tstrvec_pushf(&args, \"--quiet\");\n+\t\tstrvec_pushf(&args, \"%s\", oid_to_hex(&new_head->object.oid));\n+\n+\t\trun_command_v_opt(args.v, RUN_GIT_CMD);\n+\t\tstrvec_clear(&args);\n+\t}\n+}\n+\n static int rebase_config(const char *var, const char *value, void *data)\n {\n \tstruct rebase_options *opts = data;\n@@ -1154,6 +1343,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"automatically re-schedule any `exec` that fails\")),\n \t\tOPT_BOOL(0, \"reapply-cherry-picks\", &options.reapply_cherry_picks,\n \t\t\t N_(\"apply all changes, even those already present upstream\")),\n+\t\tOPT_BIT(0, \"recycle\", &options.flags,\n+\t\t\tN_(\"Run a recycle, if possible. Fails otherwise.\"),\n+\t\t\tREBASE_RECYCLE_OR_FAIL),\n+\t\tOPT_BIT(0, \"attempt-recycle\", &options.flags,\n+\t\t\tN_(\"Run a recycle, if possible. Continue with other approaches if it can't be done.\"),\n+\t\t\tREBASE_ATTEMPT_RECYCLE),\n \t\tOPT_END(),\n \t};\n \tint i;\n@@ -1234,6 +1429,20 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"The --edit-todo action can only be used during \"\n \t\t      \"interactive rebase.\"));\n \n+\tif (options.flags & (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE)) {\n+\t\tif ((options.flags & (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE)) ==\n+\t\t    (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE))\n+\t\t\tdie(_(\"Can't use both --recycle and --attempt-recycle.\"));\n+\t\tif (options.flags & REBASE_INTERACTIVE_EXPLICIT)\n+\t\t\tdie(_(\"Can't use --recycle/--attempt-recycle with interactive mode.\"));\n+\t\tif (options.strategy) {\n+\t\t\tdie(_(\"Can't specify a strategy when using --recycle/--atempt-recycle.\"));\n+\t\t}\n+\t\tif (options.signoff) {\n+\t\t\tdie(_(\"Can't use --signoff with --recycle/--atempt-recycle\"));\n+\t\t}\n+\t}\n+\n \tif (trace2_is_enabled()) {\n \t\tif (is_merge(&options))\n \t\t\ttrace2_cmd_mode(\"interactive\");\n@@ -1761,6 +1970,25 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tdiff_flush(&opts);\n \t}\n \n+\tif (options.flags & (REBASE_ATTEMPT_RECYCLE |\n+\t\t\t     REBASE_RECYCLE_OR_FAIL)) {\n+\t\tif (can_recycle(&options)) {\n+\t\t\tstruct commit *new_head = run_recycle(&options);\n+\t\t\tif (new_head) {\n+\t\t\t\toptions.type = REBASE_RECYCLE;\n+\t\t\t\tprintf(_(\"Recycled %s onto %s.\\n\"),\n+\t\t\t\t\tbranch_name, options.onto_name);\n+\t\t\t\trecycle_wrapup(&options, branch_name,\n+\t\t\t\t\t       new_head);\n+\t\t\t\tret = 0;\n+\t\t\t\tgoto cleanup;\n+\t\t\t}\n+\t\t} else\n+\t\t\tprintf(_(\"upstream and onto do not share the same tree. \"\n+\t\t\t\t \"Can't run a recycle.\\n\"));\n+\t\tif (options.flags & REBASE_RECYCLE_OR_FAIL)\n+\t\t\tdie(_(\"Recycle failed.\"));\n+\t}\n \tif (is_merge(&options))\n \t\tgoto run_rebase;\n \n-- \n2.35.1\n\nThis is the rebase-based implementation of my original\ngit replay concept.\n\nDecided to change the name to \"recycle\" because there is already\ncode that relates to \"replay\" in rebase... and we are \"recycling\"\ntrees so the name sounds appropriate (but might consider other\nproposals if they gather steam).\n\nThere are things that are missing like documentation\nand I will gladly add them (along with correcting anything\ncoming from code review) if this feature is interesting enough\nfor inclusion in the main line.\n\nLet me know!\n\nPS Hope I am adding this side comment at the right place.\n"},{"id":"454279","messageId":"xmqqsfq46b4p.fsf@gitster.g","threadId":"57782","inReplyTo":"20220422183744.347327-1-eantoranz@gmail.com","subject":"Re: [PATCH v1] rebase - recycle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-22T21:50:30Z","receivedAt":"2022-04-22T22:33:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n>  builtin/rebase.c | 230 ++++++++++++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 229 insertions(+), 1 deletion(-)\n\nHmph, somebody other than myself would have a nice thing to say\nabout this.  I have a feeling that this case is too narrow to be\nworth adding 230 lines, if it requires end-user intervention.\n\nWithout any additional end-user input like \"--recycle\", is it\npossible to dynamically and cheaply tell if the recycle mechanism\ncan be silently and transparently usable as a more efficient\nalternative that would produce exactly the same result as the normal\nrebase mechanism?  Using\n\n (1) the range being rebased,\n (2) the tree of the \"onto\" commit, and\n (3) the desired shape of the resulting history\n\nas input, wouldn't you be able to tell?  IOW, with\n\n\t$ git rebase --onto=A B C \n\nif there is no clear bottom of the range being rebased (i.e. \"git\nmerge-base B C\" may have multiple commits), or if the bottom of the\nrange being rebased has a tree different from that of commit A, then\nwe know \"recycle\" would not work even without trying.\n\nAlso, when the range B..C is not a single strand of pearls and we\nended up choosing REBASE_APPLY due to what the command line or\nconfigration variable said, we cannot use recycle even if the tree\nof the bottom of the range matches the tree of A, because\nREBASE_APPLY wants to lineralize the history, while recycle\nmechinery is about replanting the whole bush structure verbatim, so\nit is a bad match.\n\nBut when (i) there is a clear single bottom of the range B..C (let's\ncall that X), and (ii) the tree of that bottom matches the tree of A\n(i.e. X^{tree} == A^{tree}) and (iii) either we are asked to do\nREBASE_MERGE or the history being rebased B..C is linear, then\n\n              B\n             /\n        o---X---M---N---Q---C\n         \\       \\     /\n          \\       O---P\n           \\\n            A\n\nwe should be able to exercise the recycle engine _without_ even\ntelling the user that we did, and the only visible effect to the\nend-user and to the resulting history is that we (hopefully) did a\nbetter job with smaller amount of CPU cycles, no?\n\n              B\n             /\n        o---X\n         \\ \n          \\\n           \\\n            A---M'--N'--Q'--C\n                 \\     /\n                  O'--P'\n\nWithout thinking too much about it, I do not think there is any case\nwhere you cannot tell mechanically that recycle would be usable as a\npure optimization.  And if that is the case, forcing end-user to say\n\"try recycle, it might work, and otherwise fall-back\" does not help\nanybody.  If it is automatable easily, we should spend extra brain\ncycles to automate it and not bother the users.\n\nThat way, you do not even need to add a single line of\ndocumentation, even though you still need to have tests.\n\n> This is the rebase-based implementation of my original\n> git replay concept.\n> \n> Decided to change the name to \"recycle\" because there is already\n> code that relates to \"replay\" in rebase... and we are \"recycling\"\n> trees so the name sounds appropriate (but might consider other\n> proposals if they gather steam).\n\nI saw \"recycle commit\" in the code, but you are indeed recycling\ntrees.  But I prefer to see us think it through---I have this\nfeeling that we do not have to expose any of the candidate words\nrecycle, replay, replant, ... to end users and just use the new code\nas a special codepath that does not call out to the true merge\nengine.\n\n> There are things that are missing like documentation\n> and I will gladly add them (along with correcting anything\n> coming from code review) if this feature is interesting enough\n> for inclusion in the main line.\n\nThe last thing I want to hear from contributors in the open source\ndevelopment setting: I'll polish it more if you promise this will be\nincluded.\n\nIf it is NOT even interesting and useful enough to make you want to\npolish and perfect it, even when you were the only user, why should\nwe be interested?  Even if your userbase starts at zero (or one,\ncounting yourself), if you make it so good, other people will come\nto you, begging you to add that to the public tool.\n"},{"id":"454313","messageId":"220423.86bkwsra4a.gmgdl@evledraar.gmail.com","threadId":"57782","inReplyTo":"20220422183744.347327-1-eantoranz@gmail.com","subject":"Re: [PATCH v1] rebase - recycle","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-22T22:57:12Z","receivedAt":"2022-04-22T23:25:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 22 2022, Edmundo Carmona Antoranz wrote:\n\nJust minor nits from a quick read. Not on the main logic, just syntax\netc. issues:\n\n>  #define DEFAULT_REFLOG_ACTION \"rebase\"\n>  \n> @@ -49,7 +52,8 @@ static GIT_PATH_FUNC(merge_dir, \"rebase-merge\")\n>  enum rebase_type {\n>  \tREBASE_UNSPECIFIED = -1,\n>  \tREBASE_APPLY,\n> -\tREBASE_MERGE\n> +\tREBASE_MERGE,\n> +\tREBASE_RECYCLE\n>  };\n>  \n>  enum empty_type {\n> @@ -83,6 +87,8 @@ struct rebase_options {\n>  \t\tREBASE_DIFFSTAT = 1<<2,\n>  \t\tREBASE_FORCE = 1<<3,\n>  \t\tREBASE_INTERACTIVE_EXPLICIT = 1<<4,\n> +\t\tREBASE_RECYCLE_OR_FAIL = 1 << 5,\n> +\t\tREBASE_ATTEMPT_RECYCLE = 1<<6,\n\nNeeds consistent whitespace for <<.\n\n>  \t} flags;\n>  \tstruct strvec git_am_opts;\n>  \tconst char *action;\n> @@ -104,6 +110,16 @@ struct rebase_options {\n>  \tint fork_point;\n>  };\n>  \n> +struct recycle_parent_mapping {\n> +\tstruct oidmap_entry e;\n> +\tstruct commit *new_parent;\n> +};\n> +\n> +struct recycle_progress_info {\n> +\tstruct progress *progress;\n> +\tint commits;\n> +};\n> +\n>  #define REBASE_OPTIONS_INIT {\t\t\t  \t\\\n>  \t\t.type = REBASE_UNSPECIFIED,\t  \t\\\n>  \t\t.empty = EMPTY_UNSPECIFIED,\t  \t\\\n> @@ -384,6 +400,12 @@ static int is_merge(struct rebase_options *opts)\n>  \treturn opts->type == REBASE_MERGE;\n>  }\n>  \n> +static int can_recycle(struct rebase_options *opts)\n> +{\n> +\treturn oideq(get_commit_tree_oid(opts->onto),\n> +\t\t     get_commit_tree_oid(opts->upstream));\n> +}\n> +\n>  static void imply_merge(struct rebase_options *opts, const char *option)\n>  {\n>  \tswitch (opts->type) {\n> @@ -771,6 +793,173 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)\n>  \treturn status ? -1 : 0;\n>  }\n>  \n> +static struct commit *recycle_commit(struct commit *orig_commit,\n> +\t\t\t\t     struct oidmap *parents)\n> +{\n> +\tconst char *body;\n> +\tsize_t body_length;\n> +\tconst char *author_raw;\n> +\tsize_t author_length;\n> +\tstruct strbuf author = STRBUF_INIT;\n> +\tconst char *message;\n> +\tsize_t message_length;\n> +\tint result;\n> +\n\nWe tend not to \\n\\n-break the variables.\n\n\n> +\tif (result)\n> +\t\tdie(\"Could not create a recycled revision for %s\\n\",\n\nError messages should not start wth capital letters, so \"could..\" not\n\"Could\". Also needs _() markings for translation. Then don't add a \\n.\n\n> +\tstruct recycle_parent_mapping *mapping;\n> +\tmapping = xmalloc(sizeof(*mapping));\n\nAdd \\n\\n between variable decls & code.\n\nIn this case though we can just put the xmalloc() with decl...\n\n\n> +\tstruct commit *orig_head;\n> +\tstruct commit *new_head = NULL;\n> +\tstruct commit *commit;\n> +\tstruct commit_list *old_commits = NULL;\n> +\tstruct commit_list *old_commit;\n> +\tstruct oidmap parents;\n> +\tstruct progress *progress = NULL;\n> +\tint commit_counter = 0;\n> +\n> +\tinit_revisions(&revs, NULL);\n> +\trevs.commit_format = CMIT_FMT_RAW;\n> +\torig_head = lookup_commit_or_die(&opts->orig_head, \"head\");\n\nJust declare this with the variable, seems it doesn't need\ninit_revisions, or does it...?\n\n> +\n> +\topts->upstream->object.flags |= UNINTERESTING;\n> +\tadd_pending_object(&revs, &opts->upstream->object, \"upstream\");\n> +\tadd_pending_object(&revs, &orig_head->object, \"head\");\n> +\n> +\tif (prepare_revision_walk(&revs))\n> +\t\tdie(\"Could not get commits to recycle\");\n\nditto capital letters, _() etc.\n\n> +\tif (isatty(2)) {\n\nSkip {} for one-statement if's.\n\n> +\t\tstart_delayed_progress(_(\"Recycling commits\"),\n\nMissing the assignment to progerss, so this never works, presumably...\n\n> +\t\t\t\t       commit_list_count(old_commits));\n> +\t}\n> +\n> +\told_commit = old_commits;\n> +\twhile (old_commit) {\n> +\t\tdisplay_progress(progress, ++commit_counter);\n\nI.e. still NULL here...\n\n> +\t\tnew_head = recycle_commit(old_commit->item, &parents);\n> +\t\trecycle_save_parent_mapping(&parents, old_commit->item,\n> +\t\t\t\t\t    new_head);\n> +\t\told_commit = old_commit->next;\n> +\t}\n> +\n> +\tstop_progress(&progress);\n> +\n> +\treturn new_head;\n> +}\n> +\n> +static void recycle_wrapup(struct rebase_options *opts,\n> +\t\t\t   const char *branch_name, struct commit *new_head)\n> +{\n> +\tstruct strvec args = STRVEC_INIT;\n\n\\n\\n again.\n\n> +\tif (opts->head_name) {\n> +\t\tstruct wt_status s = { 0 };\n> +\n> +\t\ts.show_branch = 1;\n> +\t\twt_status_prepare(the_repository, &s);\n> +\t\twt_status_collect(&s);\n> +\t\tif (!strcmp(s.branch, opts->head_name)) {\n> +\t\t\tstruct reset_head_opts ropts = { 0 };\n> +\t\t\tstruct strbuf msg = STRBUF_INIT;\n\nditto.\n\n> +\t\t\tstrbuf_addf(&msg, \"rebase recycle: \"\n> +\t\t\t\t    \"moving to %s\",\n> +\t\t\t\t    oid_to_hex(&new_head->object.oid));\n> +\t\t\tropts.oid = &new_head->object.oid;\n> +\t\t\tropts.orig_head = &opts->orig_head,\n> +\t\t\tropts.flags = RESET_HEAD_HARD |\n> +\t\t\t\t      RESET_HEAD_RUN_POST_CHECKOUT_HOOK;\n> +\t\t\tropts.head_msg = msg.buf;\n> +\t\t\tropts.default_reflog_action = DEFAULT_REFLOG_ACTION;\n> +\t\t\tif (reset_head(the_repository, &ropts))\n> +\t\t\t\tdie(_(\"Could not reset\"));\n> +\t\t\tstrbuf_release(&msg);\n> +\t\t} else {\n> +\t\t\tupdate_ref(NULL, opts->head_name,\n> +\t\t\t\t   &new_head->object.oid, NULL,\n> +\t\t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n> +\n> +\t\t\tstrvec_pushf(&args, \"checkout\");\n> +\t\t\tstrvec_pushf(&args, \"--quiet\");\n> +\t\t\tstrvec_pushf(&args, \"%s\", branch_name);\n\nYou want just strvec_pushl() here., or strvec_push(), but definitely not\nstrvec_pushf(). You're not using the formatting.\n\n> +\n> +\t\t\trun_command_v_opt(args.v, RUN_GIT_CMD);\n> +\t\t\tstrvec_clear(&args);\n> +\t\t}\n> +\t} else {\n> +\t\tstrvec_pushf(&args, \"checkout\");\n> +\t\tstrvec_pushf(&args, \"--quiet\");\n> +\t\tstrvec_pushf(&args, \"%s\", oid_to_hex(&new_head->object.oid));\n\nDitto.\n\n> +\n> +\t\trun_command_v_opt(args.v, RUN_GIT_CMD);\n> +\t\tstrvec_clear(&args);\n> +\t}\n> +}\n> +\n>  static int rebase_config(const char *var, const char *value, void *data)\n>  {\n>  \tstruct rebase_options *opts = data;\n> @@ -1154,6 +1343,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  \t\t\t N_(\"automatically re-schedule any `exec` that fails\")),\n>  \t\tOPT_BOOL(0, \"reapply-cherry-picks\", &options.reapply_cherry_picks,\n>  \t\t\t N_(\"apply all changes, even those already present upstream\")),\n> +\t\tOPT_BIT(0, \"recycle\", &options.flags,\n> +\t\t\tN_(\"Run a recycle, if possible. Fails otherwise.\"),\n\nrun not Run, and drop the \".\" at the end.\n\n> +\t\t\tREBASE_RECYCLE_OR_FAIL),\n> +\t\tOPT_BIT(0, \"attempt-recycle\", &options.flags,\n> +\t\t\tN_(\"Run a recycle, if possible. Continue with other approaches if it can't be done.\"),\n\nDitto.\n\nAlso I think you want \"OPT_BOOL\"...?\n\n> +\t\t\tREBASE_ATTEMPT_RECYCLE),\n>  \t\tOPT_END(),\n>  \t};\n>  \tint i;\n> @@ -1234,6 +1429,20 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>  \t\tdie(_(\"The --edit-todo action can only be used during \"\n>  \t\t      \"interactive rebase.\"));\n>  \n> +\tif (options.flags & (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE)) {\n> +\t\tif ((options.flags & (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE)) ==\n> +\t\t    (REBASE_RECYCLE_OR_FAIL | REBASE_ATTEMPT_RECYCLE))\n> +\t\t\tdie(_(\"Can't use both --recycle and --attempt-recycle.\"));\n\nYou can use OPT_CMDMODE() to declare flags that are mutually exclusive,\nbut maybe it's not a good fit in this case.\n\n> +\t\tif (options.flags & REBASE_INTERACTIVE_EXPLICIT)\n> +\t\t\tdie(_(\"Can't use --recycle/--attempt-recycle with interactive mode.\"));\n> +\t\tif (options.strategy) {\n> +\t\t\tdie(_(\"Can't specify a strategy when using --recycle/--atempt-recycle.\"));\n> +\t\t}\n> +\t\tif (options.signoff) {\n> +\t\t\tdie(_(\"Can't use --signoff with --recycle/--atempt-recycle\"));\n> +\t\t}\n\nAside from capital, _() etc. these can also drop {}'s\n\n> +\t\t\t\tprintf(_(\"Recycled %s onto %s.\\n\"),\n> +\t\t\t\t\tbranch_name, options.onto_name);\n> +\t\t\t\trecycle_wrapup(&options, branch_name,\n> +\t\t\t\t\t       new_head);\n> +\t\t\t\tret = 0;\n> +\t\t\t\tgoto cleanup;\n> +\t\t\t}\n> +\t\t} else\n> +\t\t\tprintf(_(\"upstream and onto do not share the same tree. \"\n> +\t\t\t\t \"Can't run a recycle.\\n\"));\n\nDon't use printf() when puts() would do (the latter case).\n\nThe else is missing {} (see CodingGuidelines). I.e. if one arm gets it\nall of them get it.\n"},{"id":"454327","messageId":"CAOc6etYcByq=n=k+3r=mBR8q=8i-i_G+3LYESN8u0=Krqxt8Fg@mail.gmail.com","threadId":"57782","inReplyTo":"220423.86bkwsra4a.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v1] rebase - recycle","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-23T09:12:49Z","receivedAt":"2022-04-23T09:13:05Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Apr 23, 2022 at 1:06 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n>\nThanks for all the feedback. I'll go through it.\n"},{"id":"454328","messageId":"CAOc6etYMyA3JWX7ZtvQoB2e66f7QVyabOrLTpwgP9XBRoipgfQ@mail.gmail.com","threadId":"57782","inReplyTo":"xmqqsfq46b4p.fsf@gitster.g","subject":"Re: [PATCH v1] rebase - recycle","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-23T09:17:02Z","receivedAt":"2022-04-23T09:17:20Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Fri, Apr 22, 2022 at 11:50 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n>\n> That way, you do not even need to add a single line of\n> documentation, even though you still need to have tests.\n\nThank you for putting the time to think about it. I will take all that\ninput to turn it into a full optimization without user intervention\nand see how it looks.\n\n\n>\n> The last thing I want to hear from contributors in the open source\n> development setting: I'll polish it more if you promise this will be\n> included.\n>\n> If it is NOT even interesting and useful enough to make you want to\n> polish and perfect it, even when you were the only user, why should\n> we be interested?  Even if your userbase starts at zero (or one,\n> counting yourself), if you make it so good, other people will come\n> to you, begging you to add that to the public tool.\n\nThat is on top of \"let's fork this project\"? That is saying something\n:-D (point taken, just in case).\n\nGiven your feedback, I _think_ there is a window of opportunity for\nthis? Let me give it a shot. I will first try to create an equivalent\nof this technique into a per-commit basis to make a broader usecase\nand see if has an impact (performance or avoiding conflicts for\nmerges). Will let you know.\n"},{"id":"454342","messageId":"xmqq4k2j3giw.fsf@gitster.g","threadId":"57782","inReplyTo":"CAOc6etYMyA3JWX7ZtvQoB2e66f7QVyabOrLTpwgP9XBRoipgfQ@mail.gmail.com","subject":"Re: [PATCH v1] rebase - recycle","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-23T16:34:31Z","receivedAt":"2022-04-23T16:34:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n>> If it is NOT even interesting and useful enough to make you want to\n>> polish and perfect it, even when you were the only user, why should\n>> we be interested?  Even if your userbase starts at zero (or one,\n>> counting yourself), if you make it so good, other people will come\n>> to you, begging you to add that to the public tool.\n>\n> That is on top of \"let's fork this project\"? That is saying something\n> :-D (point taken, just in case).\n\nAbsolutely.  If you believe in it, make it so good that people come\nbegging you for it.  Such an attitude is another thing that helps to\nconvince others that what you are doing may be worth paying attention\nto.\n\n> Given your feedback, I _think_ there is a window of opportunity for\n> this? Let me give it a shot. I will first try to create an equivalent\n> of this technique into a per-commit basis to make a broader usecase\n> and see if has an impact (performance or avoiding conflicts for\n> merges). Will let you know.\n\nYup, if we can decide if the \"recycle\" short-cut is worth taking\ncheaply enough, that may be ideal, as the end-user does not have to\ndo or know anything and get an improved behaviour.  Thanks.\n"}]}