{"thread":{"id":"65389","subject":"[PATCH] unpack-trees: use explicit repository in trace2 calls","startedAt":"2026-03-30T20:13:30Z","lastAt":"2026-06-09T21:41:06Z","messageCount":9,"participants":["Jayesh Daga via GitGitGadget","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"540425","messageId":"pull.2258.git.git.1774901607564.gitgitgadget@gmail.com","threadId":"65389","inReplyTo":null,"subject":"[PATCH] unpack-trees: use explicit repository in trace2 calls","fromName":"Jayesh Daga via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-30T20:13:27Z","receivedAt":"2026-03-30T20:13:30Z","isPatch":true,"body":"From: Jayesh Daga <jayeshdaga99@gmail.com>\n\ntrace2 calls in unpack-trees.c use the global 'the_repository',\neven though the relevant context provides an explicit repository\npointer via 'istate->repo' or the local 'repo' variable.\n\nUsing the global repository can result in incorrect trace2 output\nwhen multiple repository instances are in use, as events may be\nattributed to the wrong repository.\n\nUse explicit repository pointers instead to ensure correct\nrepository attribution.\n\nSigned-off-by: Jayesh Daga <jayeshdaga99@gmail.com>\n---\n    unpack-trees: use explicit repository in trace2 calls\n    \n    trace2 calls in unpack-trees.c use the global 'the_repository', even\n    though the relevant context provides an explicit repository pointer via\n    'istate->repo' or the local 'repo' variable.\n    \n    Using the global repository can result in incorrect trace2 output when\n    multiple repository instances are in use, as events may be attributed to\n    the wrong repository.\n    \n    Use explicit repository pointers instead in these call sites to ensure\n    correct repository attribution.\n    \n    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com\n    \n    cc :Karthik Nayak karthik.188@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2258%2Fjayesh0104%2Funpack-trees-trace2-repo-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2258/jayesh0104/unpack-trees-trace2-repo-v1\nPull-Request: https://github.com/git/git/pull/2258\n\n unpack-trees.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 998a1e6dc7..191b9d4769 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1780,14 +1780,14 @@ static int clear_ce_flags(struct index_state *istate,\n \n \txsnprintf(label, sizeof(label), \"clear_ce_flags(0x%08lx,0x%08lx)\",\n \t\t  (unsigned long)select_mask, (unsigned long)clear_mask);\n-\ttrace2_region_enter(\"unpack_trees\", label, the_repository);\n+\ttrace2_region_enter(\"unpack_trees\", label, istate->repo);\n \trval = clear_ce_flags_1(istate,\n \t\t\t\tistate->cache,\n \t\t\t\tistate->cache_nr,\n \t\t\t\t&prefix,\n \t\t\t\tselect_mask, clear_mask,\n \t\t\t\tpl, 0, 0);\n-\ttrace2_region_leave(\"unpack_trees\", label, the_repository);\n+\ttrace2_region_leave(\"unpack_trees\", label, istate->repo);\n \n \tstop_progress(&istate->progress);\n \treturn rval;\n@@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tBUG(\"o->df_conflict_entry is an output only field\");\n \n \ttrace_performance_enter();\n-\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", the_repository);\n+\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", repo);\n \n \tprepare_repo_settings(repo);\n \tif (repo->settings.command_requires_full_index) {\n@@ -2007,9 +2007,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t}\n \n \t\ttrace_performance_enter();\n-\t\ttrace2_region_enter(\"unpack_trees\", \"traverse_trees\", the_repository);\n+\t\ttrace2_region_enter(\"unpack_trees\", \"traverse_trees\", repo);\n \t\tret = traverse_trees(o->src_index, len, t, &info);\n-\t\ttrace2_region_leave(\"unpack_trees\", \"traverse_trees\", the_repository);\n+\t\ttrace2_region_leave(\"unpack_trees\", \"traverse_trees\", repo);\n \t\ttrace_performance_leave(\"traverse_trees\");\n \t\tif (ret < 0)\n \t\t\tgoto return_failed;\n@@ -2106,7 +2106,7 @@ done:\n \t\tdir_clear(o->internal.dir);\n \t\to->internal.dir = NULL;\n \t}\n-\ttrace2_region_leave(\"unpack_trees\", \"unpack_trees\", the_repository);\n+\ttrace2_region_leave(\"unpack_trees\", \"unpack_trees\", repo);\n \ttrace_performance_leave(\"unpack_trees\");\n \treturn ret;\n \n\nbase-commit: 5361983c075154725be47b65cca9a2421789e410\n-- \ngitgitgadget\n"},{"id":"540465","messageId":"actcHT_ZHkb58ndi@pks.im","threadId":"65389","inReplyTo":"pull.2258.git.git.1774901607564.gitgitgadget@gmail.com","subject":"Re: [PATCH] unpack-trees: use explicit repository in trace2 calls","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-31T05:31:09Z","receivedAt":"2026-03-31T05:31:15Z","isPatch":true,"body":"On Mon, Mar 30, 2026 at 08:13:27PM +0000, Jayesh Daga via GitGitGadget wrote:\n> From: Jayesh Daga <jayeshdaga99@gmail.com>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 998a1e6dc7..191b9d4769 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \t\tBUG(\"o->df_conflict_entry is an output only field\");\n>  \n>  \ttrace_performance_enter();\n> -\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", the_repository);\n> +\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", repo);\n>  \n>  \tprepare_repo_settings(repo);\n>  \tif (repo->settings.command_requires_full_index) {\n\nThe changes in `unpack_trees()` are a bit misleading -- while it reads\nas if we don't use `the_repository` anymore, we still do because the\nfunction starts with:\n\n  int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)\n  {\n  \tstruct repository *repo = the_repository;\n\nSo would it make sense to maybe have a separate patch where we inject a\nrepository as a parameter to `unpack_trees()`?\n\nOnce that's done we only have a handful of other places, and in all but\ntwo cases we have a repository available via the index. Do we maybe want\nto go all the way so that we can drop `USE_THE_REPOSITORY_VARIABLE` at\nthe end of this series?\n\nThanks!\n\nPatrick\n"},{"id":"540525","messageId":"xmqqy0j82ex8.fsf@gitster.g","threadId":"65389","inReplyTo":"actcHT_ZHkb58ndi@pks.im","subject":"Re: [PATCH] unpack-trees: use explicit repository in trace2 calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-31T15:32:03Z","receivedAt":"2026-03-31T15:32:08Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The changes in `unpack_trees()` are a bit misleading -- while it reads\n> as if we don't use `the_repository` anymore, we still do because the\n> function starts with:\n>\n>   int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)\n>   {\n>   \tstruct repository *repo = the_repository;\n>\n> So would it make sense to maybe have a separate patch where we inject a\n> repository as a parameter to `unpack_trees()`?\n\nWe can see that \"struct unpack_trees_options\" is rich enough in the\nmerge context that it would be a natural place to have it unless it\nis already tehre.\n\nIn fact, o->dst_index->repo should probably be what you want, and\nbecause it would be insane to start from an index in a repo and\nstore the resulting updated index in another repo, there probably\nneeds an assert(o->dst_index->repo == o->src_index->repo) somewhere.\n"},{"id":"540526","messageId":"pull.2258.v2.git.git.1774971267.gitgitgadget@gmail.com","threadId":"65389","inReplyTo":"pull.2258.git.git.1774901607564.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] unpack-trees: use explicit repository in trace2 calls","fromName":"Jayesh Daga via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-31T15:34:25Z","receivedAt":"2026-03-31T15:34:30Z","isPatch":true,"body":"trace2 calls in unpack-trees.c use the global 'the_repository', even though\nthe relevant context provides an explicit repository pointer via\n'istate->repo' or the local 'repo' variable.\n\nUsing the global repository can result in incorrect trace2 output when\nmultiple repository instances are in use, as events may be attributed to the\nwrong repository.\n\nUse explicit repository pointers instead in these call sites to ensure\ncorrect repository attribution.\n\nSigned-off-by: Jayesh Daga jayeshdaga99@gmail.com\n\nv2:\n\n * Use repository from src_index instead of the_repository\n * Address review feedback from Patrick Steinhardt\n * Avoid introducing new API or struct fields\n\ncc :Karthik Nayak karthik.188@gmail.com\n\nJayesh Daga (2):\n  unpack-trees: use repository from index instead of global\n  unpack-trees: use repository from index instead of global\n\n unpack-trees.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\n\nbase-commit: 5361983c075154725be47b65cca9a2421789e410\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2258%2Fjayesh0104%2Funpack-trees-trace2-repo-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2258/jayesh0104/unpack-trees-trace2-repo-v2\nPull-Request: https://github.com/git/git/pull/2258\n\nRange-diff vs v1:\n\n 1:  717da16044 ! 1:  f03ea194e3 unpack-trees: use explicit repository in trace2 calls\n     @@ Metadata\n      Author: Jayesh Daga <jayeshdaga99@gmail.com>\n      \n       ## Commit message ##\n     -    unpack-trees: use explicit repository in trace2 calls\n     +    unpack-trees: use repository from index instead of global\n      \n     -    trace2 calls in unpack-trees.c use the global 'the_repository',\n     -    even though the relevant context provides an explicit repository\n     -    pointer via 'istate->repo' or the local 'repo' variable.\n     +    unpack_trees() currently initializes its repository from the\n     +    global 'the_repository', even though a repository instance is\n     +    already available via the source index.\n      \n     -    Using the global repository can result in incorrect trace2 output\n     -    when multiple repository instances are in use, as events may be\n     -    attributed to the wrong repository.\n     +    Use 'o->src_index->repo' instead of the global variable,\n     +    reducing reliance on global repository state.\n      \n     -    Use explicit repository pointers instead to ensure correct\n     -    repository attribution.\n     +    This is a step towards eliminating global repository usage in\n     +    unpack_trees().\n      \n     +    Suggested-by: Patrick Steinhardt <ps@pks.im>\n          Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>\n      \n       ## unpack-trees.c ##\n -:  ---------- > 2:  fbdf3271b7 unpack-trees: use repository from index instead of global\n\n-- \ngitgitgadget\n"},{"id":"540527","messageId":"f03ea194e34bef7e398a5f1142cee0a439464cc5.1774971267.git.gitgitgadget@gmail.com","threadId":"65389","inReplyTo":"pull.2258.v2.git.git.1774971267.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] unpack-trees: use repository from index instead of global","fromName":"Jayesh Daga via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-31T15:34:26Z","receivedAt":"2026-03-31T15:34:32Z","isPatch":true,"body":"From: Jayesh Daga <jayeshdaga99@gmail.com>\n\nunpack_trees() currently initializes its repository from the\nglobal 'the_repository', even though a repository instance is\nalready available via the source index.\n\nUse 'o->src_index->repo' instead of the global variable,\nreducing reliance on global repository state.\n\nThis is a step towards eliminating global repository usage in\nunpack_trees().\n\nSuggested-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Jayesh Daga <jayeshdaga99@gmail.com>\n---\n unpack-trees.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 998a1e6dc7..191b9d4769 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1780,14 +1780,14 @@ static int clear_ce_flags(struct index_state *istate,\n \n \txsnprintf(label, sizeof(label), \"clear_ce_flags(0x%08lx,0x%08lx)\",\n \t\t  (unsigned long)select_mask, (unsigned long)clear_mask);\n-\ttrace2_region_enter(\"unpack_trees\", label, the_repository);\n+\ttrace2_region_enter(\"unpack_trees\", label, istate->repo);\n \trval = clear_ce_flags_1(istate,\n \t\t\t\tistate->cache,\n \t\t\t\tistate->cache_nr,\n \t\t\t\t&prefix,\n \t\t\t\tselect_mask, clear_mask,\n \t\t\t\tpl, 0, 0);\n-\ttrace2_region_leave(\"unpack_trees\", label, the_repository);\n+\ttrace2_region_leave(\"unpack_trees\", label, istate->repo);\n \n \tstop_progress(&istate->progress);\n \treturn rval;\n@@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\tBUG(\"o->df_conflict_entry is an output only field\");\n \n \ttrace_performance_enter();\n-\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", the_repository);\n+\ttrace2_region_enter(\"unpack_trees\", \"unpack_trees\", repo);\n \n \tprepare_repo_settings(repo);\n \tif (repo->settings.command_requires_full_index) {\n@@ -2007,9 +2007,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t}\n \n \t\ttrace_performance_enter();\n-\t\ttrace2_region_enter(\"unpack_trees\", \"traverse_trees\", the_repository);\n+\t\ttrace2_region_enter(\"unpack_trees\", \"traverse_trees\", repo);\n \t\tret = traverse_trees(o->src_index, len, t, &info);\n-\t\ttrace2_region_leave(\"unpack_trees\", \"traverse_trees\", the_repository);\n+\t\ttrace2_region_leave(\"unpack_trees\", \"traverse_trees\", repo);\n \t\ttrace_performance_leave(\"traverse_trees\");\n \t\tif (ret < 0)\n \t\t\tgoto return_failed;\n@@ -2106,7 +2106,7 @@ done:\n \t\tdir_clear(o->internal.dir);\n \t\to->internal.dir = NULL;\n \t}\n-\ttrace2_region_leave(\"unpack_trees\", \"unpack_trees\", the_repository);\n+\ttrace2_region_leave(\"unpack_trees\", \"unpack_trees\", repo);\n \ttrace_performance_leave(\"unpack_trees\");\n \treturn ret;\n \n-- \ngitgitgadget\n\n"},{"id":"540528","messageId":"fbdf3271b7b05d9e3afc9aa3d476ccd64f867d08.1774971267.git.gitgitgadget@gmail.com","threadId":"65389","inReplyTo":"pull.2258.v2.git.git.1774971267.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] unpack-trees: use repository from index instead of global","fromName":"Jayesh Daga via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-31T15:34:27Z","receivedAt":"2026-03-31T15:34:34Z","isPatch":true,"body":"From: Jayesh Daga <jayeshdaga99@gmail.com>\n\nunpack_trees() currently initializes its repository from the\nglobal 'the_repository', even though a repository instance is\nalready available via the source index.\n\nUse 'o->src_index->repo' instead of the global variable,\nreducing reliance on global repository state.\n\nThis is a step towards eliminating global repository usage in\nunpack_trees().\n\nSuggested-by: Patrick Steinhardt <ps@pks.im>\nSigned-off-by: Jayesh Daga <jayeshdaga99@gmail.com>\n---\n unpack-trees.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 191b9d4769..b42020f16b 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1882,7 +1882,7 @@ static int verify_absent(const struct cache_entry *,\n  */\n int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)\n {\n-\tstruct repository *repo = the_repository;\n+\tstruct repository *repo = o->src_index->repo;\n \tint i, ret;\n \tstatic struct cache_entry *dfc;\n \tstruct pattern_list pl;\n-- \ngitgitgadget\n"},{"id":"540549","messageId":"xmqqjyurzquq.fsf@gitster.g","threadId":"65389","inReplyTo":"xmqqy0j82ex8.fsf@gitster.g","subject":"Re: [PATCH] unpack-trees: use explicit repository in trace2 calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-31T20:27:57Z","receivedAt":"2026-03-31T20:28:01Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n>> The changes in `unpack_trees()` are a bit misleading -- while it reads\n>> as if we don't use `the_repository` anymore, we still do because the\n>> function starts with:\n>>\n>>   int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)\n>>   {\n>>   \tstruct repository *repo = the_repository;\n>>\n>> So would it make sense to maybe have a separate patch where we inject a\n>> repository as a parameter to `unpack_trees()`?\n>\n> We can see that \"struct unpack_trees_options\" is rich enough in the\n> merge context that it would be a natural place to have it unless it\n> is already tehre.\n>\n> In fact, o->dst_index->repo should probably be what you want, and\n> because it would be insane to start from an index in a repo and\n> store the resulting updated index in another repo, there probably\n> needs an assert(o->dst_index->repo == o->src_index->repo) somewhere.\n\nActually, assert(dst_index->repo == src_index->repo) is probably not\nwhat we want, as dst_index can legitimately be NULL, even since\n34110cd4 (Make 'unpack_trees()' have a separate source and\ndestination index, 2008-03-06) introduced srparete src/dst indices\nto unpack_trees() API.\n\n    We will always unpack into our own internal index, but we will take the\n    source from wherever specified, and we will optionally write the result\n    to a specified index (optionally, because not everybody even _wants_ any\n    result: the index diffing really wants to just walk the tree and index\n    in parallel).\n\nSo o->src_index->repo is what we want in this case, I think.\n"},{"id":"540562","messageId":"xmqqldf7y95a.fsf@gitster.g","threadId":"65389","inReplyTo":"pull.2258.v2.git.git.1774971267.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] unpack-trees: use explicit repository in trace2 calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-31T21:35:45Z","receivedAt":"2026-03-31T21:35:48Z","isPatch":true,"body":"\"Jayesh Daga via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Jayesh Daga (2):\n>   unpack-trees: use repository from index instead of global\n>   unpack-trees: use repository from index instead of global\n\nThat is unusual to have two commits with identical title and\nidentical proposed log messages yet with different patch text.\n\nDo you perhaps want to squash them into a single commit?\n"},{"id":"545095","messageId":"xmqqqzmfz91r.fsf@gitster.g","threadId":"65389","inReplyTo":"xmqqldf7y95a.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] unpack-trees: use explicit repository in trace2 calls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-09T21:41:04Z","receivedAt":"2026-06-09T21:41:06Z","isPatch":true,"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Jayesh Daga via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n>> Jayesh Daga (2):\n>>   unpack-trees: use repository from index instead of global\n>>   unpack-trees: use repository from index instead of global\n>\n> That is unusual to have two commits with identical title and\n> identical proposed log messages yet with different patch text.\n>\n> Do you perhaps want to squash them into a single commit?\n\nAfter not hearing anything for full two months, I just decided to\nsquash these two patches into one, as the second one looked clearly\nlike \"ooops, I screwed up and left one pice behind, so here is an\nincremental update\".  I'll mark the result for 'next'.\n\n"}]}