From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 28 Apr 2011 21:11:35 +0000 (-0700)
Subject: Merge branch 'en/merge-recursive'
X-Git-Tag: v1.7.6-rc0~137
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/ac9666f84a597973ef29dc7f0f7edcb3e1a3aa45?hp=-c

Merge branch 'en/merge-recursive'

* en/merge-recursive:
merge-recursive: tweak magic band-aid
merge-recursive: When we detect we can skip an update, actually skip it
t6022: New test checking for unnecessary updates of files in D/F conflicts
t6022: New test checking for unnecessary updates of renamed+modified files
---

ac9666f84a597973ef29dc7f0f7edcb3e1a3aa45
diff --combined merge-recursive.c
index af131508ec,59482ffc87..7c12673553
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@@ -22,11 -22,6 +22,11 @@@
  #include "dir.h"
  #include "submodule.h"
  
 +static const char rename_limit_advice[] =
 +"inexact rename detection was skipped because there were too many\n"
 +"  files. You may want to set your merge.renamelimit variable to at least\n"
 +"  %d and retry this merge.";
 +
  static struct tree *shift_tree_object(struct tree *one, struct tree *two,
  				      const char *subtree_shift)
  {
@@@ -88,8 -83,10 +88,8 @@@ struct rename_df_conflict_info 
   * Since we want to write the index eventually, we cannot reuse the index
   * for these (temporary) data.
   */
 -struct stage_data
 -{
 -	struct
 -	{
 +struct stage_data {
 +	struct {
  		unsigned mode;
  		unsigned char sha[20];
  	} stages[4];
@@@ -140,6 -137,7 +140,6 @@@ static void flush_output(struct merge_o
  __attribute__((format (printf, 3, 4)))
  static void output(struct merge_options *o, int v, const char *fmt, ...)
  {
 -	int len;
  	va_list ap;
  
  	if (!show(o, v))
@@@ -150,9 -148,21 +150,9 @@@
  	strbuf_setlen(&o->obuf, o->obuf.len + o->call_depth * 2);
  
  	va_start(ap, fmt);
 -	len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
 +	strbuf_vaddf(&o->obuf, fmt, ap);
  	va_end(ap);
  
 -	if (len < 0)
 -		len = 0;
 -	if (len >= strbuf_avail(&o->obuf)) {
 -		strbuf_grow(&o->obuf, len + 2);
 -		va_start(ap, fmt);
 -		len = vsnprintf(o->obuf.buf + o->obuf.len, strbuf_avail(&o->obuf), fmt, ap);
 -		va_end(ap);
 -		if (len >= strbuf_avail(&o->obuf)) {
 -			die("this should not happen, your snprintf is broken");
 -		}
 -	}
 -	strbuf_setlen(&o->obuf, o->obuf.len + len);
  	strbuf_add(&o->obuf, "\n", 1);
  	if (!o->buffer_output)
  		flush_output(o);
@@@ -344,10 -354,11 +344,11 @@@ static void make_room_for_directories_o
  	 * make room for the corresponding directory.  Such paths will
  	 * later be processed in process_df_entry() at the end.  If
  	 * the corresponding directory ends up being removed by the
- 	 * merge, then the file will be reinstated at that time;
- 	 * otherwise, if the file is not supposed to be removed by the
- 	 * merge, the contents of the file will be placed in another
- 	 * unique filename.
+ 	 * merge, then the file will be reinstated at that time
+ 	 * (albeit with a different timestamp!); otherwise, if the
+ 	 * file is not supposed to be removed by the merge, the
+ 	 * contents of the file will be placed in another unique
+ 	 * filename.
  	 *
  	 * NOTE: This function relies on the fact that entries for a
  	 * D/F conflict will appear adjacent in the index, with the
@@@ -356,8 -367,16 +357,15 @@@
  	 */
  	const char *last_file = NULL;
  	int last_len = 0;
 -	struct stage_data *last_e;
  	int i;
  
+ 	/*
+ 	 * Do not do any of this crazyness during the recursive; we don't
+ 	 * even write anything to the working tree!
+ 	 */
+ 	if (o->call_depth)
+ 		return;
+ 
  	for (i = 0; i < entries->nr; i++) {
  		const char *path = entries->items[i].string;
  		int len = strlen(path);
@@@ -385,13 -404,15 +393,13 @@@
  		if (S_ISREG(e->stages[2].mode) || S_ISLNK(e->stages[2].mode)) {
  			last_file = path;
  			last_len = len;
 -			last_e = e;
  		} else {
  			last_file = NULL;
  		}
  	}
  }
  
 -struct rename
 -{
 +struct rename {
  	struct diff_filepair *pair;
  	struct stage_data *src_entry;
  	struct stage_data *dst_entry;
@@@ -421,16 -442,13 +429,16 @@@ static struct string_list *get_renames(
  	opts.detect_rename = DIFF_DETECT_RENAME;
  	opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
  			    o->diff_rename_limit >= 0 ? o->diff_rename_limit :
 -			    500;
 -	opts.warn_on_too_large_rename = 1;
 +			    1000;
 +	opts.rename_score = o->rename_score;
 +	opts.show_rename_progress = o->show_rename_progress;
  	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
  	if (diff_setup_done(&opts) < 0)
  		die("diff setup failed");
  	diff_tree_sha1(o_tree->object.sha1, tree->object.sha1, "", &opts);
  	diffcore_std(&opts);
 +	if (opts.needed_rename_limit > o->needed_rename_limit)
 +		o->needed_rename_limit = opts.needed_rename_limit;
  	for (i = 0; i < diff_queued_diff.nr; ++i) {
  		struct string_list_item *item;
  		struct rename *re;
@@@ -706,7 -724,8 +714,7 @@@ static void update_file(struct merge_op
  
  /* Low level file merging, update and removal */
  
 -struct merge_file_info
 -{
 +struct merge_file_info {
  	unsigned char sha[20];
  	unsigned mode;
  	unsigned clean:1,
@@@ -722,26 -741,22 +730,26 @@@ static int merge_3way(struct merge_opti
  		      const char *branch2)
  {
  	mmfile_t orig, src1, src2;
 +	struct ll_merge_options ll_opts = {0};
  	char *base_name, *name1, *name2;
  	int merge_status;
 -	int favor;
  
 -	if (o->call_depth)
 -		favor = 0;
 -	else {
 +	ll_opts.renormalize = o->renormalize;
 +	ll_opts.xdl_opts = o->xdl_opts;
 +
 +	if (o->call_depth) {
 +		ll_opts.virtual_ancestor = 1;
 +		ll_opts.variant = 0;
 +	} else {
  		switch (o->recursive_variant) {
  		case MERGE_RECURSIVE_OURS:
 -			favor = XDL_MERGE_FAVOR_OURS;
 +			ll_opts.variant = XDL_MERGE_FAVOR_OURS;
  			break;
  		case MERGE_RECURSIVE_THEIRS:
 -			favor = XDL_MERGE_FAVOR_THEIRS;
 +			ll_opts.variant = XDL_MERGE_FAVOR_THEIRS;
  			break;
  		default:
 -			favor = 0;
 +			ll_opts.variant = 0;
  			break;
  		}
  	}
@@@ -764,7 -779,10 +772,7 @@@
  	read_mmblob(&src2, b->sha1);
  
  	merge_status = ll_merge(result_buf, a->path, &orig, base_name,
 -				&src1, name1, &src2, name2,
 -				((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) |
 -				 (o->renormalize ? LL_OPT_RENORMALIZE : 0) |
 -				 create_ll_flag(favor)));
 +				&src1, name1, &src2, name2, &ll_opts);
  
  	free(name1);
  	free(name2);
@@@ -959,6 -977,7 +967,6 @@@ static int process_renames(struct merge
  	}
  
  	for (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {
 -		char *src;
  		struct string_list *renames1, *renames2Dst;
  		struct rename *ren1 = NULL, *ren2 = NULL;
  		const char *branch1, *branch2;
@@@ -993,6 -1012,7 +1001,6 @@@
  			ren2 = ren1;
  			ren1 = tmp;
  		}
 -		src = ren1->pair->one->path;
  
  		ren1->dst_entry->processed = 1;
  		ren1->src_entry->processed = 1;
@@@ -1260,9 -1280,13 +1268,13 @@@ static int merge_content(struct merge_o
  	}
  
  	if (mfi.clean && !df_conflict_remains &&
- 	    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode)
+ 	    sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode &&
+ 	    !o->call_depth && !lstat(path, &st)) {
  		output(o, 3, "Skipped %s (merged same as existing)", path);
- 	else
+ 		add_cacheinfo(mfi.mode, mfi.sha, path,
+ 			      0 /*stage*/, 1 /*refresh*/, 0 /*options*/);
+ 		return mfi.clean;
+ 	} else
  		output(o, 2, "Auto-merging %s", path);
  
  	if (!mfi.clean) {
@@@ -1652,8 -1676,6 +1664,8 @@@ int merge_recursive(struct merge_option
  		commit_list_insert(h2, &(*result)->parents->next);
  	}
  	flush_output(o);
 +	if (o->needed_rename_limit)
 +		warning(rename_limit_advice, o->needed_rename_limit);
  	return clean;
  }
  
@@@ -1746,37 -1768,3 +1758,37 @@@ void init_merge_options(struct merge_op
  	memset(&o->current_directory_set, 0, sizeof(struct string_list));
  	o->current_directory_set.strdup_strings = 1;
  }
 +
 +int parse_merge_opt(struct merge_options *o, const char *s)
 +{
 +	if (!s || !*s)
 +		return -1;
 +	if (!strcmp(s, "ours"))
 +		o->recursive_variant = MERGE_RECURSIVE_OURS;
 +	else if (!strcmp(s, "theirs"))
 +		o->recursive_variant = MERGE_RECURSIVE_THEIRS;
 +	else if (!strcmp(s, "subtree"))
 +		o->subtree_shift = "";
 +	else if (!prefixcmp(s, "subtree="))
 +		o->subtree_shift = s + strlen("subtree=");
 +	else if (!strcmp(s, "patience"))
 +		o->xdl_opts |= XDF_PATIENCE_DIFF;
 +	else if (!strcmp(s, "ignore-space-change"))
 +		o->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
 +	else if (!strcmp(s, "ignore-all-space"))
 +		o->xdl_opts |= XDF_IGNORE_WHITESPACE;
 +	else if (!strcmp(s, "ignore-space-at-eol"))
 +		o->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
 +	else if (!strcmp(s, "renormalize"))
 +		o->renormalize = 1;
 +	else if (!strcmp(s, "no-renormalize"))
 +		o->renormalize = 0;
 +	else if (!prefixcmp(s, "rename-threshold=")) {
 +		const char *score = s + strlen("rename-threshold=");
 +		if ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)
 +			return -1;
 +	}
 +	else
 +		return -1;
 +	return 0;
 +}
diff --combined t/t6022-merge-rename.sh
index 1ed259d864,7d955c1e12..5f3b604fd9
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@@ -99,147 -99,245 +99,147 @@@ git checkout master
  
  test_expect_success 'pull renaming branch into unrenaming one' \
  '
 -	git show-branch
 -	git pull . white && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	git ls-files -s
 -	test "$(git ls-files -u B | wc -l)" -eq 3 || {
 -		echo "BAD: should have left stages for B"
 -		return 1
 -	}
 -	test "$(git ls-files -s N | wc -l)" -eq 1 || {
 -		echo "BAD: should have merged N"
 -		return 1
 -	}
 +	git show-branch &&
 +	test_expect_code 1 git pull . white &&
 +	git ls-files -s &&
 +	git ls-files -u B >b.stages &&
 +	test_line_count = 3 b.stages &&
 +	git ls-files -s N >n.stages &&
 +	test_line_count = 1 n.stages &&
  	sed -ne "/^g/{
  	p
  	q
 -	}" B | grep master || {
 -		echo "BAD: should have listed our change first"
 -		return 1
 -	}
 -	test "$(git diff white N | wc -l)" -eq 0 || {
 -		echo "BAD: should have taken colored branch"
 -		return 1
 -	}
 +	}" B | grep master &&
 +	git diff --exit-code white N
  '
  
  test_expect_success 'pull renaming branch into another renaming one' \
  '
 -	rm -f B
 -	git reset --hard
 -	git checkout red
 -	git pull . white && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	test "$(git ls-files -u B | wc -l)" -eq 3 || {
 -		echo "BAD: should have left stages"
 -		return 1
 -	}
 -	test "$(git ls-files -s N | wc -l)" -eq 1 || {
 -		echo "BAD: should have merged N"
 -		return 1
 -	}
 +	rm -f B &&
 +	git reset --hard &&
 +	git checkout red &&
 +	test_expect_code 1 git pull . white &&
 +	git ls-files -u B >b.stages &&
 +	test_line_count = 3 b.stages &&
 +	git ls-files -s N >n.stages &&
 +	test_line_count = 1 n.stages &&
  	sed -ne "/^g/{
  	p
  	q
 -	}" B | grep red || {
 -		echo "BAD: should have listed our change first"
 -		return 1
 -	}
 -	test "$(git diff white N | wc -l)" -eq 0 || {
 -		echo "BAD: should have taken colored branch"
 -		return 1
 -	}
 +	}" B | grep red &&
 +	git diff --exit-code white N
  '
  
  test_expect_success 'pull unrenaming branch into renaming one' \
  '
 -	git reset --hard
 -	git show-branch
 -	git pull . master && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	test "$(git ls-files -u B | wc -l)" -eq 3 || {
 -		echo "BAD: should have left stages"
 -		return 1
 -	}
 -	test "$(git ls-files -s N | wc -l)" -eq 1 || {
 -		echo "BAD: should have merged N"
 -		return 1
 -	}
 +	git reset --hard &&
 +	git show-branch &&
 +	test_expect_code 1 git pull . master &&
 +	git ls-files -u B >b.stages &&
 +	test_line_count = 3 b.stages &&
 +	git ls-files -s N >n.stages &&
 +	test_line_count = 1 n.stages &&
  	sed -ne "/^g/{
  	p
  	q
 -	}" B | grep red || {
 -		echo "BAD: should have listed our change first"
 -		return 1
 -	}
 -	test "$(git diff white N | wc -l)" -eq 0 || {
 -		echo "BAD: should have taken colored branch"
 -		return 1
 -	}
 +	}" B | grep red &&
 +	git diff --exit-code white N
  '
  
  test_expect_success 'pull conflicting renames' \
  '
 -	git reset --hard
 -	git show-branch
 -	git pull . blue && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	test "$(git ls-files -u A | wc -l)" -eq 1 || {
 -		echo "BAD: should have left a stage"
 -		return 1
 -	}
 -	test "$(git ls-files -u B | wc -l)" -eq 1 || {
 -		echo "BAD: should have left a stage"
 -		return 1
 -	}
 -	test "$(git ls-files -u C | wc -l)" -eq 1 || {
 -		echo "BAD: should have left a stage"
 -		return 1
 -	}
 -	test "$(git ls-files -s N | wc -l)" -eq 1 || {
 -		echo "BAD: should have merged N"
 -		return 1
 -	}
 +	git reset --hard &&
 +	git show-branch &&
 +	test_expect_code 1 git pull . blue &&
 +	git ls-files -u A >a.stages &&
 +	test_line_count = 1 a.stages &&
 +	git ls-files -u B >b.stages &&
 +	test_line_count = 1 b.stages &&
 +	git ls-files -u C >c.stages &&
 +	test_line_count = 1 c.stages &&
 +	git ls-files -s N >n.stages &&
 +	test_line_count = 1 n.stages &&
  	sed -ne "/^g/{
  	p
  	q
 -	}" B | grep red || {
 -		echo "BAD: should have listed our change first"
 -		return 1
 -	}
 -	test "$(git diff white N | wc -l)" -eq 0 || {
 -		echo "BAD: should have taken colored branch"
 -		return 1
 -	}
 +	}" B | grep red &&
 +	git diff --exit-code white N
  '
  
  test_expect_success 'interference with untracked working tree file' '
 -
 -	git reset --hard
 -	git show-branch
 -	echo >A this file should not matter
 -	git pull . white && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	test -f A || {
 -		echo "BAD: should have left A intact"
 -		return 1
 -	}
 +	git reset --hard &&
 +	git show-branch &&
 +	echo >A this file should not matter &&
 +	test_expect_code 1 git pull . white &&
 +	test_path_is_file A
  '
  
  test_expect_success 'interference with untracked working tree file' '
 -
 -	git reset --hard
 -	git checkout white
 -	git show-branch
 -	rm -f A
 -	echo >A this file should not matter
 -	git pull . red && {
 -		echo "BAD: should have conflicted"
 -		return 1
 -	}
 -	test -f A || {
 -		echo "BAD: should have left A intact"
 -		return 1
 -	}
 +	git reset --hard &&
 +	git checkout white &&
 +	git show-branch &&
 +	rm -f A &&
 +	echo >A this file should not matter &&
 +	test_expect_code 1 git pull . red &&
 +	test_path_is_file A
  '
  
  test_expect_success 'interference with untracked working tree file' '
 -
 -	git reset --hard
 -	rm -f A M
 -	git checkout -f master
 -	git tag -f anchor
 -	git show-branch
 -	git pull . yellow || {
 -		echo "BAD: should have cleanly merged"
 -		return 1
 -	}
 -	test -f M && {
 -		echo "BAD: should have removed M"
 -		return 1
 -	}
 +	git reset --hard &&
 +	rm -f A M &&
 +	git checkout -f master &&
 +	git tag -f anchor &&
 +	git show-branch &&
 +	git pull . yellow &&
 +	test_path_is_missing M &&
  	git reset --hard anchor
  '
  
  test_expect_success 'updated working tree file should prevent the merge' '
 -
 -	git reset --hard
 -	rm -f A M
 -	git checkout -f master
 -	git tag -f anchor
 -	git show-branch
 -	echo >>M one line addition
 -	cat M >M.saved
 -	git pull . yellow && {
 -		echo "BAD: should have complained"
 -		return 1
 -	}
 -	test_cmp M M.saved || {
 -		echo "BAD: should have left M intact"
 -		return 1
 -	}
 +	git reset --hard &&
 +	rm -f A M &&
 +	git checkout -f master &&
 +	git tag -f anchor &&
 +	git show-branch &&
 +	echo >>M one line addition &&
 +	cat M >M.saved &&
 +	test_expect_code 128 git pull . yellow &&
 +	test_cmp M M.saved &&
  	rm -f M.saved
  '
  
  test_expect_success 'updated working tree file should prevent the merge' '
 -
 -	git reset --hard
 -	rm -f A M
 -	git checkout -f master
 -	git tag -f anchor
 -	git show-branch
 -	echo >>M one line addition
 -	cat M >M.saved
 -	git update-index M
 -	git pull . yellow && {
 -		echo "BAD: should have complained"
 -		return 1
 -	}
 -	test_cmp M M.saved || {
 -		echo "BAD: should have left M intact"
 -		return 1
 -	}
 +	git reset --hard &&
 +	rm -f A M &&
 +	git checkout -f master &&
 +	git tag -f anchor &&
 +	git show-branch &&
 +	echo >>M one line addition &&
 +	cat M >M.saved &&
 +	git update-index M &&
 +	test_expect_code 128 git pull . yellow &&
 +	test_cmp M M.saved &&
  	rm -f M.saved
  '
  
  test_expect_success 'interference with untracked working tree file' '
 -
 -	git reset --hard
 -	rm -f A M
 -	git checkout -f yellow
 -	git tag -f anchor
 -	git show-branch
 -	echo >M this file should not matter
 -	git pull . master || {
 -		echo "BAD: should have cleanly merged"
 -		return 1
 -	}
 -	test -f M || {
 -		echo "BAD: should have left M intact"
 -		return 1
 -	}
 -	git ls-files -s | grep M && {
 -		echo "BAD: M must be untracked in the result"
 -		return 1
 -	}
 +	git reset --hard &&
 +	rm -f A M &&
 +	git checkout -f yellow &&
 +	git tag -f anchor &&
 +	git show-branch &&
 +	echo >M this file should not matter &&
 +	git pull . master &&
 +	test_path_is_file M &&
 +	! {
 +		git ls-files -s |
 +		grep M
 +	} &&
  	git reset --hard anchor
  '
  
  test_expect_success 'merge of identical changes in a renamed file' '
 -	rm -f A M N
 +	rm -f A M N &&
  	git reset --hard &&
  	git checkout change+rename &&
  	GIT_MERGE_VERBOSITY=3 git merge change | grep "^Skipped B" &&
