Subject: Re: [PATCH 6/9] fetch: Check if all objects exist after fetching

From: Andreas Gruenbacher <[email-masked]>
Date: Thu, 18 Mar 2010 20:36:46 +0100

On Thursday 18 March 2010 20:08:16 Shawn O. Pearce wrote:
> Andreas Gruenbacher <[email-masked]> wrote:
> > Check if all objects reachable from the fetched refs exist after
> > fetching instead of before: this allows us to distinguish between a
> > repository which is not up to date and a corrupted repository, and to
> > ensure that the repository is up to date and complete after the fetch.
> 
> I'm against this particular change because it looks like it breaks
> the idea of "quickfetch", which we introduced to support faster
> fetches from the parent repository into a shared clone on the
> same disk.

I think you misunderstand the patch.  Before the patch, we were doing a rev-
list to determine if all objects needed are present.  If rev-list fails, this 
can have two reasons: (a) some of the branches or tags needed do not exist, 
(b) all the branches and tags needed do exist, but other objects further up 
the tree are missing (i.e., a corrupted repository).

The patch changes that to first check which needed objects are missing (with 
has_sha1_file()), which is very efficient, by then fetching the objects which 
surely need to be fetched, and by then checking the repository consistency 
with rev-list.  If rev-list then fails, which should only happen in the rarest 
cases, we know that we need to fetch all branches and tags so that we are sure 
to catch missing objects further up the tree.

So we never fetch more than we did before, and in some cases, we fetch less. 
We are also guaranteed to end up with a consistent repository in the end. (The 
old logic does not always guarantee that AFAICT: there seems to be one corner 
case where a fetch succeeds without retrieving missing objects further up the 
tree.)

Thanks,
Andreas