Git development
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Kristofer Karlsson via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Kristofer Karlsson <krka@spotify.com>
Subject: Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
Date: Tue, 7 Jul 2026 09:49:23 -0400	[thread overview]
Message-ID: <ak0D44nhSH/98WYD@nand.local> (raw)
In-Reply-To: <f9c1482a76493520b948a2e918de7a5481fa1043.1783418384.git.gitgitgadget@gmail.com>

On Tue, Jul 07, 2026 at 09:59:43AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/commit-graph.c b/commit-graph.c
> index 4e39a048c4..c2a711cceb 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source,
>
>  	g = prepare_commit_graph(ctx.r);
>  	for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
> -		g->topo_levels = &topo_levels;
> +		chain->topo_levels = &topo_levels;
>
>  	if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
>  		ctx.changed_paths = 1;

Looks obviously good.

I think that there is a more permanent fix, though, which would have not
allowed this bug to evade both its author, and reviewer (me). I *think*
that we may clear up some scoping issues if we removed g->topo_levels
entirely, and instead stored it in the write_commit_graph_ctx struct.

I haven't thought through the implications of doing so completely, so
it's entirely possible that this idea is bunk for some other reason. But
it was the first thing that came to mind, and so feels worth exploring
to see if it might have prevented something like this from ever
happening in the first place.

Thanks,
Taylor

  reply	other threads:[~2026-07-07 13:49 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  9:59 [PATCH 0/2] commit-graph: fix topo_levels slab propagation regression Kristofer Karlsson via GitGitGadget
2026-07-07  9:59 ` [PATCH 1/2] commit-graph: add trace2 instrumentation for generation DFS Kristofer Karlsson via GitGitGadget
2026-07-07 13:46   ` Taylor Blau
2026-07-07 14:08     ` Kristofer Karlsson
2026-07-10 22:09       ` Taylor Blau
2026-07-10 22:28         ` Junio C Hamano
2026-07-10 22:56           ` Taylor Blau
2026-07-11 21:18             ` Junio C Hamano
2026-07-13 19:55               ` Kristofer Karlsson
2026-07-13 20:42                 ` Junio C Hamano
2026-07-13 22:17                   ` Kristofer Karlsson
2026-07-07 16:55   ` Junio C Hamano
2026-07-07 17:39     ` Kristofer Karlsson
2026-07-07  9:59 ` [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers Kristofer Karlsson via GitGitGadget
2026-07-07 13:49   ` Taylor Blau [this message]
2026-07-07 14:02     ` Kristofer Karlsson
2026-07-07 14:57     ` Kristofer Karlsson
2026-07-10 22:14       ` Taylor Blau
2026-07-13  6:16         ` Patrick Steinhardt
2026-07-14  3:31           ` Taylor Blau
2026-07-07 17:00   ` Junio C Hamano
2026-07-07 17:42     ` Kristofer Karlsson
2026-07-07 20:13       ` Junio C Hamano
2026-07-09 13:43   ` Patrick Steinhardt
2026-07-09 15:02 ` [PATCH v2 0/2] commit-graph: fix topo_levels slab propagation regression Kristofer Karlsson via GitGitGadget
2026-07-09 15:03   ` [PATCH v2 1/2] commit-graph: add trace2 instrumentation for generation DFS Kristofer Karlsson via GitGitGadget
2026-07-09 15:03   ` [PATCH v2 2/2] commit-graph: propagate topo_levels slab to all chain layers Kristofer Karlsson via GitGitGadget
2026-07-10 22:15   ` [PATCH v2 0/2] commit-graph: fix topo_levels slab propagation regression Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak0D44nhSH/98WYD@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=krka@spotify.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox