threads / patch / 41454

v5, 3 partsmerge-recursive: option to disable renames

Subject: [PATCH v5 0/3] merge-recursive: option to disable renames

## tl;dr

6 messages between Feb 21, 2016 and Feb 21, 2016. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Felipe Gonçalves Assis· Feb 21, 2016, 02:34 UTC · lore

Add tests as suggested by Eric Sunshine. Also fixes an inconsitency in the last part.

Since there were no tests for the --rename-threshold option, I added them in a separate commit, which is useful in itself.

The other two commits contain the previous patches plus the relevant tests.

For the last part, I made --find-renames (without =<n>) reset the similarity index to the default, just as in git-diff.

Felipe Gonçalves Assis (3):
  merge-recursive: test rename threshold option
  merge-recursive: option to disable renames
  merge-recursive: more consistent interface
 Documentation/merge-strategies.txt                 |  12 +-
 merge-recursive.c                                  |  14 +-
 merge-recursive.h                                  |   1 +
 ...s.sh => t3032-merge-recursive-space-options.sh} |   2 +-
 t/t3034-merge-recursive-rename-options.sh          | 200 +++++++++++++++++++++
 5 files changed, 225 insertions(+), 4 deletions(-)
 rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
 create mode 100755 t/t3034-merge-recursive-rename-options.sh
-- 
2.7.1.342.gf5bb636
Felipe Gonçalves Assis· Feb 21, 2016, 02:34 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v5 1/3] merge-recursive: test rename threshold option

Commit 10ae7526bebb505ddddba01f76ec97d5f7b5e0e5 introduced this feature, but did not include any tests. This commit fixes this.

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---

This commit is independent of the proposed feature, so it might be of interest even if the rest of the patch is rejected.

 t/t3034-merge-recursive-rename-threshold.sh | 146 ++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100755 t/t3034-merge-recursive-rename-threshold.sh
Show changes to t/t3034-merge-recursive-rename-threshold.sh +146 −0
diff --git a/t/t3034-merge-recursive-rename-threshold.sh b/t/t3034-merge-recursive-rename-threshold.sh
new file mode 100755
index 0000000..f0b3f44
--- /dev/null
+++ b/t/t3034-merge-recursive-rename-threshold.sh
@@ -0,0 +1,146 @@
+#!/bin/sh
+
+test_description='merge-recursive rename threshold option
+
+Test rename detection by examining rename/delete conflicts.
+
+Similarity index:
+R100 a-old a-new
+R075 b-old b-new
+R050 c-old c-new
+R025 d-old d-new
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	get_expected_stages () {
+		git checkout rename -- $1-new &&
+		git ls-files --stage $1-new > expected-stages-undetected-$1
+		sed "s/ 0	/ 2	/
+		" < expected-stages-undetected-$1 > expected-stages-detected-$1
+		git read-tree -u --reset HEAD
+	} &&
+
+	rename_detected () {
+		git ls-files --stage $1-old $1-new > stages-actual-$1 &&
+		test_cmp expected-stages-detected-$1 stages-actual-$1
+	} &&
+
+	rename_undetected () {
+		git ls-files --stage $1-old $1-new > stages-actual-$1 &&
+		test_cmp expected-stages-undetected-$1 stages-actual-$1
+	} &&
+
+	check_common () {
+		git ls-files --stage > stages-actual &&
+		test $(wc -l < stages-actual) -eq 4
+	} &&
+
+	check_find_renames_25 () {
+		check_common &&
+		rename_detected a &&
+		rename_detected b &&
+		rename_detected c &&
+		rename_detected d
+	} &&
+
+	check_find_renames_50 () {
+		check_common
+		rename_detected a &&
+		rename_detected b &&
+		rename_detected c &&
+		rename_undetected d
+	} &&
+
+	check_find_renames_75 () {
+		check_common
+		rename_detected a &&
+		rename_detected b &&
+		rename_undetected c &&
+		rename_undetected d
+	} &&
+
+	check_find_renames_100 () {
+		check_common
+		rename_detected a &&
+		rename_undetected b &&
+		rename_undetected c &&
+		rename_undetected d
+	} &&
+
+	check_no_renames () {
+		check_common
+		rename_undetected a &&
+		rename_undetected b &&
+		rename_undetected c &&
+		rename_undetected d
+	} &&
+
+	cat <<-\EOF > a-old &&
+	aa1
+	aa2
+	aa3
+	aa4
+	EOF
+	sed s/aa/bb/ < a-old > b-old &&
+	sed s/aa/cc/ < a-old > c-old &&
+	sed s/aa/dd/ < a-old > d-old &&
+	git add [a-d]-old &&
+	test_tick &&
+	git commit -m base &&
+	git rm [a-d]-old &&
+	test_tick &&
+	git commit -m delete &&
+	git checkout -b rename HEAD^ &&
+	cp a-old a-new &&
+	sed 1,1s/./x/ < b-old > b-new &&
+	sed 1,2s/./x/ < c-old > c-new &&
+	sed 1,3s/./x/ < d-old > d-new &&
+	git add [a-d]-new &&
+	git rm [a-d]-old &&
+	test_tick &&
+	git commit -m rename &&
+	get_expected_stages a &&
+	get_expected_stages b &&
+	get_expected_stages c &&
+	get_expected_stages d
+'
+
+test_expect_success 'the default similarity index is 50%' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'low rename threshold' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
+test_expect_success 'high rename threshold' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=75 HEAD^ -- HEAD master &&
+	check_find_renames_75
+'
+
+test_expect_success 'exact renames only' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=100% HEAD^ -- HEAD master &&
+	check_find_renames_100
+'
+
+test_expect_success 'rename threshold is truncated' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=200% HEAD^ -- HEAD master &&
+	check_find_renames_100
+'
+
+test_expect_success 'last wins in --rename-threshold=<m> --rename-threshold=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 --rename-threshold=75 HEAD^ -- HEAD master &&
+	check_find_renames_75
+'
+
+test_done
-- 
2.7.1.342.gf5bb636
Felipe Gonçalves Assis· Feb 21, 2016, 02:34 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v5 2/3] merge-recursive: option to disable renames

