{"thread":{"id":"62167","subject":"[PATCH 0/2] rebase-merges: try and use branch names for labels","startedAt":"2024-09-21T18:38:03Z","lastAt":"2024-10-09T17:57:23Z","messageCount":18,"participants":["Nicolas Guichard via GitGitGadget","Junio C Hamano","Eric Sunshine","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"503200","messageId":"pull.1784.git.git.1726943880.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":null,"subject":"[PATCH 0/2] rebase-merges: try and use branch names for labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-21T18:37:58Z","receivedAt":"2024-09-21T18:38:03Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"When interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\nlabel feature0 merge -C $sha feature0 # “Merge branch 'feature0'\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is: *---. 967bfa4 (HEAD -> integration)\nIntegration |\\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2 | |_|/ |/| | | | * c88b01a\nFeature 1 | |/ |/| | * 75f3139 (feat0) Feature 0 |/ * 25c86d0 (main) Initial\ncommit which yields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise. In the example above, the labels become\nfeat0, Integration and feature2.\n\nNicolas Guichard (2):\n  sequencer.c: extract load_branch_decorations\n  rebase-merges: try and use branch names as labels\n\n sequencer.c                   | 50 ++++++++++++++++++++++-------------\n t/t3404-rebase-interactive.sh |  4 +--\n t/t3430-rebase-merges.sh      | 12 ++++-----\n 3 files changed, 40 insertions(+), 26 deletions(-)\n\n\nbase-commit: 94b60adee30619a05296cf5ed6addb0e6d4e25dc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1784%2Fnicolas-guichard%2Fuse-branch-names-for-rebase-merges-labels-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1784/nicolas-guichard/use-branch-names-for-rebase-merges-labels-v1\nPull-Request: https://github.com/git/git/pull/1784\n-- \ngitgitgadget\n"},{"id":"503201","messageId":"7f3d5e5da356f93ebef300ef73bfd6c312013e09.1726943880.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.git.git.1726943880.gitgitgadget@gmail.com","subject":"[PATCH 1/2] sequencer.c: extract load_branch_decorations","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-21T18:37:59Z","receivedAt":"2024-09-21T18:38:04Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nExtract load_branch_decorations from todo_list_add_update_ref_commands so\nit can be re-used in make_script_with_merges.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n sequencer.c | 25 +++++++++++++++----------\n 1 file changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 8d01cd50ac9..e5eb6f8cd76 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5810,6 +5810,20 @@ static const char *label_oid(struct object_id *oid, const char *label,\n \treturn string_entry->string;\n }\n \n+static void load_branch_decorations(void)\n+{\n+\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n+\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n+\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n+\tstruct decoration_filter decoration_filter = {\n+\t\t.include_ref_pattern = &decorate_refs_include,\n+\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n+\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n+\t};\n+\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n+\tload_ref_decorations(&decoration_filter, 0);\n+}\n+\n static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\t\t   struct rev_info *revs, struct strbuf *out,\n \t\t\t\t   unsigned flags)\n@@ -6403,14 +6417,6 @@ static int add_decorations_to_list(const struct commit *commit,\n static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n {\n \tint i, res;\n-\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n-\tstruct decoration_filter decoration_filter = {\n-\t\t.include_ref_pattern = &decorate_refs_include,\n-\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n-\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n-\t};\n \tstruct todo_add_branch_context ctx = {\n \t\t.buf = &todo_list->buf,\n \t\t.refs_to_oids = STRING_LIST_INIT_DUP,\n@@ -6419,8 +6425,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n \tctx.items_alloc = 2 * todo_list->nr + 1;\n \tALLOC_ARRAY(ctx.items, ctx.items_alloc);\n \n-\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n-\tload_ref_decorations(&decoration_filter, 0);\n+\tload_branch_decorations();\n \n \tfor (i = 0; i < todo_list->nr; ) {\n \t\tstruct todo_item *item = &todo_list->items[i];\n-- \ngitgitgadget\n\n"},{"id":"503202","messageId":"9afe428927d71df360929238c50284d4c59beaea.1726943880.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.git.git.1726943880.gitgitgadget@gmail.com","subject":"[PATCH 2/2] rebase-merges: try and use branch names as labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-21T18:38:00Z","receivedAt":"2024-09-21T18:38:04Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nWhen interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\n```\nlabel feature0\n\nmerge -C $sha feature0 # “Merge branch 'feature0'\n```\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is:\n```\n*---.   967bfa4 (HEAD -> integration) Integration\n|\\ \\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2\n| |_|/\n|/| |\n| | * c88b01a Feature 1\n| |/\n|/|\n| * 75f3139 (feat0) Feature 0\n|/\n* 25c86d0 (main) Initial commit\n```\nyields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise.\nIn the example above, the labels become feat0, Integration and feature2.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n sequencer.c                   | 25 +++++++++++++++++--------\n t/t3404-rebase-interactive.sh |  4 ++--\n t/t3430-rebase-merges.sh      | 12 ++++++------\n 3 files changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e5eb6f8cd76..a092bd05692 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5833,7 +5833,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n \tint skipped_commit = 0;\n \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n-\tstruct strbuf label = STRBUF_INIT;\n+\tstruct strbuf label_from_message = STRBUF_INIT;\n \tstruct commit_list *commits = NULL, **tail = &commits, *iter;\n \tstruct commit_list *tips = NULL, **tips_tail = &tips;\n \tstruct commit *commit;\n@@ -5856,6 +5856,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \toidmap_init(&state.commit2label, 0);\n \thashmap_init(&state.labels, labels_cmp, NULL, 0);\n \tstrbuf_init(&state.buf, 32);\n+\tload_branch_decorations();\n \n \tif (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) {\n \t\tstruct labels_entry *onto_label_entry;\n@@ -5916,18 +5917,18 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\tcontinue;\n \t\t}\n \n-\t\t/* Create a label */\n-\t\tstrbuf_reset(&label);\n+\t\t/* Create a label from the commit message */\n+\t\tstrbuf_reset(&label_from_message);\n \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n \t\t    (p1 = strchr(p1, '\\'')) &&\n \t\t    (p2 = strchr(++p1, '\\'')))\n-\t\t\tstrbuf_add(&label, p1, p2 - p1);\n+\t\t\tstrbuf_add(&label_from_message, p1, p2 - p1);\n \t\telse if (skip_prefix(oneline.buf, \"Merge pull request \",\n \t\t\t\t     &p1) &&\n \t\t\t (p1 = strstr(p1, \" from \")))\n-\t\t\tstrbuf_addstr(&label, p1 + strlen(\" from \"));\n+\t\t\tstrbuf_addstr(&label_from_message, p1 + strlen(\" from \"));\n \t\telse\n-\t\t\tstrbuf_addbuf(&label, &oneline);\n+\t\t\tstrbuf_addbuf(&label_from_message, &oneline);\n \n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s -C %s\",\n@@ -5935,6 +5936,14 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \n \t\t/* label the tips of merged branches */\n \t\tfor (; to_merge; to_merge = to_merge->next) {\n+\t\t\tconst char *label = label_from_message.buf;\n+\t\t\tconst struct name_decoration *decoration =\n+\t\t\t\tget_name_decoration(&to_merge->item->object);\n+\n+\t\t\tif (decoration)\n+\t\t\t\tskip_prefix(decoration->name, \"refs/heads/\",\n+\t\t\t\t\t    &label);\n+\n \t\t\toid = &to_merge->item->object.oid;\n \t\t\tstrbuf_addch(&buf, ' ');\n \n@@ -5947,7 +5956,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n \t\t\t\t\t\t\ttips_tail)->next;\n \n-\t\t\tstrbuf_addstr(&buf, label_oid(oid, label.buf, &state));\n+\t\t\tstrbuf_addstr(&buf, label_oid(oid, label, &state));\n \t\t}\n \t\tstrbuf_addf(&buf, \" # %s\", oneline.buf);\n \n@@ -6055,7 +6064,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tfree_commit_list(commits);\n \tfree_commit_list(tips);\n \n-\tstrbuf_release(&label);\n+\tstrbuf_release(&label_from_message);\n \tstrbuf_release(&oneline);\n \tstrbuf_release(&buf);\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..4896a801ee2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1870,7 +1870,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tpick $(git log -1 --format=%h branch2~1) F\n \t\tpick $(git log -1 --format=%h branch2) I\n \t\tupdate-ref refs/heads/branch2\n-\t\tlabel merge\n+\t\tlabel branch2\n \t\treset onto\n \t\tpick $(git log -1 --format=%h refs/heads/second) J\n \t\tupdate-ref refs/heads/second\n@@ -1881,7 +1881,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tupdate-ref refs/heads/third\n \t\tpick $(git log -1 --format=%h HEAD~2) M\n \t\tupdate-ref refs/heads/no-conflict-branch\n-\t\tmerge -C $(git log -1 --format=%h HEAD~1) merge # merge\n+\t\tmerge -C $(git log -1 --format=%h HEAD~1) branch2 # merge\n \t\tupdate-ref refs/heads/merge-branch\n \t\tEOF\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 2aa8593f77a..cb891eeb5fd 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -108,19 +108,19 @@ test_expect_success 'generate correct todo list' '\n \n \treset onto\n \tpick $b B\n-\tlabel E\n+\tlabel first\n \n \treset onto\n \tpick $c C\n \tlabel branch-point\n \tpick $f F\n \tpick $g G\n-\tlabel H\n+\tlabel second\n \n \treset branch-point # C\n \tpick $d D\n-\tmerge -C $e E # E\n-\tmerge -C $h H # H\n+\tmerge -C $e first # E\n+\tmerge -C $h second # H\n \n \tEOF\n \n@@ -462,11 +462,11 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '\n '\n \n test_expect_success 'labels that are object IDs are rewritten' '\n-\tgit checkout -b third B &&\n+\tgit checkout --detach B &&\n \ttest_commit I &&\n \tthird=$(git rev-parse HEAD) &&\n \tgit checkout -b labels main &&\n-\tgit merge --no-commit third &&\n+\tgit merge --no-commit $third &&\n \ttest_tick &&\n \tgit commit -m \"Merge commit '\\''$third'\\'' into labels\" &&\n \techo noop >script-from-scratch &&\n-- \ngitgitgadget\n"},{"id":"503283","messageId":"xmqq7cb217zb.fsf@gitster.g","threadId":"62167","inReplyTo":"7f3d5e5da356f93ebef300ef73bfd6c312013e09.1726943880.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] sequencer.c: extract load_branch_decorations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-23T19:32:40Z","receivedAt":"2024-09-23T19:32:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nicolas Guichard via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Nicolas Guichard <nicolas@guichard.eu>\n>\n> Extract load_branch_decorations from todo_list_add_update_ref_commands so\n> it can be re-used in make_script_with_merges.\n>\n> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n> ---\n>  sequencer.c | 25 +++++++++++++++----------\n>  1 file changed, 15 insertions(+), 10 deletions(-)\n\nThis intends no behaviour change, and it indeed does not make any\nbehaviour change.  Good.\n\n> diff --git a/sequencer.c b/sequencer.c\n> index 8d01cd50ac9..e5eb6f8cd76 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -5810,6 +5810,20 @@ static const char *label_oid(struct object_id *oid, const char *label,\n>  \treturn string_entry->string;\n>  }\n>  \n> +static void load_branch_decorations(void)\n> +{\n> +\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n> +\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n> +\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n> +\tstruct decoration_filter decoration_filter = {\n> +\t\t.include_ref_pattern = &decorate_refs_include,\n> +\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n> +\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n> +\t};\n> +\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n> +\tload_ref_decorations(&decoration_filter, 0);\n> +}\n\nThe load_ref_decorations() function can be called only once per\nprocess, so it does not matter whatever garbage your second and\nlater invocations throw at it.\n\nBut if this function is ever called N times, you'll add N copies of\n\"refs/heads/\" in the _include list, and because the variable is\nstatic, it is not reported as a leak.\n\nIt may have been OK back when this implementation was inside\ntodo_list_add_update_ref_commands(), which was called by\ncomplete_action() only once before the process exited.\n\nBut now you made it a \"reusable\" function, we should clean up after\nourselves, perhaps by dropping \"static\" from these three string_list\ninstances and clearing them after calling load_ref_decorations().\n"},{"id":"503284","messageId":"xmqq34lq17qb.fsf@gitster.g","threadId":"62167","inReplyTo":"9afe428927d71df360929238c50284d4c59beaea.1726943880.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] rebase-merges: try and use branch names as labels","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-23T19:38:04Z","receivedAt":"2024-09-23T19:38:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Running\n\n    git blame -L'/^static int make_script_with_merges/,/^}/' sequencer.c\n\ntells me that the code range this patch touches and the function\nwhose feature this patch extends was mostly introduced in 1644c73c\n(rebase-helper --make-script: introduce a flag to rebase merges,\n2018-04-25) by Johannes Schindelin (CC'ed), so let's ask input from\nhim as an area expert.\n\nThanks.\n\n\"Nicolas Guichard via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> From: Nicolas Guichard <nicolas@guichard.eu>\n>\n> When interactively rebasing merge commits, the commit message is parsed to\n> extract a probably meaningful label name. For instance if the merge commit\n> is “Merge branch 'feature0'”, then the rebase script will have thes lines:\n> ```\n> label feature0\n>\n> merge -C $sha feature0 # “Merge branch 'feature0'\n> ```\n>\n> This heuristic fails in the case of octopus merges or when the merge commit\n> message is actually unrelated to the parent commits.\n>\n> An example that combines both is:\n> ```\n> *---.   967bfa4 (HEAD -> integration) Integration\n> |\\ \\ \\\n> | | | * 2135be1 (feature2, feat2) Feature 2\n> | |_|/\n> |/| |\n> | | * c88b01a Feature 1\n> | |/\n> |/|\n> | * 75f3139 (feat0) Feature 0\n> |/\n> * 25c86d0 (main) Initial commit\n> ```\n> yields the labels Integration, Integration-2 and Integration-3.\n>\n> Fix this by using a branch name for each merge commit's parent that is the\n> tip of at least one branch, and falling back to a label derived from the\n> merge commit message otherwise.\n> In the example above, the labels become feat0, Integration and feature2.\n>\n> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n> ---\n>  sequencer.c                   | 25 +++++++++++++++++--------\n>  t/t3404-rebase-interactive.sh |  4 ++--\n>  t/t3430-rebase-merges.sh      | 12 ++++++------\n>  3 files changed, 25 insertions(+), 16 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index e5eb6f8cd76..a092bd05692 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -5833,7 +5833,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n>  \tint skipped_commit = 0;\n>  \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n> -\tstruct strbuf label = STRBUF_INIT;\n> +\tstruct strbuf label_from_message = STRBUF_INIT;\n>  \tstruct commit_list *commits = NULL, **tail = &commits, *iter;\n>  \tstruct commit_list *tips = NULL, **tips_tail = &tips;\n>  \tstruct commit *commit;\n> @@ -5856,6 +5856,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \toidmap_init(&state.commit2label, 0);\n>  \thashmap_init(&state.labels, labels_cmp, NULL, 0);\n>  \tstrbuf_init(&state.buf, 32);\n> +\tload_branch_decorations();\n>  \n>  \tif (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) {\n>  \t\tstruct labels_entry *onto_label_entry;\n> @@ -5916,18 +5917,18 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n> -\t\t/* Create a label */\n> -\t\tstrbuf_reset(&label);\n> +\t\t/* Create a label from the commit message */\n> +\t\tstrbuf_reset(&label_from_message);\n>  \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n>  \t\t    (p1 = strchr(p1, '\\'')) &&\n>  \t\t    (p2 = strchr(++p1, '\\'')))\n> -\t\t\tstrbuf_add(&label, p1, p2 - p1);\n> +\t\t\tstrbuf_add(&label_from_message, p1, p2 - p1);\n>  \t\telse if (skip_prefix(oneline.buf, \"Merge pull request \",\n>  \t\t\t\t     &p1) &&\n>  \t\t\t (p1 = strstr(p1, \" from \")))\n> -\t\t\tstrbuf_addstr(&label, p1 + strlen(\" from \"));\n> +\t\t\tstrbuf_addstr(&label_from_message, p1 + strlen(\" from \"));\n>  \t\telse\n> -\t\t\tstrbuf_addbuf(&label, &oneline);\n> +\t\t\tstrbuf_addbuf(&label_from_message, &oneline);\n>  \n>  \t\tstrbuf_reset(&buf);\n>  \t\tstrbuf_addf(&buf, \"%s -C %s\",\n> @@ -5935,6 +5936,14 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \n>  \t\t/* label the tips of merged branches */\n>  \t\tfor (; to_merge; to_merge = to_merge->next) {\n> +\t\t\tconst char *label = label_from_message.buf;\n> +\t\t\tconst struct name_decoration *decoration =\n> +\t\t\t\tget_name_decoration(&to_merge->item->object);\n> +\n> +\t\t\tif (decoration)\n> +\t\t\t\tskip_prefix(decoration->name, \"refs/heads/\",\n> +\t\t\t\t\t    &label);\n> +\n>  \t\t\toid = &to_merge->item->object.oid;\n>  \t\t\tstrbuf_addch(&buf, ' ');\n>  \n> @@ -5947,7 +5956,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n>  \t\t\t\t\t\t\ttips_tail)->next;\n>  \n> -\t\t\tstrbuf_addstr(&buf, label_oid(oid, label.buf, &state));\n> +\t\t\tstrbuf_addstr(&buf, label_oid(oid, label, &state));\n>  \t\t}\n>  \t\tstrbuf_addf(&buf, \" # %s\", oneline.buf);\n>  \n> @@ -6055,7 +6064,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>  \tfree_commit_list(commits);\n>  \tfree_commit_list(tips);\n>  \n> -\tstrbuf_release(&label);\n> +\tstrbuf_release(&label_from_message);\n>  \tstrbuf_release(&oneline);\n>  \tstrbuf_release(&buf);\n>  \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index f171af3061d..4896a801ee2 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1870,7 +1870,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n>  \t\tpick $(git log -1 --format=%h branch2~1) F\n>  \t\tpick $(git log -1 --format=%h branch2) I\n>  \t\tupdate-ref refs/heads/branch2\n> -\t\tlabel merge\n> +\t\tlabel branch2\n>  \t\treset onto\n>  \t\tpick $(git log -1 --format=%h refs/heads/second) J\n>  \t\tupdate-ref refs/heads/second\n> @@ -1881,7 +1881,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n>  \t\tupdate-ref refs/heads/third\n>  \t\tpick $(git log -1 --format=%h HEAD~2) M\n>  \t\tupdate-ref refs/heads/no-conflict-branch\n> -\t\tmerge -C $(git log -1 --format=%h HEAD~1) merge # merge\n> +\t\tmerge -C $(git log -1 --format=%h HEAD~1) branch2 # merge\n>  \t\tupdate-ref refs/heads/merge-branch\n>  \t\tEOF\n>  \n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index 2aa8593f77a..cb891eeb5fd 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -108,19 +108,19 @@ test_expect_success 'generate correct todo list' '\n>  \n>  \treset onto\n>  \tpick $b B\n> -\tlabel E\n> +\tlabel first\n>  \n>  \treset onto\n>  \tpick $c C\n>  \tlabel branch-point\n>  \tpick $f F\n>  \tpick $g G\n> -\tlabel H\n> +\tlabel second\n>  \n>  \treset branch-point # C\n>  \tpick $d D\n> -\tmerge -C $e E # E\n> -\tmerge -C $h H # H\n> +\tmerge -C $e first # E\n> +\tmerge -C $h second # H\n>  \n>  \tEOF\n>  \n> @@ -462,11 +462,11 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '\n>  '\n>  \n>  test_expect_success 'labels that are object IDs are rewritten' '\n> -\tgit checkout -b third B &&\n> +\tgit checkout --detach B &&\n>  \ttest_commit I &&\n>  \tthird=$(git rev-parse HEAD) &&\n>  \tgit checkout -b labels main &&\n> -\tgit merge --no-commit third &&\n> +\tgit merge --no-commit $third &&\n>  \ttest_tick &&\n>  \tgit commit -m \"Merge commit '\\''$third'\\'' into labels\" &&\n>  \techo noop >script-from-scratch &&\n"},{"id":"504120","messageId":"pull.1784.v2.git.git.1728084140.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.git.git.1726943880.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] rebase-merges: try and use branch names for labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-04T23:22:17Z","receivedAt":"2024-10-04T23:22:23Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"When interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\n\nlabel feature0\nmerge -C $sha feature0 # “Merge branch 'feature0'\n\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is:\n\n*---.   967bfa4 (HEAD -> integration) Integration\n|\\ \\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2\n| |_|/\n|/| |\n| | * c88b01a Feature 1\n| |/\n|/|\n| * 75f3139 (feat0) Feature 0\n|/\n* `25c86d0` (main) Initial commit\n\n\nwhich yields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise. In the example above, the labels become\nfeat0, Integration and feature2.\n\nChanges since v1:\n\n * moved load_branch_decorations to re-use the decoration_loaded guard and\n   avoid pointlessly appending \"refs/heads/\" to a static string_list, as\n   pointed out by Junio C Hamano (thanks!)\n * fixed a leak in load_branch_decorations found by making the filter\n   string_lists non-static\n\nNicolas Guichard (3):\n  load_branch_decorations: fix memory leak with non-static filters\n  rebase-update-refs: extract load_branch_decorations\n  rebase-merges: try and use branch names as labels\n\n log-tree.c                    | 26 +++++++++++++++++++++++++\n log-tree.h                    |  1 +\n sequencer.c                   | 36 +++++++++++++++++------------------\n t/t3404-rebase-interactive.sh |  4 ++--\n t/t3430-rebase-merges.sh      | 12 ++++++------\n 5 files changed, 53 insertions(+), 26 deletions(-)\n\n\nbase-commit: 8895aca99693e22ec4856744d5b2c25b8736a111\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1784%2Fnicolas-guichard%2Fuse-branch-names-for-rebase-merges-labels-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1784/nicolas-guichard/use-branch-names-for-rebase-merges-labels-v2\nPull-Request: https://github.com/git/git/pull/1784\n\nRange-diff vs v1:\n\n -:  ----------- > 1:  6250a7f6d6c load_branch_decorations: fix memory leak with non-static filters\n 1:  7f3d5e5da35 ! 2:  167418d10d1 sequencer.c: extract load_branch_decorations\n     @@ Metadata\n      Author: Nicolas Guichard <nicolas@guichard.eu>\n      \n       ## Commit message ##\n     -    sequencer.c: extract load_branch_decorations\n     +    rebase-update-refs: extract load_branch_decorations\n      \n          Extract load_branch_decorations from todo_list_add_update_ref_commands so\n          it can be re-used in make_script_with_merges.\n      \n     +    Since it can now be called multiple times, use non-static lists and place\n     +    it next to load_ref_decorations to re-use the decoration_loaded guard.\n     +\n          Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n      \n     - ## sequencer.c ##\n     -@@ sequencer.c: static const char *label_oid(struct object_id *oid, const char *label,\n     - \treturn string_entry->string;\n     + ## log-tree.c ##\n     +@@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flags)\n     + \t}\n       }\n       \n     -+static void load_branch_decorations(void)\n     ++void load_branch_decorations(void)\n      +{\n     -+\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n     -+\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n     -+\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n     -+\tstruct decoration_filter decoration_filter = {\n     -+\t\t.include_ref_pattern = &decorate_refs_include,\n     -+\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n     -+\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n     -+\t};\n     -+\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n     -+\tload_ref_decorations(&decoration_filter, 0);\n     ++\tif (!decoration_loaded) {\n     ++\t\tstruct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n     ++\t\tstruct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n     ++\t\tstruct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n     ++\t\tstruct decoration_filter decoration_filter = {\n     ++\t\t\t.include_ref_pattern = &decorate_refs_include,\n     ++\t\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n     ++\t\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n     ++\t\t};\n     ++\n     ++\t\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n     ++\t\tload_ref_decorations(&decoration_filter, 0);\n     ++\n     ++\t\tstring_list_clear(&decorate_refs_exclude, false);\n     ++\t\tstring_list_clear(&decorate_refs_exclude_config, false);\n     ++\t\tstring_list_clear(&decorate_refs_include, false);\n     ++\t}\n      +}\n      +\n     - static int make_script_with_merges(struct pretty_print_context *pp,\n     - \t\t\t\t   struct rev_info *revs, struct strbuf *out,\n     - \t\t\t\t   unsigned flags)\n     + static void show_parents(struct commit *commit, int abbrev, FILE *file)\n     + {\n     + \tstruct commit_list *p;\n     +\n     + ## log-tree.h ##\n     +@@ log-tree.h: void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n     + \t\t\t     int *need_8bit_cte_p,\n     + \t\t\t     int maybe_multipart);\n     + void load_ref_decorations(struct decoration_filter *filter, int flags);\n     ++void load_branch_decorations(void);\n     + \n     + void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n     + void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\n     +\n     + ## sequencer.c ##\n      @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,\n       static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n       {\n 2:  9afe428927d = 3:  dfa1f0a7648 rebase-merges: try and use branch names as labels\n\n-- \ngitgitgadget\n"},{"id":"504121","messageId":"6250a7f6d6c84654093b383744c59d3559de717b.1728084140.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v2.git.git.1728084140.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] load_branch_decorations: fix memory leak with non-static filters","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-04T23:22:18Z","receivedAt":"2024-10-04T23:22:24Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nload_branch_decorations calls normalize_glob_ref on each string of filter's\nstring_lists. This effectively replaces the potentially non-owning char* of\nthose items with an owning char*.\n\nSet the strdup_string flag on those string_lists.\n\nThis was not caught until now because:\n- when passing string_lists already with the strdup_string already set, the\n  behaviour was correct\n- when passing static string_lists, the new char* remain reachable until\n  program exit\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n log-tree.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 3758e0d3b8e..cd57de2424e 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -232,6 +232,11 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n \t\t\t}\n+\n+\t\t\t// normalize_glob_ref duplicates the strings\n+\t\t\tfilter->exclude_ref_pattern->strdup_strings = true;\n+\t\t\tfilter->include_ref_pattern->strdup_strings = true;\n+\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = true;\n \t\t}\n \t\tdecoration_loaded = 1;\n \t\tdecoration_flags = flags;\n-- \ngitgitgadget\n\n"},{"id":"504122","messageId":"167418d10d137cbcd278a98c95168150df10627d.1728084140.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v2.git.git.1728084140.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] rebase-update-refs: extract load_branch_decorations","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-04T23:22:19Z","receivedAt":"2024-10-04T23:22:25Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nExtract load_branch_decorations from todo_list_add_update_ref_commands so\nit can be re-used in make_script_with_merges.\n\nSince it can now be called multiple times, use non-static lists and place\nit next to load_ref_decorations to re-use the decoration_loaded guard.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n log-tree.c  | 21 +++++++++++++++++++++\n log-tree.h  |  1 +\n sequencer.c | 11 +----------\n 3 files changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex cd57de2424e..5c17aff2265 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -248,6 +248,27 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t}\n }\n \n+void load_branch_decorations(void)\n+{\n+\tif (!decoration_loaded) {\n+\t\tstruct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n+\t\tstruct decoration_filter decoration_filter = {\n+\t\t\t.include_ref_pattern = &decorate_refs_include,\n+\t\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n+\t\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n+\t\t};\n+\n+\t\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n+\t\tload_ref_decorations(&decoration_filter, 0);\n+\n+\t\tstring_list_clear(&decorate_refs_exclude, false);\n+\t\tstring_list_clear(&decorate_refs_exclude_config, false);\n+\t\tstring_list_clear(&decorate_refs_include, false);\n+\t}\n+}\n+\n static void show_parents(struct commit *commit, int abbrev, FILE *file)\n {\n \tstruct commit_list *p;\ndiff --git a/log-tree.h b/log-tree.h\nindex 94978e2c838..ebe491c543c 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -33,6 +33,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t     int *need_8bit_cte_p,\n \t\t\t     int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n+void load_branch_decorations(void);\n \n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\ndiff --git a/sequencer.c b/sequencer.c\nindex 8d01cd50ac9..97959036b50 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6403,14 +6403,6 @@ static int add_decorations_to_list(const struct commit *commit,\n static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n {\n \tint i, res;\n-\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n-\tstruct decoration_filter decoration_filter = {\n-\t\t.include_ref_pattern = &decorate_refs_include,\n-\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n-\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n-\t};\n \tstruct todo_add_branch_context ctx = {\n \t\t.buf = &todo_list->buf,\n \t\t.refs_to_oids = STRING_LIST_INIT_DUP,\n@@ -6419,8 +6411,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n \tctx.items_alloc = 2 * todo_list->nr + 1;\n \tALLOC_ARRAY(ctx.items, ctx.items_alloc);\n \n-\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n-\tload_ref_decorations(&decoration_filter, 0);\n+\tload_branch_decorations();\n \n \tfor (i = 0; i < todo_list->nr; ) {\n \t\tstruct todo_item *item = &todo_list->items[i];\n-- \ngitgitgadget\n\n"},{"id":"504123","messageId":"dfa1f0a76488e981a0d3e75b49e7fb65c071a285.1728084140.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v2.git.git.1728084140.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] rebase-merges: try and use branch names as labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-04T23:22:20Z","receivedAt":"2024-10-04T23:22:26Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nWhen interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\n```\nlabel feature0\n\nmerge -C $sha feature0 # “Merge branch 'feature0'\n```\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is:\n```\n*---.   967bfa4 (HEAD -> integration) Integration\n|\\ \\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2\n| |_|/\n|/| |\n| | * c88b01a Feature 1\n| |/\n|/|\n| * 75f3139 (feat0) Feature 0\n|/\n* 25c86d0 (main) Initial commit\n```\nyields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise.\nIn the example above, the labels become feat0, Integration and feature2.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n sequencer.c                   | 25 +++++++++++++++++--------\n t/t3404-rebase-interactive.sh |  4 ++--\n t/t3430-rebase-merges.sh      | 12 ++++++------\n 3 files changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 97959036b50..353d804999b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5819,7 +5819,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n \tint skipped_commit = 0;\n \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n-\tstruct strbuf label = STRBUF_INIT;\n+\tstruct strbuf label_from_message = STRBUF_INIT;\n \tstruct commit_list *commits = NULL, **tail = &commits, *iter;\n \tstruct commit_list *tips = NULL, **tips_tail = &tips;\n \tstruct commit *commit;\n@@ -5842,6 +5842,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \toidmap_init(&state.commit2label, 0);\n \thashmap_init(&state.labels, labels_cmp, NULL, 0);\n \tstrbuf_init(&state.buf, 32);\n+\tload_branch_decorations();\n \n \tif (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) {\n \t\tstruct labels_entry *onto_label_entry;\n@@ -5902,18 +5903,18 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\tcontinue;\n \t\t}\n \n-\t\t/* Create a label */\n-\t\tstrbuf_reset(&label);\n+\t\t/* Create a label from the commit message */\n+\t\tstrbuf_reset(&label_from_message);\n \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n \t\t    (p1 = strchr(p1, '\\'')) &&\n \t\t    (p2 = strchr(++p1, '\\'')))\n-\t\t\tstrbuf_add(&label, p1, p2 - p1);\n+\t\t\tstrbuf_add(&label_from_message, p1, p2 - p1);\n \t\telse if (skip_prefix(oneline.buf, \"Merge pull request \",\n \t\t\t\t     &p1) &&\n \t\t\t (p1 = strstr(p1, \" from \")))\n-\t\t\tstrbuf_addstr(&label, p1 + strlen(\" from \"));\n+\t\t\tstrbuf_addstr(&label_from_message, p1 + strlen(\" from \"));\n \t\telse\n-\t\t\tstrbuf_addbuf(&label, &oneline);\n+\t\t\tstrbuf_addbuf(&label_from_message, &oneline);\n \n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s -C %s\",\n@@ -5921,6 +5922,14 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \n \t\t/* label the tips of merged branches */\n \t\tfor (; to_merge; to_merge = to_merge->next) {\n+\t\t\tconst char *label = label_from_message.buf;\n+\t\t\tconst struct name_decoration *decoration =\n+\t\t\t\tget_name_decoration(&to_merge->item->object);\n+\n+\t\t\tif (decoration)\n+\t\t\t\tskip_prefix(decoration->name, \"refs/heads/\",\n+\t\t\t\t\t    &label);\n+\n \t\t\toid = &to_merge->item->object.oid;\n \t\t\tstrbuf_addch(&buf, ' ');\n \n@@ -5933,7 +5942,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n \t\t\t\t\t\t\ttips_tail)->next;\n \n-\t\t\tstrbuf_addstr(&buf, label_oid(oid, label.buf, &state));\n+\t\t\tstrbuf_addstr(&buf, label_oid(oid, label, &state));\n \t\t}\n \t\tstrbuf_addf(&buf, \" # %s\", oneline.buf);\n \n@@ -6041,7 +6050,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tfree_commit_list(commits);\n \tfree_commit_list(tips);\n \n-\tstrbuf_release(&label);\n+\tstrbuf_release(&label_from_message);\n \tstrbuf_release(&oneline);\n \tstrbuf_release(&buf);\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..4896a801ee2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1870,7 +1870,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tpick $(git log -1 --format=%h branch2~1) F\n \t\tpick $(git log -1 --format=%h branch2) I\n \t\tupdate-ref refs/heads/branch2\n-\t\tlabel merge\n+\t\tlabel branch2\n \t\treset onto\n \t\tpick $(git log -1 --format=%h refs/heads/second) J\n \t\tupdate-ref refs/heads/second\n@@ -1881,7 +1881,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tupdate-ref refs/heads/third\n \t\tpick $(git log -1 --format=%h HEAD~2) M\n \t\tupdate-ref refs/heads/no-conflict-branch\n-\t\tmerge -C $(git log -1 --format=%h HEAD~1) merge # merge\n+\t\tmerge -C $(git log -1 --format=%h HEAD~1) branch2 # merge\n \t\tupdate-ref refs/heads/merge-branch\n \t\tEOF\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 2aa8593f77a..cb891eeb5fd 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -108,19 +108,19 @@ test_expect_success 'generate correct todo list' '\n \n \treset onto\n \tpick $b B\n-\tlabel E\n+\tlabel first\n \n \treset onto\n \tpick $c C\n \tlabel branch-point\n \tpick $f F\n \tpick $g G\n-\tlabel H\n+\tlabel second\n \n \treset branch-point # C\n \tpick $d D\n-\tmerge -C $e E # E\n-\tmerge -C $h H # H\n+\tmerge -C $e first # E\n+\tmerge -C $h second # H\n \n \tEOF\n \n@@ -462,11 +462,11 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '\n '\n \n test_expect_success 'labels that are object IDs are rewritten' '\n-\tgit checkout -b third B &&\n+\tgit checkout --detach B &&\n \ttest_commit I &&\n \tthird=$(git rev-parse HEAD) &&\n \tgit checkout -b labels main &&\n-\tgit merge --no-commit third &&\n+\tgit merge --no-commit $third &&\n \ttest_tick &&\n \tgit commit -m \"Merge commit '\\''$third'\\'' into labels\" &&\n \techo noop >script-from-scratch &&\n-- \ngitgitgadget\n"},{"id":"504130","messageId":"CAPig+cRQsai41pYV8wjPKqHz5+bF7ZTEX+UQc9zw8CG-FWUTkw@mail.gmail.com","threadId":"62167","inReplyTo":"6250a7f6d6c84654093b383744c59d3559de717b.1728084140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] load_branch_decorations: fix memory leak with non-static filters","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-05T03:43:10Z","receivedAt":"2024-10-05T03:43:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 4, 2024 at 7:22 PM Nicolas Guichard via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> load_branch_decorations calls normalize_glob_ref on each string of filter's\n> string_lists. This effectively replaces the potentially non-owning char* of\n> those items with an owning char*.\n>\n> Set the strdup_string flag on those string_lists.\n>\n> This was not caught until now because:\n> - when passing string_lists already with the strdup_string already set, the\n>   behaviour was correct\n> - when passing static string_lists, the new char* remain reachable until\n>   program exit\n>\n> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n\nNot a proper review; just a couple style nits below...\n\n> diff --git a/log-tree.c b/log-tree.c\n> @@ -232,6 +232,11 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n> +                       // normalize_glob_ref duplicates the strings\n> +                       filter->exclude_ref_pattern->strdup_strings = true;\n> +                       filter->include_ref_pattern->strdup_strings = true;\n> +                       filter->exclude_ref_config_pattern->strdup_strings = true;\n\nFor the sake of older platforms and older compilers, we're still\navoiding certain modernisms on this project. Instead:\n\n* use /* ... */ style comments instead of // style\n* use 1 instead of `true` (and 0 instead of `false`)\n"},{"id":"504131","messageId":"CAPig+cT9dofoAPbRCzp-VGHDK9qpy2r2f09aExN=WYFbR0n8aA@mail.gmail.com","threadId":"62167","inReplyTo":"167418d10d137cbcd278a98c95168150df10627d.1728084140.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/3] rebase-update-refs: extract load_branch_decorations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-05T03:44:47Z","receivedAt":"2024-10-05T03:44:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 4, 2024 at 7:22 PM Nicolas Guichard via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Extract load_branch_decorations from todo_list_add_update_ref_commands so\n> it can be re-used in make_script_with_merges.\n>\n> Since it can now be called multiple times, use non-static lists and place\n> it next to load_ref_decorations to re-use the decoration_loaded guard.\n>\n> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n> ---\n> diff --git a/log-tree.c b/log-tree.c\n> @@ -248,6 +248,27 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n> +void load_branch_decorations(void)\n> +{\n> +       if (!decoration_loaded) {\n> +               struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n> +               struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n> +               struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n> +               struct decoration_filter decoration_filter = {\n> +                       .include_ref_pattern = &decorate_refs_include,\n> +                       .exclude_ref_pattern = &decorate_refs_exclude,\n> +                       .exclude_ref_config_pattern = &decorate_refs_exclude_config,\n> +               };\n> +\n> +               string_list_append(&decorate_refs_include, \"refs/heads/\");\n> +               load_ref_decorations(&decoration_filter, 0);\n> +\n> +               string_list_clear(&decorate_refs_exclude, false);\n> +               string_list_clear(&decorate_refs_exclude_config, false);\n> +               string_list_clear(&decorate_refs_include, false);\n> +       }\n\nSame minor style nit as previous patch:\n\n* use 0 instead of `false`\n"},{"id":"504527","messageId":"pull.1784.v3.git.git.1728460700.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v2.git.git.1728084140.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] rebase-merges: try and use branch names for labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-09T07:58:17Z","receivedAt":"2024-10-09T07:58:24Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"When interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\n\nlabel feature0\nmerge -C $sha feature0 # “Merge branch 'feature0'\n\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is:\n\n*---.   967bfa4 (HEAD -> integration) Integration\n|\\ \\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2\n| |_|/\n|/| |\n| | * c88b01a Feature 1\n| |/\n|/|\n| * 75f3139 (feat0) Feature 0\n|/\n* `25c86d0` (main) Initial commit\n\n\nwhich yields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise. In the example above, the labels become\nfeat0, Integration and feature2.\n\nChanges since v1:\n\n * moved load_branch_decorations to re-use the decoration_loaded guard and\n   avoid pointlessly appending \"refs/heads/\" to a static string_list, as\n   pointed out by Junio C Hamano (thanks!)\n * fixed a leak in load_branch_decorations found by making the filter\n   string_lists non-static\n\nChanges since v2:\n\n * style changes (true/false -> 1/0 and // -> /* */)\n\nNicolas Guichard (3):\n  load_branch_decorations: fix memory leak with non-static filters\n  rebase-update-refs: extract load_branch_decorations\n  rebase-merges: try and use branch names as labels\n\n log-tree.c                    | 26 +++++++++++++++++++++++++\n log-tree.h                    |  1 +\n sequencer.c                   | 36 +++++++++++++++++------------------\n t/t3404-rebase-interactive.sh |  4 ++--\n t/t3430-rebase-merges.sh      | 12 ++++++------\n 5 files changed, 53 insertions(+), 26 deletions(-)\n\n\nbase-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1784%2Fnicolas-guichard%2Fuse-branch-names-for-rebase-merges-labels-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1784/nicolas-guichard/use-branch-names-for-rebase-merges-labels-v3\nPull-Request: https://github.com/git/git/pull/1784\n\nRange-diff vs v2:\n\n 1:  6250a7f6d6c ! 1:  e030ddd91f3 load_branch_decorations: fix memory leak with non-static filters\n     @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag\n       \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n       \t\t\t}\n      +\n     -+\t\t\t// normalize_glob_ref duplicates the strings\n     -+\t\t\tfilter->exclude_ref_pattern->strdup_strings = true;\n     -+\t\t\tfilter->include_ref_pattern->strdup_strings = true;\n     -+\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = true;\n     ++\t\t\t/* normalize_glob_ref duplicates the strings */\n     ++\t\t\tfilter->exclude_ref_pattern->strdup_strings = 1;\n     ++\t\t\tfilter->include_ref_pattern->strdup_strings = 1;\n     ++\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = 1;\n       \t\t}\n       \t\tdecoration_loaded = 1;\n       \t\tdecoration_flags = flags;\n 2:  167418d10d1 ! 2:  1dad6096eb7 rebase-update-refs: extract load_branch_decorations\n     @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag\n      +\t\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n      +\t\tload_ref_decorations(&decoration_filter, 0);\n      +\n     -+\t\tstring_list_clear(&decorate_refs_exclude, false);\n     -+\t\tstring_list_clear(&decorate_refs_exclude_config, false);\n     -+\t\tstring_list_clear(&decorate_refs_include, false);\n     ++\t\tstring_list_clear(&decorate_refs_exclude, 0);\n     ++\t\tstring_list_clear(&decorate_refs_exclude_config, 0);\n     ++\t\tstring_list_clear(&decorate_refs_include, 0);\n      +\t}\n      +}\n      +\n 3:  dfa1f0a7648 = 3:  19d8253da07 rebase-merges: try and use branch names as labels\n\n-- \ngitgitgadget\n"},{"id":"504528","messageId":"e030ddd91f365b870332467057ab054bb34aa4cd.1728460700.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v3.git.git.1728460700.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] load_branch_decorations: fix memory leak with non-static filters","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-09T07:58:18Z","receivedAt":"2024-10-09T07:58:25Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nload_branch_decorations calls normalize_glob_ref on each string of filter's\nstring_lists. This effectively replaces the potentially non-owning char* of\nthose items with an owning char*.\n\nSet the strdup_string flag on those string_lists.\n\nThis was not caught until now because:\n- when passing string_lists already with the strdup_string already set, the\n  behaviour was correct\n- when passing static string_lists, the new char* remain reachable until\n  program exit\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n log-tree.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 3758e0d3b8e..c65ebd29202 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -232,6 +232,11 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t\t\tfor_each_string_list_item(item, filter->exclude_ref_config_pattern) {\n \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n \t\t\t}\n+\n+\t\t\t/* normalize_glob_ref duplicates the strings */\n+\t\t\tfilter->exclude_ref_pattern->strdup_strings = 1;\n+\t\t\tfilter->include_ref_pattern->strdup_strings = 1;\n+\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = 1;\n \t\t}\n \t\tdecoration_loaded = 1;\n \t\tdecoration_flags = flags;\n-- \ngitgitgadget\n\n"},{"id":"504529","messageId":"1dad6096eb7f329d8b43390f1a445e28d0e609d9.1728460700.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v3.git.git.1728460700.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] rebase-update-refs: extract load_branch_decorations","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-09T07:58:19Z","receivedAt":"2024-10-09T07:58:26Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nExtract load_branch_decorations from todo_list_add_update_ref_commands so\nit can be re-used in make_script_with_merges.\n\nSince it can now be called multiple times, use non-static lists and place\nit next to load_ref_decorations to re-use the decoration_loaded guard.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n log-tree.c  | 21 +++++++++++++++++++++\n log-tree.h  |  1 +\n sequencer.c | 11 +----------\n 3 files changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex c65ebd29202..bf6d2ae40c8 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -248,6 +248,27 @@ void load_ref_decorations(struct decoration_filter *filter, int flags)\n \t}\n }\n \n+void load_branch_decorations(void)\n+{\n+\tif (!decoration_loaded) {\n+\t\tstruct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n+\t\tstruct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n+\t\tstruct decoration_filter decoration_filter = {\n+\t\t\t.include_ref_pattern = &decorate_refs_include,\n+\t\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n+\t\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n+\t\t};\n+\n+\t\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n+\t\tload_ref_decorations(&decoration_filter, 0);\n+\n+\t\tstring_list_clear(&decorate_refs_exclude, 0);\n+\t\tstring_list_clear(&decorate_refs_exclude_config, 0);\n+\t\tstring_list_clear(&decorate_refs_include, 0);\n+\t}\n+}\n+\n static void show_parents(struct commit *commit, int abbrev, FILE *file)\n {\n \tstruct commit_list *p;\ndiff --git a/log-tree.h b/log-tree.h\nindex 94978e2c838..ebe491c543c 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -33,6 +33,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t     int *need_8bit_cte_p,\n \t\t\t     int maybe_multipart);\n void load_ref_decorations(struct decoration_filter *filter, int flags);\n+void load_branch_decorations(void);\n \n void fmt_output_commit(struct strbuf *, struct commit *, struct rev_info *);\n void fmt_output_subject(struct strbuf *, const char *subject, struct rev_info *);\ndiff --git a/sequencer.c b/sequencer.c\nindex 8d01cd50ac9..97959036b50 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6403,14 +6403,6 @@ static int add_decorations_to_list(const struct commit *commit,\n static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n {\n \tint i, res;\n-\tstatic struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;\n-\tstatic struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;\n-\tstruct decoration_filter decoration_filter = {\n-\t\t.include_ref_pattern = &decorate_refs_include,\n-\t\t.exclude_ref_pattern = &decorate_refs_exclude,\n-\t\t.exclude_ref_config_pattern = &decorate_refs_exclude_config,\n-\t};\n \tstruct todo_add_branch_context ctx = {\n \t\t.buf = &todo_list->buf,\n \t\t.refs_to_oids = STRING_LIST_INIT_DUP,\n@@ -6419,8 +6411,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)\n \tctx.items_alloc = 2 * todo_list->nr + 1;\n \tALLOC_ARRAY(ctx.items, ctx.items_alloc);\n \n-\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n-\tload_ref_decorations(&decoration_filter, 0);\n+\tload_branch_decorations();\n \n \tfor (i = 0; i < todo_list->nr; ) {\n \t\tstruct todo_item *item = &todo_list->items[i];\n-- \ngitgitgadget\n\n"},{"id":"504530","messageId":"19d8253da07624aa14ec78d00b549bba8799c55c.1728460700.git.gitgitgadget@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v3.git.git.1728460700.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] rebase-merges: try and use branch names as labels","fromName":"Nicolas Guichard via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-09T07:58:20Z","receivedAt":"2024-10-09T07:58:27Z","isPatch":true,"sender":{"key":"name:Nicolas Guichard","avatar":null},"body":"From: Nicolas Guichard <nicolas@guichard.eu>\n\nWhen interactively rebasing merge commits, the commit message is parsed to\nextract a probably meaningful label name. For instance if the merge commit\nis “Merge branch 'feature0'”, then the rebase script will have thes lines:\n```\nlabel feature0\n\nmerge -C $sha feature0 # “Merge branch 'feature0'\n```\n\nThis heuristic fails in the case of octopus merges or when the merge commit\nmessage is actually unrelated to the parent commits.\n\nAn example that combines both is:\n```\n*---.   967bfa4 (HEAD -> integration) Integration\n|\\ \\ \\\n| | | * 2135be1 (feature2, feat2) Feature 2\n| |_|/\n|/| |\n| | * c88b01a Feature 1\n| |/\n|/|\n| * 75f3139 (feat0) Feature 0\n|/\n* 25c86d0 (main) Initial commit\n```\nyields the labels Integration, Integration-2 and Integration-3.\n\nFix this by using a branch name for each merge commit's parent that is the\ntip of at least one branch, and falling back to a label derived from the\nmerge commit message otherwise.\nIn the example above, the labels become feat0, Integration and feature2.\n\nSigned-off-by: Nicolas Guichard <nicolas@guichard.eu>\n---\n sequencer.c                   | 25 +++++++++++++++++--------\n t/t3404-rebase-interactive.sh |  4 ++--\n t/t3430-rebase-merges.sh      | 12 ++++++------\n 3 files changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 97959036b50..353d804999b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -5819,7 +5819,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n \tint skipped_commit = 0;\n \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n-\tstruct strbuf label = STRBUF_INIT;\n+\tstruct strbuf label_from_message = STRBUF_INIT;\n \tstruct commit_list *commits = NULL, **tail = &commits, *iter;\n \tstruct commit_list *tips = NULL, **tips_tail = &tips;\n \tstruct commit *commit;\n@@ -5842,6 +5842,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \toidmap_init(&state.commit2label, 0);\n \thashmap_init(&state.labels, labels_cmp, NULL, 0);\n \tstrbuf_init(&state.buf, 32);\n+\tload_branch_decorations();\n \n \tif (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) {\n \t\tstruct labels_entry *onto_label_entry;\n@@ -5902,18 +5903,18 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\tcontinue;\n \t\t}\n \n-\t\t/* Create a label */\n-\t\tstrbuf_reset(&label);\n+\t\t/* Create a label from the commit message */\n+\t\tstrbuf_reset(&label_from_message);\n \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n \t\t    (p1 = strchr(p1, '\\'')) &&\n \t\t    (p2 = strchr(++p1, '\\'')))\n-\t\t\tstrbuf_add(&label, p1, p2 - p1);\n+\t\t\tstrbuf_add(&label_from_message, p1, p2 - p1);\n \t\telse if (skip_prefix(oneline.buf, \"Merge pull request \",\n \t\t\t\t     &p1) &&\n \t\t\t (p1 = strstr(p1, \" from \")))\n-\t\t\tstrbuf_addstr(&label, p1 + strlen(\" from \"));\n+\t\t\tstrbuf_addstr(&label_from_message, p1 + strlen(\" from \"));\n \t\telse\n-\t\t\tstrbuf_addbuf(&label, &oneline);\n+\t\t\tstrbuf_addbuf(&label_from_message, &oneline);\n \n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s -C %s\",\n@@ -5921,6 +5922,14 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \n \t\t/* label the tips of merged branches */\n \t\tfor (; to_merge; to_merge = to_merge->next) {\n+\t\t\tconst char *label = label_from_message.buf;\n+\t\t\tconst struct name_decoration *decoration =\n+\t\t\t\tget_name_decoration(&to_merge->item->object);\n+\n+\t\t\tif (decoration)\n+\t\t\t\tskip_prefix(decoration->name, \"refs/heads/\",\n+\t\t\t\t\t    &label);\n+\n \t\t\toid = &to_merge->item->object.oid;\n \t\t\tstrbuf_addch(&buf, ' ');\n \n@@ -5933,7 +5942,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n \t\t\t\t\t\t\ttips_tail)->next;\n \n-\t\t\tstrbuf_addstr(&buf, label_oid(oid, label.buf, &state));\n+\t\t\tstrbuf_addstr(&buf, label_oid(oid, label, &state));\n \t\t}\n \t\tstrbuf_addf(&buf, \" # %s\", oneline.buf);\n \n@@ -6041,7 +6050,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \tfree_commit_list(commits);\n \tfree_commit_list(tips);\n \n-\tstrbuf_release(&label);\n+\tstrbuf_release(&label_from_message);\n \tstrbuf_release(&oneline);\n \tstrbuf_release(&buf);\n \ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..4896a801ee2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1870,7 +1870,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tpick $(git log -1 --format=%h branch2~1) F\n \t\tpick $(git log -1 --format=%h branch2) I\n \t\tupdate-ref refs/heads/branch2\n-\t\tlabel merge\n+\t\tlabel branch2\n \t\treset onto\n \t\tpick $(git log -1 --format=%h refs/heads/second) J\n \t\tupdate-ref refs/heads/second\n@@ -1881,7 +1881,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n \t\tupdate-ref refs/heads/third\n \t\tpick $(git log -1 --format=%h HEAD~2) M\n \t\tupdate-ref refs/heads/no-conflict-branch\n-\t\tmerge -C $(git log -1 --format=%h HEAD~1) merge # merge\n+\t\tmerge -C $(git log -1 --format=%h HEAD~1) branch2 # merge\n \t\tupdate-ref refs/heads/merge-branch\n \t\tEOF\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 2aa8593f77a..cb891eeb5fd 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -108,19 +108,19 @@ test_expect_success 'generate correct todo list' '\n \n \treset onto\n \tpick $b B\n-\tlabel E\n+\tlabel first\n \n \treset onto\n \tpick $c C\n \tlabel branch-point\n \tpick $f F\n \tpick $g G\n-\tlabel H\n+\tlabel second\n \n \treset branch-point # C\n \tpick $d D\n-\tmerge -C $e E # E\n-\tmerge -C $h H # H\n+\tmerge -C $e first # E\n+\tmerge -C $h second # H\n \n \tEOF\n \n@@ -462,11 +462,11 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '\n '\n \n test_expect_success 'labels that are object IDs are rewritten' '\n-\tgit checkout -b third B &&\n+\tgit checkout --detach B &&\n \ttest_commit I &&\n \tthird=$(git rev-parse HEAD) &&\n \tgit checkout -b labels main &&\n-\tgit merge --no-commit third &&\n+\tgit merge --no-commit $third &&\n \ttest_tick &&\n \tgit commit -m \"Merge commit '\\''$third'\\'' into labels\" &&\n \techo noop >script-from-scratch &&\n-- \ngitgitgadget\n"},{"id":"504569","messageId":"ca5f7ce3-f3ec-4627-b503-6b43695c9439@gmail.com","threadId":"62167","inReplyTo":"19d8253da07624aa14ec78d00b549bba8799c55c.1728460700.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 3/3] rebase-merges: try and use branch names as labels","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-09T14:21:55Z","receivedAt":"2024-10-09T14:21:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Nicolas\n\nOn 09/10/2024 08:58, Nicolas Guichard via GitGitGadget wrote:\n> From: Nicolas Guichard <nicolas@guichard.eu>\n> \n> When interactively rebasing merge commits, the commit message is parsed to\n> extract a probably meaningful label name. For instance if the merge commit\n> is “Merge branch 'feature0'”, then the rebase script will have thes lines:\n> ```\n> label feature0\n> \n> merge -C $sha feature0 # “Merge branch 'feature0'\n> ```\n> \n> This heuristic fails in the case of octopus merges or when the merge commit\n> message is actually unrelated to the parent commits.\n> \n> An example that combines both is:\n> ```\n> *---.   967bfa4 (HEAD -> integration) Integration\n> |\\ \\ \\\n> | | | * 2135be1 (feature2, feat2) Feature 2\n> | |_|/\n> |/| |\n> | | * c88b01a Feature 1\n> | |/\n> |/|\n> | * 75f3139 (feat0) Feature 0\n> |/\n> * 25c86d0 (main) Initial commit\n> ```\n> yields the labels Integration, Integration-2 and Integration-3.\n> \n> Fix this by using a branch name for each merge commit's parent that is the\n> tip of at least one branch, and falling back to a label derived from the\n> merge commit message otherwise.\n> In the example above, the labels become feat0, Integration and feature2.\n\nThis looks like a nicely described useful improvement, thank you for \nworking on it. The way the code is structured means we always calculate \nthe fallback label before seeing if there is a branch name we could use \ninstead but as calculating the fallback is cheap I don't think that's a \nproblem in practice.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Nicolas Guichard <nicolas@guichard.eu>\n> ---\n>   sequencer.c                   | 25 +++++++++++++++++--------\n>   t/t3404-rebase-interactive.sh |  4 ++--\n>   t/t3430-rebase-merges.sh      | 12 ++++++------\n>   3 files changed, 25 insertions(+), 16 deletions(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index 97959036b50..353d804999b 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -5819,7 +5819,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \tint root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;\n>   \tint skipped_commit = 0;\n>   \tstruct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;\n> -\tstruct strbuf label = STRBUF_INIT;\n> +\tstruct strbuf label_from_message = STRBUF_INIT;\n>   \tstruct commit_list *commits = NULL, **tail = &commits, *iter;\n>   \tstruct commit_list *tips = NULL, **tips_tail = &tips;\n>   \tstruct commit *commit;\n> @@ -5842,6 +5842,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \toidmap_init(&state.commit2label, 0);\n>   \thashmap_init(&state.labels, labels_cmp, NULL, 0);\n>   \tstrbuf_init(&state.buf, 32);\n> +\tload_branch_decorations();\n>   \n>   \tif (revs->cmdline.nr && (revs->cmdline.rev[0].flags & BOTTOM)) {\n>   \t\tstruct labels_entry *onto_label_entry;\n> @@ -5902,18 +5903,18 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \t\t\tcontinue;\n>   \t\t}\n>   \n> -\t\t/* Create a label */\n> -\t\tstrbuf_reset(&label);\n> +\t\t/* Create a label from the commit message */\n> +\t\tstrbuf_reset(&label_from_message);\n>   \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n>   \t\t    (p1 = strchr(p1, '\\'')) &&\n>   \t\t    (p2 = strchr(++p1, '\\'')))\n> -\t\t\tstrbuf_add(&label, p1, p2 - p1);\n> +\t\t\tstrbuf_add(&label_from_message, p1, p2 - p1);\n>   \t\telse if (skip_prefix(oneline.buf, \"Merge pull request \",\n>   \t\t\t\t     &p1) &&\n>   \t\t\t (p1 = strstr(p1, \" from \")))\n> -\t\t\tstrbuf_addstr(&label, p1 + strlen(\" from \"));\n> +\t\t\tstrbuf_addstr(&label_from_message, p1 + strlen(\" from \"));\n>   \t\telse\n> -\t\t\tstrbuf_addbuf(&label, &oneline);\n> +\t\t\tstrbuf_addbuf(&label_from_message, &oneline);\n>   \n>   \t\tstrbuf_reset(&buf);\n>   \t\tstrbuf_addf(&buf, \"%s -C %s\",\n> @@ -5921,6 +5922,14 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \n>   \t\t/* label the tips of merged branches */\n>   \t\tfor (; to_merge; to_merge = to_merge->next) {\n> +\t\t\tconst char *label = label_from_message.buf;\n> +\t\t\tconst struct name_decoration *decoration =\n> +\t\t\t\tget_name_decoration(&to_merge->item->object);\n> +\n> +\t\t\tif (decoration)\n> +\t\t\t\tskip_prefix(decoration->name, \"refs/heads/\",\n> +\t\t\t\t\t    &label);\n> +\n>   \t\t\toid = &to_merge->item->object.oid;\n>   \t\t\tstrbuf_addch(&buf, ' ');\n>   \n> @@ -5933,7 +5942,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n>   \t\t\t\t\t\t\ttips_tail)->next;\n>   \n> -\t\t\tstrbuf_addstr(&buf, label_oid(oid, label.buf, &state));\n> +\t\t\tstrbuf_addstr(&buf, label_oid(oid, label, &state));\n>   \t\t}\n>   \t\tstrbuf_addf(&buf, \" # %s\", oneline.buf);\n>   \n> @@ -6041,7 +6050,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n>   \tfree_commit_list(commits);\n>   \tfree_commit_list(tips);\n>   \n> -\tstrbuf_release(&label);\n> +\tstrbuf_release(&label_from_message);\n>   \tstrbuf_release(&oneline);\n>   \tstrbuf_release(&buf);\n>   \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index f171af3061d..4896a801ee2 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1870,7 +1870,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n>   \t\tpick $(git log -1 --format=%h branch2~1) F\n>   \t\tpick $(git log -1 --format=%h branch2) I\n>   \t\tupdate-ref refs/heads/branch2\n> -\t\tlabel merge\n> +\t\tlabel branch2\n>   \t\treset onto\n>   \t\tpick $(git log -1 --format=%h refs/heads/second) J\n>   \t\tupdate-ref refs/heads/second\n> @@ -1881,7 +1881,7 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '\n>   \t\tupdate-ref refs/heads/third\n>   \t\tpick $(git log -1 --format=%h HEAD~2) M\n>   \t\tupdate-ref refs/heads/no-conflict-branch\n> -\t\tmerge -C $(git log -1 --format=%h HEAD~1) merge # merge\n> +\t\tmerge -C $(git log -1 --format=%h HEAD~1) branch2 # merge\n>   \t\tupdate-ref refs/heads/merge-branch\n>   \t\tEOF\n>   \n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> index 2aa8593f77a..cb891eeb5fd 100755\n> --- a/t/t3430-rebase-merges.sh\n> +++ b/t/t3430-rebase-merges.sh\n> @@ -108,19 +108,19 @@ test_expect_success 'generate correct todo list' '\n>   \n>   \treset onto\n>   \tpick $b B\n> -\tlabel E\n> +\tlabel first\n>   \n>   \treset onto\n>   \tpick $c C\n>   \tlabel branch-point\n>   \tpick $f F\n>   \tpick $g G\n> -\tlabel H\n> +\tlabel second\n>   \n>   \treset branch-point # C\n>   \tpick $d D\n> -\tmerge -C $e E # E\n> -\tmerge -C $h H # H\n> +\tmerge -C $e first # E\n> +\tmerge -C $h second # H\n>   \n>   \tEOF\n>   \n> @@ -462,11 +462,11 @@ test_expect_success 'A root commit can be a cousin, treat it that way' '\n>   '\n>   \n>   test_expect_success 'labels that are object IDs are rewritten' '\n> -\tgit checkout -b third B &&\n> +\tgit checkout --detach B &&\n>   \ttest_commit I &&\n>   \tthird=$(git rev-parse HEAD) &&\n>   \tgit checkout -b labels main &&\n> -\tgit merge --no-commit third &&\n> +\tgit merge --no-commit $third &&\n>   \ttest_tick &&\n>   \tgit commit -m \"Merge commit '\\''$third'\\'' into labels\" &&\n>   \techo noop >script-from-scratch &&\n\n"},{"id":"504570","messageId":"c94213e3-f8a6-4f16-9096-f83dbd4e7b7e@gmail.com","threadId":"62167","inReplyTo":"pull.1784.v3.git.git.1728460700.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/3] rebase-merges: try and use branch names for labels","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-09T14:22:02Z","receivedAt":"2024-10-09T14:22:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Nicolas\n\nThanks for working on this. This version looks good to me\n\nThanks\n\nPhillip\n\nOn 09/10/2024 08:58, Nicolas Guichard via GitGitGadget wrote:\n> When interactively rebasing merge commits, the commit message is parsed to\n> extract a probably meaningful label name. For instance if the merge commit\n> is “Merge branch 'feature0'”, then the rebase script will have thes lines:\n> \n> label feature0\n> merge -C $sha feature0 # “Merge branch 'feature0'\n> \n> \n> This heuristic fails in the case of octopus merges or when the merge commit\n> message is actually unrelated to the parent commits.\n> \n> An example that combines both is:\n> \n> *---.   967bfa4 (HEAD -> integration) Integration\n> |\\ \\ \\\n> | | | * 2135be1 (feature2, feat2) Feature 2\n> | |_|/\n> |/| |\n> | | * c88b01a Feature 1\n> | |/\n> |/|\n> | * 75f3139 (feat0) Feature 0\n> |/\n> * `25c86d0` (main) Initial commit\n> \n> \n> which yields the labels Integration, Integration-2 and Integration-3.\n> \n> Fix this by using a branch name for each merge commit's parent that is the\n> tip of at least one branch, and falling back to a label derived from the\n> merge commit message otherwise. In the example above, the labels become\n> feat0, Integration and feature2.\n> \n> Changes since v1:\n> \n>   * moved load_branch_decorations to re-use the decoration_loaded guard and\n>     avoid pointlessly appending \"refs/heads/\" to a static string_list, as\n>     pointed out by Junio C Hamano (thanks!)\n>   * fixed a leak in load_branch_decorations found by making the filter\n>     string_lists non-static\n> \n> Changes since v2:\n> \n>   * style changes (true/false -> 1/0 and // -> /* */)\n> \n> Nicolas Guichard (3):\n>    load_branch_decorations: fix memory leak with non-static filters\n>    rebase-update-refs: extract load_branch_decorations\n>    rebase-merges: try and use branch names as labels\n> \n>   log-tree.c                    | 26 +++++++++++++++++++++++++\n>   log-tree.h                    |  1 +\n>   sequencer.c                   | 36 +++++++++++++++++------------------\n>   t/t3404-rebase-interactive.sh |  4 ++--\n>   t/t3430-rebase-merges.sh      | 12 ++++++------\n>   5 files changed, 53 insertions(+), 26 deletions(-)\n> \n> \n> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1784%2Fnicolas-guichard%2Fuse-branch-names-for-rebase-merges-labels-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1784/nicolas-guichard/use-branch-names-for-rebase-merges-labels-v3\n> Pull-Request: https://github.com/git/git/pull/1784\n> \n> Range-diff vs v2:\n> \n>   1:  6250a7f6d6c ! 1:  e030ddd91f3 load_branch_decorations: fix memory leak with non-static filters\n>       @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag\n>         \t\t\t\tnormalize_glob_ref(item, NULL, item->string);\n>         \t\t\t}\n>        +\n>       -+\t\t\t// normalize_glob_ref duplicates the strings\n>       -+\t\t\tfilter->exclude_ref_pattern->strdup_strings = true;\n>       -+\t\t\tfilter->include_ref_pattern->strdup_strings = true;\n>       -+\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = true;\n>       ++\t\t\t/* normalize_glob_ref duplicates the strings */\n>       ++\t\t\tfilter->exclude_ref_pattern->strdup_strings = 1;\n>       ++\t\t\tfilter->include_ref_pattern->strdup_strings = 1;\n>       ++\t\t\tfilter->exclude_ref_config_pattern->strdup_strings = 1;\n>         \t\t}\n>         \t\tdecoration_loaded = 1;\n>         \t\tdecoration_flags = flags;\n>   2:  167418d10d1 ! 2:  1dad6096eb7 rebase-update-refs: extract load_branch_decorations\n>       @@ log-tree.c: void load_ref_decorations(struct decoration_filter *filter, int flag\n>        +\t\tstring_list_append(&decorate_refs_include, \"refs/heads/\");\n>        +\t\tload_ref_decorations(&decoration_filter, 0);\n>        +\n>       -+\t\tstring_list_clear(&decorate_refs_exclude, false);\n>       -+\t\tstring_list_clear(&decorate_refs_exclude_config, false);\n>       -+\t\tstring_list_clear(&decorate_refs_include, false);\n>       ++\t\tstring_list_clear(&decorate_refs_exclude, 0);\n>       ++\t\tstring_list_clear(&decorate_refs_exclude_config, 0);\n>       ++\t\tstring_list_clear(&decorate_refs_include, 0);\n>        +\t}\n>        +}\n>        +\n>   3:  dfa1f0a7648 = 3:  19d8253da07 rebase-merges: try and use branch names as labels\n> \n\n"},{"id":"504618","messageId":"xmqqr08ptb0e.fsf@gitster.g","threadId":"62167","inReplyTo":"ca5f7ce3-f3ec-4627-b503-6b43695c9439@gmail.com","subject":"Re: [PATCH v3 3/3] rebase-merges: try and use branch names as labels","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-10-09T17:57:21Z","receivedAt":"2024-10-09T17:57:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Nicolas\n>\n> On 09/10/2024 08:58, Nicolas Guichard via GitGitGadget wrote:\n>> From: Nicolas Guichard <nicolas@guichard.eu>\n>> When interactively rebasing merge commits, the commit message is\n>> parsed to\n>> extract a probably meaningful label name. For instance if the merge commit\n>> is “Merge branch 'feature0'”, then the rebase script will have thes lines:\n>> ```\n>> label feature0\n>> merge -C $sha feature0 # “Merge branch 'feature0'\n>> ```\n>> This heuristic fails in the case of octopus merges or when the merge\n>> commit\n>> message is actually unrelated to the parent commits.\n>> An example that combines both is:\n>> ```\n>> *---.   967bfa4 (HEAD -> integration) Integration\n>> |\\ \\ \\\n>> | | | * 2135be1 (feature2, feat2) Feature 2\n>> | |_|/\n>> |/| |\n>> | | * c88b01a Feature 1\n>> | |/\n>> |/|\n>> | * 75f3139 (feat0) Feature 0\n>> |/\n>> * 25c86d0 (main) Initial commit\n>> ```\n>> yields the labels Integration, Integration-2 and Integration-3.\n>> Fix this by using a branch name for each merge commit's parent that\n>> is the\n>> tip of at least one branch, and falling back to a label derived from the\n>> merge commit message otherwise.\n>> In the example above, the labels become feat0, Integration and feature2.\n>\n> This looks like a nicely described useful improvement, thank you for\n> working on it. The way the code is structured means we always\n> calculate the fallback label before seeing if there is a branch name\n> we could use instead but as calculating the fallback is cheap I don't\n> think that's a problem in practice.\n\nThanks, both of you.\n\nWill queue.\n"}]}