From: Antonin Delpeuch <antonin@xxxxxxxxxxx>
The documentation claims that "recursive defaults to the diff.algorithm
config setting", but this is currently not the case. This fixes it,
ensuring that diff.algorithm is used when -Xdiff-algorithm is not
supplied. This affects the following porcelain commands: "merge",
"rebase", "cherry-pick", "pull", "stash", "log", "am" and "checkout".
It also affects the "merge-tree" ancillary interrogator.
This change also affects the "replay" and "merge-recursive" plumbing
commands, which happen to call 'merge_recursive_config' and therefore
are also affected by other configuration variables read in this
function. For instance theay read "diff.renames", classified in diff.c
as a diff "UI" config variable. Removing the reliance of those
commands on this set of configuration variables feels like a bigger
change and introducing an argument to 'merge_recursive_config' to
prevent only the newly added diff.algorithm to be read by plumbing
commands feels like muddying the architecture, as this function
should likely not be called at all by plumbing commands.
Signed-off-by: Antonin Delpeuch <antonin@xxxxxxxxxxx>
---
merge-recursive: honor diff.algorithm
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1743%2Fwetneb%2Frecursive_respects_diff.algorithm-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1743/wetneb/recursive_respects_diff.algorithm-v1
Pull-Request: https://github.com/git/git/pull/1743
builtin/merge-recursive.c | 4 ++++
builtin/replay.c | 4 ++++
merge-recursive.c | 7 ++++++
t/t3515-cherry-pick-diff.sh | 41 +++++++++++++++++++++++++++++++++++
t/t3515/base.c | 17 +++++++++++++++
t/t3515/ours.c | 17 +++++++++++++++
t/t3515/theirs.c | 17 +++++++++++++++
t/t7615-merge-diff.sh | 43 +++++++++++++++++++++++++++++++++++++
8 files changed, 150 insertions(+)
create mode 100755 t/t3515-cherry-pick-diff.sh
create mode 100644 t/t3515/base.c
create mode 100644 t/t3515/ours.c
create mode 100644 t/t3515/theirs.c
create mode 100755 t/t7615-merge-diff.sh
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index c2ce044a201..c14158fd1db 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -31,6 +31,10 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED)
char *better1, *better2;
struct commit *result;
+ /*
+ * FIXME: This reads various config variables,
+ * which 'merge-recursive' should ignore as a plumbing command
+ */
init_merge_options(&o, the_repository);
if (argv[0] && ends_with(argv[0], "-subtree"))
o.subtree_shift = "";
diff --git a/builtin/replay.c b/builtin/replay.c
index 6bf0691f15d..98feb6f6320 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -373,6 +373,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
goto cleanup;
}
+ /*
+ * FIXME: This reads various config variables,
+ * which 'replay' should ignore as a plumbing command
+ */
init_merge_options(&merge_opt, the_repository);
memset(&result, 0, sizeof(result));
merge_opt.show_rename_progress = 0;
diff --git a/merge-recursive.c b/merge-recursive.c
index 46ee364af73..205fb8aa72d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3930,6 +3930,13 @@ static void merge_recursive_config(struct merge_options *opt)
} /* avoid erroring on values from future versions of git */
free(value);
}
+ if (!git_config_get_string("diff.algorithm", &value)) {
+ long diff_algorithm = parse_algorithm_value(value);
+ if (diff_algorithm < 0)
+ die(_("unknown value for config '%s': %s"), "diff.algorithm", value);
+ opt->xdl_opts = (opt->xdl_opts & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm;
+ free(value);
+ }
git_config(git_xmerge_config, NULL);
}
diff --git a/t/t3515-cherry-pick-diff.sh b/t/t3515-cherry-pick-diff.sh
new file mode 100755
index 00000000000..caeaa01c590
--- /dev/null
+++ b/t/t3515-cherry-pick-diff.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='git cherry-pick
+
+Testing the influence of the diff algorithm on the merge output.'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ cp "$TEST_DIRECTORY"/t3515/base.c file.c &&
+ git add file.c &&
+ git commit -m c0 &&
+ git tag c0 &&
+ cp "$TEST_DIRECTORY"/t3515/ours.c file.c &&
+ git add file.c &&
+ git commit -m c1 &&
+ git tag c1 &&
+ git reset --hard c0 &&
+ cp "$TEST_DIRECTORY"/t3515/theirs.c file.c &&
+ git add file.c &&
+ git commit -m c2 &&
+ git tag c2
+'
+
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
+ git reset --hard c1 &&
+ test_must_fail git cherry-pick -s recursive c2
+'
+
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '
+ git reset --hard c1 &&
+ git cherry-pick --strategy recursive -Xdiff-algorithm=histogram c2
+'
+
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy succeeds with diff.algorithm = histogram' '
+ git reset --hard c1 &&
+ git config diff.algorithm histogram &&
+ git cherry-pick --strategy recursive c2
+'
+test_done
diff --git a/t/t3515/base.c b/t/t3515/base.c
new file mode 100644
index 00000000000..c64abc59366
--- /dev/null
+++ b/t/t3515/base.c
@@ -0,0 +1,17 @@
+int f(int x, int y)
+{
+ if (x == 0)
+ {
+ return y;
+ }
+ return x;
+}
+
+int g(size_t u)
+{
+ while (u < 30)
+ {
+ u++;
+ }
+ return u;
+}
diff --git a/t/t3515/ours.c b/t/t3515/ours.c
new file mode 100644
index 00000000000..44d82513970
--- /dev/null
+++ b/t/t3515/ours.c
@@ -0,0 +1,17 @@
+int g(size_t u)
+{
+ while (u < 30)
+ {
+ u++;
+ }
+ return u;
+}
+
+int h(int x, int y, int z)
+{
+ if (z == 0)
+ {
+ return x;
+ }
+ return y;
+}
diff --git a/t/t3515/theirs.c b/t/t3515/theirs.c
new file mode 100644
index 00000000000..85f02146fee
--- /dev/null
+++ b/t/t3515/theirs.c
@@ -0,0 +1,17 @@
+int f(int x, int y)
+{
+ if (x == 0)
+ {
+ return y;
+ }
+ return x;
+}
+
+int g(size_t u)
+{
+ while (u > 34)
+ {
+ u--;
+ }
+ return u;
+}
diff --git a/t/t7615-merge-diff.sh b/t/t7615-merge-diff.sh
new file mode 100755
index 00000000000..be335c7c3d1
--- /dev/null
+++ b/t/t7615-merge-diff.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git merge
+
+Testing the influence of the diff algorithm on the merge output.'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ cp "$TEST_DIRECTORY"/t3515/base.c file.c &&
+ git add file.c &&
+ git commit -m c0 &&
+ git tag c0 &&
+ cp "$TEST_DIRECTORY"/t3515/ours.c file.c &&
+ git add file.c &&
+ git commit -m c1 &&
+ git tag c1 &&
+ git reset --hard c0 &&
+ cp "$TEST_DIRECTORY"/t3515/theirs.c file.c &&
+ git add file.c &&
+ git commit -m c2 &&
+ git tag c2
+'
+
+GIT_TEST_MERGE_ALGORITHM=recursive
+
+test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
+ git reset --hard c1 &&
+ test_must_fail git merge -s recursive c2
+'
+
+test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '
+ git reset --hard c1 &&
+ git merge --strategy recursive -Xdiff-algorithm=histogram c2
+'
+
+test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with diff.algorithm = histogram' '
+ git reset --hard c1 &&
+ git config diff.algorithm histogram &&
+ git merge --strategy recursive c2
+'
+test_done
base-commit: 06e570c0dfb2a2deb64d217db78e2ec21672f558
--
gitgitgadget
