From: John Keeping Date: Thu, 28 Mar 2013 09:34:58 GMT Subject: Re: [PATCH] merge-tree: fix "same file added in subdir" Message-ID: <20130328093458.GV2286@serenity.lan> In-Reply-To: <20130327225739.GT2286@serenity.lan> On Wed, Mar 27, 2013 at 10:57:39PM +0000, John Keeping wrote: > On Wed, Mar 27, 2013 at 03:42:40PM -0700, Junio C Hamano wrote: > > John Keeping writes: > > > > > When the same file is added with identical content at the top level, > > > git-merge-tree prints "added in both" with the details. But if the file > > > is added in an existing subdirectory, threeway_callback() bails out early > > > because the two trees have been modified identically. > > > > > > In order to detect this, we need to fall through and recurse into the > > > subtree in this case. > > > > > > Signed-off-by: John Keeping > > > > The rationale the above description gives is internally consistent, > > but it is rather sad to see this optimization go. The primary > > motivation behind this program, which does not use the usual > > unpack-trees machinery, is to allow us to cull the identical result > > at a shallow level of the traversal when the both sides changed (not > > added) a file deep in a subdirectory hierarchy. > > > > The patch makes me wonder if we should go the other way around, > > resolving the "both added identically" case at the top cleanly > > without complaint. > > I don't use merge-tree so I have no opinion on this, just wanted to fix > an inconsistency :-) Having re-read the manpage, I think you're right that we should just resolve the "both added identically" case cleanly, but I wonder whether some of the other cases should also be resolved cleanly. git-merge-tree(1) says: the output from the command omits entries that match the tree. so you could argue that "added in branch1", "changed in branch1, unmodified in branch2" and "removed in branch1, unchanged in branch2" should also print no output. But as I said above I don't use git-merge-tree so perhaps people who do would like to explain what they expect in these cases.