Re: [PATCH] read-tree A B C: do not create a bogus index and do not segfault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, 12 Mar 2009, Daniel Barkalow wrote:
> 
> I think one of the later refactorings may have given up on seeing 
> conflicts while reading trees, but didn't drop the flag (perhaps because 
> Linus knew at the time that my assumption that conflicts would actually 
> have been recognized was false, and didn't realize that it was also the 
> source of the flag).

That is very thoughtful of you, but I suspect the real reason was a much 
simpler one: redoing the read-tree logic was a huge pain in the posterior, 
and my primary goal at the time was to make the code more readable while 
passing all the tests.

So while I tried to make patches minimal (which may well explain why the 
flag remained), it wasn't the main goal, and trying to sort out the 
read-tree logic into something more understandable was quite test-driven.

So I think the big issue to take away from this is that I think this is 
such an odd case that if we want to support it, it would need explicit 
tests. Or:

> I think it might be a good idea to take this as evidence that nobody is 
> using read-tree with multiple trees without merge, and just disallow it. 

Hmm. It _has_ been used. It's been useful for really odd things 
historically, namely trying to merge different trees by hand. So while I 
agree that we could probably remove it, it _is_ a very interesting 
feature in theory, and since we have the code.. 

So I'd say "add a few tests for the known horrible cases" should be the 
first approach. If it ever actually breaks again and becomes a big 
maintenance headache, maybe _then_ remove the feature as not being worth 
the pain?

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux