> > That is the way it should work, but after thinking about it once more, I > > realize that it isn't. > > > > opt->shallow_file is not set to anything. And fetch-pack updates the > > shallow file by itself (at least, that is my understanding of > > update_shallow() in fetch-pack.c) before fetch calls check_connected(), > > meaning that if check_connected() fails, there is still no rollback of > > the shallow file. > > Ouch. We need to fix that; otherwise, a broken server will keep > giving you a corrupt repository even with this fix, no?
Yes, that is true - the repository will be left in a corrupt state. I did some more investigation, and usually things are OK because unpack-trees runs a fsck_walk on all the objects it unpacks (with --shallow-file appropriately set). Things are bad only if the packfile is empty (or, presumably, if the packfile only has unrelated objects). I managed to use the one-time-sed mechanism to craft a response that triggers this case. This both enables me to avoid the brittle check of "rev-list" appearing in the GIT_TRACE output (as discussed in [1]), and to verify that a rollback of the shallow file works, once such a patch is written. I'll look into writing such a patch, so feel free to hold off on this until that patch is done. [1] https://public-inbox.org/git/20180627225105.155996-1-jonathanta...@google.com/