@@@ -609,4 -707,67 +609,67 @@@ test_expect_success 'check handling of 
  	! test -f original
  '
  
+ test_expect_success 'setup avoid unnecessary update, normal rename' '
+ 	git reset --hard &&
+ 	git checkout --orphan avoid-unnecessary-update-1 &&
+ 	git rm -rf . &&
+ 	git clean -fdqx &&
+ 
+ 	printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" >original &&
+ 	git add -A &&
+ 	git commit -m "Common commmit" &&
+ 
+ 	git mv original rename &&
+ 	echo 11 >>rename &&
+ 	git add -u &&
+ 	git commit -m "Renamed and modified" &&
+ 
+ 	git checkout -b merge-branch-1 HEAD~1 &&
+ 	echo "random content" >random-file &&
+ 	git add -A &&
+ 	git commit -m "Random, unrelated changes"
+ '
+ 
+ test_expect_success 'avoid unnecessary update, normal rename' '
+ 	git checkout -q avoid-unnecessary-update-1^0 &&
+ 	test-chmtime =1000000000 rename &&
+ 	test-chmtime -v +0 rename >expect &&
+ 	git merge merge-branch-1 &&
+ 	test-chmtime -v +0 rename >actual &&
+ 	test_cmp expect actual # "rename" should have stayed intact
+ '
+ 
+ test_expect_success 'setup to test avoiding unnecessary update, with D/F conflict' '
+ 	git reset --hard &&
+ 	git checkout --orphan avoid-unnecessary-update-2 &&
+ 	git rm -rf . &&
+ 	git clean -fdqx &&
+ 
+ 	mkdir df &&
+ 	printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" >df/file &&
+ 	git add -A &&
+ 	git commit -m "Common commmit" &&
+ 
+ 	git mv df/file temp &&
+ 	rm -rf df &&
+ 	git mv temp df &&
+ 	echo 11 >>df &&
+ 	git add -u &&
+ 	git commit -m "Renamed and modified" &&
+ 
+ 	git checkout -b merge-branch-2 HEAD~1 &&
+ 	>unrelated-change &&
+ 	git add unrelated-change &&
+ 	git commit -m "Only unrelated changes"
+ '
+ 
+ test_expect_failure 'avoid unnecessary update, with D/F conflict' '
+ 	git checkout -q avoid-unnecessary-update-2^0 &&
+ 	test-chmtime =1000000000 df &&
+ 	test-chmtime -v +0 df >expect &&
+ 	git merge merge-branch-2 &&
+ 	test-chmtime -v +0 df >actual &&
+ 	test_cmp expect actual # "df" should have stayed intact
+ '
+ 
  test_done