Broken directory pathname pruning..

Broken directory pathname pruning..

[Linus Torvalds]

The directory-based pathname pruning doesn't work any more.

I used to just say

	git-whatchanged -v -p drivers/usb/

and I just noticed that it doesn't work any more.

It _does_ work if I leave the final '/' off the thing.

diff-tree itself does this right: that's shown by the fact that with the
slash in place, we still do get the right _changelog_ that is restricted
to drivers/usb changes, but the diffs themselves are missing.

So it seems to be purely "diffcore_pathspec()" that is broken.

Btw, I don't think we should call "diffcore_pathspec()" at all in
diff-tree, because diff-tree already handles "interesting" paths correctly
(and has to do so for performance reasons, since it's not acceptable to
expand all the trees and diff them).

So in the "git-whatchanged"  case the fix is as simple as just removing
those two lines, but the bug then continues to exist in the other *diff* 
programs..

Junio?

		Linus

diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -268,8 +268,6 @@ static int call_diff_flush(void)
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
 	}
-	if (nr_paths)
-		diffcore_pathspec(paths);
 	if (header) {
 		if (diff_output_format == DIFF_FORMAT_MACHINE) {
 			const char *ep, *cp;

Re: Broken directory pathname pruning..

[Junio C Hamano]

Agreed in both counts.

[PATCH] allow pathspec to end with a slash

[Junio C Hamano]

Recent rewrite broke "git-whatchanged -v -p drivers/usb/" but 
"git-whatchanged -v -p drivers/usb" still works.  Just strip out
the trailing slashes internally to make it work again.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
# - linus: [PATCH] Make ls-* output consistent with diff-* output format.
# + (working tree)
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -45,8 +45,12 @@ void diffcore_pathspec(const char **path
 	speccnt = i;
 	spec = xmalloc(sizeof(*spec) * speccnt);
 	for (i = 0; pathspec[i]; i++) {
+		int l;
 		spec[i].spec = pathspec[i];
-		spec[i].len = strlen(pathspec[i]);
+		l = strlen(pathspec[i]);
+		while (pathspec[i][l-1] == '/')
+			l--;
+		spec[i].len = l;
 	}
 
 	for (i = 0; i < q->nr; i++) {

Compilation finished at Thu May 26 17:46:56

[PATCH] allow pathspec to end with a slash (take #2)

[Junio C Hamano]

Oops, wrong patch; I am an idiot.

------------
Recent rewrite broke "git-whatchanged -v -p drivers/usb/" but 
"git-whatchanged -v -p drivers/usb" still works.  Just strip out
the trailing slashes internally to make it work again.

It uses compare-thing-with-number comparison order instead of
visual comparison order ;-).

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
cd /opt/packrat/playpen/public/in-place/git/git.junio/
jit-diff 1: diffcore-pathspec.c
# - linus: [PATCH] Make ls-* output consistent with diff-* output format.
# + (working tree)
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -45,8 +45,12 @@ void diffcore_pathspec(const char **path
 	speccnt = i;
 	spec = xmalloc(sizeof(*spec) * speccnt);
 	for (i = 0; pathspec[i]; i++) {
+		int l;
 		spec[i].spec = pathspec[i];
-		spec[i].len = strlen(pathspec[i]);
+		l = strlen(pathspec[i]);
+		while (l > 0 && pathspec[i][l-1] == '/')
+			l--;
+		spec[i].len = l;
 	}
 
 	for (i = 0; i < q->nr; i++) {

Compilation finished at Thu May 26 17:50:40

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

[Junio C Hamano]

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.

The problem was fixed by changing the way diffcore-rename
records whether the source file remains in the resulting tree.

This patch also introduces an optimization for diff-tree -M and
diff-tree -C to throw away implausible rename/copy source
candidate by keeping the filesize information of already seen
SHA1 object IDs in a cache.  This cache is activated only when
diff-tree is reading from --stdin (i.e. processing sequence of
tree pairs) and when rename detection is used.

Pickaxe acquired another option, --pickaxe-all, to diff-* three
brothers.  When a search string is touched, instead of showing
only the changed file that touches that searched string, it
shows the entire changeset.  This is useful to see the change in
context.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-cache.c             |   13 ++
diff-files.c             |   11 +-
diff-helper.c            |    9 +-
diff-tree.c              |   16 ++-
diff.c                   |  209 ++++++++++++++++++++++++++++++-----------------
diff.h                   |   18 ++--
diffcore-pathspec.c      |    5 -
diffcore-pickaxe.c       |   79 +++++++++++++----
diffcore-rename.c        |   91 +++++++++-----------
diffcore.h               |   14 +--
git-external-diff-script |    2 
t/t4007-rename-3.sh      |  103 +++++++++++++++++++++++
12 files changed, 392 insertions(+), 178 deletions(-)
new file (100755): t/t4007-rename-3.sh

diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -5,9 +5,10 @@ static int cached_only = 0;
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int match_nonexisting = 0;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -202,13 +203,17 @@ int main(int argc, const char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "-R")) {
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 			continue;
 		}
 		if (!strcmp(arg, "-S")) {
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strcmp(arg, "--pickaxe-all")) {
+			pickaxe_opts = DIFF_PICKAXE_ALL;
+			continue;
+		}
 		if (!strcmp(arg, "-m")) {
 			match_nonexisting = 1;
 			continue;
@@ -224,7 +229,7 @@ int main(int argc, const char **argv)
 		usage(diff_cache_usage);
 
 	/* The rest is for paths restriction. */
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 
 	mark_merge_entries();
 
@@ -238,7 +243,7 @@ int main(int argc, const char **argv)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (pathspec)
 		diffcore_pathspec(pathspec);
 	diff_flush(diff_output_format, 1);
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -11,9 +11,10 @@ static const char *diff_files_usage =
 
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -51,9 +52,11 @@ int main(int argc, const char **argv)
 		else if (!strcmp(argv[1], "-z"))
 			diff_output_format = DIFF_FORMAT_MACHINE;
 		else if (!strcmp(argv[1], "-R"))
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 		else if (!strcmp(argv[1], "-S"))
 			pickaxe = argv[1] + 2;
+		else if (!strcmp(argv[1], "--pickaxe-all"))
+			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else if (!strncmp(argv[1], "-M", 2)) {
 			diff_score_opt = diff_scoreopt_parse(argv[1]);
 			detect_rename = DIFF_DETECT_RENAME;
@@ -75,7 +78,7 @@ int main(int argc, const char **argv)
 		exit(1);
 	}
 
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 
 	for (i = 0; i < entries; i++) {
 		struct stat st;
@@ -116,7 +119,7 @@ int main(int argc, const char **argv)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (1 < argc)
 		diffcore_pathspec(argv + 1);
 	diff_flush(diff_output_format, 1);
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -4,9 +4,9 @@
 #include "cache.h"
 #include "strbuf.h"
 #include "diff.h"
-#include "diffcore.h" /* just for MAX_SCORE */
 
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static int line_termination = '\n';
 static int inter_name_termination = '\t';
 
@@ -24,6 +24,8 @@ int main(int ac, const char **av) {
 		else if (av[1][1] == 'S') {
 			pickaxe = av[1] + 2;
 		}
+		else if (!strcmp(av[1], "--pickaxe-all"))
+			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else
 			usage(diff_helper_usage);
 		ac--; av++;
@@ -78,7 +80,6 @@ int main(int ac, const char **av) {
 			if (status == 'R' || status == 'C') {
 				two_paths = 1;
 				sscanf(cp, "%d", &score);
-				score = score * MAX_SCORE / 100;
 				if (line_termination) {
 					cp = strchr(cp,
 						    inter_name_termination);
@@ -129,14 +130,14 @@ int main(int ac, const char **av) {
 			continue;
 		}
 		if (pickaxe)
-			diffcore_pickaxe(pickaxe);
+			diffcore_pickaxe(pickaxe, pickaxe_opts);
 		if (1 < ac)
 			diffcore_pathspec(av + 1);
 		diff_flush(DIFF_FORMAT_PATCH, 0);
 		printf("%s\n", sb.buf);
 	}
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (1 < ac)
 		diffcore_pathspec(av + 1);
 	diff_flush(DIFF_FORMAT_PATCH, 0);
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -10,9 +10,10 @@ static int show_tree_entry_in_recursive 
 static int read_stdin = 0;
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static const char *header = NULL;
 static const char *header_prefix = "";
 
@@ -255,7 +256,7 @@ static int diff_tree_sha1(const unsigned
 
 static void call_diff_setup(void)
 {
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 }
 
 static int call_diff_flush(void)
@@ -263,7 +264,7 @@ static int call_diff_flush(void)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (diff_queue_is_empty()) {
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
@@ -504,7 +505,7 @@ int main(int argc, const char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "-R")) {
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 			continue;
 		}
 		if (!strcmp(arg, "-p")) {
@@ -516,6 +517,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strcmp(arg, "--pickaxe-all")) {
+			pickaxe_opts = DIFF_PICKAXE_ALL;
+			continue;
+		}
 		if (!strncmp(arg, "-M", 2)) {
 			detect_rename = DIFF_DETECT_RENAME;
 			diff_score_opt = diff_scoreopt_parse(arg);
@@ -580,6 +585,9 @@ int main(int argc, const char **argv)
 	if (!read_stdin)
 		return 0;
 
+	if (detect_rename)
+		diff_setup_opt |= (DIFF_SETUP_USE_SIZE_CACHE |
+				   DIFF_SETUP_USE_CACHE);
 	while (fgets(line, sizeof(line), stdin))
 		diff_tree_stdin(line);
 
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -12,6 +12,7 @@ static const char *diff_opts = "-pu";
 static unsigned char null_sha1[20] = { 0, };
 
 static int reverse_diff;
+static int use_size_cache;
 
 static const char *external_diff(void)
 {
@@ -141,7 +142,7 @@ static void builtin_diff(const char *nam
 			printf("new mode %s\n", temp[1].mode);
 		}
 		if (xfrm_msg && xfrm_msg[0])
-			fputs(xfrm_msg, stdout);
+			puts(xfrm_msg);
 
 		if (strncmp(temp[0].mode, temp[1].mode, 3))
 			/* we do not run diff between different kind
@@ -222,12 +223,60 @@ static int work_tree_matches(const char 
 	return 1;
 }
 
+static struct sha1_size_cache {
+	unsigned char sha1[20];
+	unsigned long size;
+} **sha1_size_cache;
+static int sha1_size_cache_nr, sha1_size_cache_alloc;
+
+static struct sha1_size_cache *locate_size_cache(unsigned char *sha1,
+						 unsigned long size)
+{
+	int first, last;
+	struct sha1_size_cache *e;
+
+	first = 0;
+	last = sha1_size_cache_nr;
+	while (last > first) {
+		int next = (last + first) >> 1;
+		e = sha1_size_cache[next];
+		int cmp = memcmp(e->sha1, sha1, 20);
+		if (!cmp)
+			return e;
+		if (cmp < 0) {
+			last = next;
+			continue;
+		}
+		first = next+1;
+	}
+	/* not found */
+	if (size == UINT_MAX)
+		return NULL;
+	/* insert to make it at "first" */
+	if (sha1_size_cache_alloc <= sha1_size_cache_nr) {
+		sha1_size_cache_alloc = alloc_nr(sha1_size_cache_alloc);
+		sha1_size_cache = xrealloc(sha1_size_cache,
+					   sha1_size_cache_alloc *
+					   sizeof(*sha1_size_cache));
+	}
+	sha1_size_cache_nr++;
+	if (first < sha1_size_cache_nr)
+		memmove(sha1_size_cache + first + 1, sha1_size_cache + first,
+			(sha1_size_cache_nr - first - 1) *
+			sizeof(*sha1_size_cache));
+	e = xmalloc(sizeof(struct sha1_size_cache));
+	sha1_size_cache[first] = e;
+	memcpy(e->sha1, sha1, 20);
+	e->size = size;
+	return e;
+}
+
 /*
  * While doing rename detection and pickaxe operation, we may need to
  * grab the data for the blob (or file) for our own in-core comparison.
  * diff_filespec has data and size fields for this purpose.
  */
-int diff_populate_filespec(struct diff_filespec *s)
+int diff_populate_filespec(struct diff_filespec *s, int size_only)
 {
 	int err = 0;
 	if (!DIFF_FILE_VALID(s))
@@ -235,6 +284,9 @@ int diff_populate_filespec(struct diff_f
 	if (S_ISDIR(s->mode))
 		return -1;
 
+	if (!use_size_cache)
+		size_only = 0;
+
 	if (s->data)
 		return err;
 	if (!s->sha1_valid ||
@@ -254,6 +306,8 @@ int diff_populate_filespec(struct diff_f
 		s->size = st.st_size;
 		if (!s->size)
 			goto empty;
+		if (size_only)
+			return 0;
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
 			s->data = xmalloc(s->size);
@@ -273,9 +327,21 @@ int diff_populate_filespec(struct diff_f
 		close(fd);
 	}
 	else {
+		/* We cannot do size only for SHA1 blobs */
 		char type[20];
+		struct sha1_size_cache *e;
+
+		if (size_only) {
+			e = locate_size_cache(s->sha1, UINT_MAX);
+			if (e) {
+				s->size = e->size;
+				return 0;
+			}
+		}
 		s->data = read_sha1_file(s->sha1, type, &s->size);
 		s->should_free = 1;
+		if (s->data && size_only)
+			locate_size_cache(s->sha1, s->size);
 	}
 	return 0;
 }
@@ -361,7 +427,7 @@ static void prepare_temp_file(const char
 		return;
 	}
 	else {
-		if (diff_populate_filespec(one))
+		if (diff_populate_filespec(one, 0))
 			die("cannot read data blob for %s", one->path);
 		prep_temp_blob(temp, one->data, one->size,
 			       one->sha1, one->mode);
@@ -492,9 +558,23 @@ static void run_diff(const char *name,
 		run_external_diff(pgm, name, other, one, two, xfrm_msg);
 }
 
-void diff_setup(int reverse_diff_)
+void diff_setup(int flags)
 {
-	reverse_diff = reverse_diff_;
+	if (flags & DIFF_SETUP_REVERSE)
+		reverse_diff = 1;
+	if (flags & DIFF_SETUP_USE_CACHE) {
+		if (!active_cache)
+			/* read-cache does not die even when it fails
+			 * so it is safe for us to do this here.  Also
+			 * it does not smudge active_cache or active_nr
+			 * when it fails, so we do not have to worry about
+			 * cleaning it up oufselves either.
+			 */
+			read_cache();
+	}
+	if (flags & DIFF_SETUP_USE_SIZE_CACHE)
+		use_size_cache = 1;
+	
 }
 
 struct diff_queue_struct diff_queued_diff;
@@ -517,10 +597,18 @@ struct diff_filepair *diff_queue(struct 
 	dp->one = one;
 	dp->two = two;
 	dp->score = 0;
+	dp->source_stays = 0;
 	diff_q(queue, dp);
 	return dp;
 }
 
+void diff_free_filepair(struct diff_filepair *p)
+{
+	diff_free_filespec_data(p->one);
+	diff_free_filespec_data(p->two);
+	free(p);
+}
+
 static void diff_flush_raw(struct diff_filepair *p,
 			   int line_termination,
 			   int inter_name_termination)
@@ -615,7 +703,7 @@ static void diff_flush_patch(struct diff
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"copy from %s\n"
-			"copy to %s\n",
+			"copy to %s",
 			(int)(0.5 + p->score * 100.0/MAX_SCORE),
 			p->one->path, p->two->path);
 		msg = msg_;
@@ -624,7 +712,7 @@ static void diff_flush_patch(struct diff
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"rename old %s\n"
-			"rename new %s\n",
+			"rename new %s",
 			(int)(0.5 + p->score * 100.0/MAX_SCORE),
 			p->one->path, p->two->path);
 		msg = msg_;
@@ -639,28 +727,6 @@ static void diff_flush_patch(struct diff
 		run_diff(name, other, p->one, p->two, msg);
 }
 
-int diff_needs_to_stay(struct diff_queue_struct *q, int i,
-		       struct diff_filespec *it)
-{
-	/* If it will be used in later entry (either stay or used
-	 * as the source of rename/copy), we need to copy, not rename.
-	 */
-	while (i < q->nr) {
-		struct diff_filepair *p = q->queue[i++];
-		if (!DIFF_FILE_VALID(p->two))
-			continue; /* removed is fine */
-		if (strcmp(p->one->path, it->path))
-			continue; /* not relevant */
-
-		/* p has its src set to *it and it is not a delete;
-		 * it will be used for in-place change, rename/copy,
-		 * or just stays there.  We cannot rename it out.
-		 */
-		return 1;
-	}
-	return 0;
-}
-
 int diff_queue_is_empty(void)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
@@ -689,8 +755,8 @@ void diff_debug_filepair(const struct di
 {
 	diff_debug_filespec(p->one, i, "one");
 	diff_debug_filespec(p->two, i, "two");
-	fprintf(stderr, "score %d, status %c\n",
-		p->score, p->status ? : '?');
+	fprintf(stderr, "score %d, status %c source_stays %d\n",
+		p->score, p->status ? : '?', p->source_stays);
 }
 
 void diff_debug_queue(const char *msg, struct diff_queue_struct *q)
@@ -712,8 +778,6 @@ static void diff_resolve_rename_copy(voi
 	struct diff_filepair *p, *pp;
 	struct diff_queue_struct *q = &diff_queued_diff;
 
-	/* This should not depend on the ordering of things. */
-
 	diff_debug_queue("resolve-rename-copy", q);
 
 	for (i = 0; i < q->nr; i++) {
@@ -721,23 +785,28 @@ static void diff_resolve_rename_copy(voi
 		p->status = 0; /* undecided */
 		if (DIFF_PAIR_UNMERGED(p))
 			p->status = 'U';
-		else if (!DIFF_FILE_VALID((p)->one))
+		else if (!DIFF_FILE_VALID(p->one))
 			p->status = 'N';
-		else if (!DIFF_FILE_VALID((p)->two)) {
-			/* Deletion record should be omitted if there
-			 * are rename/copy entries using this one as
-			 * the source.  Then we can say one of them
-			 * is a rename and the rest are copies.
+		else if (!DIFF_FILE_VALID(p->two)) {
+			/* Deleted entry may have been picked up by
+			 * another rename-copy entry.  So we scan the
+			 * queue and if we find one that uses us as the
+			 * source we do not say delete for this entry.
 			 */
-			p->status = 'D';
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
-				if (!strcmp(pp->one->path, p->one->path) &&
-				    strcmp(pp->one->path, pp->two->path)) {
+				if (!strcmp(p->one->path, pp->one->path) &&
+				    pp->score) {
+					/* rename/copy are always valid
+					 * so we do not say DIFF_FILE_VALID()
+					 * on pp->one and pp->two.
+					 */
 					p->status = 'X';
 					break;
 				}
 			}
+			if (!p->status)
+				p->status = 'D';
 		}
 		else if (DIFF_PAIR_TYPE_CHANGED(p))
 			p->status = 'T';
@@ -746,33 +815,24 @@ static void diff_resolve_rename_copy(voi
 		 * whose both sides are valid and of the same type, i.e.
 		 * either in-place edit or rename/copy edit.
 		 */
-		else if (strcmp(p->one->path, p->two->path)) {
-			/* See if there is somebody else anywhere that
-			 * will keep the path (either modified or
-			 * unmodified).  If so, we have to be a copy,
-			 * not a rename.  In addition, if there is
-			 * some other rename or copy that comes later
-			 * than us that uses the same source, we
-			 * have to be a copy, not a rename.
+		else if (p->score) {
+			if (p->source_stays) {
+				p->status = 'C';
+				continue;
+			}
+			/* See if there is some other filepair that
+			 * copies from the same source as us.  If so
+			 * we are a copy.  Otherwise we are a rename.
 			 */
-			for (j = 0; j < q->nr; j++) {
+			for (j = i + 1; j < q->nr; j++) {
 				pp = q->queue[j];
 				if (strcmp(pp->one->path, p->one->path))
-					continue;
-				if (!strcmp(pp->one->path, pp->two->path)) {
-					if (DIFF_FILE_VALID(pp->two)) {
-						/* non-delete */
-						p->status = 'C';
-						break;
-					}
-					continue;
-				}
-				/* pp is a rename/copy ... */
-				if (i < j) {
-					/* ... and comes later than us */
-					p->status = 'C';
-					break;
-				}
+					continue; /* not us */
+				if (!pp->score)
+					continue; /* not a rename/copy */
+				/* pp is a rename/copy from the same source */
+				p->status = 'C';
+				break;
 			}
 			if (!p->status)
 				p->status = 'R';
@@ -781,8 +841,11 @@ static void diff_resolve_rename_copy(voi
 			 p->one->mode != p->two->mode)
 			p->status = 'M';
 		else
-			/* this is a "no-change" entry */
-			p->status = 'X';
+			/* this is a "no-change" entry.
+			 * should not happen anymore.
+			 * p->status = 'X';
+			 */
+			die("internal error in diffcore: unmodified entry remains");
 	}
 	diff_debug_queue("resolve-rename-copy done", q);
 }
@@ -817,12 +880,8 @@ void diff_flush(int diff_output_style, i
 			break;
 		}
 	}
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		diff_free_filespec_data(p->one);
-		diff_free_filespec_data(p->two);
-		free(p);
-	}
+	for (i = 0; i < q->nr; i++)
+		diff_free_filepair(q->queue[i]);
 	free(q->queue);
 	q->queue = NULL;
 	q->nr = q->alloc = 0;
@@ -883,7 +942,7 @@ void diff_helper_input(unsigned old_mode
 	if (new_mode)
 		fill_filespec(two, new_sha1, new_mode);
 	dp = diff_queue(&diff_queued_diff, one, two);
-	dp->score = score;
+	dp->score = score * MAX_SCORE / 100;
 	dp->status = status;
 }
 
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -28,22 +28,28 @@ extern void diff_unmerge(const char *pat
 
 extern int diff_scoreopt_parse(const char *opt);
 
-#define DIFF_FORMAT_HUMAN	0
-#define DIFF_FORMAT_MACHINE	1
-#define DIFF_FORMAT_PATCH	2
-#define DIFF_FORMAT_NO_OUTPUT	3
-extern void diff_setup(int reverse);
+#define DIFF_SETUP_REVERSE      	1
+#define DIFF_SETUP_USE_CACHE		2
+#define DIFF_SETUP_USE_SIZE_CACHE	4
+extern void diff_setup(int flags);
 
 #define DIFF_DETECT_RENAME	1
 #define DIFF_DETECT_COPY	2
 
 extern void diffcore_rename(int rename_copy, int minimum_score);
 
-extern void diffcore_pickaxe(const char *needle);
+#define DIFF_PICKAXE_ALL	1
+extern void diffcore_pickaxe(const char *needle, int opts);
+
 extern void diffcore_pathspec(const char **pathspec);
 
 extern int diff_queue_is_empty(void);
 
+#define DIFF_FORMAT_HUMAN	0
+#define DIFF_FORMAT_MACHINE	1
+#define DIFF_FORMAT_PATCH	2
+#define DIFF_FORMAT_NO_OUTPUT	3
+
 extern void diff_flush(int output_style, int resolve_rename_copy);
 
 #endif /* DIFF_H */
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -55,11 +55,10 @@ void diffcore_pathspec(const char **path
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		if (matches_pathspec(p->one->path, spec, speccnt) ||
-		    matches_pathspec(p->two->path, spec, speccnt))
+		if (matches_pathspec(p->two->path, spec, speccnt))
 			diff_q(&outq, p);
 		else
-			free(p);
+			diff_free_filepair(p);
 	}
 	free(q->queue);
 	*q = outq;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -11,7 +11,7 @@ static int contains(struct diff_filespec
 {
 	unsigned long offset, sz;
 	const char *data;
-	if (diff_populate_filespec(one))
+	if (diff_populate_filespec(one, 0))
 		return 0;
 	sz = one->size;
 	data = one->data;
@@ -21,36 +21,73 @@ static int contains(struct diff_filespec
 	return 0;
 }
 
-void diffcore_pickaxe(const char *needle)
+void diffcore_pickaxe(const char *needle, int opts)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	unsigned long len = strlen(needle);
-	int i;
+	int i, has_changes;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
 
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		int onum = outq.nr;
-		if (!DIFF_FILE_VALID(p->one)) {
-			if (!DIFF_FILE_VALID(p->two))
-				continue; /* ignore nonsense */
-			/* created */
-			if (contains(p->two, needle, len))
-				diff_q(&outq, p);
+	if (opts & DIFF_PICKAXE_ALL) {
+		/* Showing the whole changeset if needle exists */
+		for (i = has_changes = 0; !has_changes && i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (!DIFF_FILE_VALID(p->one)) {
+				if (!DIFF_FILE_VALID(p->two))
+					continue; /* ignore unmerged */
+				/* created */
+				if (contains(p->two, needle, len))
+					has_changes++;
+			}
+			else if (!DIFF_FILE_VALID(p->two)) {
+				if (contains(p->one, needle, len))
+					has_changes++;
+			}
+			else if (!diff_unmodified_pair(p) &&
+				 contains(p->one, needle, len) !=
+				 contains(p->two, needle, len))
+				has_changes++;
 		}
-		else if (!DIFF_FILE_VALID(p->two)) {
-			if (contains(p->one, needle, len))
+		if (has_changes)
+			return; /* not munge the queue */
+
+		/* otherwise we will clear the whole queue
+		 * by copying the empty outq at the end of this
+		 * function, but first clear the current entries
+		 * in the queue.
+		 */
+		for (i = 0; i < q->nr; i++)
+			diff_free_filepair(q->queue[i]);
+	}
+	else 
+		/* Showing only the filepairs that has the needle */
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			has_changes = 0;
+			if (!DIFF_FILE_VALID(p->one)) {
+				if (!DIFF_FILE_VALID(p->two))
+					; /* ignore unmerged */
+				/* created */
+				else if (contains(p->two, needle, len))
+					has_changes = 1;
+			}
+			else if (!DIFF_FILE_VALID(p->two)) {
+				if (contains(p->one, needle, len))
+					has_changes = 1;
+			}
+			else if (!diff_unmodified_pair(p) &&
+				 contains(p->one, needle, len) !=
+				 contains(p->two, needle, len))
+				has_changes = 1;
+
+			if (has_changes)
 				diff_q(&outq, p);
+			else
+				diff_free_filepair(p);
 		}
-		else if (!diff_unmodified_pair(p) &&
-			 contains(p->one, needle, len) !=
-			 contains(p->two, needle, len))
-			diff_q(&outq, p);
-		if (onum == outq.nr)
-			free(p);
-	}
+
 	free(q->queue);
 	*q = outq;
 	return;
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -52,14 +52,15 @@ static struct diff_rename_dst *locate_re
 	return &(rename_dst[first]);
 }
 
+/* Table of rename/copy src files */
 static struct diff_rename_src {
 	struct diff_filespec *one;
-	unsigned src_used : 1;
+	unsigned src_stays : 1;
 } *rename_src;
 static int rename_src_nr, rename_src_alloc;
 
-static struct diff_rename_src *locate_rename_src(struct diff_filespec *one,
-						 int insert_ok)
+static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
+						   int src_stays)
 {
 	int first, last;
 
@@ -77,9 +78,7 @@ static struct diff_rename_src *locate_re
 		}
 		first = next+1;
 	}
-	/* not found */
-	if (!insert_ok)
-		return NULL;
+
 	/* insert to make it at "first" */
 	if (rename_src_alloc <= rename_src_nr) {
 		rename_src_alloc = alloc_nr(rename_src_alloc);
@@ -91,7 +90,7 @@ static struct diff_rename_src *locate_re
 		memmove(rename_src + first + 1, rename_src + first,
 			(rename_src_nr - first - 1) * sizeof(*rename_src));
 	rename_src[first].one = one;
-	rename_src[first].src_used = 0;
+	rename_src[first].src_stays = src_stays;
 	return &(rename_src[first]);
 }
 
@@ -100,8 +99,11 @@ static int is_exact_match(struct diff_fi
 	if (src->sha1_valid && dst->sha1_valid &&
 	    !memcmp(src->sha1, dst->sha1, 20))
 		return 1;
-	if (diff_populate_filespec(src) || diff_populate_filespec(dst))
-		/* this is an error but will be caught downstream */
+	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
+		return 0;
+	if (src->size != dst->size)
+		return 0;
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
 		return 0;
 	if (src->size == dst->size &&
 	    !memcmp(src->data, dst->data, src->size))
@@ -113,7 +115,6 @@ struct diff_score {
 	int src; /* index in rename_src */
 	int dst; /* index in rename_dst */
 	int score;
-	int rank;
 };
 
 static int estimate_similarity(struct diff_filespec *src,
@@ -127,9 +128,11 @@ static int estimate_similarity(struct di
 	 * dst, and then some edit has been applied to dst.
 	 *
 	 * Compare them and return how similar they are, representing
-	 * the score as an integer between 0 and 10000, except
-	 * where they match exactly it is considered better than anything
-	 * else.
+	 * the score as an integer between 0 and MAX_SCORE.
+	 *
+	 * When there is an exact match, it is considered a better
+	 * match than anything else; the destination does not even
+	 * call into this function in that case.
 	 */
 	void *delta;
 	unsigned long delta_size, base_size;
@@ -149,6 +152,7 @@ static int estimate_similarity(struct di
 	/* We would not consider edits that change the file size so
 	 * drastically.  delta_size must be smaller than
 	 * (MAX_SCORE-minimum_score)/MAX_SCORE * min(src->size, dst->size).
+	 *
 	 * Note that base_size == 0 case is handled here already
 	 * and the final score computation below would not have a
 	 * divide-by-zero issue.
@@ -156,6 +160,9 @@ static int estimate_similarity(struct di
 	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
+		return 0; /* error but caught downstream */
+
 	delta = diff_delta(src->data, src->size,
 			   dst->data, dst->size,
 			   &delta_size);
@@ -163,7 +170,7 @@ static int estimate_similarity(struct di
 	/* A delta that has a lot of literal additions would have
 	 * big delta_size no matter what else it does.
 	 */
-	if (minimum_score < MAX_SCORE * delta_size / base_size)
+	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
 	/* Estimate the edit size by interpreting delta. */
@@ -200,15 +207,14 @@ static void record_rename_pair(struct di
 	fill_filespec(two, dst->sha1, dst->mode);
 
 	dp = diff_queue(renq, one, two);
-	dp->score = score;
-
-	rename_src[src_index].src_used = 1;
+	dp->score = score ? : 1; /* make sure it is at least 1 */
+	dp->source_stays = rename_src[src_index].src_stays;
 	rename_dst[dst_index].pair = dp;
 }
 
 /*
  * We sort the rename similarity matrix with the score, in descending
- * order (more similar first).
+ * order (the most similar first).
  */
 static int score_compare(const void *a_, const void *b_)
 {
@@ -223,7 +229,7 @@ int diff_scoreopt_parse(const char *opt)
 		return -1; /* that is not a -M nor -C option */
 	diglen = strspn(opt+2, "0123456789");
 	if (diglen == 0 || strlen(opt+2) != diglen)
-		return 0; /* use default */
+		return DEFAULT_MINIMUM_SCORE; /* use default */
 	sscanf(opt+2, "%d", &num);
 	for (i = 0, scale = 1; i < diglen; i++)
 		scale *= 10;
@@ -255,9 +261,9 @@ void diffcore_rename(int detect_rename, 
 			else
 				locate_rename_dst(p->two, 1);
 		else if (!DIFF_FILE_VALID(p->two))
-			locate_rename_src(p->one, 1);
-		else if (1 < detect_rename) /* find copy, too */
-			locate_rename_src(p->one, 1);
+			register_rename_src(p->one, 0);
+		else if (detect_rename == DIFF_DETECT_COPY)
+			register_rename_src(p->one, 1);
 	}
 	if (rename_dst_nr == 0)
 		goto cleanup; /* nothing to do */
@@ -308,7 +314,7 @@ void diffcore_rename(int detect_rename, 
 		if (dst->pair)
 			continue; /* already done, either exact or fuzzy. */
 		if (mx[i].score < minimum_score)
-			break; /* there is not any more diffs applicable. */
+			break; /* there is no more usable pair. */
 		record_rename_pair(&renq, mx[i].dst, mx[i].src, mx[i].score);
 	}
 	free(mx);
@@ -317,28 +323,21 @@ void diffcore_rename(int detect_rename, 
  flush_rest:
 	/* At this point, we have found some renames and copies and they
 	 * are kept in renq.  The original list is still in *q.
-	 *
-	 * Scan the original list and move them into the outq; we will sort
-	 * outq and swap it into the queue supplied to pass that to
-	 * downstream, so we assign the sort keys in this loop.
-	 *
-	 * See comments at the top of record_rename_pair for numbers used
-	 * to assign rename_rank.
 	 */
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		struct diff_rename_src *src = locate_rename_src(p->one, 0);
 		struct diff_rename_dst *dst = locate_rename_dst(p->two, 0);
 		struct diff_filepair *pair_to_free = NULL;
 
 		if (dst) {
 			/* creation */
 			if (dst->pair) {
-				/* renq has rename/copy already to produce
-				 * this file, so we do not emit the creation
-				 * record in the output.
+				/* renq has rename/copy to produce
+				 * this file already, so we do not
+				 * emit the creation record in the
+				 * output.
 				 */
 				diff_q(&outq, dst->pair);
 				pair_to_free = p;
@@ -350,25 +349,17 @@ void diffcore_rename(int detect_rename, 
 				diff_q(&outq, p);
 		}
 		else if (!diff_unmodified_pair(p))
-			/* all the other cases need to be recorded as is */
+			/* all the usual ones need to be kept */
 			diff_q(&outq, p);
-		else {
-			/* unmodified pair needs to be recorded only if
-			 * it is used as the source of rename/copy
-			 */
-			if (src && src->src_used)
-				diff_q(&outq, p);
-			else
-				pair_to_free = p;
-		}
-		if (pair_to_free) {
-			diff_free_filespec_data(pair_to_free->one);
-			diff_free_filespec_data(pair_to_free->two);
-			free(pair_to_free);
-		}
+		else
+			/* no need to keep unmodified pairs */
+			pair_to_free = p;
+
+		if (pair_to_free)
+			diff_free_filepair(pair_to_free);
 	}
 	diff_debug_queue("done copying original", &outq);
-
+	
 	free(renq.queue);
 	free(q->queue);
 	*q = outq;
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -33,14 +33,17 @@ extern struct diff_filespec *alloc_files
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
 			  unsigned short);
 
-extern int diff_populate_filespec(struct diff_filespec *);
+extern int diff_populate_filespec(struct diff_filespec *, int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 
 struct diff_filepair {
 	struct diff_filespec *one;
 	struct diff_filespec *two;
-	int score; /* only valid when one and two are different paths */
-	int status; /* M C R N D U (see Documentation/diff-format.txt) */
+	unsigned short int score; /* only valid when one and two are
+				   * different paths
+				   */
+	char source_stays; /* all of R/C are copies */
+	char status; /* M C R N D U (see Documentation/diff-format.txt) */
 };
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
@@ -54,6 +57,8 @@ struct diff_filepair {
 	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
 	S_ISLNK(mode) ? S_IFLNK : S_IFDIR)
 
+extern void diff_free_filepair(struct diff_filepair *);
+
 extern int diff_unmodified_pair(struct diff_filepair *);
 
 struct diff_queue_struct {
@@ -68,9 +73,6 @@ extern struct diff_filepair *diff_queue(
 					struct diff_filespec *);
 extern void diff_q(struct diff_queue_struct *, struct diff_filepair *);
 
-extern int diff_needs_to_stay(struct diff_queue_struct *, int,
-			      struct diff_filespec *);
-
 #define DIFF_DEBUG 0
 #if DIFF_DEBUG
 void diff_debug_filespec(struct diff_filespec *, int, const char *);
diff --git a/git-external-diff-script b/git-external-diff-script
--- a/git-external-diff-script
+++ b/git-external-diff-script
@@ -59,7 +59,7 @@ then
     echo "new mode $mode2"
     if test "$xfrm_msg" != ""
     then
-	echo -n $xfrm_msg
+	echo "$xfrm_msg"
     fi
 fi
 diff ${GIT_DIFF_OPTS-'-pu'} -L "a/$name1" -L "b/$name2" "$tmp1" "$tmp2"
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
new file mode 100755
--- /dev/null
+++ b/t/t4007-rename-3.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Rename interaction with pathspec.
+
+'
+. ./test-lib.sh
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]*	/ \1 \1 \2#	/'
+compare_diff_raw () {
+    # When heuristics are improved, the score numbers would change.
+    # Ignore them while comparing.
+    # Also we do not check SHA1 hash generation in this test, which
+    # is a job for t0000-basic.sh
+
+    sed -e "$sanitize_diff_raw" <"$1" >.tmp-1
+    sed -e "$sanitize_diff_raw" <"$2" >.tmp-2
+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
+test_expect_success \
+    'prepare reference tree' \
+    'mkdir path0 path1 &&
+     cp ../../COPYING path0/COPYING &&
+     git-update-cache --add path0/COPYING &&
+    tree=$(git-write-tree) &&
+    echo $tree'
+
+test_expect_success \
+    'prepare work tree' \
+    'cp path0/COPYING path1/COPYING &&
+     git-update-cache --add --remove path0/COPYING path1/COPYING'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 and
+# path1 both have COPYING and the latter is a copy of path0/COPYING.
+# Comparing the full tree with cache should tell us so.
+
+git-diff-cache -C $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 and
+# path1 both have COPYING and the latter is a copy of path0/COPYING.
+# When we omit output from path0 it should still be able to tell us
+# that path1/COPYING is result from a copy from path0/COPYING, not
+# rename, which would imply path0/COPYING is now gone.
+
+git-diff-cache -C $tree path1 >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'tweak work tree' \
+    'rm -f path0/COPYING &&
+     git-update-cache --remove path0/COPYING'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 does
+# not have COPYING anymore and path1 has COPYING which is a copy of
+# path0/COPYING.  Showing the full tree with cache should tell us about
+# the rename.
+
+git-diff-cache -C $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 does
+# not have COPYING anymore and path1 has COPYING which is a copy of
+# path0/COPYING.  Even if we restrict the output to path1, it still
+# should show us the rename.
+
+git-diff-cache -C $tree path1 >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+test_done
\ No newline at end of file
------------------------------------------------

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

[Linus Torvalds]

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

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

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

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

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

Enlightment!

My initial thinking was to do rename/copy first, deliberately
ignoring pathspec (I was thinking about tweaking diff-tree to
break out of its built-in pathspec limit to feed diffcore
_everything_, even things outside of the specified directories
when --detect-copies-harder is used), to cast a wider net so
that it can come up with "the best matches".  But what you just
said made me realize that the definition of "the best matches" I
had in mind was not best at all.  I agree with you 100% that
pathspec should come first before everything else to limit the
world diffcore operates in, not just to limit the set of output
paths.

I still want to do what this patch does for another reason.  It
is so much simpler if I do not have to carry around _some_
unmatched pair after rename/copy.  The earlier representation
was forcing all the downstream diffcore filters to be aware of
what rename/copy did, which was simply _wrong_.  They should not
have to care.

About ordering of pickaxe and rename/copy, I think for most uses
of diffcore filters, not limited to pickaxe, would make more
sense if they come after rename/copy when rename/copy detection
is in effect.  What I really wanted to do (I made a side note
comment in the earlier discussion about this, saying "something
like streams", but have not pursued it further) is to make these
diffcore filters/transformations stackable so that the main
programs can control not just if each of them is used or not,
but the order of application of the used ones.  I cannot offhand
think of a good use case of having pickaxe come _before_
rename/copy, but there may be a case the user want to have
things in that order, and having the application order not
hardcoded in three diff-* brothers is a major problem if we
wanted to give that option.

[PATCH 00/12] Diff updates

[Junio C Hamano]

This series consists of the following 12 patches.  Most of them
are bugfixes and cleanups.  The last one is somewhat iffy,
although it does not break things, and lies somewhere between a
request for inclusion and a request for comments.

    [PATCH 01/12] Fix math thinko in similarity estimator.
    [PATCH 02/12] Introduce diff_free_filepair() funcion.
    [PATCH 03/12] Make pathspec only care about the detination tree.
    [PATCH 04/12] Remove unused rank field from diff_core structure.
    [PATCH 05/12] Do not expose internal scaling to diff-helper.
    [PATCH 06/12] Remove final newline from the value of xfrm_msg variable.
    [PATCH 07/12] Clean up diff_setup() to make it more extensible.
    [PATCH 08/12] Remove a function not used anymore.
    [PATCH 09/12] Add --pickaxe-all to diff-* brothers.
    [PATCH 10/12] Fix the way diffcore-rename records unremoved source.
    [PATCH 11/12] Move pathspec to the beginning of the diffcore chain.
    [PATCH 12/12] Optimize diff-tree -[CM] --stdin

[PATCH 01/12] Fix math thinko in similarity estimator.

[Junio C Hamano]

The math to reject delta that is too big was confused.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diffcore-rename.c |    2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -163,7 +163,7 @@ static int estimate_similarity(struct di
 	/* A delta that has a lot of literal additions would have
 	 * big delta_size no matter what else it does.
 	 */
-	if (minimum_score < MAX_SCORE * delta_size / base_size)
+	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
 	/* Estimate the edit size by interpreting delta. */
------------------------------------------------

[PATCH 02/12] Introduce diff_free_filepair() funcion.

[Junio C Hamano]

This introduces a new function to free a common data structure,
and plugs some leaks.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c              |   15 +++++++++------
diffcore-pathspec.c |    2 +-
diffcore-pickaxe.c  |    2 +-
diffcore-rename.c   |    7 ++-----
diffcore.h          |    2 ++
5 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -521,6 +521,13 @@ struct diff_filepair *diff_queue(struct 
 	return dp;
 }
 
+void diff_free_filepair(struct diff_filepair *p)
+{
+	diff_free_filespec_data(p->one);
+	diff_free_filespec_data(p->two);
+	free(p);
+}
+
 static void diff_flush_raw(struct diff_filepair *p,
 			   int line_termination,
 			   int inter_name_termination)
@@ -817,12 +824,8 @@ void diff_flush(int diff_output_style, i
 			break;
 		}
 	}
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		diff_free_filespec_data(p->one);
-		diff_free_filespec_data(p->two);
-		free(p);
-	}
+	for (i = 0; i < q->nr; i++)
+		diff_free_filepair(q->queue[i]);
 	free(q->queue);
 	q->queue = NULL;
 	q->nr = q->alloc = 0;
diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -59,7 +59,7 @@ void diffcore_pathspec(const char **path
 		    matches_pathspec(p->two->path, spec, speccnt))
 			diff_q(&outq, p);
 		else
-			free(p);
+			diff_free_filepair(p);
 	}
 	free(q->queue);
 	*q = outq;
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -49,7 +49,7 @@ void diffcore_pickaxe(const char *needle
 			 contains(p->two, needle, len))
 			diff_q(&outq, p);
 		if (onum == outq.nr)
-			free(p);
+			diff_free_filepair(p);
 	}
 	free(q->queue);
 	*q = outq;
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -361,11 +361,8 @@ void diffcore_rename(int detect_rename, 
 			else
 				pair_to_free = p;
 		}
-		if (pair_to_free) {
-			diff_free_filespec_data(pair_to_free->one);
-			diff_free_filespec_data(pair_to_free->two);
-			free(pair_to_free);
-		}
+		if (pair_to_free)
+			diff_free_filepair(pair_to_free);
 	}
 	diff_debug_queue("done copying original", &outq);
 
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -54,6 +54,8 @@ struct diff_filepair {
 	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
 	S_ISLNK(mode) ? S_IFLNK : S_IFDIR)
 
+extern void diff_free_filepair(struct diff_filepair *);
+
 extern int diff_unmodified_pair(struct diff_filepair *);
 
 struct diff_queue_struct {
------------------------------------------------

[PATCH 03/12] Make pathspec only care about the detination tree.

[Junio C Hamano]

Earlier it had a misguided attempt to include paths that matches
either source tree or destination tree after the rename/copy
detection.  The new semantics will be that pathspec defines a
narrowed down world the diffcore operates in, so it should not
even look at where in the source tree the path came from.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diffcore-pathspec.c |    3 +--
1 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/diffcore-pathspec.c b/diffcore-pathspec.c
--- a/diffcore-pathspec.c
+++ b/diffcore-pathspec.c
@@ -55,8 +55,7 @@ void diffcore_pathspec(const char **path
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		if (matches_pathspec(p->one->path, spec, speccnt) ||
-		    matches_pathspec(p->two->path, spec, speccnt))
+		if (matches_pathspec(p->two->path, spec, speccnt))
 			diff_q(&outq, p);
 		else
 			diff_free_filepair(p);
------------------------------------------------

[PATCH 04/12] Remove unused rank field from diff_core structure.

[Junio C Hamano]

This removes a field that is no longer used from diff_score
structure.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diffcore-rename.c |    1 -
1 files changed, 1 deletion(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -113,7 +113,6 @@ struct diff_score {
 	int src; /* index in rename_src */
 	int dst; /* index in rename_dst */
 	int score;
-	int rank;
 };
 
 static int estimate_similarity(struct diff_filespec *src,
------------------------------------------------

[PATCH 05/12] Do not expose internal scaling to diff-helper.

[Junio C Hamano]

Instead we can normalize what diff-raw records at the diffcore
side.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-helper.c |    2 --
diff.c        |    2 +-
2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -4,7 +4,6 @@
 #include "cache.h"
 #include "strbuf.h"
 #include "diff.h"
-#include "diffcore.h" /* just for MAX_SCORE */
 
 static const char *pickaxe = NULL;
 static int line_termination = '\n';
@@ -78,7 +77,6 @@ int main(int ac, const char **av) {
 			if (status == 'R' || status == 'C') {
 				two_paths = 1;
 				sscanf(cp, "%d", &score);
-				score = score * MAX_SCORE / 100;
 				if (line_termination) {
 					cp = strchr(cp,
 						    inter_name_termination);
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -886,7 +886,7 @@ void diff_helper_input(unsigned old_mode
 	if (new_mode)
 		fill_filespec(two, new_sha1, new_mode);
 	dp = diff_queue(&diff_queued_diff, one, two);
-	dp->score = score;
+	dp->score = score * MAX_SCORE / 100;
 	dp->status = status;
 }
 
------------------------------------------------

[PATCH 06/12] Remove final newline from the value of xfrm_msg variable.

[Junio C Hamano]

This change makes the implementation of git-external-diff-script
cleaner.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c                   |    6 +++---
git-external-diff-script |    2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -141,7 +141,7 @@ static void builtin_diff(const char *nam
 			printf("new mode %s\n", temp[1].mode);
 		}
 		if (xfrm_msg && xfrm_msg[0])
-			fputs(xfrm_msg, stdout);
+			puts(xfrm_msg);
 
 		if (strncmp(temp[0].mode, temp[1].mode, 3))
 			/* we do not run diff between different kind
@@ -622,7 +622,7 @@ static void diff_flush_patch(struct diff
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"copy from %s\n"
-			"copy to %s\n",
+			"copy to %s",
 			(int)(0.5 + p->score * 100.0/MAX_SCORE),
 			p->one->path, p->two->path);
 		msg = msg_;
@@ -631,7 +631,7 @@ static void diff_flush_patch(struct diff
 		sprintf(msg_,
 			"similarity index %d%%\n"
 			"rename old %s\n"
-			"rename new %s\n",
+			"rename new %s",
 			(int)(0.5 + p->score * 100.0/MAX_SCORE),
 			p->one->path, p->two->path);
 		msg = msg_;
diff --git a/git-external-diff-script b/git-external-diff-script
--- a/git-external-diff-script
+++ b/git-external-diff-script
@@ -59,7 +59,7 @@ then
     echo "new mode $mode2"
     if test "$xfrm_msg" != ""
     then
-	echo -n $xfrm_msg
+	echo "$xfrm_msg"
     fi
 fi
 diff ${GIT_DIFF_OPTS-'-pu'} -L "a/$name1" -L "b/$name2" "$tmp1" "$tmp2"
------------------------------------------------

[PATCH 07/12] Clean up diff_setup() to make it more extensible.

[Junio C Hamano]

This changes the argument of diff_setup() from an integer that
says if we are feeding reversed diff to a bitmask, so that later
global options can be added more easily.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-cache.c |    6 +++---
diff-files.c |    6 +++---
diff-tree.c  |    6 +++---
diff.c       |    5 +++--
diff.h       |   12 +++++++-----
5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -5,7 +5,7 @@ static int cached_only = 0;
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int match_nonexisting = 0;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 
@@ -202,7 +202,7 @@ int main(int argc, const char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "-R")) {
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 			continue;
 		}
 		if (!strcmp(arg, "-S")) {
@@ -224,7 +224,7 @@ int main(int argc, const char **argv)
 		usage(diff_cache_usage);
 
 	/* The rest is for paths restriction. */
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 
 	mark_merge_entries();
 
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -11,7 +11,7 @@ static const char *diff_files_usage =
 
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int silent = 0;
@@ -51,7 +51,7 @@ int main(int argc, const char **argv)
 		else if (!strcmp(argv[1], "-z"))
 			diff_output_format = DIFF_FORMAT_MACHINE;
 		else if (!strcmp(argv[1], "-R"))
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 		else if (!strcmp(argv[1], "-S"))
 			pickaxe = argv[1] + 2;
 		else if (!strncmp(argv[1], "-M", 2)) {
@@ -75,7 +75,7 @@ int main(int argc, const char **argv)
 		exit(1);
 	}
 
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 
 	for (i = 0; i < entries; i++) {
 		struct stat st;
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -10,7 +10,7 @@ static int show_tree_entry_in_recursive 
 static int read_stdin = 0;
 static int diff_output_format = DIFF_FORMAT_HUMAN;
 static int detect_rename = 0;
-static int reverse_diff = 0;
+static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static const char *header = NULL;
@@ -255,7 +255,7 @@ static int diff_tree_sha1(const unsigned
 
 static void call_diff_setup(void)
 {
-	diff_setup(reverse_diff);
+	diff_setup(diff_setup_opt);
 }
 
 static int call_diff_flush(void)
@@ -497,7 +497,7 @@ int main(int argc, const char **argv)
 			continue;
 		}
 		if (!strcmp(arg, "-R")) {
-			reverse_diff = 1;
+			diff_setup_opt |= DIFF_SETUP_REVERSE;
 			continue;
 		}
 		if (!strcmp(arg, "-p")) {
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -492,9 +492,10 @@ static void run_diff(const char *name,
 		run_external_diff(pgm, name, other, one, two, xfrm_msg);
 }
 
-void diff_setup(int reverse_diff_)
+void diff_setup(int flags)
 {
-	reverse_diff = reverse_diff_;
+	if (flags & DIFF_SETUP_REVERSE)
+		reverse_diff = 1;
 }
 
 struct diff_queue_struct diff_queued_diff;
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -28,11 +28,8 @@ extern void diff_unmerge(const char *pat
 
 extern int diff_scoreopt_parse(const char *opt);
 
-#define DIFF_FORMAT_HUMAN	0
-#define DIFF_FORMAT_MACHINE	1
-#define DIFF_FORMAT_PATCH	2
-#define DIFF_FORMAT_NO_OUTPUT	3
-extern void diff_setup(int reverse);
+#define DIFF_SETUP_REVERSE      	1
+extern void diff_setup(int flags);
 
 #define DIFF_DETECT_RENAME	1
 #define DIFF_DETECT_COPY	2
@@ -44,6 +41,11 @@ extern void diffcore_pathspec(const char
 
 extern int diff_queue_is_empty(void);
 
+#define DIFF_FORMAT_HUMAN	0
+#define DIFF_FORMAT_MACHINE	1
+#define DIFF_FORMAT_PATCH	2
+#define DIFF_FORMAT_NO_OUTPUT	3
+
 extern void diff_flush(int output_style, int resolve_rename_copy);
 
 #endif /* DIFF_H */
------------------------------------------------

[PATCH 08/12] Remove a function not used anymore.

[Junio C Hamano]

Earlier rename/copy detection left unmodified filepair in the
output and forced downstream to keep them even when they are
filtering, and the diff_needs_to_stay() function was used for
the logic.  It is not used anymore, so remove it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c     |   22 ----------------------
diffcore.h |    3 ---
2 files changed, 25 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -647,28 +647,6 @@ static void diff_flush_patch(struct diff
 		run_diff(name, other, p->one, p->two, msg);
 }
 
-int diff_needs_to_stay(struct diff_queue_struct *q, int i,
-		       struct diff_filespec *it)
-{
-	/* If it will be used in later entry (either stay or used
-	 * as the source of rename/copy), we need to copy, not rename.
-	 */
-	while (i < q->nr) {
-		struct diff_filepair *p = q->queue[i++];
-		if (!DIFF_FILE_VALID(p->two))
-			continue; /* removed is fine */
-		if (strcmp(p->one->path, it->path))
-			continue; /* not relevant */
-
-		/* p has its src set to *it and it is not a delete;
-		 * it will be used for in-place change, rename/copy,
-		 * or just stays there.  We cannot rename it out.
-		 */
-		return 1;
-	}
-	return 0;
-}
-
 int diff_queue_is_empty(void)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -70,9 +70,6 @@ extern struct diff_filepair *diff_queue(
 					struct diff_filespec *);
 extern void diff_q(struct diff_queue_struct *, struct diff_filepair *);
 
-extern int diff_needs_to_stay(struct diff_queue_struct *, int,
-			      struct diff_filespec *);
-
 #define DIFF_DEBUG 0
 #if DIFF_DEBUG
 void diff_debug_filespec(struct diff_filespec *, int, const char *);
------------------------------------------------

[PATCH 09/12] Add --pickaxe-all to diff-* brothers.

[Junio C Hamano]

When --pickaxe-all is given in addition to -S, pickaxe shows the
entire diffs contained in the changeset, not just the diffs for
the filepair that touched the sought-after string.  This is
useful to see the changes in context.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-cache.c       |    7 ++++
diff-files.c       |    5 ++-
diff-helper.c      |    7 +++-
diff-tree.c        |    7 ++++
diff.h             |    4 ++
diffcore-pickaxe.c |   77 +++++++++++++++++++++++++++++++++++++++--------------
6 files changed, 81 insertions(+), 26 deletions(-)

diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -8,6 +8,7 @@ static int detect_rename = 0;
 static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -209,6 +210,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strcmp(arg, "--pickaxe-all")) {
+			pickaxe_opts = DIFF_PICKAXE_ALL;
+			continue;
+		}
 		if (!strcmp(arg, "-m")) {
 			match_nonexisting = 1;
 			continue;
@@ -238,7 +243,7 @@ int main(int argc, const char **argv)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (pathspec)
 		diffcore_pathspec(pathspec);
 	diff_flush(diff_output_format, 1);
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -14,6 +14,7 @@ static int detect_rename = 0;
 static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -54,6 +55,8 @@ int main(int argc, const char **argv)
 			diff_setup_opt |= DIFF_SETUP_REVERSE;
 		else if (!strcmp(argv[1], "-S"))
 			pickaxe = argv[1] + 2;
+		else if (!strcmp(argv[1], "--pickaxe-all"))
+			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else if (!strncmp(argv[1], "-M", 2)) {
 			diff_score_opt = diff_scoreopt_parse(argv[1]);
 			detect_rename = DIFF_DETECT_RENAME;
@@ -116,7 +119,7 @@ int main(int argc, const char **argv)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (1 < argc)
 		diffcore_pathspec(argv + 1);
 	diff_flush(diff_output_format, 1);
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -6,6 +6,7 @@
 #include "diff.h"
 
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static int line_termination = '\n';
 static int inter_name_termination = '\t';
 
@@ -23,6 +24,8 @@ int main(int ac, const char **av) {
 		else if (av[1][1] == 'S') {
 			pickaxe = av[1] + 2;
 		}
+		else if (!strcmp(av[1], "--pickaxe-all"))
+			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else
 			usage(diff_helper_usage);
 		ac--; av++;
@@ -127,14 +130,14 @@ int main(int ac, const char **av) {
 			continue;
 		}
 		if (pickaxe)
-			diffcore_pickaxe(pickaxe);
+			diffcore_pickaxe(pickaxe, pickaxe_opts);
 		if (1 < ac)
 			diffcore_pathspec(av + 1);
 		diff_flush(DIFF_FORMAT_PATCH, 0);
 		printf("%s\n", sb.buf);
 	}
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (1 < ac)
 		diffcore_pathspec(av + 1);
 	diff_flush(DIFF_FORMAT_PATCH, 0);
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -13,6 +13,7 @@ static int detect_rename = 0;
 static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
+static int pickaxe_opts = 0;
 static const char *header = NULL;
 static const char *header_prefix = "";
 
@@ -263,7 +264,7 @@ static int call_diff_flush(void)
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
-		diffcore_pickaxe(pickaxe);
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (diff_queue_is_empty()) {
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
@@ -509,6 +510,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strcmp(arg, "--pickaxe-all")) {
+			pickaxe_opts = DIFF_PICKAXE_ALL;
+			continue;
+		}
 		if (!strncmp(arg, "-M", 2)) {
 			detect_rename = DIFF_DETECT_RENAME;
 			diff_score_opt = diff_scoreopt_parse(arg);
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -36,7 +36,9 @@ extern void diff_setup(int flags);
 
 extern void diffcore_rename(int rename_copy, int minimum_score);
 
-extern void diffcore_pickaxe(const char *needle);
+#define DIFF_PICKAXE_ALL	1
+extern void diffcore_pickaxe(const char *needle, int opts);
+
 extern void diffcore_pathspec(const char **pathspec);
 
 extern int diff_queue_is_empty(void);
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -21,36 +21,73 @@ static int contains(struct diff_filespec
 	return 0;
 }
 
-void diffcore_pickaxe(const char *needle)
+void diffcore_pickaxe(const char *needle, int opts)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	unsigned long len = strlen(needle);
-	int i;
+	int i, has_changes;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
 
-	for (i = 0; i < q->nr; i++) {
-		struct diff_filepair *p = q->queue[i];
-		int onum = outq.nr;
-		if (!DIFF_FILE_VALID(p->one)) {
-			if (!DIFF_FILE_VALID(p->two))
-				continue; /* ignore nonsense */
-			/* created */
-			if (contains(p->two, needle, len))
-				diff_q(&outq, p);
+	if (opts & DIFF_PICKAXE_ALL) {
+		/* Showing the whole changeset if needle exists */
+		for (i = has_changes = 0; !has_changes && i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (!DIFF_FILE_VALID(p->one)) {
+				if (!DIFF_FILE_VALID(p->two))
+					continue; /* ignore unmerged */
+				/* created */
+				if (contains(p->two, needle, len))
+					has_changes++;
+			}
+			else if (!DIFF_FILE_VALID(p->two)) {
+				if (contains(p->one, needle, len))
+					has_changes++;
+			}
+			else if (!diff_unmodified_pair(p) &&
+				 contains(p->one, needle, len) !=
+				 contains(p->two, needle, len))
+				has_changes++;
 		}
-		else if (!DIFF_FILE_VALID(p->two)) {
-			if (contains(p->one, needle, len))
+		if (has_changes)
+			return; /* not munge the queue */
+
+		/* otherwise we will clear the whole queue
+		 * by copying the empty outq at the end of this
+		 * function, but first clear the current entries
+		 * in the queue.
+		 */
+		for (i = 0; i < q->nr; i++)
+			diff_free_filepair(q->queue[i]);
+	}
+	else 
+		/* Showing only the filepairs that has the needle */
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			has_changes = 0;
+			if (!DIFF_FILE_VALID(p->one)) {
+				if (!DIFF_FILE_VALID(p->two))
+					; /* ignore unmerged */
+				/* created */
+				else if (contains(p->two, needle, len))
+					has_changes = 1;
+			}
+			else if (!DIFF_FILE_VALID(p->two)) {
+				if (contains(p->one, needle, len))
+					has_changes = 1;
+			}
+			else if (!diff_unmodified_pair(p) &&
+				 contains(p->one, needle, len) !=
+				 contains(p->two, needle, len))
+				has_changes = 1;
+
+			if (has_changes)
 				diff_q(&outq, p);
+			else
+				diff_free_filepair(p);
 		}
-		else if (!diff_unmodified_pair(p) &&
-			 contains(p->one, needle, len) !=
-			 contains(p->two, needle, len))
-			diff_q(&outq, p);
-		if (onum == outq.nr)
-			diff_free_filepair(p);
-	}
+
 	free(q->queue);
 	*q = outq;
 	return;
------------------------------------------------

[PATCH 10/12] Fix the way diffcore-rename records unremoved source.

[Junio C Hamano]

Earier version of diffcore-rename used to keep unmodified
filepair in its output so that the last stage of the processing
that tells renames from copies can make all of rename/copy to
copies.  However this had a bad interaction with other diffcore
filters that wanted to run after diffcore-rename, in that such
unmodified filepair must be retained for proper distinction
between renames and copies to happen.

This patch fixes the problem by changing the way diffcore-rename
records the information needed to distinguish "all are copies"
case and "the last one is a rename" case.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c              |   76 ++++++++++++++++++--------------------
diffcore-rename.c   |   63 ++++++++++++-------------------
diffcore.h          |    7 ++-
t/t4007-rename-3.sh |  103 ++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 169 insertions(+), 80 deletions(-)
new file (100644): t/t4007-rename-3.sh

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -518,6 +518,7 @@ struct diff_filepair *diff_queue(struct 
 	dp->one = one;
 	dp->two = two;
 	dp->score = 0;
+	dp->source_stays = 0;
 	diff_q(queue, dp);
 	return dp;
 }
@@ -675,8 +676,8 @@ void diff_debug_filepair(const struct di
 {
 	diff_debug_filespec(p->one, i, "one");
 	diff_debug_filespec(p->two, i, "two");
-	fprintf(stderr, "score %d, status %c\n",
-		p->score, p->status ? : '?');
+	fprintf(stderr, "score %d, status %c source_stays %d\n",
+		p->score, p->status ? : '?', p->source_stays);
 }
 
 void diff_debug_queue(const char *msg, struct diff_queue_struct *q)
@@ -698,8 +699,6 @@ static void diff_resolve_rename_copy(voi
 	struct diff_filepair *p, *pp;
 	struct diff_queue_struct *q = &diff_queued_diff;
 
-	/* This should not depend on the ordering of things. */
-
 	diff_debug_queue("resolve-rename-copy", q);
 
 	for (i = 0; i < q->nr; i++) {
@@ -707,23 +706,28 @@ static void diff_resolve_rename_copy(voi
 		p->status = 0; /* undecided */
 		if (DIFF_PAIR_UNMERGED(p))
 			p->status = 'U';
-		else if (!DIFF_FILE_VALID((p)->one))
+		else if (!DIFF_FILE_VALID(p->one))
 			p->status = 'N';
-		else if (!DIFF_FILE_VALID((p)->two)) {
-			/* Deletion record should be omitted if there
-			 * are rename/copy entries using this one as
-			 * the source.  Then we can say one of them
-			 * is a rename and the rest are copies.
+		else if (!DIFF_FILE_VALID(p->two)) {
+			/* Deleted entry may have been picked up by
+			 * another rename-copy entry.  So we scan the
+			 * queue and if we find one that uses us as the
+			 * source we do not say delete for this entry.
 			 */
-			p->status = 'D';
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
-				if (!strcmp(pp->one->path, p->one->path) &&
-				    strcmp(pp->one->path, pp->two->path)) {
+				if (!strcmp(p->one->path, pp->one->path) &&
+				    pp->score) {
+					/* rename/copy are always valid
+					 * so we do not say DIFF_FILE_VALID()
+					 * on pp->one and pp->two.
+					 */
 					p->status = 'X';
 					break;
 				}
 			}
+			if (!p->status)
+				p->status = 'D';
 		}
 		else if (DIFF_PAIR_TYPE_CHANGED(p))
 			p->status = 'T';
@@ -732,33 +736,24 @@ static void diff_resolve_rename_copy(voi
 		 * whose both sides are valid and of the same type, i.e.
 		 * either in-place edit or rename/copy edit.
 		 */
-		else if (strcmp(p->one->path, p->two->path)) {
-			/* See if there is somebody else anywhere that
-			 * will keep the path (either modified or
-			 * unmodified).  If so, we have to be a copy,
-			 * not a rename.  In addition, if there is
-			 * some other rename or copy that comes later
-			 * than us that uses the same source, we
-			 * have to be a copy, not a rename.
+		else if (p->score) {
+			if (p->source_stays) {
+				p->status = 'C';
+				continue;
+			}
+			/* See if there is some other filepair that
+			 * copies from the same source as us.  If so
+			 * we are a copy.  Otherwise we are a rename.
 			 */
-			for (j = 0; j < q->nr; j++) {
+			for (j = i + 1; j < q->nr; j++) {
 				pp = q->queue[j];
 				if (strcmp(pp->one->path, p->one->path))
-					continue;
-				if (!strcmp(pp->one->path, pp->two->path)) {
-					if (DIFF_FILE_VALID(pp->two)) {
-						/* non-delete */
-						p->status = 'C';
-						break;
-					}
-					continue;
-				}
-				/* pp is a rename/copy ... */
-				if (i < j) {
-					/* ... and comes later than us */
-					p->status = 'C';
-					break;
-				}
+					continue; /* not us */
+				if (!pp->score)
+					continue; /* not a rename/copy */
+				/* pp is a rename/copy from the same source */
+				p->status = 'C';
+				break;
 			}
 			if (!p->status)
 				p->status = 'R';
@@ -767,8 +762,11 @@ static void diff_resolve_rename_copy(voi
 			 p->one->mode != p->two->mode)
 			p->status = 'M';
 		else
-			/* this is a "no-change" entry */
-			p->status = 'X';
+			/* this is a "no-change" entry.
+			 * should not happen anymore.
+			 * p->status = 'X';
+			 */
+			die("internal error in diffcore: unmodified entry remains");
 	}
 	diff_debug_queue("resolve-rename-copy done", q);
 }
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -52,14 +52,15 @@ static struct diff_rename_dst *locate_re
 	return &(rename_dst[first]);
 }
 
+/* Table of rename/copy src files */
 static struct diff_rename_src {
 	struct diff_filespec *one;
-	unsigned src_used : 1;
+	unsigned src_stays : 1;
 } *rename_src;
 static int rename_src_nr, rename_src_alloc;
 
-static struct diff_rename_src *locate_rename_src(struct diff_filespec *one,
-						 int insert_ok)
+static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
+						   int src_stays)
 {
 	int first, last;
 
@@ -77,9 +78,7 @@ static struct diff_rename_src *locate_re
 		}
 		first = next+1;
 	}
-	/* not found */
-	if (!insert_ok)
-		return NULL;
+
 	/* insert to make it at "first" */
 	if (rename_src_alloc <= rename_src_nr) {
 		rename_src_alloc = alloc_nr(rename_src_alloc);
@@ -91,7 +90,7 @@ static struct diff_rename_src *locate_re
 		memmove(rename_src + first + 1, rename_src + first,
 			(rename_src_nr - first - 1) * sizeof(*rename_src));
 	rename_src[first].one = one;
-	rename_src[first].src_used = 0;
+	rename_src[first].src_stays = src_stays;
 	return &(rename_src[first]);
 }
 
@@ -199,15 +198,14 @@ static void record_rename_pair(struct di
 	fill_filespec(two, dst->sha1, dst->mode);
 
 	dp = diff_queue(renq, one, two);
-	dp->score = score;
-
-	rename_src[src_index].src_used = 1;
+	dp->score = score ? : 1; /* make sure it is at least 1 */
+	dp->source_stays = rename_src[src_index].src_stays;
 	rename_dst[dst_index].pair = dp;
 }
 
 /*
  * We sort the rename similarity matrix with the score, in descending
- * order (more similar first).
+ * order (the most similar first).
  */
 static int score_compare(const void *a_, const void *b_)
 {
@@ -254,9 +252,9 @@ void diffcore_rename(int detect_rename, 
 			else
 				locate_rename_dst(p->two, 1);
 		else if (!DIFF_FILE_VALID(p->two))
-			locate_rename_src(p->one, 1);
-		else if (1 < detect_rename) /* find copy, too */
-			locate_rename_src(p->one, 1);
+			register_rename_src(p->one, 0);
+		else if (detect_rename == DIFF_DETECT_COPY)
+			register_rename_src(p->one, 1);
 	}
 	if (rename_dst_nr == 0)
 		goto cleanup; /* nothing to do */
@@ -280,7 +278,7 @@ void diffcore_rename(int detect_rename, 
 	 * doing the delta matrix altogether.
 	 */
 	if (renq.nr == rename_dst_nr)
-		goto flush_rest;
+		goto cleanup;
 
 	num_create = (rename_dst_nr - renq.nr);
 	num_src = rename_src_nr;
@@ -307,37 +305,30 @@ void diffcore_rename(int detect_rename, 
 		if (dst->pair)
 			continue; /* already done, either exact or fuzzy. */
 		if (mx[i].score < minimum_score)
-			break; /* there is not any more diffs applicable. */
+			break; /* there is no more usable pair. */
 		record_rename_pair(&renq, mx[i].dst, mx[i].src, mx[i].score);
 	}
 	free(mx);
 	diff_debug_queue("done detecting fuzzy", &renq);
 
- flush_rest:
+ cleanup:
 	/* At this point, we have found some renames and copies and they
 	 * are kept in renq.  The original list is still in *q.
-	 *
-	 * Scan the original list and move them into the outq; we will sort
-	 * outq and swap it into the queue supplied to pass that to
-	 * downstream, so we assign the sort keys in this loop.
-	 *
-	 * See comments at the top of record_rename_pair for numbers used
-	 * to assign rename_rank.
 	 */
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		struct diff_rename_src *src = locate_rename_src(p->one, 0);
 		struct diff_rename_dst *dst = locate_rename_dst(p->two, 0);
 		struct diff_filepair *pair_to_free = NULL;
 
 		if (dst) {
 			/* creation */
 			if (dst->pair) {
-				/* renq has rename/copy already to produce
-				 * this file, so we do not emit the creation
-				 * record in the output.
+				/* renq has rename/copy to produce
+				 * this file already, so we do not
+				 * emit the creation record in the
+				 * output.
 				 */
 				diff_q(&outq, dst->pair);
 				pair_to_free = p;
@@ -349,17 +340,12 @@ void diffcore_rename(int detect_rename, 
 				diff_q(&outq, p);
 		}
 		else if (!diff_unmodified_pair(p))
-			/* all the other cases need to be recorded as is */
+			/* all the usual ones need to be kept */
 			diff_q(&outq, p);
-		else {
-			/* unmodified pair needs to be recorded only if
-			 * it is used as the source of rename/copy
-			 */
-			if (src && src->src_used)
-				diff_q(&outq, p);
-			else
-				pair_to_free = p;
-		}
+		else
+			/* no need to keep unmodified pairs */
+			pair_to_free = p;
+
 		if (pair_to_free)
 			diff_free_filepair(pair_to_free);
 	}
@@ -370,7 +356,6 @@ void diffcore_rename(int detect_rename, 
 	*q = outq;
 	diff_debug_queue("done collapsing", q);
 
- cleanup:
 	free(rename_dst);
 	rename_dst = NULL;
 	rename_dst_nr = rename_dst_alloc = 0;
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -39,8 +39,11 @@ extern void diff_free_filespec_data(stru
 struct diff_filepair {
 	struct diff_filespec *one;
 	struct diff_filespec *two;
-	int score; /* only valid when one and two are different paths */
-	int status; /* M C R N D U (see Documentation/diff-format.txt) */
+	unsigned short int score; /* only valid when one and two are
+				   * different paths
+				   */
+	char source_stays; /* all of R/C are copies */
+	char status; /* M C R N D U (see Documentation/diff-format.txt) */
 };
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
new file mode 100644
--- /dev/null
+++ b/t/t4007-rename-3.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Rename interaction with pathspec.
+
+'
+. ./test-lib.sh
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]*	/ \1 \1 \2#	/'
+compare_diff_raw () {
+    # When heuristics are improved, the score numbers would change.
+    # Ignore them while comparing.
+    # Also we do not check SHA1 hash generation in this test, which
+    # is a job for t0000-basic.sh
+
+    sed -e "$sanitize_diff_raw" <"$1" >.tmp-1
+    sed -e "$sanitize_diff_raw" <"$2" >.tmp-2
+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
+test_expect_success \
+    'prepare reference tree' \
+    'mkdir path0 path1 &&
+     cp ../../COPYING path0/COPYING &&
+     git-update-cache --add path0/COPYING &&
+    tree=$(git-write-tree) &&
+    echo $tree'
+
+test_expect_success \
+    'prepare work tree' \
+    'cp path0/COPYING path1/COPYING &&
+     git-update-cache --add --remove path0/COPYING path1/COPYING'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 and
+# path1 both have COPYING and the latter is a copy of path0/COPYING.
+# Comparing the full tree with cache should tell us so.
+
+git-diff-cache -C $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 and
+# path1 both have COPYING and the latter is a copy of path0/COPYING.
+# When we omit output from path0 it should still be able to tell us
+# that path1/COPYING is result from a copy from path0/COPYING, not
+# rename, which would imply path0/COPYING is now gone.
+
+git-diff-cache -C $tree path1 >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'tweak work tree' \
+    'rm -f path0/COPYING &&
+     git-update-cache --remove path0/COPYING'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 does
+# not have COPYING anymore and path1 has COPYING which is a copy of
+# path0/COPYING.  Showing the full tree with cache should tell us about
+# the rename.
+
+git-diff-cache -C $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+# In the tree, there is only path0/COPYING.  In the cache, path0 does
+# not have COPYING anymore and path1 has COPYING which is a copy of
+# path0/COPYING.  Even if we restrict the output to path1, it still
+# should show us the rename.
+
+git-diff-cache -C $tree path1 >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	path0/COPYING	path1/COPYING
+EOF
+
+test_expect_success \
+    'validate the result' \
+    'compare_diff_raw current expected'
+
+test_done
------------------------------------------------

[PATCH 11/12] Move pathspec to the beginning of the diffcore chain.

[Junio C Hamano]

This changes the way how pathspec is used in the three diff-*
brothers.  Earlier, they tried to grab as much information from
the original input and used pathspec to limit the output.  This
version uses pathspec upfront to narrow the world diffcore
operates in, so "git-diff-* <arguments> some-directory" does not
look at things outside the specified subtree when finding
rename/copy or running pickaxe.

Since diff-tree already takes this view and does not feed
anything outside the specified directotires to begin with, this
patch does not have to touch that command.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-cache.c        |    4 ++--
diff-files.c        |    4 ++--
diff-helper.c       |    8 ++++----
t/t4007-rename-3.sh |   22 +++++++++++-----------
4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -240,12 +240,12 @@ int main(int argc, const char **argv)
 		die("unable to read tree object %s", tree_name);
 
 	ret = diff_cache(active_cache, active_nr);
+	if (pathspec)
+		diffcore_pathspec(pathspec);
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
-	if (pathspec)
-		diffcore_pathspec(pathspec);
 	diff_flush(diff_output_format, 1);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -116,12 +116,12 @@ int main(int argc, const char **argv)
 		show_modified(oldmode, mode, ce->sha1, null_sha1,
 			      ce->name);
 	}
+	if (1 < argc)
+		diffcore_pathspec(argv + 1);
 	if (detect_rename)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
-	if (1 < argc)
-		diffcore_pathspec(argv + 1);
 	diff_flush(diff_output_format, 1);
 	return 0;
 }
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -129,17 +129,17 @@ int main(int ac, const char **av) {
 					  new_path);
 			continue;
 		}
-		if (pickaxe)
-			diffcore_pickaxe(pickaxe, pickaxe_opts);
 		if (1 < ac)
 			diffcore_pathspec(av + 1);
+		if (pickaxe)
+			diffcore_pickaxe(pickaxe, pickaxe_opts);
 		diff_flush(DIFF_FORMAT_PATCH, 0);
 		printf("%s\n", sb.buf);
 	}
-	if (pickaxe)
-		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (1 < ac)
 		diffcore_pathspec(av + 1);
+	if (pickaxe)
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	diff_flush(DIFF_FORMAT_PATCH, 0);
 	return 0;
 }
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -46,23 +46,23 @@ cat >expected <<\EOF
 EOF
 
 test_expect_success \
-    'validate the result' \
+    'validate the result (#1)' \
     'compare_diff_raw current expected'
 
 # In the tree, there is only path0/COPYING.  In the cache, path0 and
 # path1 both have COPYING and the latter is a copy of path0/COPYING.
-# When we omit output from path0 it should still be able to tell us
-# that path1/COPYING is result from a copy from path0/COPYING, not
-# rename, which would imply path0/COPYING is now gone.
+# However when we say we care only about path1, we should just see
+# path1/COPYING suddenly appearing from nowhere, not detected as
+# a copy from path0/COPYING.
 
 git-diff-cache -C $tree path1 >current
 
 cat >expected <<\EOF
-:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 C100	path0/COPYING	path1/COPYING
+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N	path1/COPYING
 EOF
 
 test_expect_success \
-    'validate the result' \
+    'validate the result (#2)' \
     'compare_diff_raw current expected'
 
 test_expect_success \
@@ -82,22 +82,22 @@ cat >expected <<\EOF
 EOF
 
 test_expect_success \
-    'validate the result' \
+    'validate the result (#3)' \
     'compare_diff_raw current expected'
 
 # In the tree, there is only path0/COPYING.  In the cache, path0 does
 # not have COPYING anymore and path1 has COPYING which is a copy of
-# path0/COPYING.  Even if we restrict the output to path1, it still
-# should show us the rename.
+# path0/COPYING.  When we say we care only about path1, we should just
+# see path1/COPYING appearing from nowhere.
 
 git-diff-cache -C $tree path1 >current
 
 cat >expected <<\EOF
-:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	path0/COPYING	path1/COPYING
+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N	path1/COPYING
 EOF
 
 test_expect_success \
-    'validate the result' \
+    'validate the result (#4)' \
     'compare_diff_raw current expected'
 
 test_done
------------------------------------------------

[PATCH 12/12] Optimize diff-tree -[CM] --stdin

[Junio C Hamano]

This attempts to optimize "diff-tree -[CM] --stdin", which
compares successible tree pairs.  This optimization does not
make much sense for other commands in the diff-* brothers.

When reading from --stdin and using rename/copy detection, the
patch makes diff-tree to read the current index file first.
This is done to reuse the optimization used by diff-cache in the
non-cached case.  Similarity estimator can avoid expanding a
blob if the index says what is in the work tree has an exact
copy of that blob already expanded.

Another optimization the patch makes is to check only file sizes
first to terminate similarity estimation early.  In order for
this to work, it needs a way to tell the size of the blob
without expanding it.  Since an obvious way of doing it, which
is to keep all the blobs previously used in the memory, is too
costly, it does so by keeping the filesize for each object it
has already seen in memory.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-tree.c        |    3 +
diff.c             |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++--
diff.h             |    2 +
diffcore-pickaxe.c |    2 -
diffcore-rename.c  |   19 ++++++++----
diffcore.h         |    2 -
6 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -578,6 +578,9 @@ int main(int argc, const char **argv)
 	if (!read_stdin)
 		return 0;
 
+	if (detect_rename)
+		diff_setup_opt |= (DIFF_SETUP_USE_SIZE_CACHE |
+				   DIFF_SETUP_USE_CACHE);
 	while (fgets(line, sizeof(line), stdin))
 		diff_tree_stdin(line);
 
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -12,6 +12,7 @@ static const char *diff_opts = "-pu";
 static unsigned char null_sha1[20] = { 0, };
 
 static int reverse_diff;
+static int use_size_cache;
 
 static const char *external_diff(void)
 {
@@ -222,12 +223,60 @@ static int work_tree_matches(const char 
 	return 1;
 }
 
+static struct sha1_size_cache {
+	unsigned char sha1[20];
+	unsigned long size;
+} **sha1_size_cache;
+static int sha1_size_cache_nr, sha1_size_cache_alloc;
+
+static struct sha1_size_cache *locate_size_cache(unsigned char *sha1,
+						 unsigned long size)
+{
+	int first, last;
+	struct sha1_size_cache *e;
+
+	first = 0;
+	last = sha1_size_cache_nr;
+	while (last > first) {
+		int next = (last + first) >> 1;
+		e = sha1_size_cache[next];
+		int cmp = memcmp(e->sha1, sha1, 20);
+		if (!cmp)
+			return e;
+		if (cmp < 0) {
+			last = next;
+			continue;
+		}
+		first = next+1;
+	}
+	/* not found */
+	if (size == UINT_MAX)
+		return NULL;
+	/* insert to make it at "first" */
+	if (sha1_size_cache_alloc <= sha1_size_cache_nr) {
+		sha1_size_cache_alloc = alloc_nr(sha1_size_cache_alloc);
+		sha1_size_cache = xrealloc(sha1_size_cache,
+					   sha1_size_cache_alloc *
+					   sizeof(*sha1_size_cache));
+	}
+	sha1_size_cache_nr++;
+	if (first < sha1_size_cache_nr)
+		memmove(sha1_size_cache + first + 1, sha1_size_cache + first,
+			(sha1_size_cache_nr - first - 1) *
+			sizeof(*sha1_size_cache));
+	e = xmalloc(sizeof(struct sha1_size_cache));
+	sha1_size_cache[first] = e;
+	memcpy(e->sha1, sha1, 20);
+	e->size = size;
+	return e;
+}
+
 /*
  * While doing rename detection and pickaxe operation, we may need to
  * grab the data for the blob (or file) for our own in-core comparison.
  * diff_filespec has data and size fields for this purpose.
  */
-int diff_populate_filespec(struct diff_filespec *s)
+int diff_populate_filespec(struct diff_filespec *s, int size_only)
 {
 	int err = 0;
 	if (!DIFF_FILE_VALID(s))
@@ -235,6 +284,9 @@ int diff_populate_filespec(struct diff_f
 	if (S_ISDIR(s->mode))
 		return -1;
 
+	if (!use_size_cache)
+		size_only = 0;
+
 	if (s->data)
 		return err;
 	if (!s->sha1_valid ||
@@ -254,6 +306,8 @@ int diff_populate_filespec(struct diff_f
 		s->size = st.st_size;
 		if (!s->size)
 			goto empty;
+		if (size_only)
+			return 0;
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
 			s->data = xmalloc(s->size);
@@ -273,9 +327,21 @@ int diff_populate_filespec(struct diff_f
 		close(fd);
 	}
 	else {
+		/* We cannot do size only for SHA1 blobs */
 		char type[20];
+		struct sha1_size_cache *e;
+
+		if (size_only) {
+			e = locate_size_cache(s->sha1, UINT_MAX);
+			if (e) {
+				s->size = e->size;
+				return 0;
+			}
+		}
 		s->data = read_sha1_file(s->sha1, type, &s->size);
 		s->should_free = 1;
+		if (s->data && size_only)
+			locate_size_cache(s->sha1, s->size);
 	}
 	return 0;
 }
@@ -361,7 +427,7 @@ static void prepare_temp_file(const char
 		return;
 	}
 	else {
-		if (diff_populate_filespec(one))
+		if (diff_populate_filespec(one, 0))
 			die("cannot read data blob for %s", one->path);
 		prep_temp_blob(temp, one->data, one->size,
 			       one->sha1, one->mode);
@@ -496,6 +562,19 @@ void diff_setup(int flags)
 {
 	if (flags & DIFF_SETUP_REVERSE)
 		reverse_diff = 1;
+	if (flags & DIFF_SETUP_USE_CACHE) {
+		if (!active_cache)
+			/* read-cache does not die even when it fails
+			 * so it is safe for us to do this here.  Also
+			 * it does not smudge active_cache or active_nr
+			 * when it fails, so we do not have to worry about
+			 * cleaning it up oufselves either.
+			 */
+			read_cache();
+	}
+	if (flags & DIFF_SETUP_USE_SIZE_CACHE)
+		use_size_cache = 1;
+	
 }
 
 struct diff_queue_struct diff_queued_diff;
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -29,6 +29,8 @@ extern void diff_unmerge(const char *pat
 extern int diff_scoreopt_parse(const char *opt);
 
 #define DIFF_SETUP_REVERSE      	1
+#define DIFF_SETUP_USE_CACHE		2
+#define DIFF_SETUP_USE_SIZE_CACHE	4
 extern void diff_setup(int flags);
 
 #define DIFF_DETECT_RENAME	1
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -11,7 +11,7 @@ static int contains(struct diff_filespec
 {
 	unsigned long offset, sz;
 	const char *data;
-	if (diff_populate_filespec(one))
+	if (diff_populate_filespec(one, 0))
 		return 0;
 	sz = one->size;
 	data = one->data;
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -99,8 +99,11 @@ static int is_exact_match(struct diff_fi
 	if (src->sha1_valid && dst->sha1_valid &&
 	    !memcmp(src->sha1, dst->sha1, 20))
 		return 1;
-	if (diff_populate_filespec(src) || diff_populate_filespec(dst))
-		/* this is an error but will be caught downstream */
+	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
+		return 0;
+	if (src->size != dst->size)
+		return 0;
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
 		return 0;
 	if (src->size == dst->size &&
 	    !memcmp(src->data, dst->data, src->size))
@@ -125,9 +128,11 @@ static int estimate_similarity(struct di
 	 * dst, and then some edit has been applied to dst.
 	 *
 	 * Compare them and return how similar they are, representing
-	 * the score as an integer between 0 and 10000, except
-	 * where they match exactly it is considered better than anything
-	 * else.
+	 * the score as an integer between 0 and MAX_SCORE.
+	 *
+	 * When there is an exact match, it is considered a better
+	 * match than anything else; the destination does not even
+	 * call into this function in that case.
 	 */
 	void *delta;
 	unsigned long delta_size, base_size;
@@ -147,6 +152,7 @@ static int estimate_similarity(struct di
 	/* We would not consider edits that change the file size so
 	 * drastically.  delta_size must be smaller than
 	 * (MAX_SCORE-minimum_score)/MAX_SCORE * min(src->size, dst->size).
+	 *
 	 * Note that base_size == 0 case is handled here already
 	 * and the final score computation below would not have a
 	 * divide-by-zero issue.
@@ -154,6 +160,9 @@ static int estimate_similarity(struct di
 	if (base_size * (MAX_SCORE-minimum_score) < delta_size * MAX_SCORE)
 		return 0;
 
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
+		return 0; /* error but caught downstream */
+
 	delta = diff_delta(src->data, src->size,
 			   dst->data, dst->size,
 			   &delta_size);
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -33,7 +33,7 @@ extern struct diff_filespec *alloc_files
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
 			  unsigned short);
 
-extern int diff_populate_filespec(struct diff_filespec *);
+extern int diff_populate_filespec(struct diff_filespec *, int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 
 struct diff_filepair {
------------------------------------------------

Re: [PATCH 00/12] Diff updates

[Junio C Hamano]

I've said "I am done with diff" twice on this list already.
Aside from unevitable bug reports ;-) I think this time I am
done.  At least the major ones I wanted to do.

Except one thing.  What do you think about the current behaviour
of "diff-cache -p (uncached") in a work tree which was freshly
checked-out, unmodified but you "touch"ed some files to make
them stat-dirty?

I think ancient diff-cache did not report those files, but with
the new "diff --git" headers it will show the "diff --git"
header mentioning those files followed by no content nor mode
changes.  Admittedly this matches the diff-raw output behaviour
more closely, but I find it a bit distracting.  Do you care
about cleaning this up?

[PATCH] Do not show empty diff in diff-cache uncached.

[Junio C Hamano]

Pre- "diff --git" built-in diff did not add any extended header
on its own, so it did not show anything for unmodified but
stat-dirty file from diff-cache command without --cached flag.

Recent diff-cache produces "diff --git" header internally before
calling the "diff" command, which results in an empty diff for
such a file, cluttering the output.  This patch fixes this.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c            |   26 ++++++++++++++++++++++++++
diffcore-rename.c |   17 -----------------
diffcore.h        |    2 ++
3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -684,6 +684,23 @@ int diff_unmodified_pair(struct diff_fil
 	return 0;
 }
 
+int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst)
+{
+	if (src->sha1_valid && dst->sha1_valid &&
+	    !memcmp(src->sha1, dst->sha1, 20))
+		return 1;
+	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
+		return 0;
+	if (src->size != dst->size)
+		return 0;
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
+		return 0;
+	if (src->size == dst->size &&
+	    !memcmp(src->data, dst->data, src->size))
+		return 1;
+	return 0;
+}
+
 static void diff_flush_patch(struct diff_filepair *p)
 {
 	const char *name, *other;
@@ -692,6 +709,15 @@ static void diff_flush_patch(struct diff
 	if (diff_unmodified_pair(p))
 		return;
 
+	/* When diff-raw would have said "look at the filesystem", we
+	 * need to see if the two are the same and if so not to emit
+	 * anything at all.  Avoid is_exact_match() comparison when it
+	 * does not matter.
+	 */
+	if ((DIFF_FILE_VALID(p->two) && !p->two->sha1_valid) &&
+	    is_exact_match(p->one, p->two))
+		return;
+
 	name = p->one->path;
 	other = (strcmp(name, p->two->path) ? p->two->path : NULL);
 	if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -94,23 +94,6 @@ static struct diff_rename_src *register_
 	return &(rename_src[first]);
 }
 
-static int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst)
-{
-	if (src->sha1_valid && dst->sha1_valid &&
-	    !memcmp(src->sha1, dst->sha1, 20))
-		return 1;
-	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
-		return 0;
-	if (src->size != dst->size)
-		return 0;
-	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
-		return 0;
-	if (src->size == dst->size &&
-	    !memcmp(src->data, dst->data, src->size))
-		return 1;
-	return 0;
-}
-
 struct diff_score {
 	int src; /* index in rename_src */
 	int dst; /* index in rename_dst */
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -29,6 +29,8 @@ struct diff_filespec {
 	unsigned should_munmap : 1; /* data should be munmap()'ed */
 };
 
+extern int is_exact_match(struct diff_filespec *, struct diff_filespec *);
+
 extern struct diff_filespec *alloc_filespec(const char *);
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
 			  unsigned short);
------------------------------------------------

[PATCH] Diff: two fixes.

[Junio C Hamano]

The count-delta routine sometimes overcounted the copied source
material which resulted in unsigned int wraparound.

The latest diff-cache (uncached) fix to eliminate empty diffs
from the output revealed that is_exact_match() was not careful
enough, which resulted in a sanity check routine triggering when
a file is added to an index.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

count-delta.c |    3 +++
diff.c        |    3 +++
2 files changed, 6 insertions(+)

diff --git a/count-delta.c b/count-delta.c
--- a/count-delta.c
+++ b/count-delta.c
@@ -88,5 +88,8 @@ unsigned long count_delta(void *delta_bu
 	/* delete size is what was _not_ copied from source.
 	 * edit size is that and literal additions.
 	 */
+	if (src_size + added_literal < copied_from_source)
+		/* we ended up overcounting and underflowed */
+		return 0;
 	return (src_size - copied_from_source) + added_literal;
 }
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -689,6 +689,9 @@ int is_exact_match(struct diff_filespec 
 	if (src->sha1_valid && dst->sha1_valid &&
 	    !memcmp(src->sha1, dst->sha1, 20))
 		return 1;
+	/* if either is invalid they cannot match */
+	if (!DIFF_FILE_VALID(src) || !DIFF_FILE_VALID(dst))
+		return 0;
 	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
 		return 0;
 	if (src->size != dst->size)
------------------------------------------------

[PATCH] diff-helper: fix R/C score parsing under -z flag.

[Junio C Hamano]

The score number that follows R/C status letter was parsed but
the parse pointer was not updated, causing the entire line to
become unrecognized.  This patch fixes this problem.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-helper.c |   17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -80,17 +80,16 @@ int main(int ac, const char **av) {
 			if (!strchr("MCRNDU", status))
 				break;
 			two_paths = score = 0;
-			if (status == 'R' || status == 'C') {
+			if (status == 'R' || status == 'C')
 				two_paths = 1;
-				sscanf(cp, "%d", &score);
-				if (line_termination) {
-					cp = strchr(cp,
-						    inter_name_termination);
-					if (!cp)
-						break;
-				}
-			}
 
+			/* pick up score if exists */
+			if (sscanf(cp, "%d", &score) != 1)
+				score = 0;
+			cp = strchr(cp,
+				    inter_name_termination);
+			if (!cp)
+				break;
 			if (*cp++ != inter_name_termination)
 				break;
 
------------------------------------------------

[PATCH] diff-cache: diff-patch (-p) format fixes.

[Junio C Hamano]

This fixes two more bugs.

 - When diff-cache is run with -R and without --cached, a
   stat-dirty but otherwise unmodified file still produced an
   empty "diff --git" header (earlier fix was only for the case
   without -R).

 - When diff-cache is run without --cached, an unmodifed file
   that has different mode bits from the tree incorrectly picked
   up the mode bits from the filesystem, when it should have
   used what was recorded in the tree object.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c |   26 ++++++++++++++++++++++----
1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -183,6 +183,9 @@ void fill_filespec(struct diff_filespec 
  * Given a name and sha1 pair, if the dircache tells us the file in
  * the work tree has that object contents, return true, so that
  * prepare_temp_file() does not have to inflate and extract.
+ *
+ * NOTE: this function does not use the mode bits, so diff_filespec
+ * users must be careful about mode handling!
  */
 static int work_tree_matches(const char *name, const unsigned char *sha1)
 {
@@ -394,6 +397,10 @@ static void prepare_temp_file(const char
 
 	if (!one->sha1_valid ||
 	    work_tree_matches(name, one->sha1)) {
+		/* NOTE: we only say the matching file has the same
+		 * contents.  It may have a different mode, in which
+		 * case we should not pick it up from the filesystem!
+		 */
 		struct stat st;
 		if (lstat(name, &st) < 0) {
 			if (errno == ENOENT)
@@ -421,8 +428,15 @@ static void prepare_temp_file(const char
 				strcpy(temp->hex, sha1_to_hex(null_sha1));
 			else
 				strcpy(temp->hex, sha1_to_hex(one->sha1));
-			sprintf(temp->mode, "%06o",
-				S_IFREG |ce_permissions(st.st_mode));
+			/* even though we borrow the contents from the
+			 * work tree, we want our mode if we are not told
+			 * to look at the filesystem.
+			 */
+			if (one->sha1_valid)
+				sprintf(temp->mode, "%06o", one->mode);
+			else
+				sprintf(temp->mode, "%06o",
+					S_IFREG | ce_permissions(st.st_mode));
 		}
 		return;
 	}
@@ -716,9 +730,13 @@ static void diff_flush_patch(struct diff
 	 * need to see if the two are the same and if so not to emit
 	 * anything at all.  Avoid is_exact_match() comparison when it
 	 * does not matter.
+	 * Notice that "look at the filesystem" can happen on p->one
+	 * if we are operating under reverse-diff option.
 	 */
-	if ((DIFF_FILE_VALID(p->two) && !p->two->sha1_valid) &&
-	    is_exact_match(p->one, p->two))
+	if ( ((DIFF_FILE_VALID(p->one) && !p->one->sha1_valid) ||
+	      (DIFF_FILE_VALID(p->two) && !p->two->sha1_valid)) &&
+	     is_exact_match(p->one, p->two) &&
+	     (p->one->mode == p->two->mode) )
 		return;
 
 	name = p->one->path;
------------------------------------------------

[PATCH] diff: code clean-up.

[Junio C Hamano]

 - The previous fix for the "diff-cache -p mode bits problem"
   had an unnecessary conditional statement not to use the mode
   that came from the calling program when we get 0{40} SHA1.
   This was totally unnecessary, since the caller gives the
   valid mode bits even when calling us with 0{40} SHA1.
   This mistake did not break anything, since the mode bits we
   got ourselves were the same one taken from the filesystem,
   but it was doing unnecessary work.  This has been fixed.

 - DIFF_PAIR_RENAME() macro is introduced to distinguish a filepair
   that is a rename/copy (the definition of which is src and dst
   are different paths, of course).  This removes the hack used
   in the record_rename_pair() to always put some non-zero value
   in the score field.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c            |   17 +++++++----------
diffcore-rename.c |    2 +-
diffcore.h        |    6 +++---
3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -429,14 +429,11 @@ static void prepare_temp_file(const char
 			else
 				strcpy(temp->hex, sha1_to_hex(one->sha1));
 			/* even though we borrow the contents from the
-			 * work tree, we want our mode if we are not told
-			 * to look at the filesystem.
+			 * work tree, we always want our mode.  mode is
+			 * trustworthy even when !(one->sha1_valid), as
+			 * long as DIFF_FILE_VALID(one).
 			 */
-			if (one->sha1_valid)
-				sprintf(temp->mode, "%06o", one->mode);
-			else
-				sprintf(temp->mode, "%06o",
-					S_IFREG | ce_permissions(st.st_mode));
+			sprintf(temp->mode, "%06o", one->mode);
 		}
 		return;
 	}
@@ -843,7 +840,7 @@ static void diff_resolve_rename_copy(voi
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
 				if (!strcmp(p->one->path, pp->one->path) &&
-				    pp->score) {
+				    DIFF_PAIR_RENAME(pp)) {
 					/* rename/copy are always valid
 					 * so we do not say DIFF_FILE_VALID()
 					 * on pp->one and pp->two.
@@ -862,7 +859,7 @@ static void diff_resolve_rename_copy(voi
 		 * whose both sides are valid and of the same type, i.e.
 		 * either in-place edit or rename/copy edit.
 		 */
-		else if (p->score) {
+		else if (DIFF_PAIR_RENAME(p)) {
 			if (p->source_stays) {
 				p->status = 'C';
 				continue;
@@ -875,7 +872,7 @@ static void diff_resolve_rename_copy(voi
 				pp = q->queue[j];
 				if (strcmp(pp->one->path, p->one->path))
 					continue; /* not us */
-				if (!pp->score)
+				if (!DIFF_PAIR_RENAME(pp))
 					continue; /* not a rename/copy */
 				/* pp is a rename/copy from the same source */
 				p->status = 'C';
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -190,7 +190,7 @@ static void record_rename_pair(struct di
 	fill_filespec(two, dst->sha1, dst->mode);
 
 	dp = diff_queue(renq, one, two);
-	dp->score = score ? : 1; /* make sure it is at least 1 */
+	dp->score = score;
 	dp->source_stays = rename_src[src_index].src_stays;
 	rename_dst[dst_index].pair = dp;
 }
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -42,15 +42,15 @@ extern void diff_free_filespec_data(stru
 struct diff_filepair {
 	struct diff_filespec *one;
 	struct diff_filespec *two;
-	unsigned short int score; /* only valid when one and two are
-				   * different paths
-				   */
+	unsigned short int score;
 	char source_stays; /* all of R/C are copies */
 	char status; /* M C R N D U (see Documentation/diff-format.txt) */
 };
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
 
+#define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path))
+
 #define DIFF_PAIR_TYPE_CHANGED(p) \
 	((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode))
 
------------------------------------------------

Re: [PATCH] Do not show empty diff in diff-cache uncached.

[Linus Torvalds]

On Sat, 28 May 2005, Junio C Hamano wrote:
>
> Pre- "diff --git" built-in diff did not add any extended header
> on its own, so it did not show anything for unmodified but
> stat-dirty file from diff-cache command without --cached flag.
> 
> Recent diff-cache produces "diff --git" header internally before
> calling the "diff" command, which results in an empty diff for
> such a file, cluttering the output.  This patch fixes this.

I'm not sure I like this.

I actually _expect_ that "git-diff-files" will show files that don't match 
the index, even if they happen to have the exact content that the index 
points to. It's how I know whether the index is up-to-date or not.

The exact same thing is trye of git-diff-cache. If something isn't 
up-to-date in the cache, you should show it, since certain operations 
depend on the cache being updated.

		Linus

Re: [PATCH] Do not show empty diff in diff-cache uncached.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> I'm not sure I like this.

LT> I actually _expect_ that "git-diff-files" will show files that don't match 
LT> the index, even if they happen to have the exact content that the index 
LT> points to. It's how I know whether the index is up-to-date or not.

LT> The exact same thing is trye of git-diff-cache. If something isn't 
LT> up-to-date in the cache, you should show it, since certain operations 
LT> depend on the cache being updated.

Let me make sure that we are on the same page.  I am only
talking about '-p' (diff-patch) case in this patch.  The code
should continue to show the 0{40} SHA1 on the right hand side in
diff-raw output.  Do you still want to see the header in that
case to match the diff-raw exactly?

Re: [PATCH] Do not show empty diff in diff-cache uncached.

[Junio C Hamano]

>>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:

JCH> Let me make sure that we are on the same page.

Linus,

    now I've examined what are and what aren't merged in your
tree, I think I know the answer to that question.  The set you
picked look sensible to me.

I'm taking a look at the resulting tree to see if there are
fixes and cleanups that I still think should be merged.  I'll
feed them afresh to you later, if there are any, after rebasing
the patch to the tip of your tree.

Please disregard the patches you have already discarded so far;
this request-to-discard includes -O and -B enhancements.

[PATCH 0/3] Leftover bits after 12-series

[Junio C Hamano]

>>>>> "JCH" == Junio C Hamano <junkio@cox.net> writes:

JCH> I'm taking a look at the resulting tree to see if there are
JCH> fixes and cleanups that I still think should be merged.  I'll
JCH> feed them afresh to you later, if there are any, after rebasing
JCH> the patch to the tip of your tree.

I'll be submitting a set of cleanup and bugfix patches.  The
first one is a real bugfix.  The latter two are cleanups.

    [PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag.
    [PATCH 2/3] diff: consolidate various calls into diffcore.
    [PATCH 3/3] diff: code clean-up.

[PATCH 1/3] diff-helper: Fix R/C score parsing under -z flag.

[Junio C Hamano]

The score number that follow R/C status were parsed but the
parse pointer was not updated, causing the entire line to become
unrecognized.  This patch fixes this problem.

There was a test missing to catch this breakage, which this
commit adds as t4009-diff-rename-4.sh.  The diff-raw tests used
in related t4005-diff-rename-2.sh (the same test without -z) and
t4007-rename-3.sh were stricter than necessarily, despite that
the comment for the tests said otherwise.  This patch also
corrects them.

The documentation is updated to say that the status can
optionally be followed by a number called "score"; it does not
have to stay similarity index forever and there is no reason to
limit it only to C and R.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

Documentation/diff-format.txt |    2 
diff-helper.c                 |   17 +--
t/t4005-diff-rename-2.sh      |   20 ++--
t/t4007-rename-3.sh           |    2 
t/t4009-diff-rename-4.sh      |  196 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 219 insertions(+), 18 deletions(-)
new file (100755): t/t4009-diff-rename-4.sh

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -36,7 +36,7 @@ That is, from the left to the right:
   (6) sha1 for "src"; 0{40} if creation or unmerged.
   (7) a space.
   (8) sha1 for "dst"; 0{40} if creation, unmerged or "look at work tree".
-  (9) status, followed by similarlity index number only for C and R.
+  (9) status, followed by optional "score" number.
  (10) a tab or a NUL when '-z' option is used.
  (11) path for "src"
  (12) a tab or a NUL when '-z' option is used; only exists for C or R.
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -80,17 +80,16 @@ int main(int ac, const char **av) {
 			if (!strchr("MCRNDU", status))
 				break;
 			two_paths = score = 0;
-			if (status == 'R' || status == 'C') {
+			if (status == 'R' || status == 'C')
 				two_paths = 1;
-				sscanf(cp, "%d", &score);
-				if (line_termination) {
-					cp = strchr(cp,
-						    inter_name_termination);
-					if (!cp)
-						break;
-				}
-			}
 
+			/* pick up score if exists */
+			if (sscanf(cp, "%d", &score) != 1)
+				score = 0;
+			cp = strchr(cp,
+				    inter_name_termination);
+			if (!cp)
+				break;
 			if (*cp++ != inter_name_termination)
 				break;
 
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -8,11 +8,17 @@ test_description='Same rename detection 
 '
 . ./test-lib.sh
 
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]*	/ X X \1#	/'
 compare_diff_raw () {
     # When heuristics are improved, the score numbers would change.
     # Ignore them while comparing.
-    sed -e 's/ \([CR]\)[0-9]*	/\1#/' <"$1" >.tmp-1
-    sed -e 's/ \([CR]\)[0-9]*	/\1#/' <"$2" >.tmp-2
+    # Also we do not check SHA1 hash generation in this test, which
+    # is a job for t0000-basic.sh
+
+    sed -e "$sanitize_diff_raw" <"$1" >.tmp-1
+    sed -e "$sanitize_diff_raw" <"$2" >.tmp-2
     diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
 }
 
@@ -109,11 +115,6 @@ test_expect_success \
     'validate output from rename/copy detection (#2)' \
     'compare_diff_raw current expected'
 
-test_expect_success \
-    'prepare work tree once again' \
-    'cat ../../COPYING >COPYING &&
-     git-update-cache --add --remove COPYING COPYING.1'
-
 # make sure diff-helper can grok it.
 mv expected diff-raw
 GIT_DIFF_OPTS=--unified=0 git-diff-helper <diff-raw >current
@@ -151,6 +152,11 @@ test_expect_success \
 # anything about rezrov nor COPYING, since the revised again diff-raw
 # nows how to say Copy.
 
+test_expect_success \
+    'prepare work tree once again' \
+    'cat ../../COPYING >COPYING &&
+     git-update-cache --add --remove COPYING COPYING.1'
+
 git-diff-cache -C $tree >current
 cat >expected <<\EOF
 :100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234	COPYING	COPYING.1
diff --git a/t/t4007-rename-3.sh b/t/t4007-rename-3.sh
--- a/t/t4007-rename-3.sh
+++ b/t/t4007-rename-3.sh
@@ -10,7 +10,7 @@ test_description='Rename interaction wit
 
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
-sanitize_diff_raw='s/ \('"$_x40"'\) \1 \([CR]\)[0-9]*	/ \1 \1 \2#	/'
+sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]*	/ X X \1#	/'
 compare_diff_raw () {
     # When heuristics are improved, the score numbers would change.
     # Ignore them while comparing.
diff --git a/t/t4009-diff-rename-4.sh b/t/t4009-diff-rename-4.sh
new file mode 100755
--- /dev/null
+++ b/t/t4009-diff-rename-4.sh
@@ -0,0 +1,196 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Same rename detection as t4003 but testing diff-raw -z.
+
+'
+. ./test-lib.sh
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sanitize_diff_raw='/^:/s/ '"$_x40"' '"$_x40"' \([A-Z]\)[0-9]*$/ X X \1#/'
+compare_diff_raw () {
+    # When heuristics are improved, the score numbers would change.
+    # Ignore them while comparing.
+    # Also we do not check SHA1 hash generation in this test, which
+    # is a job for t0000-basic.sh
+
+    tr '\0' '\012' <"$1" | sed -e "$sanitize_diff_raw" >.tmp-1
+    tr '\0' '\012' <"$2" | sed -e "$sanitize_diff_raw" >.tmp-2
+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
+compare_diff_patch () {
+    # When heuristics are improved, the score numbers would change.
+    # Ignore them while comparing.
+    sed -e '/^similarity index [0-9]*%$/d' <"$1" >.tmp-1
+    sed -e '/^similarity index [0-9]*%$/d' <"$2" >.tmp-2
+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
+test_expect_success \
+    'prepare reference tree' \
+    'cat ../../COPYING >COPYING &&
+     echo frotz >rezrov &&
+    git-update-cache --add COPYING rezrov &&
+    tree=$(git-write-tree) &&
+    echo $tree'
+
+test_expect_success \
+    'prepare work tree' \
+    'sed -e 's/HOWEVER/However/' <COPYING >COPYING.1 &&
+    sed -e 's/GPL/G.P.L/g' <COPYING >COPYING.2 &&
+    rm -f COPYING &&
+    git-update-cache --add --remove COPYING COPYING.?'
+
+# tree has COPYING and rezrov.  work tree has COPYING.1 and COPYING.2,
+# both are slightly edited, and unchanged rezrov.  We say COPYING.1
+# and COPYING.2 are based on COPYING, and do not say anything about
+# rezrov.
+
+git-diff-cache -z -M $tree >current
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
+COPYING
+COPYING.1
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 R1234
+COPYING
+COPYING.2
+EOF
+
+test_expect_success \
+    'validate output from rename/copy detection (#1)' \
+    'compare_diff_raw current expected'
+
+# make sure diff-helper can grok it.
+mv current diff-raw
+GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING.1
+copy from COPYING
+copy to COPYING.1
+--- a/COPYING
++++ b/COPYING.1
+@@ -6 +6 @@
+- HOWEVER, in order to allow a migration to GPLv3 if that seems like
++ However, in order to allow a migration to GPLv3 if that seems like
+diff --git a/COPYING b/COPYING.2
+rename old COPYING
+rename new COPYING.2
+--- a/COPYING
++++ b/COPYING.2
+@@ -2 +2 @@
+- Note that the only valid version of the GPL as far as this project
++ Note that the only valid version of the G.P.L as far as this project
+@@ -6 +6 @@
+- HOWEVER, in order to allow a migration to GPLv3 if that seems like
++ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
+@@ -12 +12 @@
+-	This file is licensed under the GPL v2, or a later version
++	This file is licensed under the G.P.L v2, or a later version
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#1)' \
+    'compare_diff_patch current expected'
+
+################################################################
+
+test_expect_success \
+    'prepare work tree again' \
+    'mv COPYING.2 COPYING &&
+     git-update-cache --add --remove COPYING COPYING.1 COPYING.2'
+
+# tree has COPYING and rezrov.  work tree has COPYING and COPYING.1,
+# both are slightly edited, and unchanged rezrov.  We say COPYING.1
+# is based on COPYING and COPYING is still there, and do not say anything
+# about rezrov.
+
+git-diff-cache -z -C $tree >current
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 06c67961bbaed34a127f76d261f4c0bf73eda471 M
+COPYING
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
+COPYING
+COPYING.1
+EOF
+
+test_expect_success \
+    'validate output from rename/copy detection (#2)' \
+    'compare_diff_raw current expected'
+
+# make sure diff-helper can grok it.
+mv current diff-raw
+GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING
+--- a/COPYING
++++ b/COPYING
+@@ -2 +2 @@
+- Note that the only valid version of the GPL as far as this project
++ Note that the only valid version of the G.P.L as far as this project
+@@ -6 +6 @@
+- HOWEVER, in order to allow a migration to GPLv3 if that seems like
++ HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
+@@ -12 +12 @@
+-	This file is licensed under the GPL v2, or a later version
++	This file is licensed under the G.P.L v2, or a later version
+diff --git a/COPYING b/COPYING.1
+copy from COPYING
+copy to COPYING.1
+--- a/COPYING
++++ b/COPYING.1
+@@ -6 +6 @@
+- HOWEVER, in order to allow a migration to GPLv3 if that seems like
++ However, in order to allow a migration to GPLv3 if that seems like
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#2)' \
+    'compare_diff_patch current expected'
+
+################################################################
+
+# tree has COPYING and rezrov.  work tree has the same COPYING and
+# copy-edited COPYING.1, and unchanged rezrov.  We should not say
+# anything about rezrov nor COPYING, since the revised again diff-raw
+# nows how to say Copy.
+
+test_expect_success \
+    'prepare work tree once again' \
+    'cat ../../COPYING >COPYING &&
+     git-update-cache --add --remove COPYING COPYING.1'
+
+git-diff-cache -z -C $tree >current
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0603b3238a076dc6c8022aedc6648fa523a17178 C1234
+COPYING
+COPYING.1
+EOF
+
+test_expect_success \
+    'validate output from rename/copy detection (#3)' \
+    'compare_diff_raw current expected'
+
+# make sure diff-helper can grok it.
+mv current diff-raw
+GIT_DIFF_OPTS=--unified=0 git-diff-helper -z <diff-raw >current
+cat >expected <<\EOF
+diff --git a/COPYING b/COPYING.1
+copy from COPYING
+copy to COPYING.1
+--- a/COPYING
++++ b/COPYING.1
+@@ -6 +6 @@
+- HOWEVER, in order to allow a migration to GPLv3 if that seems like
++ However, in order to allow a migration to GPLv3 if that seems like
+EOF
+
+test_expect_success \
+    'validate output from diff-helper (#3)' \
+    'compare_diff_patch current expected'
+
+test_done
------------------------------------------------

[PATCH 2/3] diff: consolidate various calls into diffcore.

[Junio C Hamano]

The three diff-* brothers had a sequence of calls into diffcore
that were almost identical.  Introduce a new diffcore_std()
function that takes all the necessary arguments to consolidate
it.  This will make later enhancements and changing the order of
diffcore application simpler.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff-cache.c |    9 +++------
diff-files.c |    9 +++------
diff-tree.c  |    7 +++----
diff.c       |   12 ++++++++++++
diff.h       |    4 ++++
5 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -240,12 +240,9 @@ int main(int argc, const char **argv)
 		die("unable to read tree object %s", tree_name);
 
 	ret = diff_cache(active_cache, active_nr);
-	if (pathspec)
-		diffcore_pathspec(pathspec);
-	if (detect_rename)
-		diffcore_rename(detect_rename, diff_score_opt);
-	if (pickaxe)
-		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	diffcore_std(pathspec,
+		     detect_rename, diff_score_opt,
+		     pickaxe, pickaxe_opts);
 	diff_flush(diff_output_format, 1);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -116,12 +116,9 @@ int main(int argc, const char **argv)
 		show_modified(oldmode, mode, ce->sha1, null_sha1,
 			      ce->name);
 	}
-	if (1 < argc)
-		diffcore_pathspec(argv + 1);
-	if (detect_rename)
-		diffcore_rename(detect_rename, diff_score_opt);
-	if (pickaxe)
-		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	diffcore_std(argv + 1,
+		     detect_rename, diff_score_opt,
+		     pickaxe, pickaxe_opts);
 	diff_flush(diff_output_format, 1);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -261,10 +261,9 @@ static void call_diff_setup(void)
 
 static int call_diff_flush(void)
 {
-	if (detect_rename)
-		diffcore_rename(detect_rename, diff_score_opt);
-	if (pickaxe)
-		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	diffcore_std(0,
+		     detect_rename, diff_score_opt,
+		     pickaxe, pickaxe_opts);
 	if (diff_queue_is_empty()) {
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -887,6 +887,18 @@ void diff_flush(int diff_output_style, i
 	q->nr = q->alloc = 0;
 }
 
+void diffcore_std(const char **paths,
+		  int detect_rename, int rename_score,
+		  const char *pickaxe, int pickaxe_opts)
+{
+	if (paths && paths[0])
+		diffcore_pathspec(paths);
+	if (detect_rename)
+		diffcore_rename(detect_rename, rename_score);
+	if (pickaxe)
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
+}
+
 void diff_addremove(int addremove, unsigned mode,
 		    const unsigned char *sha1,
 		    const char *base, const char *path)
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -43,6 +43,10 @@ extern void diffcore_pickaxe(const char 
 
 extern void diffcore_pathspec(const char **pathspec);
 
+extern void diffcore_std(const char **paths,
+			 int detect_rename, int rename_score,
+			 const char *pickaxe, int pickaxe_opts);
+
 extern int diff_queue_is_empty(void);
 
 #define DIFF_FORMAT_HUMAN	0

[PATCH 3/3] diff: code clean-up and removal of rename hack.

[Junio C Hamano]

A new macro, DIFF_PAIR_RENAME(), is introduced to distinguish a
filepair that is a rename/copy (the definition of which is src
and dst are different paths, of course).  This removes the hack
used in the record_rename_pair() to always put a non-zero value
in the score field.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c            |    6 +++---
diffcore-rename.c |    2 +-
diffcore.h        |    6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -796,7 +796,7 @@ static void diff_resolve_rename_copy(voi
 			for (j = 0; j < q->nr; j++) {
 				pp = q->queue[j];
 				if (!strcmp(p->one->path, pp->one->path) &&
-				    pp->score) {
+				    DIFF_PAIR_RENAME(pp)) {
 					/* rename/copy are always valid
 					 * so we do not say DIFF_FILE_VALID()
 					 * on pp->one and pp->two.
@@ -815,7 +815,7 @@ static void diff_resolve_rename_copy(voi
 		 * whose both sides are valid and of the same type, i.e.
 		 * either in-place edit or rename/copy edit.
 		 */
-		else if (p->score) {
+		else if (DIFF_PAIR_RENAME(p)) {
 			if (p->source_stays) {
 				p->status = 'C';
 				continue;
@@ -828,7 +828,7 @@ static void diff_resolve_rename_copy(voi
 				pp = q->queue[j];
 				if (strcmp(pp->one->path, p->one->path))
 					continue; /* not us */
-				if (!pp->score)
+				if (!DIFF_PAIR_RENAME(pp))
 					continue; /* not a rename/copy */
 				/* pp is a rename/copy from the same source */
 				p->status = 'C';
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -207,7 +207,7 @@ static void record_rename_pair(struct di
 	fill_filespec(two, dst->sha1, dst->mode);
 
 	dp = diff_queue(renq, one, two);
-	dp->score = score ? : 1; /* make sure it is at least 1 */
+	dp->score = score;
 	dp->source_stays = rename_src[src_index].src_stays;
 	rename_dst[dst_index].pair = dp;
 }
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -39,15 +39,15 @@ extern void diff_free_filespec_data(stru
 struct diff_filepair {
 	struct diff_filespec *one;
 	struct diff_filespec *two;
-	unsigned short int score; /* only valid when one and two are
-				   * different paths
-				   */
+	unsigned short int score;
 	char source_stays; /* all of R/C are copies */
 	char status; /* M C R N D U (see Documentation/diff-format.txt) */
 };
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
 
+#define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path))
+
 #define DIFF_PAIR_TYPE_CHANGED(p) \
 	((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode))
 

Re: [PATCH] Do not show empty diff in diff-cache uncached.

[Linus Torvalds]

On Sun, 29 May 2005, Junio C Hamano wrote:
> 
> Please disregard the patches you have already discarded so far;
> this request-to-discard includes -O and -B enhancements.

I actually like -B, it's just that that patch depended on -O and also came 
with a separate patch that was the reason I liked -B in the first place..

IO, if we have

	/* start out with files "a" and "b" */
	mv b c
	mv a b
	git-update-cache --add --remove a b c
	git-diff-cache -B HEAD

then I htink you're 100% right that it should show up as two renames in
a diffs, and "-B" would catch it. I think that's a great thing.

		Linus

Re: [PATCH] Do not show empty diff in diff-cache uncached.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On Sun, 29 May 2005, Junio C Hamano wrote:
>> 
>> Please disregard the patches you have already discarded so far;
>> this request-to-discard includes -O and -B enhancements.

LT> I actually like -B, it's just that that patch depended on -O and also came 
LT> with a separate patch that was the reason I liked -B in the first place..

Good.  I actually just finished testing the complete reorder of
the patches and about to start the final review before throwing
the bundle at you again.  In the new set, -B comes before -O.

[PATCH 0/4]

[Junio C Hamano]

Linus,

	as promised, I am sending a couple of further cleanup,
and a pair of new diffcore routines.

I am assuming that you would have applied the three clean-up
patches I sent after you rejected some of my 12-piece set,
before you use these four.  They come on top of the previous
three.  Although I do not think they depend on the previous
three, I am letting you know that that was how I tested these
four:

    [PATCH 1/4] diff: further cleanup.
    [PATCH 2/4] diff: fix the culling of unneeded delete record.
    [PATCH 3/4] Add -B flag to diff-* brothers.
    [PATCH 4/4] Add -O<orderfile> option to diff-* brothers.

The third one is the gem of this series.  I think I covered the
basics I can think of in the new test script, but there could
still be cases that rename/copy detector does something
interesting when broken pairs are involved.  Please give it a
good beating before you use it for anything important.  This
being diff routine, it obviously cannot corrupt your data,
though.

The fourth one was what both you and Petr expressed reluctance,
although Thomas was supportive.  I admit it is of "nice to have"
category not "great we need to have it inside" category, but it
is my favorite.

Oh, before I forget, I was wondering if you want me to mark
broken pair in any special way, just line I mark matched
rename/copy pairs.  Something along the lines of:

    diff --git a/foo b/foo
    dissimilarity index 100%
    deleted file mode 100644
    --- a/foo
    +++ /dev/null
    @@ ...
    diff --git a/foo b/foo
    dissimilarity index 100%
    new file mode 100644
    --- /dev/null
    +++ a/foo
    @@ ...

[PATCH 1/4] diff: further cleanup.

[Junio C Hamano]

When preparing data to feed the external diff, we should give
the mode we obtained from the caller, even when we are dealing
with a file with 0{40} SHA1 (i.e. the caller said "look at the
filesystem"), since the mode passed by the caller via
diff_addremove() or diff_change() is always trustworthy.

This is _not_ a bugfix --- the existing code stat() on the file
ifself and does the same computation on st.st_mode to compute
the mode the same way the caller did to give the original mode.
We cannot remove the stat() call from here, but the extra
computation to create the mode value is unnecessary.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 diff.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

9c88ea020142346a56e36cea87e349814675f3f5 (from f96e0e2250f29f5ba2ae06c6b401a83fa1b828b4)
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -421,8 +421,13 @@ static void prepare_temp_file(const char
 				strcpy(temp->hex, sha1_to_hex(null_sha1));
 			else
 				strcpy(temp->hex, sha1_to_hex(one->sha1));
-			sprintf(temp->mode, "%06o",
-				S_IFREG |ce_permissions(st.st_mode));
+			/* Even though we may sometimes borrow the
+			 * contents from the work tree, we always want
+			 * one->mode.  mode is trustworthy even when
+			 * !(one->sha1_valid), as long as
+			 * DIFF_FILE_VALID(one).
+			 */
+			sprintf(temp->mode, "%06o", one->mode);
 		}
 		return;
 	}

[PATCH 2/4] diff: fix the culling of unneeded delete record.

[Junio C Hamano]

The commit 15d061b435a7e3b6bead39df3889f4af78c4b00a

    [PATCH] Fix the way diffcore-rename records unremoved source.

still leaves unneeded delete records in its output stream by
mistake, which was covered up by having an extra check to turn
such a delete into a no-op downstream.  Fix the check in the
diffcore-rename to simplify the output routine.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 diff.c            |   23 ++---------------------
 diffcore-rename.c |   44 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 35 insertions(+), 32 deletions(-)

3fde726e95a828e680c95297f185d6d25fcf853a (from 9c88ea020142346a56e36cea87e349814675f3f5)
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -792,27 +792,8 @@ static void diff_resolve_rename_copy(voi
 			p->status = 'U';
 		else if (!DIFF_FILE_VALID(p->one))
 			p->status = 'N';
-		else if (!DIFF_FILE_VALID(p->two)) {
-			/* Deleted entry may have been picked up by
-			 * another rename-copy entry.  So we scan the
-			 * queue and if we find one that uses us as the
-			 * source we do not say delete for this entry.
-			 */
-			for (j = 0; j < q->nr; j++) {
-				pp = q->queue[j];
-				if (!strcmp(p->one->path, pp->one->path) &&
-				    DIFF_PAIR_RENAME(pp)) {
-					/* rename/copy are always valid
-					 * so we do not say DIFF_FILE_VALID()
-					 * on pp->one and pp->two.
-					 */
-					p->status = 'X';
-					break;
-				}
-			}
-			if (!p->status)
-				p->status = 'D';
-		}
+		else if (!DIFF_FILE_VALID(p->two))
+			p->status = 'D';
 		else if (DIFF_PAIR_TYPE_CHANGED(p))
 			p->status = 'T';
 
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -328,26 +328,48 @@ void diffcore_rename(int detect_rename, 
 	outq.nr = outq.alloc = 0;
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		struct diff_rename_dst *dst = locate_rename_dst(p->two, 0);
 		struct diff_filepair *pair_to_free = NULL;
 
-		if (dst) {
-			/* creation */
-			if (dst->pair) {
-				/* renq has rename/copy to produce
-				 * this file already, so we do not
-				 * emit the creation record in the
-				 * output.
-				 */
+		if (!DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
+			/*
+			 * Creation
+			 *
+			 * We would output this create record if it has
+			 * not been turned into a rename/copy already.
+			 */
+			struct diff_rename_dst *dst =
+				locate_rename_dst(p->two, 0);
+			if (dst && dst->pair) {
 				diff_q(&outq, dst->pair);
 				pair_to_free = p;
 			}
 			else
-				/* no matching rename/copy source, so record
-				 * this as a creation.
+				/* no matching rename/copy source, so
+				 * record this as a creation.
 				 */
 				diff_q(&outq, p);
 		}
+		else if (DIFF_FILE_VALID(p->one) && !DIFF_FILE_VALID(p->two)) {
+			/*
+			 * Deletion
+			 *
+			 * We would output this delete record if renq
+			 * does not have a rename/copy to move
+			 * p->one->path out.
+			 */
+			for (j = 0; j < renq.nr; j++)
+				if (!strcmp(renq.queue[j]->one->path,
+					    p->one->path))
+					break;
+			if (j < renq.nr)
+				/* this path remains */
+				pair_to_free = p;
+
+			if (pair_to_free)
+				;
+			else
+				diff_q(&outq, p);
+		}
 		else if (!diff_unmodified_pair(p))
 			/* all the usual ones need to be kept */
 			diff_q(&outq, p);

[PATCH 3/4] Add -B flag to diff-* brothers.

[Junio C Hamano]

A new diffcore transformation, diffcore-break.c, is introduced.

When the -B flag is given, a patch that represents a complete
rewrite is broken into a deletion followed by a creation.  This
makes it easier to review such a complete rewrite patch.

The -B flag takes the same syntax as the -M and -C flags to
specify the minimum amount of non-source material the resulting
file needs to have to be considered a complete rewrite, and
defaults to 99% if not specified.

As the new test t4008-diff-break-rewrite.sh demonstrates, if a
file is a complete rewrite, it is broken into a delete/create
pair, which can further be subjected to the usual rename
detection if -M or -C is used.  For example, if file0 gets
completely rewritten to make it as if it were rather based on
file1 which itself disappeared, the following happens:

    The original change looks like this:

	file0     --> file0' (quite different from file0)
	file1     --> /dev/null

    After diffcore-break runs, it would become this:

	file0     --> /dev/null
	/dev/null --> file0'
	file1     --> /dev/null

    Then diffcore-rename matches them up:

	file1     --> file0'

The internal score values are finer grained now.  Earlier
maximum of 10000 has been raised to 60000; there is no user
visible changes but there is no reason to waste available bits.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-diff-cache.txt |    5 
 Documentation/git-diff-files.txt |    5 
 Documentation/git-diff-tree.txt  |    5 
 Makefile                         |    3 
 diff-cache.c                     |   11 +-
 diff-files.c                     |    8 +
 diff-tree.c                      |    8 +
 diff.c                           |   21 +++
 diff.h                           |    5 
 diffcore-break.c                 |  127 +++++++++++++++++++++++
 diffcore-rename.c                |   45 ++++++--
 diffcore.h                       |   12 +-
 t/t4008-diff-break-rewrite.sh    |  207 +++++++++++++++++++++++++++++++++++++++
 13 files changed, 433 insertions(+), 29 deletions(-)

96c0ba825059e65743bfce718e54413c4583ff35 (from 3fde726e95a828e680c95297f185d6d25fcf853a)
diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt
--- a/Documentation/git-diff-cache.txt
+++ b/Documentation/git-diff-cache.txt
@@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo
 
 SYNOPSIS
 --------
-'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
+'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
 
 DESCRIPTION
 -----------
@@ -35,6 +35,9 @@ OPTIONS
 -z::
 	\0 line termination on output
 
+-B::
+	Break complete rewrite changes into pairs of delete and create.
+
 -M::
 	Detect renames.
 
diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt
--- a/Documentation/git-diff-files.txt
+++ b/Documentation/git-diff-files.txt
@@ -9,7 +9,7 @@ git-diff-files - Compares files in the w
 
 SYNOPSIS
 --------
-'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...]
+'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...]
 
 DESCRIPTION
 -----------
@@ -29,6 +29,9 @@ OPTIONS
 -R::
 	Output diff in reverse.
 
+-B::
+	Break complete rewrite changes into pairs of delete and create.
+
 -M::
 	Detect renames.
 
diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -9,7 +9,7 @@ git-diff-tree - Compares the content and
 
 SYNOPSIS
 --------
-'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
+'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
 
 DESCRIPTION
 -----------
@@ -33,6 +33,9 @@ OPTIONS
 	generate patch (see section on generating patches).  For
 	git-diff-tree, this flag implies '-r' as well.
 
+-B::
+	Break complete rewrite changes into pairs of delete and create.
+
 -M::
 	Detect renames.
 
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o
 
 LIB_H += diff.h count-delta.h
 LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \
-	count-delta.o
+	count-delta.o diffcore-break.o
 
 LIB_OBJS += gitenv.o
 
@@ -130,6 +130,7 @@ diff.o: $(LIB_H) diffcore.h
 diffcore-rename.o : $(LIB_H) diffcore.h
 diffcore-pathspec.o : $(LIB_H) diffcore.h
 diffcore-pickaxe.o : $(LIB_H) diffcore.h
+diffcore-break.o : $(LIB_H) diffcore.h
 
 test: all
 	$(MAKE) -C t/ all
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -9,6 +9,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static int diff_break_opt = -1;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -188,6 +189,10 @@ int main(int argc, const char **argv)
 			diff_output_format = DIFF_FORMAT_PATCH;
 			continue;
 		}
+		if (!strncmp(arg, "-B", 2)) {
+			diff_break_opt = diff_scoreopt_parse(arg);
+			continue;
+		}
 		if (!strncmp(arg, "-M", 2)) {
 			detect_rename = DIFF_DETECT_RENAME;
 			diff_score_opt = diff_scoreopt_parse(arg);
@@ -240,9 +245,11 @@ int main(int argc, const char **argv)
 		die("unable to read tree object %s", tree_name);
 
 	ret = diff_cache(active_cache, active_nr);
-	diffcore_std(pathspec,
+
+	diffcore_std(pathspec ? : NULL,
 		     detect_rename, diff_score_opt,
-		     pickaxe, pickaxe_opts);
+		     pickaxe, pickaxe_opts,
+		     diff_break_opt);
 	diff_flush(diff_output_format, 1);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -15,6 +15,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static int diff_break_opt = -1;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -57,6 +58,8 @@ int main(int argc, const char **argv)
 			pickaxe = argv[1] + 2;
 		else if (!strcmp(argv[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
+		else if (!strncmp(argv[1], "-B", 2))
+			diff_break_opt = diff_scoreopt_parse(argv[1]);
 		else if (!strncmp(argv[1], "-M", 2)) {
 			diff_score_opt = diff_scoreopt_parse(argv[1]);
 			detect_rename = DIFF_DETECT_RENAME;
@@ -116,9 +119,10 @@ int main(int argc, const char **argv)
 		show_modified(oldmode, mode, ce->sha1, null_sha1,
 			      ce->name);
 	}
-	diffcore_std(argv + 1,
+	diffcore_std((1 < argc) ? argv + 1 : NULL,
 		     detect_rename, diff_score_opt,
-		     pickaxe, pickaxe_opts);
+		     pickaxe, pickaxe_opts,
+		     diff_break_opt);
 	diff_flush(diff_output_format, 1);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -14,6 +14,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static int diff_break_opt = -1;
 static const char *header = NULL;
 static const char *header_prefix = "";
 
@@ -263,7 +264,8 @@ static int call_diff_flush(void)
 {
 	diffcore_std(0,
 		     detect_rename, diff_score_opt,
-		     pickaxe, pickaxe_opts);
+		     pickaxe, pickaxe_opts,
+		     diff_break_opt);
 	if (diff_queue_is_empty()) {
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
@@ -523,6 +525,10 @@ int main(int argc, const char **argv)
 			diff_score_opt = diff_scoreopt_parse(arg);
 			continue;
 		}
+		if (!strncmp(arg, "-B", 2)) {
+			diff_break_opt = diff_scoreopt_parse(arg);
+			continue;
+		}
 		if (!strcmp(arg, "-z")) {
 			diff_output_format = DIFF_FORMAT_MACHINE;
 			continue;
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -603,6 +603,7 @@ struct diff_filepair *diff_queue(struct 
 	dp->two = two;
 	dp->score = 0;
 	dp->source_stays = 0;
+	dp->broken_pair = 0;
 	diff_q(queue, dp);
 	return dp;
 }
@@ -637,6 +638,16 @@ static void diff_flush_raw(struct diff_f
 		sprintf(status, "%c%03d", p->status,
 			(int)(0.5 + p->score * 100.0/MAX_SCORE));
 		break;
+	case 'N': case 'D':
+		two_paths = 0;
+		if (p->score)
+			sprintf(status, "%c%03d", p->status,
+				(int)(0.5 + p->score * 100.0/MAX_SCORE));
+		else {
+			status[0] = p->status;
+			status[1] = 0;
+		}
+		break;
 	default:
 		two_paths = 0;
 		status[0] = p->status;
@@ -760,8 +771,9 @@ void diff_debug_filepair(const struct di
 {
 	diff_debug_filespec(p->one, i, "one");
 	diff_debug_filespec(p->two, i, "two");
-	fprintf(stderr, "score %d, status %c source_stays %d\n",
-		p->score, p->status ? : '?', p->source_stays);
+	fprintf(stderr, "score %d, status %c stays %d broken %d\n",
+		p->score, p->status ? : '?',
+		p->source_stays, p->broken_pair);
 }
 
 void diff_debug_queue(const char *msg, struct diff_queue_struct *q)
@@ -875,10 +887,13 @@ void diff_flush(int diff_output_style, i
 
 void diffcore_std(const char **paths,
 		  int detect_rename, int rename_score,
-		  const char *pickaxe, int pickaxe_opts)
+		  const char *pickaxe, int pickaxe_opts,
+		  int break_opt)
 {
 	if (paths && paths[0])
 		diffcore_pathspec(paths);
+	if (0 <= break_opt)
+		diffcore_break(break_opt);
 	if (detect_rename)
 		diffcore_rename(detect_rename, rename_score);
 	if (pickaxe)
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -43,9 +43,12 @@ extern void diffcore_pickaxe(const char 
 
 extern void diffcore_pathspec(const char **pathspec);
 
+extern void diffcore_break(int);
+
 extern void diffcore_std(const char **paths,
 			 int detect_rename, int rename_score,
-			 const char *pickaxe, int pickaxe_opts);
+			 const char *pickaxe, int pickaxe_opts,
+			 int break_opt);
 
 extern int diff_queue_is_empty(void);
 
diff --git a/diffcore-break.c b/diffcore-break.c
new file mode 100644
--- /dev/null
+++ b/diffcore-break.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
+#include "cache.h"
+#include "diff.h"
+#include "diffcore.h"
+#include "delta.h"
+#include "count-delta.h"
+
+static int very_different(struct diff_filespec *src,
+			  struct diff_filespec *dst,
+			  int min_score)
+{
+	/* dst is recorded as a modification of src.  Are they so
+	 * different that we are better off recording this as a pair
+	 * of delete and create?  min_score is the minimum amount of
+	 * new material that must exist in the dst and not in src for
+	 * the pair to be considered a complete rewrite, and recommended
+	 * to be set to a very high value, 99% or so.
+	 *
+	 * The value we return represents the amount of new material
+	 * that is in dst and not in src.  We return 0 when we do not
+	 * want to get the filepair broken.
+	 */
+	void *delta;
+	unsigned long delta_size, base_size;
+
+	if (!S_ISREG(src->mode) || !S_ISREG(dst->mode))
+		return 0; /* leave symlink rename alone */
+
+	if (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))
+		return 0; /* error but caught downstream */
+
+	delta_size = ((src->size < dst->size) ?
+		      (dst->size - src->size) : (src->size - dst->size));
+
+	/* Notice that we use max of src and dst as the base size,
+	 * unlike rename similarity detection.  This is so that we do
+	 * not mistake a large addition as a complete rewrite.
+	 */
+	base_size = ((src->size < dst->size) ? dst->size : src->size);
+
+	/*
+	 * If file size difference is too big compared to the
+	 * base_size, we declare this a complete rewrite.
+	 */
+	if (base_size * min_score < delta_size * MAX_SCORE)
+		return MAX_SCORE;
+
+	if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
+		return 0; /* error but caught downstream */
+
+	delta = diff_delta(src->data, src->size,
+			   dst->data, dst->size,
+			   &delta_size);
+
+	/* A delta that has a lot of literal additions would have
+	 * big delta_size no matter what else it does.
+	 */
+	if (base_size * min_score < delta_size * MAX_SCORE)
+		return MAX_SCORE;
+
+	/* Estimate the edit size by interpreting delta. */
+	delta_size = count_delta(delta, delta_size);
+	free(delta);
+	if (delta_size == UINT_MAX)
+		return 0; /* error in delta computation */
+
+	if (base_size < delta_size)
+		return MAX_SCORE;
+
+	return delta_size * MAX_SCORE / base_size; 
+}
+
+void diffcore_break(int min_score)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	int i;
+
+	if (!min_score)
+		min_score = DEFAULT_BREAK_SCORE;
+
+	outq.nr = outq.alloc = 0;
+	outq.queue = NULL;
+
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		int score;
+
+		/* We deal only with in-place edit of non directory.
+		 * We do not break anything else.
+		 */
+		if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two) &&
+		    !S_ISDIR(p->one->mode) && !S_ISDIR(p->two->mode) &&
+		    !strcmp(p->one->path, p->two->path)) {
+			score = very_different(p->one, p->two, min_score);
+			if (min_score <= score) {
+				/* Split this into delete and create */
+				struct diff_filespec *null_one, *null_two;
+				struct diff_filepair *dp;
+
+				/* deletion of one */
+				null_one = alloc_filespec(p->one->path);
+				dp = diff_queue(&outq, p->one, null_one);
+				dp->score = score;
+				dp->broken_pair = 1;
+
+				/* creation of two */
+				null_two = alloc_filespec(p->two->path);
+				dp = diff_queue(&outq, null_two, p->two);
+				dp->score = score;
+				dp->broken_pair = 1;
+
+				free(p); /* not diff_free_filepair(), we are
+					  * reusing one and two here.
+					  */
+				continue;
+			}
+		}
+		diff_q(&outq, p);
+	}
+	free(q->queue);
+	*q = outq;
+
+	return;
+}
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -225,8 +225,8 @@ static int score_compare(const void *a_,
 int diff_scoreopt_parse(const char *opt)
 {
 	int diglen, num, scale, i;
-	if (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C'))
-		return -1; /* that is not a -M nor -C option */
+	if (opt[0] != '-' || (opt[1] != 'M' && opt[1] != 'C' && opt[1] != 'B'))
+		return -1; /* that is not a -M, -C nor -B option */
 	diglen = strspn(opt+2, "0123456789");
 	if (diglen == 0 || strlen(opt+2) != diglen)
 		return 0; /* use default */
@@ -249,7 +249,7 @@ void diffcore_rename(int detect_rename, 
 	int num_create, num_src, dst_cnt;
 
 	if (!minimum_score)
-		minimum_score = DEFAULT_MINIMUM_SCORE;
+		minimum_score = DEFAULT_RENAME_SCORE;
 	renq.queue = NULL;
 	renq.nr = renq.alloc = 0;
 
@@ -353,17 +353,36 @@ void diffcore_rename(int detect_rename, 
 			/*
 			 * Deletion
 			 *
-			 * We would output this delete record if renq
-			 * does not have a rename/copy to move
-			 * p->one->path out.
+			 * We would output this delete record if:
+			 *
+			 * (1) this is a broken delete and the counterpart
+			 *     broken create remains in the output; or
+			 * (2) this is not a broken delete, and renq does
+			 *     not have a rename/copy to move p->one->path
+			 *     out.
+			 *
+			 * Otherwise, the counterpart broken create
+			 * has been turned into a rename-edit; or
+			 * delete did not have a matching create to
+			 * begin with.
 			 */
-			for (j = 0; j < renq.nr; j++)
-				if (!strcmp(renq.queue[j]->one->path,
-					    p->one->path))
-					break;
-			if (j < renq.nr)
-				/* this path remains */
-				pair_to_free = p;
+			if (DIFF_PAIR_BROKEN(p)) {
+				/* broken delete */
+				struct diff_rename_dst *dst =
+					locate_rename_dst(p->one, 0);
+				if (dst && dst->pair)
+					/* counterpart is now rename/copy */
+					pair_to_free = p;
+			}
+			else {
+				for (j = 0; j < renq.nr; j++)
+					if (!strcmp(renq.queue[j]->one->path,
+						    p->one->path))
+						break;
+				if (j < renq.nr)
+					/* this path remains */
+					pair_to_free = p;
+			}
 
 			if (pair_to_free)
 				;
diff --git a/diffcore.h b/diffcore.h
--- a/diffcore.h
+++ b/diffcore.h
@@ -8,8 +8,9 @@
  * (e.g. diffcore-rename, diffcore-pickaxe).  Never include this header
  * in anything else.
  */
-#define MAX_SCORE 10000
-#define DEFAULT_MINIMUM_SCORE 5000
+#define MAX_SCORE 60000
+#define DEFAULT_RENAME_SCORE 30000 /* rename/copy similarity minimum (50%) */
+#define DEFAULT_BREAK_SCORE  59400 /* minimum for break to happen (99%)*/
 
 #define RENAME_DST_MATCHED 01
 
@@ -40,14 +41,19 @@ struct diff_filepair {
 	struct diff_filespec *one;
 	struct diff_filespec *two;
 	unsigned short int score;
-	char source_stays; /* all of R/C are copies */
 	char status; /* M C R N D U (see Documentation/diff-format.txt) */
+	unsigned source_stays : 1; /* all of R/C are copies */
+	unsigned broken_pair : 1;
 };
 #define DIFF_PAIR_UNMERGED(p) \
 	(!DIFF_FILE_VALID((p)->one) && !DIFF_FILE_VALID((p)->two))
 
 #define DIFF_PAIR_RENAME(p) (strcmp((p)->one->path, (p)->two->path))
 
+#define DIFF_PAIR_BROKEN(p) \
+	( (!DIFF_FILE_VALID((p)->one) != !DIFF_FILE_VALID((p)->two)) && \
+	  ((p)->broken_pair != 0) )
+
 #define DIFF_PAIR_TYPE_CHANGED(p) \
 	((S_IFMT & (p)->one->mode) != (S_IFMT & (p)->two->mode))
 
diff --git a/t/t4008-diff-break-rewrite.sh b/t/t4008-diff-break-rewrite.sh
new file mode 100755
--- /dev/null
+++ b/t/t4008-diff-break-rewrite.sh
@@ -0,0 +1,207 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Break and then rename
+
+We have two very different files, file0 and file1, registered in a tree.
+
+We update file1 so drastically that it is more similar to file0, and
+then remove file0.  With -B, changes to file1 should be broken into
+separate delete and create, resulting in removal of file0, removal of
+original file1 and creation of completely rewritten file1.
+
+Further, with -B and -M together, these three modifications should
+turn into rename-edit of file0 into file1.
+
+Starting from the same two files in the tree, we swap file0 and file1.
+With -B, this should be detected as two complete rewrites, resulting in
+four changes in total.
+
+Further, with -B and -M together, these should turn into two renames.
+'
+. ./test-lib.sh
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+sanitize_diff_raw='s/ '"$_x40"' '"$_x40"' \([CDNR]\)[0-9]*	/ X X \1#	/'
+compare_diff_raw () {
+    # When heuristics are improved, the score numbers would change.
+    # Ignore them while comparing.
+    # Also we do not check SHA1 hash generation in this test, which
+    # is a job for t0000-basic.sh
+
+    sed -e "$sanitize_diff_raw" <"$1" >.tmp-1
+    sed -e "$sanitize_diff_raw" <"$2" >.tmp-2
+    diff -u .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2
+}
+
+test_expect_success \
+    setup \
+    'cat ../../README >file0 &&
+     cat ../../COPYING >file1 &&
+    git-update-cache --add file0 file1 &&
+    tree=$(git-write-tree) &&
+    echo "$tree"'
+
+test_expect_success \
+    'change file1 with copy-edit of file0 and remove file0' \
+    'sed -e "s/git/GIT/" file0 >file1 &&
+     rm -f file0 &&
+    git-update-cache --remove file0 file1'
+
+test_expect_success \
+    'run diff with -B' \
+    'git-diff-cache -B --cached "$tree" >current'
+
+cat >expected <<\EOF
+:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D	file0
+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100	file1
+:000000 100644 0000000000000000000000000000000000000000 11e331465a89c394dc25c780de230043750c1ec8 N100	file1
+EOF
+
+test_expect_success \
+    'validate result of -B (#1)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'run diff with -B and -M' \
+    'git-diff-cache -B -M "$tree" >current'
+
+cat >expected <<\EOF
+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c R100	file0	file1
+EOF
+
+test_expect_success \
+    'validate result of -B -M (#2)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'swap file0 and file1' \
+    'rm -f file0 file1 &&
+     git-read-tree -m $tree &&
+     git-checkout-cache -f -u -a &&
+     mv file0 tmp &&
+     mv file1 file0 &&
+     mv tmp file1 &&
+     git-update-cache file0 file1'
+
+test_expect_success \
+    'run diff with -B' \
+    'git-diff-cache -B "$tree" >current'
+
+cat >expected <<\EOF
+:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D100	file0
+:000000 100644 0000000000000000000000000000000000000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 N100	file0
+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100	file1
+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100	file1
+EOF
+
+test_expect_success \
+    'validate result of -B (#3)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'run diff with -B and -M' \
+    'git-diff-cache -B -M "$tree" >current'
+
+cat >expected <<\EOF
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 6ff87c4664981e4397625791c8ea3bbb5f2279a3 R100	file1	file0
+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 R100	file0	file1
+EOF
+
+test_expect_success \
+    'validate result of -B -M (#4)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'make file0 into something completely different' \
+    'rm -f file0 &&
+     ln -s frotz file0 &&
+     git-update-cache file0 file1'
+
+test_expect_success \
+    'run diff with -B' \
+    'git-diff-cache -B "$tree" >current'
+
+cat >expected <<\EOF
+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T	file0
+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100	file1
+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100	file1
+EOF
+
+test_expect_success \
+    'validate result of -B (#5)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'run diff with -B' \
+    'git-diff-cache -B -M "$tree" >current'
+
+# This should not mistake file0 as the copy source of new file1
+# due to type differences.
+cat >expected <<\EOF
+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T	file0
+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100	file1
+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N100	file1
+EOF
+
+test_expect_success \
+    'validate result of -B -M (#6)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'run diff with -M' \
+    'git-diff-cache -M "$tree" >current'
+
+# This should not mistake file0 as the copy source of new file1
+# due to type differences.
+cat >expected <<\EOF
+:100644 120000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 67be421f88824578857624f7b3dc75e99a8a1481 T	file0
+:100644 100644 6ff87c4664981e4397625791c8ea3bbb5f2279a3 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 M	file1
+EOF
+
+test_expect_success \
+    'validate result of -M (#7)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'file1 edited to look like file0 and file0 rename-edited to file2' \
+    'rm -f file0 file1 &&
+     git-read-tree -m $tree &&
+     git-checkout-cache -f -u -a &&
+     sed -e "s/git/GIT/" file0 >file1 &&
+     sed -e "s/git/GET/" file0 >file2 &&
+     rm -f file0
+     git-update-cache --add --remove file0 file1 file2'
+
+test_expect_success \
+    'run diff with -B' \
+    'git-diff-cache -B "$tree" >current'
+
+cat >expected <<\EOF
+:100644 000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 0000000000000000000000000000000000000000 D	file0
+:100644 000000 6ff87c4664981e4397625791c8ea3bbb5f2279a3 0000000000000000000000000000000000000000 D100	file1
+:000000 100644 0000000000000000000000000000000000000000 08bb2fb671deff4c03a4d4a0a1315dff98d5732c N100	file1
+:000000 100644 0000000000000000000000000000000000000000 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 N	file2
+EOF
+
+test_expect_success \
+    'validate result of -B (#8)' \
+    'compare_diff_raw current expected'
+
+test_expect_success \
+    'run diff with -B -M' \
+    'git-diff-cache -B -M "$tree" >current'
+
+cat >expected <<\EOF
+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 08bb2fb671deff4c03a4d4a0a1315dff98d5732c C095	file0	file1
+:100644 100644 f5deac7be59e7eeab8657fd9ae706fd6a57daed2 59f832e5c8b3f7e486be15ad0cd3e95ba9af8998 R095	file0	file2
+EOF
+
+test_expect_success \
+    'validate result of -B -M (#9)' \
+    'compare_diff_raw current expected'
+
+test_done

[PATCH 4/4] Add -O<orderfile> option to diff-* brothers.

[Junio C Hamano]

A new diffcore filter diffcore-order is introduced.  This takes
a text file each of whose line is a shell glob pattern.  Patches
that match a glob pattern on an earlier line in the file are
output before patches that match a later line, and patches that
do not match any glob pattern are output last.

A typical orderfile for git project probably should look like
this:

    README
    Makefile
    Documentation
    *.h
    *.c

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 Documentation/git-diff-cache.txt |    6 +
 Documentation/git-diff-files.txt |    6 +
 Documentation/git-diff-tree.txt  |    6 +
 Makefile                         |    3 
 diff-cache.c                     |    8 ++
 diff-files.c                     |    6 +
 diff-tree.c                      |    8 ++
 diff.c                           |    5 +
 diff.h                           |    7 +-
 diffcore-order.c                 |  122 +++++++++++++++++++++++++++++++++++++++
 10 files changed, 167 insertions(+), 10 deletions(-)

6575b8d66ea48891aaba3fba3eb8a4fddc596c2c (from 96c0ba825059e65743bfce718e54413c4583ff35)
diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt
--- a/Documentation/git-diff-cache.txt
+++ b/Documentation/git-diff-cache.txt
@@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo
 
 SYNOPSIS
 --------
-'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
+'git-diff-cache' [-p] [-r] [-z] [-m] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
 
 DESCRIPTION
 -----------
@@ -52,6 +52,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -R::
 	Output diff in reverse.
 
diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt
--- a/Documentation/git-diff-files.txt
+++ b/Documentation/git-diff-files.txt
@@ -9,7 +9,7 @@ git-diff-files - Compares files in the w
 
 SYNOPSIS
 --------
-'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...]
+'git-diff-files' [-p] [-q] [-r] [-z] [-B] [-M] [-C] [-R] [-O<orderfile>] [-S<string>] [--pickaxe-all] [<pattern>...]
 
 DESCRIPTION
 -----------
@@ -46,6 +46,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -r::
 	This flag does not mean anything.  It is there only to match
 	git-diff-tree.  Unlike git-diff-tree, git-diff-files always looks
diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -9,7 +9,7 @@ git-diff-tree - Compares the content and
 
 SYNOPSIS
 --------
-'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
+'git-diff-tree' [-p] [-r] [-z] [--stdin] [-B] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
 
 DESCRIPTION
 -----------
@@ -53,6 +53,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -r::
 	recurse
 
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o
 
 LIB_H += diff.h count-delta.h
 LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \
-	count-delta.o diffcore-break.o
+	count-delta.o diffcore-break.o diffcore-order.o
 
 LIB_OBJS += gitenv.o
 
@@ -131,6 +131,7 @@ diffcore-rename.o : $(LIB_H) diffcore.h
 diffcore-pathspec.o : $(LIB_H) diffcore.h
 diffcore-pickaxe.o : $(LIB_H) diffcore.h
 diffcore-break.o : $(LIB_H) diffcore.h
+diffcore-order.o : $(LIB_H) diffcore.h
 
 test: all
 	$(MAKE) -C t/ all
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -10,6 +10,7 @@ static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
+static const char *orderfile = NULL;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -215,6 +216,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "-O", 2)) {
+			orderfile = arg + 2;
+			continue;
+		}
 		if (!strcmp(arg, "--pickaxe-all")) {
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 			continue;
@@ -249,7 +254,8 @@ int main(int argc, const char **argv)
 	diffcore_std(pathspec ? : NULL,
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
-		     diff_break_opt);
+		     diff_break_opt,
+		     orderfile);
 	diff_flush(diff_output_format, 1);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -16,6 +16,7 @@ static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
+static const char *orderfile = NULL;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -56,6 +57,8 @@ int main(int argc, const char **argv)
 			diff_setup_opt |= DIFF_SETUP_REVERSE;
 		else if (!strncmp(argv[1], "-S", 2))
 			pickaxe = argv[1] + 2;
+		else if (!strncmp(argv[1], "-O", 2))
+			orderfile = argv[1] + 2;
 		else if (!strcmp(argv[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else if (!strncmp(argv[1], "-B", 2))
@@ -122,7 +125,8 @@ int main(int argc, const char **argv)
 	diffcore_std((1 < argc) ? argv + 1 : NULL,
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
-		     diff_break_opt);
+		     diff_break_opt,
+		     orderfile);
 	diff_flush(diff_output_format, 1);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -15,6 +15,7 @@ static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
+static const char *orderfile = NULL;
 static const char *header = NULL;
 static const char *header_prefix = "";
 
@@ -265,7 +266,8 @@ static int call_diff_flush(void)
 	diffcore_std(0,
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
-		     diff_break_opt);
+		     diff_break_opt,
+		     orderfile);
 	if (diff_queue_is_empty()) {
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
@@ -511,6 +513,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "-O", 2)) {
+			orderfile = arg + 2;
+			continue;
+		}
 		if (!strcmp(arg, "--pickaxe-all")) {
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 			continue;
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -888,7 +888,8 @@ void diff_flush(int diff_output_style, i
 void diffcore_std(const char **paths,
 		  int detect_rename, int rename_score,
 		  const char *pickaxe, int pickaxe_opts,
-		  int break_opt)
+		  int break_opt,
+		  const char *orderfile)
 {
 	if (paths && paths[0])
 		diffcore_pathspec(paths);
@@ -898,6 +899,8 @@ void diffcore_std(const char **paths,
 		diffcore_rename(detect_rename, rename_score);
 	if (pickaxe)
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	if (orderfile)
+		diffcore_order(orderfile);
 }
 
 void diff_addremove(int addremove, unsigned mode,
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -43,12 +43,15 @@ extern void diffcore_pickaxe(const char 
 
 extern void diffcore_pathspec(const char **pathspec);
 
-extern void diffcore_break(int);
+extern void diffcore_order(const char *orderfile);
+
+extern void diffcore_break(int max_score);
 
 extern void diffcore_std(const char **paths,
 			 int detect_rename, int rename_score,
 			 const char *pickaxe, int pickaxe_opts,
-			 int break_opt);
+			 int break_opt,
+			 const char *orderfile);
 
 extern int diff_queue_is_empty(void);
 
diff --git a/diffcore-order.c b/diffcore-order.c
new file mode 100644
--- /dev/null
+++ b/diffcore-order.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
+#include "cache.h"
+#include "diff.h"
+#include "diffcore.h"
+#include <fnmatch.h>
+
+static char **order;
+static int order_cnt;
+
+static void prepare_order(const char *orderfile)
+{
+	int fd, cnt, pass;
+	void *map;
+	char *cp, *endp;
+	struct stat st;
+
+	if (order)
+		return;
+
+	fd = open(orderfile, O_RDONLY);
+	if (fd < 0)
+		return;
+	if (fstat(fd, &st)) {
+		close(fd);
+		return;
+	}
+	map = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+	close(fd);
+	if (-1 == (int)(long)map)
+		return;
+	endp = map + st.st_size;
+	for (pass = 0; pass < 2; pass++) {
+		cnt = 0;
+		cp = map;
+		while (cp < endp) {
+			char *ep;
+			for (ep = cp; ep < endp && *ep != '\n'; ep++)
+				;
+			/* cp to ep has one line */
+			if (*cp == '\n' || *cp == '#')
+				; /* comment */
+			else if (pass == 0)
+				cnt++;
+			else {
+				if (*ep == '\n') {
+					*ep = 0;
+					order[cnt] = cp;
+				}
+				else {
+					order[cnt] = xmalloc(ep-cp+1);
+					memcpy(order[cnt], cp, ep-cp);
+					order[cnt][ep-cp] = 0;
+				}
+				cnt++;
+			}
+			if (ep < endp)
+				ep++;
+			cp = ep;
+		}
+		if (pass == 0) {
+			order_cnt = cnt;
+			order = xmalloc(sizeof(*order) * cnt);
+		}
+	}
+}
+
+struct pair_order {
+	struct diff_filepair *pair;
+	int orig_order;
+	int order;
+};
+
+static int match_order(const char *path)
+{
+	int i;
+	char p[PATH_MAX];
+
+	for (i = 0; i < order_cnt; i++) {
+		strcpy(p, path);
+		while (p[0]) {
+			char *cp;
+			if (!fnmatch(order[i], p, 0))
+				return i;
+			cp = strrchr(p, '/');
+			if (!cp)
+				break;
+			*cp = 0;
+		}
+	}
+	return order_cnt;
+}
+
+static int compare_pair_order(const void *a_, const void *b_)
+{
+	struct pair_order const *a, *b;
+	a = (struct pair_order const *)a_;
+	b = (struct pair_order const *)b_;
+	if (a->order != b->order)
+		return a->order - b->order;
+	return a->orig_order - b->orig_order;
+}
+
+void diffcore_order(const char *orderfile)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	int i;
+
+	prepare_order(orderfile);
+	for (i = 0; i < q->nr; i++) {
+		o[i].pair = q->queue[i];
+		o[i].orig_order = i;
+		o[i].order = match_order(o[i].pair->two->path);
+	}
+	qsort(o, q->nr, sizeof(*o), compare_pair_order);
+	for (i = 0; i < q->nr; i++)
+		q->queue[i] = o[i].pair;
+	free(o);
+	return;
+}

Re: [PATCH 12/12] Optimize diff-tree -[CM] --stdin

[Yoichi Yuasa]

Hi,

On Fri, 27 May 2005 15:56:38 -0700
Junio C Hamano <junkio@cox.net> wrote:

>

<snip>

> diff --git a/diff.c b/diff.c
> --- a/diff.c
> +++ b/diff.c
> @@ -12,6 +12,7 @@ static const char *diff_opts = "-pu";
>  static unsigned char null_sha1[20] = { 0, };
>  
>  static int reverse_diff;
> +static int use_size_cache;
>  
>  static const char *external_diff(void)
>  {
> @@ -222,12 +223,60 @@ static int work_tree_matches(const char 
>  	return 1;
>  }
>  
> +static struct sha1_size_cache {
> +	unsigned char sha1[20];
> +	unsigned long size;
> +} **sha1_size_cache;
> +static int sha1_size_cache_nr, sha1_size_cache_alloc;
> +
> +static struct sha1_size_cache *locate_size_cache(unsigned char *sha1,
> +						 unsigned long size)
> +{
> +	int first, last;
> +	struct sha1_size_cache *e;
> +
> +	first = 0;
> +	last = sha1_size_cache_nr;
> +	while (last > first) {
> +		int next = (last + first) >> 1;
> +		e = sha1_size_cache[next];
> +		int cmp = memcmp(e->sha1, sha1, 20);

I got build error in diff.c
Please apply the following patch.

Thanks,

Yoichi

Signed-off-by: Yoichi Yuasa <yuasa@hh.iij4u.or.jp>

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -244,9 +244,9 @@ static struct sha1_size_cache *locate_si
 	first = 0;
 	last = sha1_size_cache_nr;
 	while (last > first) {
-		int next = (last + first) >> 1;
+		int cmp, next = (last + first) >> 1;
 		e = sha1_size_cache[next];
-		int cmp = memcmp(e->sha1, sha1, 20);
+		cmp = memcmp(e->sha1, sha1, 20);
 		if (!cmp)
 			return e;
 		if (cmp < 0) {

[PATCH] apply.c: tolerate diff from a dirty but unchanged path

[Junio C Hamano]

I just noticed a problem with empty "diff --git" header a dirty
but otherwise unmodified path spits out.  The problem is related
to an old topic:

    Re: [PATCH] Do not show empty diff in diff-cache uncached.
    >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

    LT> On Sat, 28 May 2005, Junio C Hamano wrote:
    >> Recent diff-cache produces "diff --git" header internally before
    >> calling the "diff" command, which results in an empty diff for
    >> such a file, cluttering the output.  This patch fixes this.

    LT> I actually _expect_ that "git-diff-files" will show
    LT> files that don't match the index, even if they happen to
    LT> have the exact content that the index points to. It's
    LT> how I know whether the index is up-to-date or not.

I now agree with the reasoning and think showing an empty "diff
--git" header is a good thing.  I had something like this, while
fixing a rename/copy bug:

    ...
     #endif

     static void diff_resolve_rename_copy(void)
    diff --git a/diffcore-break.c b/diffcore-break.c
    diff --git a/diffcore-rename.c b/diffcore-rename.c
    --- a/diffcore-rename.c
    +++ b/diffcore-rename.c
    @@ -249,8 +249,12 @@ void diffcore_rename(int detect_rename, 
    ...

I touched diffcore-break.c and decided I should not change it so
I reverted, which is why the empty "diff --git" for that file
shows that the cache is dirty.  The problem is that currently,
"git-apply --stat" seems to barf reading such a patch.

Here is a proposed fix.

------------
apply.c: tolerate diff from a dirty but otherwise unchanged cache.

For such an entry, "diff --git" line without anything else is
output, but the next "diff --git" header is not really treated
as an unrecognized diff.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/apply.c b/apply.c
--- a/apply.c
+++ b/apply.c
@@ -343,7 +343,7 @@ static int gitdiff_dissimilarity(const c
  */
 static int gitdiff_unrecognized(const char *line, struct patch *patch)
 {
-	return -1;
+	return -2;
 }
 
 static char *git_header_name(char *line)
@@ -453,8 +453,12 @@ static int parse_git_header(char *line, 
 			int oplen = strlen(p->str);
 			if (len < oplen || memcmp(p->str, line, oplen))
 				continue;
-			if (p->fn(line + oplen, patch) < 0)
+			switch (p->fn(line + oplen, patch)) {
+			case -1:
 				return offset;
+			case -2:
+				return -1;
+			}
 			break;
 		}
 	}

Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path

[Linus Torvalds]

On Sat, 11 Jun 2005, Junio C Hamano wrote:
> 
> Here is a proposed fix.

This is wrong. It will make us reject all patches that don't end the magic 
sequence of "@@ -", ie it will make us reject pure renames etc.

I'll fix it up,

		Linus

Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path

[Linus Torvalds]

On Sun, 12 Jun 2005, Linus Torvalds wrote:
> 
> I'll fix it up,

One-liner fix checked in: we should ignore all git headers that are just a 
single line. A valid git header is _always_ multiple lines: either you 
have the "---/+++" lines of a diff, or you have the old/new lines of a 
mode or name change.

So the

	if (git_hdr_len < 0)

test of the return value of "parse_git_header()" was changed into

	if (git_hdr_len <= len)

and that cleanly solves the problem.

		Linus

Re: [PATCH] apply.c: tolerate diff from a dirty but unchanged path

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> One-liner fix checked in: we should ignore all git headers that are just a 
LT> single line. A valid git header is _always_ multiple lines: either you 
LT> have the "---/+++" lines of a diff, or you have the old/new lines of a 
LT> mode or name change.

Yup.  I was about to say "it always amazes me" but by now I
should instead say "I am used to see" that your fix to my
crapola is always cleaner, simpler, and shorter.

By the way, just to help you sort the heap of recent patches
from me, here is the list of patches from me not in your tree
that I think you _could_ consider.  Other patches I posted to
the list that have not been merged are mostly earlier attempts
that I consider you have already discarded; there _could_ be
something I am forgetting, but if even I myself forget, they
probably do not matter ;-):

------------
This is a fix for a bug that would embarrass me unless fixed.

    Subject: [PATCH] Fix rename/copy when dealing with temporarily broken...
    Date: Sat, 11 Jun 2005 20:55:20 -0700
    Message-ID: <7vwtp0p6tz.fsf@assigned-by-dhcp.cox.net>

A good test case is to run and compare these two in the GIT
repository.  The latter case is mishandled with unpatched code:

    commit=7ef76925d9c19ef74874e1735e2436e56d0c4897
    git-diff-tree -C $commit
    git-diff-tree -B -C $commit

------------
This one adjusts what the tutorial tells the user to expect to
the reality with the "corrected" -B behaviour we already have in
your tree.  We must have this before 1.0 in order not to confuse
the reader.  Unless you drop -B from git-status-script, that is.

    Subject: [PATCH 5/4] Tutorial update to adjust for -B fix
    Date: Fri, 03 Jun 2005 12:11:07 -0700
    Message-ID: <7vd5r3l0hg.fsf_-_@assigned-by-dhcp.cox.net>

------------
These two are enhancements that would help writing the
ultra-smart three-way merge that would look at not just merge
base and two heads, but the changes in the ancestry chain:

    Message-ID: <7vpsut7k89.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH] diff-tree: --find-copies-harder
    Date: Fri, 10 Jun 2005 18:31:02 -0700

    Message-ID: <7vr7f8p6qu.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH] Add --diff-filter= output restriction to diff-* family.
    Date: Sat, 11 Jun 2005 20:57:13 -0700

I mentioned them in the reply to this message from you:

    Date: Thu, 9 Jun 2005 08:15:52 -0700 (PDT)
    Subject: Re: [PATCH 3/3] read-tree -m 3-way: handle more trivial merges
    Message-ID: <Pine.LNX.4.58.0506090800580.2286@ppc970.osdl.org>

    No, I think this is quite possibly wrong for several reasons.
    ...
    So I just need a little convincing that this is a good idea.

------------
These are reworked "help carrying forward local changes in 2-way
fast forward".

    Message-ID: <7vd5qt7k2d.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH 1/3] Clean up read-tree two-way tests.
    Date: Fri, 10 Jun 2005 18:34:34 -0700

    Message-ID: <7v7jh17jzr.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH 2/3] read-tree --emu23.
    Date: Fri, 10 Jun 2005 18:36:08 -0700

    Message-ID: <7v1x797jx0.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH 3/3] read-tree: fix too strong index requirement #5ALT
    Date: Fri, 10 Jun 2005 18:37:47 -0700

    Message-ID: <7voeadxlvo.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH 4/3] Finish making --emu23 equivalent to pure 2-way merge.
    Date: Sat, 11 Jun 2005 02:50:51 -0700

    Message-ID: <7vfyvpxlqi.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH 5/3] read-tree: loosen too strict index requirements
    Date: Sat, 11 Jun 2005 02:53:57 -0700

The first four implement --emu23, a two-tree fast forward
emulated internally using the three-way mechanism.  This is to
help cases where the user has local changes since the old head
by not refusing to run in many cases straight 2-tree fast
forward would refuse, and letting the user use the usual
merge-cache three-way merge postprocessing machinery to carry
local changes forward instead.

The changes these four introduce do affect 3-way in 3 cases,
which you objected, but this set has smaller impact than my
earlier attempts.  It only resolves 3 extra cases the original
3-way refuses to touch (my earlier one made it to resolve 3
cases that involve removed paths as well, none of which this
round touches, and leaves stages in the resulting index).  The
cases new code internally resolves, instead of refusing to run,
are:

 - only "merged head" created a new path, and the index happened
   to have the same change; we resolve it internally by taking
   the "merged head" and keep the index dirty if it was (#2ALT).

 - only "our head" created a new path, and the cache entry is
   dirty (i.e. user has local changes in the work tree file); we
   resolve it internally by taking "our head" and keep the index
   dirty if it was (#3ALT).

 - both "merged head" and "our head" created a new path
   identically, and the cache entry is dirty; we resolve it
   internally by taking "our head" and keep the index dirty if
   it was (#5ALT).

I think your earlier objection of "closing the door to the
clever merge algorithm" would not apply to these three cases.
Note that --emu23 does not need 3-way code to resolve the #2ALT
case, but needs the other two cases not refused; the patch makes
it resolve #2ALT case only to keep things symmetric with #3ALT
case --- otherwise users of 3-way merge would get confusing (but
still correct) results.

The last one [5/3] deals with the case where a path is only
changed in the merged head in 3-way case, and the index already
happens to have a version from the merged head (probably the
user was playing with a patch floating around in the mailing
list before initiating the merge).  In this case we can make the
3-way merge succeed instead of refusing to run, and that is what
this patch is about.  This does not change the outcome of the
merge, and I think your "closing the door" objection would not
apply to it.

------------
This is to enhance the git-*-script suite.  I do not mind too
much if it stays outside, but I think it would be useful to
other people as well, not just me.

    Message-ID: <7vll5h7k5t.fsf@assigned-by-dhcp.cox.net>
    Subject: [PATCH] Add script for patch submission via e-mail.
    Date: Fri, 10 Jun 2005 18:32:30 -0700

------------
This attempts to split the "too big main() that does it all"
which you hated.  Purely cosmetic at this point but as you
pointed out it is a good discipline to help later maintenance to
have a set of smaller single-task functions.

    Message-ID: <7vu0k56517.fsf_-_@assigned-by-dhcp.cox.net>
    Subject: [PATCH] diff-stages: unuglify the too big main() function.
    Date: Fri, 10 Jun 2005 18:44:36 -0700