The recursive strategy turns on rename detection by default. Add a strategy option to disable rename detection even for exact renames.

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
Added tests.

For consistent naming, I renamed and slightly edited the test of whitespace options: t/t3032-merge-recursive-options.sh -> t/t3032-merge-recursive-space-options.sh

 Documentation/merge-strategies.txt                   |  6 ++++++
 merge-recursive.c                                    |  7 +++++++
 merge-recursive.h                                    |  1 +
 ...ons.sh => t3032-merge-recursive-space-options.sh} |  2 +-
 ...ld.sh => t3034-merge-recursive-rename-options.sh} | 20 +++++++++++++++++++-
 5 files changed, 34 insertions(+), 2 deletions(-)
 rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
 rename t/{t3034-merge-recursive-rename-threshold.sh => t3034-merge-recursive-rename-options.sh} (83%)
Show changes to 7 files +34 −2

Documentation/merge-strategies.txt, merge-recursive.c, merge-recursive.h, t/t3032-merge-recursive-options.sh, t/t3032-merge-recursive-space-options.sh, t/t3034-merge-recursive-rename-threshold.sh, t/t3034-merge-recursive-rename-options.sh

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 7bbd19b..1a5e197 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -81,8 +81,14 @@ no-renormalize;;
 	Disables the `renormalize` option.  This overrides the
 	`merge.renormalize` configuration variable.
 
+no-renames;;
+	Turn off rename detection.
+	See also linkgit:git-diff[1] `--no-renames`.
+
 rename-threshold=<n>;;
 	Controls the similarity threshold used for rename detection.
+	Re-enables rename detection if disabled by a preceding
+	`no-renames`.
 	See also linkgit:git-diff[1] `-M`.
 
 subtree[=<path>];;
diff --git a/merge-recursive.c b/merge-recursive.c
index 8eabde2..6dd0a11 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -482,6 +482,9 @@ static struct string_list *get_renames(struct merge_options *o,
 	struct diff_options opts;
 
 	renames = xcalloc(1, sizeof(struct string_list));
+	if (!o->detect_rename)
+		return renames;
+
 	diff_setup(&opts);
 	DIFF_OPT_SET(&opts, RECURSIVE);
 	DIFF_OPT_CLR(&opts, RENAME_EMPTY);
@@ -2039,6 +2042,7 @@ void init_merge_options(struct merge_options *o)
 	o->diff_rename_limit = -1;
 	o->merge_rename_limit = -1;
 	o->renormalize = 0;
+	o->detect_rename = 1;
 	merge_recursive_config(o);
 	if (getenv("GIT_MERGE_VERBOSITY"))
 		o->verbosity =
@@ -2088,9 +2092,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)
 		o->renormalize = 1;
 	else if (!strcmp(s, "no-renormalize"))
 		o->renormalize = 0;
