git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Linus Torvalds <torvalds@osdl.org>
To: Junio C Hamano <junkio@cox.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] Diff updates, fixing pathspec and rename/copy interaction.
Date: Fri, 27 May 2005 08:56:35 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.58.0505270848220.17402@ppc970.osdl.org> (raw)
In-Reply-To: <7vsm091887.fsf@assigned-by-dhcp.cox.net>



On Thu, 26 May 2005, Junio C Hamano wrote:
>
> 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

  reply	other threads:[~2005-05-27 15:53 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-27  0:41 Broken directory pathname pruning Linus Torvalds
2005-05-27  0:42 ` Junio C Hamano
2005-05-27  0:49 ` [PATCH] allow pathspec to end with a slash Junio C Hamano
2005-05-27  0:52   ` [PATCH] allow pathspec to end with a slash (take #2) Junio C Hamano
2005-05-27  6:41 ` [PATCH] Diff updates, fixing pathspec and rename/copy interaction Junio C Hamano
2005-05-27 15:56   ` Linus Torvalds [this message]
2005-05-27 18:22     ` Junio C Hamano
2005-05-27 22:43     ` [PATCH 00/12] Diff updates Junio C Hamano
2005-05-27 22:49       ` [PATCH 01/12] Fix math thinko in similarity estimator Junio C Hamano
2005-05-27 22:50       ` [PATCH 02/12] Introduce diff_free_filepair() funcion Junio C Hamano
2005-05-27 22:51       ` [PATCH 03/12] Make pathspec only care about the detination tree Junio C Hamano
2005-05-27 22:52       ` [PATCH 04/12] Remove unused rank field from diff_core structure Junio C Hamano
2005-05-27 22:53       ` [PATCH 05/12] Do not expose internal scaling to diff-helper Junio C Hamano
2005-05-27 22:54       ` [PATCH 06/12] Remove final newline from the value of xfrm_msg variable Junio C Hamano
2005-05-27 22:54       ` [PATCH 07/12] Clean up diff_setup() to make it more extensible Junio C Hamano
2005-05-27 22:55       ` [PATCH 08/12] Remove a function not used anymore Junio C Hamano
2005-05-27 22:55       ` [PATCH 09/12] Add --pickaxe-all to diff-* brothers Junio C Hamano
2005-05-27 22:55       ` [PATCH 10/12] Fix the way diffcore-rename records unremoved source Junio C Hamano
2005-05-27 22:56       ` [PATCH 11/12] Move pathspec to the beginning of the diffcore chain Junio C Hamano
2005-05-27 22:56       ` [PATCH 12/12] Optimize diff-tree -[CM] --stdin Junio C Hamano
2005-06-04  2:17         ` Yoichi Yuasa
2005-05-27 23:03       ` [PATCH 00/12] Diff updates Junio C Hamano
2005-05-28 10:11         ` [PATCH] Do not show empty diff in diff-cache uncached Junio C Hamano
2005-05-28 19:22           ` [PATCH] Diff: two fixes Junio C Hamano
2005-05-29  4:20             ` [PATCH] diff-helper: fix R/C score parsing under -z flag Junio C Hamano
2005-05-29  5:23               ` [PATCH] diff-cache: diff-patch (-p) format fixes Junio C Hamano
2005-05-29  9:10                 ` [PATCH] diff: code clean-up Junio C Hamano
2005-05-29 18:53           ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds
2005-05-29 20:09             ` Junio C Hamano
2005-05-29 21:52               ` Junio C Hamano
2005-05-29 23:41                 ` [PATCH 0/3] Leftover bits after 12-series Junio C Hamano
2005-05-29 23:54                   ` [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag Junio C Hamano
2005-05-29 23:56                   ` [PATCH 2/3] diff: consolidate various calls into diffcore Junio C Hamano
2005-05-29 23:56                   ` [PATCH 3/3] diff: code clean-up and removal of rename hack Junio C Hamano
2005-05-30  6:58                   ` [PATCH 0/4] Junio C Hamano
2005-05-30  7:07                     ` [PATCH 1/4] diff: further cleanup Junio C Hamano
2005-05-30  7:08                     ` [PATCH 2/4] diff: fix the culling of unneeded delete record Junio C Hamano
2005-05-30  7:08                     ` [PATCH 3/4] Add -B flag to diff-* brothers Junio C Hamano
2005-05-30  7:09                     ` [PATCH 4/4] Add -O<orderfile> option " Junio C Hamano
2005-05-30  5:34                 ` [PATCH] Do not show empty diff in diff-cache uncached Linus Torvalds
2005-05-30  5:53                   ` Junio C Hamano
2005-06-11 23:27             ` [PATCH] apply.c: tolerate diff from a dirty but unchanged path Junio C Hamano
2005-06-12 16:14               ` Linus Torvalds
2005-06-12 17:05                 ` Linus Torvalds
2005-06-12 18:34                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.58.0505270848220.17402@ppc970.osdl.org \
    --to=torvalds@osdl.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).