From: Linus Torvalds Date: Thu, 02 Feb 2006 07:51:34 GMT Subject: Re: The merge from hell... Message-ID: In-Reply-To: <7v8xsuuto5.fsf@assigned-by-dhcp.cox.net> On Wed, 1 Feb 2006, Junio C Hamano wrote: > > > It's commit 9fdb62af92c741addbea15545f214a6e89460865, and passing it to > > git-diff-tree with the "--cc" option seems to do the largely the right > > thing (although arguably, since one of the parents always matches the end > > result in all the files, it shouldn't have shown anything at all, so I > > think it could do with some tweaking). > > Hmph. Do you mean a hunk like this? > > diff --cc kernel/sys.c > @@@@@@@@@@@@@ +33,7 @@@@@@@@@@@@@ > > #include > #include > + +++ #include > > #include > #include Yes. > Currently I cull hunks that have changes from only one parent, > or when the changes are the same from all but one parent. This > hunk does not match either criteria. Correct. I think "git-diff-tree --cc" actually works as you advertized, but that we should change the rules. More akin to - if there are only two versions of a hunk, and one of those versions ended up in the result, the hunk is considered successfully merged and should be ignored under "--cc" Under that rule, the kernel/sys.c hunk you specify is indeed uninteresting, because there are only two versions: the version that didn't have the addition, and the version that did. > or "plus and minus in the same set of columns" hunks. If I did > so, this would become uninteresting: > > diff --cc arch/ia64/pci/pci.c > @@@@@@@@@@@@@ +709,7 @@@@@@@@@@@@@ > */ > int ia64_pci_legacy_write(struct pci_dev *bus, u16... > { > ------ - int ret = 0; > ++++++ + int ret = size; > > switch (size) { > case 1: Yes. Again, only two versions. Either it was "ret = 0" or it was "ret = size". No other version existed in any branch or in the end result, so there really were only two versions, and one of them ended up being the end result. > But this is still interesting: > > @@@@@@@@@@@@@ +308,35 @@@@@@@@@@@@@ > goto end; > } > } > -- cx->usage++; > -- > > +++ #ifdef CONFIG_HOTPLUG_CPU > +++ /* Not under my suggested "two version" rule. The above has _three_ versions: the one with no changes, the one with "cx->usage++" removed and the one with "#ifdef CONFIG_HOTPLUG_CPU" added. So under the two-version rule, the last one would still be interesting. > > git-diff-tree takes almost three seconds to get its result, though. > > One trivial thing I should be able to do to speed things up is > to reuse previous diff with other parents. Yes. In this example, it would apparently cut down the work by a third for that file, although it migt make it harder to calculate the proper "-/+" patterns.. > I did not expect anybody to be _that_ sick (eh, pardon my > language) to do a 12-way octpus, so I did not consider this > optimization possibility, but I should be doing only 4 diffs to > format -c for this commit. Currently I do 12. I agree, I wouldn't have considered that 12-way merge sane either, but hey, it worked, and our octopus merge rules are simple enough that I guess there's not really any downside to it. And I was actually happy to have "git-diff-tree --cc" to look at it, and that "gitk" eventually gave results that looked decent. I didn't go through it all that closely, but the ones I _looked_ at looked fine, and it was mighty pleasant to have the tools to look more closely. So thanks for "--cc" yet again. > > So think of it as a correctness/scalability test. > > Heh. I like this one as a practice. Thanks! Thank Len. He may have done it as a way to avoid having extra merges, since I complained about those last time ;) Linus