merge-recursive: honor diff.algorithm
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 refactors the initialization of merge options to introduce two functions, "init_merge_ui_options" and "init_merge_basic_options" instead of just one "init_merge_options". This design follows the approach used in diff.c, providing initialization methods for porcelain and plumbing commands respectively. Thanks to that, the "replay" and "merge-recursive" plumbing commands remain unaffected by diff.algorithm. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Antonin Delpeuch committed
Jul 13, 2024 at 16:51 UTC
9c93ba4d0aee1bc8c663a13552afd2b2c22863a9
15 files changed
+150
-15
builtin/am.c
+1
-1
@@ -1630,7 +1630,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
1630
* changes.
1631
*/
1632
1633
- init_merge_options(&o, the_repository);
1633
+ init_ui_merge_options(&o, the_repository);
1634
1635
o.branch1 = "HEAD";
1636
their_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
builtin/checkout.c
+1
-1
@@ -884,7 +884,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
884
885
add_files_to_cache(the_repository, NULL, NULL, NULL, 0,
886
0);
887
- init_merge_options(&o, the_repository);
887
+ init_ui_merge_options(&o, the_repository);
888
o.verbosity = 0;
889
work = write_in_core_index_as_tree(the_repository);
890
builtin/merge-recursive.c
+1
-1
@@ -31,7 +31,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED)
31
char *better1, *better2;
32
struct commit *result;
33
34
- init_merge_options(&o, the_repository);
34
+ init_basic_merge_options(&o, the_repository);
35
if (argv[0] && ends_with(argv[0], "-subtree"))
36
o.subtree_shift = "";
37
builtin/merge-tree.c
+1
-1
@@ -571,7 +571,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
571
};
572
573
/* Init merge options */
574
- init_merge_options(&o.merge_options, the_repository);
574
+ init_ui_merge_options(&o.merge_options, the_repository);
575
576
/* Parse arguments */
577
original_argc = argc - 1; /* ignoring argv[0] */
builtin/merge.c
+1
-1
@@ -724,7 +724,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
724
return 2;
725
}
726
727
- init_merge_options(&o, the_repository);
727
+ init_ui_merge_options(&o, the_repository);
728
if (!strcmp(strategy, "subtree"))
729
o.subtree_shift = "";
730
builtin/replay.c
+1
-1
@@ -377,7 +377,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix)
377
goto cleanup;
378
}
379
380
- init_merge_options(&merge_opt, the_repository);
380
+ init_basic_merge_options(&merge_opt, the_repository);
381
memset(&result, 0, sizeof(result));
382
merge_opt.show_rename_progress = 0;
383
last_commit = onto;
builtin/stash.c
+1
-1
@@ -574,7 +574,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
574
}
575
}
576
577
- init_merge_options(&o, the_repository);
577
+ init_ui_merge_options(&o, the_repository);
578
579
o.branch1 = "Updated upstream";
580
o.branch2 = "Stashed changes";
log-tree.c
+1
-1
@@ -1025,7 +1025,7 @@ static int do_remerge_diff(struct rev_info *opt,
1025
struct strbuf parent2_desc = STRBUF_INIT;
1026
1027
/* Setup merge options */
1028
- init_merge_options(&o, the_repository);
1028
+ init_ui_merge_options(&o, the_repository);
1029
o.show_rename_progress = 0;
1030
o.record_conflict_msgs_as_headers = 1;
1031
o.msg_header_prefix = "remerge";
merge-recursive.c
+25
-4
@@ -3921,7 +3921,7 @@ int merge_recursive_generic(struct merge_options *opt,
3921
return clean ? 0 : 1;
3922
}
3923
3924
-static void merge_recursive_config(struct merge_options *opt)
3924
+static void merge_recursive_config(struct merge_options *opt, int ui)
3925
{
3926
char *value = NULL;
3927
int renormalize = 0;
@@ -3950,11 +3950,20 @@ static void merge_recursive_config(struct merge_options *opt)
3950
} /* avoid erroring on values from future versions of git */
3951
free(value);
3952
}
3953
+ if (ui) {
3954
+ if (!git_config_get_string("diff.algorithm", &value)) {
3955
+ long diff_algorithm = parse_algorithm_value(value);
3956
+ if (diff_algorithm < 0)
3957
+ die(_("unknown value for config '%s': %s"), "diff.algorithm", value);
3958
+ opt->xdl_opts = (opt->xdl_opts & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm;
3959
+ free(value);
3960
+ }
3961
+ }
3962
git_config(git_xmerge_config, NULL);
3963
}
3964
3956
-void init_merge_options(struct merge_options *opt,
3957
- struct repository *repo)
3965
+static void init_merge_options(struct merge_options *opt,
3966
+ struct repository *repo, int ui)
3967
{
3968
const char *merge_verbosity;
3969
memset(opt, 0, sizeof(struct merge_options));
@@ -3973,7 +3982,7 @@ void init_merge_options(struct merge_options *opt,
3982
3983
opt->conflict_style = -1;
3984
3976
- merge_recursive_config(opt);
3985
+ merge_recursive_config(opt, ui);
3986
merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
3987
if (merge_verbosity)
3988
opt->verbosity = strtol(merge_verbosity, NULL, 10);
@@ -3981,6 +3990,18 @@ void init_merge_options(struct merge_options *opt,
3990
opt->buffer_output = 0;
3991
}
3992
3993
+void init_ui_merge_options(struct merge_options *opt,
3994
+ struct repository *repo)
3995
+{
3996
+ init_merge_options(opt, repo, 1);
3997
+}
3998
+
3999
+void init_basic_merge_options(struct merge_options *opt,
4000
+ struct repository *repo)
4001
+{
4002
+ init_merge_options(opt, repo, 0);
4003
+}
4004
+
4005
/*
4006
* For now, members of merge_options do not need deep copying, but
4007
* it may change in the future, in which case we would need to update
merge-recursive.h
+4
-1
@@ -54,7 +54,10 @@ struct merge_options {
54
struct merge_options_internal *priv;
55
};
56
57
-void init_merge_options(struct merge_options *opt, struct repository *repo);
57
+/* for use by porcelain commands */
58
+void init_ui_merge_options(struct merge_options *opt, struct repository *repo);
59
+/* for use by plumbing commands */
60
+void init_basic_merge_options(struct merge_options *opt, struct repository *repo);
61
62
void copy_merge_options(struct merge_options *dst, struct merge_options *src);
63
void clear_merge_options(struct merge_options *opt);
sequencer.c
+2
-2
@@ -762,7 +762,7 @@ static int do_recursive_merge(struct repository *r,
762
763
repo_read_index(r);
764
765
- init_merge_options(&o, r);
765
+ init_ui_merge_options(&o, r);
766
o.ancestor = base ? base_label : "(empty tree)";
767
o.branch1 = "HEAD";
768
o.branch2 = next ? next_label : "(empty tree)";
@@ -4309,7 +4309,7 @@ static int do_merge(struct repository *r,
4309
bases = reverse_commit_list(bases);
4310
4311
repo_read_index(r);
4312
- init_merge_options(&o, r);
4312
+ init_ui_merge_options(&o, r);
4313
o.branch1 = "HEAD";
4314
o.branch2 = ref_name.buf;
4315
o.buffer_output = 2;
t/t7615-diff-algo-with-mergy-operations.sh
new
+60
@@ -0,0 +1,60 @@
1
+#!/bin/sh
2
+
3
+test_description='git merge and other operations that rely on merge
4
+
5
+Testing the influence of the diff algorithm on the merge output.'
6
+
7
+TEST_PASSES_SANITIZE_LEAK=true
8
+. ./test-lib.sh
9
+
10
+test_expect_success 'setup' '
11
+ cp "$TEST_DIRECTORY"/t7615/base.c file.c &&
12
+ git add file.c &&
13
+ git commit -m c0 &&
14
+ git tag c0 &&
15
+ cp "$TEST_DIRECTORY"/t7615/ours.c file.c &&
16
+ git add file.c &&
17
+ git commit -m c1 &&
18
+ git tag c1 &&
19
+ git reset --hard c0 &&
20
+ cp "$TEST_DIRECTORY"/t7615/theirs.c file.c &&
21
+ git add file.c &&
22
+ git commit -m c2 &&
23
+ git tag c2
24
+'
25
+
26
+GIT_TEST_MERGE_ALGORITHM=recursive
27
+
28
+test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
29
+ git reset --hard c1 &&
30
+ test_must_fail git merge -s recursive c2
31
+'
32
+
33
+test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '
34
+ git reset --hard c1 &&
35
+ git merge --strategy recursive -Xdiff-algorithm=histogram c2
36
+'
37
+
38
+test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with diff.algorithm = histogram' '
39
+ git reset --hard c1 &&
40
+ git config diff.algorithm histogram &&
41
+ git merge --strategy recursive c2
42
+'
43
+
44
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
45
+ git reset --hard c1 &&
46
+ test_must_fail git cherry-pick -s recursive c2
47
+'
48
+
49
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '
50
+ git reset --hard c1 &&
51
+ git cherry-pick --strategy recursive -Xdiff-algorithm=histogram c2
52
+'
53
+
54
+test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy succeeds with diff.algorithm = histogram' '
55
+ git reset --hard c1 &&
56
+ git config diff.algorithm histogram &&
57
+ git cherry-pick --strategy recursive c2
58
+'
59
+
60
+test_done
t/t7615/base.c
new
+17
@@ -0,0 +1,17 @@
1
+int f(int x, int y)
2
+{
3
+ if (x == 0)
4
+ {
5
+ return y;
6
+ }
7
+ return x;
8
+}
9
+
10
+int g(size_t u)
11
+{
12
+ while (u < 30)
13
+ {
14
+ u++;
15
+ }
16
+ return u;
17
+}
t/t7615/ours.c
new
+17
@@ -0,0 +1,17 @@
1
+int g(size_t u)
2
+{
3
+ while (u < 30)
4
+ {
5
+ u++;
6
+ }
7
+ return u;
8
+}
9
+
10
+int h(int x, int y, int z)
11
+{
12
+ if (z == 0)
13
+ {
14
+ return x;
15
+ }
16
+ return y;
17
+}
t/t7615/theirs.c
new
+17
@@ -0,0 +1,17 @@
1
+int f(int x, int y)
2
+{
3
+ if (x == 0)
4
+ {
5
+ return y;
6
+ }
7
+ return x;
8
+}
9
+
10
+int g(size_t u)
11
+{
12
+ while (u > 34)
13
+ {
14
+ u--;
15
+ }
16
+ return u;
17
+}