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

[Linus Torvalds <torvalds@osdl.org>]

On Tue, 17 Oct 2006, Nicolas Pitre wrote:
> 
> 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?

I only looked at it from the patches, and the actual data structure, 
and they didn't have it, so..

> There is _no_ confusion possible.

Ok. Good.

> 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?

If you go back to that discussion, I actually pointed out several times 
that the whole bug _was_ actually introduced exactly because the xdiff 
code used things that behave differently depending on word-size.

My suggestion for a _proper_ fix was to not use "unsigned long" for that, 
and the patch I suggested (and eventually got merged) was to use the _low_ 
bits of the hash, exactly because the low bits are the ones that act the 
same, regardless of wordsize.

> But of course not.  We want it to behave differently on 64-bit than 
> 32-bit.

No, we actually don't. Not for xdiff, at least. The last thing you want is 
for different architectures to get different results. It's horrible. It 
means that bugs are hard to reproduce, and it means that even code that is 
"tested" is actually tested only for a particular architecture.

So the bug in xdiff was _exactly_ that somebody - totally incorrectly - 
thought it should work "better" on 64-bits. 

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

Maybe the code is fine. Maybe the particular detail wasn't important. But 
the original code didn't have _any_ dependencies on things like structure 
alignment that caused it to do strange things. 

And dammit, the fact is, I think the new format is just worse. I think it 
was a good thing to have the full SHA1 in the pack-file. I think the code 
got less understandable, and had more special cases, just because now we 
have two totally different kinds of deltas. So maybe I'm reacting to the 
fact that I think the bug happened in the first place for a very simple 
reason: the data structure wasn't unambiguous any more. 

		Linus