Re: heads-up: git-index-pack in "next" is broken

[Nicolas Pitre <nico@cam.org>]

On Tue, 17 Oct 2006, Linus Torvalds wrote:

> 
> 
> On Tue, 17 Oct 2006, Nicolas Pitre wrote:
> > > 
> > > .. and it sorts _differently_ on a big-endian vs little-endian thing, 
> > > doesn't it?
> > 
> > Sure.  But who cares?  The sorting is just there to 1) perform binary 
> > searches on the list of deltas based from a given object, and 2) find a 
> > list of all deltas with the same base object.
> 
> _I_ care.

OK, So I do.

> The new code is messy. It's fragile, and already showed one very 
> fundamental bug which depended on architectures.

My stance is that it is not fragile.  Sure it had one bug that depended 
on an architecture difference, but so was commit ac58c7b18e about.

The code also has many consistency checks all over so that it doesn't 
write out garbage if such bugs arise.  And that fundamental bug was 
actually a trivial one that was caught right away by such consistency 
check.

> These things matter. We have had very few bugs in git, and one of the 
> reasons is (I believe) that we haven't had ad-hoc code. I get _very_ 
> nervous when you mix up SHA1 names with somethign totally different 
> without even a flag to say which one it is. That's just nasty.

But there _is_ a flag for damn sake.  Did you at least try to understand 
the code and not just skim over it from 10000 feet above?

It is really simple:

 - if the found union content matches with a reference union initialized 
   through the sha1 member then deltas[j].obj->type == OBJ_REF_DELTA 
   must be true.

 - if the found union content matches with a reference union initialized 
   through the sha1 member then deltas[j].obj->type == OBJ_OFS_DELTA 
   must be true.

 - For all deltas with deltas[j].obj->type == OBJ_REF_DELTA there can 
   not be more than one of them with the same union value.

 - For all deltas with deltas[j].obj->type == OBJ_OFS_DELTA there can 
   not be more than one of them with the same union value.

There is _no_ confusion possible.

> The fact that the code then behaves (and behave_d_) differently on 
> different architectures is just a sign of the problem.

Does this mean that, with your own change to xdiff that has just been 
committed, you actually created a "problem"?  Because this is a change 
that creates different behaviors whether a 32-bit or 64-bit architecture 
is used, Right?

But of course not.  We want it to behave differently on 64-bit than 
32-bit.  My code is in the _same_ camp since I explicitly want it to 
sort numbers differently whether it is a little endian or big endian 
machine.

So this is not a problem this is a feature, and very by design.

> "Who cares?" is not a good question to ask for a SCM. 

Please just try to understand why I'm claming this is not important in 
this very case.  Please do me this favor.


Nicolas