+	else if (!strcmp(s, "no-renames"))
+		o->detect_rename = 0;
 	else if (skip_prefix(s, "rename-threshold=", &arg)) {
 		if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
 			return -1;
+		o->detect_rename = 1;
 	}
 	else
 		return -1;
diff --git a/merge-recursive.h b/merge-recursive.h
index 9e090a3..52f0201 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -17,6 +17,7 @@ struct merge_options {
 	unsigned renormalize : 1;
 	long xdl_opts;
 	int verbosity;
+	int detect_rename;
 	int diff_rename_limit;
 	int merge_rename_limit;
 	int rename_score;
diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-space-options.sh
similarity index 99%
rename from t/t3032-merge-recursive-options.sh
rename to t/t3032-merge-recursive-space-options.sh
index 4029c9c..b56180e 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-space-options.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='merge-recursive options
+test_description='merge-recursive space options
 
 * [master] Clarify
  ! [remote] Remove cruft
diff --git a/t/t3034-merge-recursive-rename-threshold.sh b/t/t3034-merge-recursive-rename-options.sh
similarity index 83%
rename from t/t3034-merge-recursive-rename-threshold.sh
rename to t/t3034-merge-recursive-rename-options.sh
index f0b3f44..2f10fa7 100755
--- a/t/t3034-merge-recursive-rename-threshold.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='merge-recursive rename threshold option
+test_description='merge-recursive rename options
 
 Test rename detection by examining rename/delete conflicts.
 
@@ -137,10 +137,28 @@ test_expect_success 'rename threshold is truncated' '
 	check_find_renames_100
 '
 
+test_expect_success 'disabled rename detection' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
 test_expect_success 'last wins in --rename-threshold=<m> --rename-threshold=<n>' '
 	git read-tree --reset -u HEAD &&
 	test_must_fail git merge-recursive --rename-threshold=25 --rename-threshold=75 HEAD^ -- HEAD master &&
 	check_find_renames_75
 '
 
+test_expect_success 'last wins in --no-renames --rename-threshold=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --no-renames --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
+test_expect_success 'last wins in --rename-threshold=<n> --no-renames' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --rename-threshold=25 --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
 test_done
-- 
2.7.1.342.gf5bb636
Felipe Gonçalves Assis· Feb 21, 2016, 02:34 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v5 3/3] merge-recursive: more consistent interface

Add strategy option find-renames, following git-diff interface. This makes the option rename-threshold redundant.

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
Added tests and made --find-renames reset the similarity index to the default.
 Documentation/merge-strategies.txt        | 10 ++++---
 merge-recursive.c                         |  7 ++++-
 t/t3034-merge-recursive-rename-options.sh | 48 +++++++++++++++++++++++++++----
 3 files changed, 54 insertions(+), 11 deletions(-)
Show changes to 3 files +54 −11

Documentation/merge-strategies.txt, merge-recursive.c, t/t3034-merge-recursive-rename-options.sh

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 1a5e197..2eb92b9 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -85,11 +85,13 @@ no-renames;;
 	Turn off rename detection.
 	See also linkgit:git-diff[1] `--no-renames`.
 
+find-renames[=<n>];;
+	Turn on rename detection, optionally setting the similarity
+	threshold.  This is the default.
+	See also linkgit:git-diff[1] `--find-renames`.
+
 rename-threshold=<n>;;
-	Controls the similarity threshold used for rename detection.
-	Re-enables rename detection if disabled by a preceding
-	`no-renames`.
-	See also linkgit:git-diff[1] `-M`.
+	Deprecated synonym for `find-renames=<n>`.
 
 subtree[=<path>];;
 	This option is a more advanced form of 'subtree' strategy, where
diff --git a/merge-recursive.c b/merge-recursive.c
index 6dd0a11..63b8ba8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2094,7 +2094,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)
 		o->renormalize = 0;
 	else if (!strcmp(s, "no-renames"))
 		o->detect_rename = 0;
