Re: [RFC PATCH 4/6] commit-graph.c: unconditionally load Bloom filters

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 21, 2023, 20:40 UTC
Message-ID: <ZOPLphXoMjq876Wk@nand.local>
In-Reply-To: <20230811220030.3329161-1-jonathantanmy@google.com>

On Fri, Aug 11, 2023 at 03:00:30PM -0700, Jonathan Tan wrote:

Show 17 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
> > diff --git a/commit-graph.c b/commit-graph.c
> > index 38edb12669..60e5f9ada7 100644
> > --- a/commit-graph.c
> > +++ b/commit-graph.c
> > @@ -317,12 +317,6 @@ static int graph_read_bloom_data(const unsigned char *chunk_start,
> >  	uint32_t hash_version;
> >  	hash_version = get_be32(chunk_start);
> >
> > -	if (*c->commit_graph_changed_paths_version == -1) {
> > -		*c->commit_graph_changed_paths_version = hash_version;
> > -	} else if (hash_version != *c->commit_graph_changed_paths_version) {
> > -		return 0;
> > -	}
>
> Lots of things to notice in this patch, but the summary is that this
> patch looks correct.

Thanks for the detailed analysis here. This is definitely the patch that I was most nervous about while writing, but I appreciate the second set of eyes.

> (Also, I don't think we need the commit_graph_changed_paths_version
> variable anymore.)

Great catch, thanks. I cleaned that up in a separate commit after this one, since I think that this patch is already intricate enough.

Thanks, Taylor