Re: [PATCH] Diff updates, fixing pathspec and rename/copy interaction.

From: Linus Torvalds <torvalds@osdl.org>
Date: May 27, 2005, 15:56 UTC
Message-ID: <Pine.LNX.4.58.0505270848220.17402@ppc970.osdl.org>
In-Reply-To: <7vsm091887.fsf@assigned-by-dhcp.cox.net>

On Thu, 26 May 2005, Junio C Hamano wrote:

Show 15 quoted lines

>
> During the mailing list discussion about diff-tree omitting to
> call diffcore-pathspec, I realized that the current rename/copy
> differentiator has a major flaw interacting with pathspec (or
> any other filepair filters, including pickaxe).
> 
> The problem was that in order to tell if the rename-copy source
> still remains in the resulting tree (that is what determines if
> one of the rename-copy can become a rename or everybody needs to
> be a copy), diffcore-rename was sending a filepair that records
> unmodified source downstream, expecting that it reaches
> resolve_rename_copy() which happens as the final stage before
> actual output happens.  Of course, pathspec and pickaxe can
> interfere and happily remove that entry, in which case what
> should be shown as a copy suddenly becomes a rename.

Umm.

I would much prefer a _much_ simpler fix at least for the pathname part, which is to just always require that pathspec handling is done _first_.

Why? Because that's fundamentally how git-diff-tree has to work, and it's how my mental model has always been: the path limitations are a first-order filter, and if you give a directory, the end result should always look exactly as if that directory was a project of its own.

In other words, if you limit yourself to a directory, and there was a rename that moved a file from outside that directory into it, then it is not a rename at all, it is a "create". The stuff outside the pathspec limit simply doesn't exist.

This is fundamentally how git-diff-tree has to work for pathspec to make any sense at all, and it's also the only usage that makes any sense (ie when I say "git-whatchanged -p arch/i386 include/asm-i386", I expect that the patches that show up _only_ concern themselves with what happened in x86, and there's no cross-pollination with other stuff at all, even if rename detection is enabled).

That in turn implies that the other pathspec users have to work the same way, for the thing to be consistent.

Now, I don't know how you want pickaxe to work, and maybe you want that to run _after_ rename detection, I dunno. So I suspect you still want to do what this patch does, but I really don't want pathspec to be involved in this thing..

		Linus