-	else if (skip_prefix(s, "rename-threshold=", &arg)) {
+	else if (!strcmp(s, "find-renames")) {
+		o->detect_rename = 1;
+		o->rename_score = 0;
+	}
+	else if (skip_prefix(s, "find-renames=", &arg) ||
+	         skip_prefix(s, "rename-threshold=", &arg)) {
 		if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
 			return -1;
 		o->detect_rename = 1;
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index 2f10fa7..4e3fefd 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -115,25 +115,25 @@ test_expect_success 'the default similarity index is 50%' '
 
 test_expect_success 'low rename threshold' '
 	git read-tree --reset -u HEAD &&
-	test_must_fail git merge-recursive --rename-threshold=25 HEAD^ -- HEAD master &&
+	test_must_fail git merge-recursive --find-renames=25 HEAD^ -- HEAD master &&
 	check_find_renames_25
 '
 
 test_expect_success 'high rename threshold' '
 	git read-tree --reset -u HEAD &&
-	test_must_fail git merge-recursive --rename-threshold=75 HEAD^ -- HEAD master &&
+	test_must_fail git merge-recursive --find-renames=75 HEAD^ -- HEAD master &&
 	check_find_renames_75
 '
 
 test_expect_success 'exact renames only' '
 	git read-tree --reset -u HEAD &&
-	test_must_fail git merge-recursive --rename-threshold=100% HEAD^ -- HEAD master &&
+	test_must_fail git merge-recursive --find-renames=100% HEAD^ -- HEAD master &&
 	check_find_renames_100
 '
 
 test_expect_success 'rename threshold is truncated' '
 	git read-tree --reset -u HEAD &&
-	test_must_fail git merge-recursive --rename-threshold=200% HEAD^ -- HEAD master &&
+	test_must_fail git merge-recursive --find-renames=200% HEAD^ -- HEAD master &&
 	check_find_renames_100
 '
 
@@ -143,12 +143,36 @@ test_expect_success 'disabled rename detection' '
 	check_no_renames
 '
 
-test_expect_success 'last wins in --rename-threshold=<m> --rename-threshold=<n>' '
+test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' '
 	git read-tree --reset -u HEAD &&
-	test_must_fail git merge-recursive --rename-threshold=25 --rename-threshold=75 HEAD^ -- HEAD master &&
+	test_must_fail git merge-recursive --find-renames=25 --find-renames=75 HEAD^ -- HEAD master &&
 	check_find_renames_75
 '
 
+test_expect_success '--find-renames is equivalent to --find-renames=5' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=25 --find-renames HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'last wins in --no-renames --find-renames' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --no-renames --find-renames HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'last wins in --find-renames --no-renames' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --find-renames --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
+test_expect_success 'rename-threshold=<n> is a synonym for find-renames=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
 test_expect_success 'last wins in --no-renames --rename-threshold=<n>' '
 	git read-tree --reset -u HEAD &&
 	test_must_fail git merge-recursive --no-renames --rename-threshold=25 HEAD^ -- HEAD master &&
@@ -161,4 +185,16 @@ test_expect_success 'last wins in --rename-threshold=<n> --no-renames' '
 	check_no_renames
 '
 
+test_expect_success 'last wins in --rename-threshold=<n> --find-renames' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 --find-renames HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'last wins in --find-renames --rename-threshold=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
 test_done
-- 
2.7.1.342.gf5bb636
Junio C Hamano· Feb 21, 2016, 07:40 UTC · re: Felipe Gonçalves Assis · lore

Re: [PATCH v5 0/3] merge-recursive: option to disable renames

Yikes, your previous round is already in 'next', so could you make this series an incremental on top of what is already queued up to 1b47ad16 (merge-recursive: more consistent interface, 2016-02-17)?

Thanks.
Felipe Gonçalves Assis· Feb 21, 2016, 15:07 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 0/3] merge-recursive: option to disable renames

On 21 February 2016 at 04:40, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> Yikes, your previous round is already in 'next', so could you make
> this series an incremental on top of what is already queued up to
> 1b47ad16 (merge-recursive: more consistent interface, 2016-02-17)?
>
> Thanks.
>
Oh, sorry. I did not notice that. I will submit a new series shortly.

← back to recent threads