Junio C Hamano <junkio@xxxxxxx> wrote: > I haven't looked your patch very closely, but two successive > "git-repack -a -d" would produce the packfile under the same > name, but with different parameters (--window or --depth), the > old .idx and new .pack may not match. > > Can this be possible (time flows from top to bottom): > > git-repack -a -d somebody else > > produces a new .pack/.idx > > mv new .pack to objects/pack > > mmap .idx (old) > finds .pack does not match > > mv new .idx to objects/pack Gah. That's sick. And *so* totally possible. > Is this something we care about? Yes and no. I've pushed the idea that `git repack -a -d` is totally safe to run in a live repository. Its clearly not. *Especially* if you repack with the same set of objects (as you do above) or if you repack in rapid succession faster than readers can make progress. I think both of these are just outside of the realm of "good ideas to do to live things". Its like taking your cat to the zoo play with a lion. Maybe not the best thing for the cat... In this particular case that you highlight above this patch would mark the pack as invalid and never permit it to be accessed again by the 'somebody else'. But Git has always been broken for this case. prepare_packed_git_one would refuse to open the new index, as it already has a packed_git. Prior to this patch we'd try to access the packfile and die(), as it did not match the index that we have. Now we'd at least try to locate some other source, or die() over a missing object, but only after listing the idx/pack mismatch error too. The only fix for the case above is to unlink the invalid packed_git from packed_git, so that if we fail to find the object and wind up calling reprepare_packed_git() we can reload the new index. I did consider doing that over the invalid flag, but flipped a coin and added invalid. FWIW, maybe invalid is the wrong way to go. We still have a problem though, as the above case could still happen to us during the fallback search in read_sha1_file(). In other words two searches ain't enough. As Dscho pointed out in the original thread, we probably need to loop until no new packs are identified before giving up. I almost submitted a patch to do that tonight, but I couldn't decide on behavior: should we scan known packs, then try for loose, then scan packs again until no object or no new pack is found? Probably. -- Shawn. - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html