Re: [PATCH v3 08/13] bloom: use provided 'struct bloom_filter_settings'

On Thu, Sep 17, 2020 at 10:59:27PM -0400, Taylor Blau wrote:

When 'get_or_compute_bloom_filter()' needs to compute a Bloom filter
from scratch, it looks to the default 'struct bloom_filter_settings' in
order to determine the maximum number of changed paths, number of bits
per entry, and so on.

All of these values have so far been constant, and so there was no need
to pass in a pointer from the caller (eg., the one that is stored in the
'struct write_commit_graph_context').

Start passing in a 'struct bloom_filter_settings *' instead of using the
default values to respect graph-specific settings (eg., in the case of
setting 'GIT_TEST_BLOOM_SETTINGS_MAX_CHANGED_PATHS').

I think this description misses the most important aspect of this
patch: it fixes, even if only partially, the half-broken fix in
0087a87ba8 (commit-graph: persist existence of changed-paths,
2020-07-01).

That commit, among other things, tried to make sure that we use the
same Bloom filter settings to compute new Bloom filters that have been
used in already existing filters.  However, it only read those
settings from the header of the existing BDAT chunk and wrote them to
the header of the new BDAT chunk (and printed them to trace2 output).
Unfortunately, it didn't actually use those settings to compute Bloom
filters, because it left get_bloom_filter() unchanged to use the
hardcoded default Bloom filter settings.  This can result in bogus
commits-graphs and, in turn, pathspec-limited revision walks omitting
commits that do modify the specified path.