On Tue, 3 Apr 2007, Junio C Hamano wrote: > Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > > > That whole "verify no SHA1 hash collision" code is really pretty damn > > paranoid. Maybe we shouldn't have it enabled by default. > > > > So how about this updated patch? We could certainly make "git pull" imply > > "--paranoid" if we want to, but even that is likely pretty unnecessary. > > It's not like anybody has ever shown a SHA1 collision, and if the *local* > > repository is corrupt (and has an object with the wrong SHA1 - that's what > > the testsuite checks for), then it's probably good to get the valid object > > from the remote.. > > I agree with that reasoning. For the record, I don't agree. I stated why in my other email. > We did not do paranoid in git-pull long after we introduced the .keep > thing anyway, That doesn't make it more "correct". > so I do not > think the following patch is even needed, but I am throwing it > out just for discussion. 1) None of the objects in a pack should exist in the local repo when fetching, meaning that the paranoia code should not be executed normally. 2) Running index-pack on a pack _inside_ a repository is a dubious thing to do with questionable usefulness already. 3) It is unefficient to run pack-objects with --stdout just to feed the result to index-pack afterwards while repack-objects can create the index itself, which is the source of this discussion. 4) I invite you to read the commit log for 8685da42561 where the _perception_ of GIT's security is discussed which led to the paranoia check, and sometimes the perception is more valuable than the reality, especially when it is free. Therefore Linus' patch and this one are working around the wrong issue as described in (3) IMHO. What could be done instead, if really really needed, is to have the paranoia test be made conditional on index-pack --stdin instead. But please no bogus extra switches pretty please. Nicolas - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html