From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 26 Aug 2014 18:16:26 +0000 (-0700)
Subject: Merge branch 'jk/diff-tree-t-fix'
X-Git-Tag: v2.1.1~14
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/4109c28e055dba27d73cefb956bea5e611f66ec0?ds=inline;hp=-c

Merge branch 'jk/diff-tree-t-fix'

Fix (rarely used) "git diff-tree -t" regression in 2.0.

* jk/diff-tree-t-fix:
intersect_paths: respect mode in git's tree-sort
---

4109c28e055dba27d73cefb956bea5e611f66ec0
diff --combined combine-diff.c
index f9975d2c2e,4e043060f0..60cb4f81f9
--- a/combine-diff.c
+++ b/combine-diff.c
@@@ -12,6 -12,16 +12,16 @@@
  #include "sha1-array.h"
  #include "revision.h"
  
+ static int compare_paths(const struct combine_diff_path *one,
+ 			  const struct diff_filespec *two)
+ {
+ 	if (!S_ISDIR(one->mode) && !S_ISDIR(two->mode))
+ 		return strcmp(one->path, two->path);
+ 
+ 	return base_name_compare(one->path, strlen(one->path), one->mode,
+ 				 two->path, strlen(two->path), two->mode);
+ }
+ 
  static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
  {
  	struct diff_queue_struct *q = &diff_queued_diff;
@@@ -52,7 -62,7 +62,7 @@@
  	i = 0;
  	while ((p = *tail) != NULL) {
  		cmp = ((i >= q->nr)
- 		       ? -1 : strcmp(p->path, q->queue[i]->two->path));
+ 		       ? -1 : compare_paths(p, q->queue[i]->two));
  
  		if (cmp < 0) {
  			/* p->path not in q->queue[]; drop it */
@@@ -1301,81 -1311,6 +1311,81 @@@ static const char *path_path(void *obj
  	return path->path;
  }
  
 +
 +/* find set of paths that every parent touches */
 +static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,
 +	const struct sha1_array *parents, struct diff_options *opt)
 +{
 +	struct combine_diff_path *paths = NULL;
 +	int i, num_parent = parents->nr;
 +
 +	int output_format = opt->output_format;
 +	const char *orderfile = opt->orderfile;
 +
 +	opt->output_format = DIFF_FORMAT_NO_OUTPUT;
 +	/* tell diff_tree to emit paths in sorted (=tree) order */
 +	opt->orderfile = NULL;
 +
 +	/* D(A,P1...Pn) = D(A,P1) ^ ... ^ D(A,Pn)  (wrt paths) */
 +	for (i = 0; i < num_parent; i++) {
 +		/*
 +		 * show stat against the first parent even when doing
 +		 * combined diff.
 +		 */
 +		int stat_opt = (output_format &
 +				(DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT));
 +		if (i == 0 && stat_opt)
 +			opt->output_format = stat_opt;
 +		else
 +			opt->output_format = DIFF_FORMAT_NO_OUTPUT;
 +		diff_tree_sha1(parents->sha1[i], sha1, "", opt);
 +		diffcore_std(opt);
 +		paths = intersect_paths(paths, i, num_parent);
 +
 +		/* if showing diff, show it in requested order */
 +		if (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&
 +		    orderfile) {
 +			diffcore_order(orderfile);
 +		}
 +
 +		diff_flush(opt);
 +	}
 +
 +	opt->output_format = output_format;
 +	opt->orderfile = orderfile;
 +	return paths;
 +}
 +
 +
 +/*
 + * find set of paths that everybody touches, assuming diff is run without
 + * rename/copy detection, etc, comparing all trees simultaneously (= faster).
 + */
 +static struct combine_diff_path *find_paths_multitree(
 +	const unsigned char *sha1, const struct sha1_array *parents,
 +	struct diff_options *opt)
 +{
 +	int i, nparent = parents->nr;
 +	const unsigned char **parents_sha1;
 +	struct combine_diff_path paths_head;
 +	struct strbuf base;
 +
 +	parents_sha1 = xmalloc(nparent * sizeof(parents_sha1[0]));
 +	for (i = 0; i < nparent; i++)
 +		parents_sha1[i] = parents->sha1[i];
 +
 +	/* fake list head, so worker can assume it is non-NULL */
 +	paths_head.next = NULL;
 +
 +	strbuf_init(&base, PATH_MAX);
 +	diff_tree_paths(&paths_head, sha1, parents_sha1, nparent, &base, opt);
 +
 +	strbuf_release(&base);
 +	free(parents_sha1);
 +	return paths_head.next;
 +}
 +
 +
  void diff_tree_combined(const unsigned char *sha1,
  			const struct sha1_array *parents,
  			int dense,
@@@ -1383,83 -1318,49 +1393,83 @@@
  {
  	struct diff_options *opt = &rev->diffopt;
  	struct diff_options diffopts;
 -	struct combine_diff_path *p, *paths = NULL;
 +	struct combine_diff_path *p, *paths;
  	int i, num_paths, needsep, show_log_first, num_parent = parents->nr;
 +	int need_generic_pathscan;
 +
 +	/* nothing to do, if no parents */
 +	if (!num_parent)
 +		return;
 +
 +	show_log_first = !!rev->loginfo && !rev->no_commit_id;
 +	needsep = 0;
 +	if (show_log_first) {
 +		show_log(rev);
 +
 +		if (rev->verbose_header && opt->output_format &&
 +		    opt->output_format != DIFF_FORMAT_NO_OUTPUT)
 +			printf("%s%c", diff_line_prefix(opt),
 +			       opt->line_termination);
 +	}
  
  	diffopts = *opt;
  	copy_pathspec(&diffopts.pathspec, &opt->pathspec);
 -	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
  	DIFF_OPT_SET(&diffopts, RECURSIVE);
  	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
 -	/* tell diff_tree to emit paths in sorted (=tree) order */
 -	diffopts.orderfile = NULL;
  
 -	show_log_first = !!rev->loginfo && !rev->no_commit_id;
 -	needsep = 0;
 -	/* find set of paths that everybody touches */
 -	for (i = 0; i < num_parent; i++) {
 -		/* show stat against the first parent even
 +	/* find set of paths that everybody touches
 +	 *
 +	 * NOTE
 +	 *
 +	 * Diffcore transformations are bound to diff_filespec and logic
 +	 * comparing two entries - i.e. they do not apply directly to combine
 +	 * diff.
 +	 *
 +	 * If some of such transformations is requested - we launch generic
 +	 * path scanning, which works significantly slower compared to
 +	 * simultaneous all-trees-in-one-go scan in find_paths_multitree().
 +	 *
 +	 * TODO some of the filters could be ported to work on
 +	 * combine_diff_paths - i.e. all functionality that skips paths, so in
 +	 * theory, we could end up having only multitree path scanning.
 +	 *
 +	 * NOTE please keep this semantically in sync with diffcore_std()
 +	 */
 +	need_generic_pathscan = opt->skip_stat_unmatch	||
 +			DIFF_OPT_TST(opt, FOLLOW_RENAMES)	||
 +			opt->break_opt != -1	||
 +			opt->detect_rename	||
 +			opt->pickaxe		||
 +			opt->filter;
 +
 +
 +	if (need_generic_pathscan) {
 +		/*
 +		 * NOTE generic case also handles --stat, as it computes
 +		 * diff(sha1,parent_i) for all i to do the job, specifically
 +		 * for parent0.
 +		 */
 +		paths = find_paths_generic(sha1, parents, &diffopts);
 +	}
 +	else {
 +		int stat_opt;
 +		paths = find_paths_multitree(sha1, parents, &diffopts);
 +
 +		/*
 +		 * show stat against the first parent even
  		 * when doing combined diff.
  		 */
 -		int stat_opt = (opt->output_format &
 +		stat_opt = (opt->output_format &
  				(DIFF_FORMAT_NUMSTAT|DIFF_FORMAT_DIFFSTAT));
 -		if (i == 0 && stat_opt)
 +		if (stat_opt) {
  			diffopts.output_format = stat_opt;
 -		else
 -			diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
 -		diff_tree_sha1(parents->sha1[i], sha1, "", &diffopts);
 -		diffcore_std(&diffopts);
 -		paths = intersect_paths(paths, i, num_parent);
  
 -		if (show_log_first && i == 0) {
 -			show_log(rev);
 -
 -			if (rev->verbose_header && opt->output_format)
 -				printf("%s%c", diff_line_prefix(opt),
 -				       opt->line_termination);
 +			diff_tree_sha1(parents->sha1[0], sha1, "", &diffopts);
 +			diffcore_std(&diffopts);
 +			if (opt->orderfile)
 +				diffcore_order(opt->orderfile);
 +			diff_flush(&diffopts);
  		}
 -
 -		/* if showing diff, show it in requested order */
 -		if (diffopts.output_format != DIFF_FORMAT_NO_OUTPUT &&
 -		    opt->orderfile) {
 -			diffcore_order(opt->orderfile);
 -		}
 -
 -		diff_flush(&diffopts);
  	}
  
  	/* find out number of surviving paths */
diff --combined t/t4038-diff-combined.sh
index 41913c3aa3,c5eacb80f6..0b4f7dfdc6
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@@ -94,7 -94,7 +94,7 @@@ test_expect_success 'setup for --cc --r
  	blob=$(echo file | git hash-object --stdin -w) &&
  	base_tree=$(echo "100644 blob $blob	file" | git mktree) &&
  	trees= &&
 -	for i in `test_seq 1 40`
 +	for i in $(test_seq 1 40)
  	do
  		blob=$(echo file$i | git hash-object --stdin -w) &&
  		trees="$trees$(echo "100644 blob $blob	file" | git mktree)$LF"
@@@ -401,4 -401,38 +401,38 @@@ test_expect_success 'combine diff missi
  	compare_diff_patch expected actual
  '
  
+ test_expect_success 'combine diff gets tree sorting right' '
+ 	# create a directory and a file that sort differently in trees
+ 	# versus byte-wise (implied "/" sorts after ".")
+ 	git checkout -f master &&
+ 	mkdir foo &&
+ 	echo base >foo/one &&
+ 	echo base >foo/two &&
+ 	echo base >foo.ext &&
+ 	git add foo foo.ext &&
+ 	git commit -m base &&
+ 
+ 	# one side modifies a file in the directory, along with the root
+ 	# file...
+ 	echo master >foo/one &&
+ 	echo master >foo.ext &&
+ 	git commit -a -m master &&
+ 
+ 	# the other side modifies the other file in the directory
+ 	git checkout -b other HEAD^ &&
+ 	echo other >foo/two &&
+ 	git commit -a -m other &&
+ 
+ 	# And now we merge. The files in the subdirectory will resolve cleanly,
+ 	# meaning that a combined diff will not find them interesting. But it
+ 	# will find the tree itself interesting, because it had to be merged.
+ 	git checkout master &&
+ 	git merge other &&
+ 
+ 	printf "MM\tfoo\n" >expect &&
+ 	git diff-tree -c --name-status -t HEAD >actual.tmp &&
+ 	sed 1d <actual.tmp >actual &&
+ 	test_cmp expect actual
+ '
+ 
  test_done