Re: [PATCH] fetch: when deepening, check connectivity fully

[Jonathan Tan <jonathantanmy@google.com>]

> > 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-jonathantanmy@google.com/