On Fri, Jan 06, 2023 at 09:40:22PM +0900, Junio C Hamano wrote: > > I don't think we have any fsck checks that the packed-refs file is in > > sorted order. It might be reasonable to have them. Likewise, when > > pack-refs rewrites the file, it should be able to cheaply double-check > > that the input is sorted by comparing each entry against its previous. > > True. I would not mind a patch to make us do so in the code path > where we rewrite the file and add "sorted" trait to the file. > refs/packed-backend.c::sort_snapshot() seems to be already equipped > to do this? I think it may be a little trickier than that. Yes, sort_snapshot() knows about sorting, but it only kicks in when the file isn't already marked as sorted (and we sort on the fly so that the rest of the code can use the same lookup routines). And it's possible that we can just sort_snapshot() before writing, even if the original claims to be sorted. But I'm not sure what performance impact that might have on the normal case that everything is already in good order. Maybe it's not a big deal; the write is already O(n), so adding an O(n log n) might not be the end of the world. But I was thinking more that write_with_updates() would, while iterating through the existing entries, check that the values it gets from the ref_iterator are indeed in sorted order. And if not, I think it needs to actually bail, since we might already have written a partially-confused result. And there "bail" may mean "write a warning to the user, abort the current write, call sort_snapshot(), and then try again". All of which is to say I don't think it's conceptually _too_ hard, but it was not simple enough that I was comfortable dashing off a one-liner and saying "probably something like this". ;) > So we can conclude that this discussion thread has an > incorrect Subject: and the symptom was caused by human error? That's my read on it. -Peff
