On Mon, Apr 29, 2019 at 08:48:51AM -0400, Derrick Stolee wrote: > Q: How should we handle a detectably-invalid commit-graph? > > I think most of your patches have done a good job so far of detecting > an invalid header, and responding by ignoring the commit-graph. This case > of a detectable error in the chunk data itself is not something we can > check on the first load without serious performance issues. > > I hope we can decide on a good solution. I think it's OK to die() unceremoniously in these cases. We already have similar behavior when dealing with packfiles. While we try to just return an error for bit-flips in data (so we can try finding the object elsewhere), if there are things like nonsensical file offsets, we generally just give up. So I think it makes sense to start with the _safe_ thing, which is just giving up on the operation and informing the user. If somebody wants to then loosen things up on a case by case basis (after seeing that it wouldn't produce incorrect results to do so), that can be future work. For the commit-graph, though, and given that we're talking about file corruption, I don't know that it's even worth the effort. The solution is basically always going to be "delete the commit-graph file and regenerate it". It's perhaps slightly inconvenient compared to falling back, but this just isn't something we'd expect to happen often. -Peff
