On 6/13/2023 3:21 PM, Junio C Hamano wrote: > Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > >> Thanks Ramsay for spotting the errors and mentioning that I can use >> octal escapes. Here's an update taking into account their comments. > > The changes look good. Will queue. > > Stolee, you had comments on an earlier round---how does this one > look? I'm sorry I'm so late to this. I've been meaning to get to it, but it's been a crazy couple of weeks. This version is not ready. The backwards compatibility story is incomplete. When commitGraph.changedPathsVersion is set, it does not allow reading a previous filter version, leaving us in a poor performance state until the commit-graph file can be rewritten. While I was reviewing, it seemed reasonable to deprecate commitGraph.readChangedPaths, but this use of "also restrict writes to this version" didn't make sense to me at the time. Instead, it would be good to have this clarity between the config options: commitGraph.readChangedPaths: should we read and use the filters that exist on disk? Defaults to 'true'. commitGraph.changedPathsVersion: Which version should we _write_ when writing a new commit-graph? Defaults to '1' but will default to '2' in the next major verion, then '1' will no longer be an accepted value in the version after that. The tricky part is that during the commit-graph write, you will need to check the existing filter value to see if it matches. If not, the filters will need to be recomputed from scratch. This will change patch 4 a bit, but it's the right thing to do. Thanks, -Stolee
