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.
