Re: Bad objects error since upgrading GitHub servers to 1.6.1

["Shawn O. Pearce" <spearce@spearce.org>]

Jeff King <peff@peff.net> wrote:
> On Wed, Jan 28, 2009 at 12:05:33AM -0800, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> > >
> > >        C--D
> > >       /
> > >   A--B
> > >       \
> > >        E--F
> 
> Don't the other changes have similar parallel use cases? [2/2] also deals
> with tag lookup. Wouldn't you also expect, if you had a tag "T" pointing
> to "E" in the above scenario that "git rev-list T..D" would barf? I
> really think you don't want to ignore missing negations _ever_ unless
> the caller knows that such a miss is really only about optimization and
> not correctness.

Exactly what I just said in my other message.
 
> Side note:
> 
> As you described, we expect to reach this situation from a partial
> transfer. Which means that you don't actually have a _ref_ for "T" (or
> "F"). So it is unlikely to come up in normal use (you would have to
> manually specify the sha1 of a broken portion of the graph).

True, but in the send-pack case we are discussing the remote side
has specified the SHA-1 of broken portions of the graph to us,
and we've taken that into consideration.  So we have to fix that
assumption we've made.

> But what is more important is that your repository _is_ corrupted,

Depends.  If the SHA-1 came from the remote side during send-pack,
it doesn't matter that we have a broken chain along that path,
it may have been a dumb transport fetch that was interrupted.
Our local repository isn't corrupt, it just has some extra crap
laying around that hasn't gc'd yet.

If the SHA-1 came from the user, then it depends on the context
of why the user is giving it to us.  In pretty much every case,
yes, its a corruption and we should be aborting.  :-)

Actually, the only time where it *isn't* a corruption is when its
input to "git bundle create A.bdl ... -not $SOMEBADID" as that is
the exact same thing as coming from the other side via send-pack.

> I
> think we are losing an important method by which the user finds out. Git
> is usually very good at informing you of a problem in the repo early,
> and I think unconditionally ignoring missing objects would lose that.

Yup, I agree.  But as you and Junio have already pointed out, C Git
can miss some types of corruption because the revision machinary has
some gaps.  *sigh*

I'd really like to see those gaps closed.  But I don't have a good
enough handle on the code structure of the C Git revision machinary
to do that myself in a short period of time.  I know JGit's well...
but that's only because I wrote it.  ;-)

Its now on my wish list of things I wish I had time for in C Git.
But perhaps someone who is more familiar with the revision machinary
will get to it first.

-- 
Shawn.