@matheusbsbs / matheus-bagliano / commits / 14c4586c2d

merge,rebase,revert: select ort or recursive by config or environment

Allow the testsuite to run where it treats requests for "recursive" or the default merge algorithm via consulting the environment variable GIT_TEST_MERGE_ALGORITHM which is expected to either be "recursive" (the old traditional algorithm) or "ort" (the new algorithm). Also, allow folks to pick the new algorithm via config setting. It turns out builtin/merge.c already had a way to allow users to specify a different default merge algorithm: pull.twohead. Rather odd configuration name (especially to be in the 'pull' namespace rather than 'merge') but it's there. Add that same configuration to rebase, cherry-pick, and revert. This required updating the various callsites that called merge_trees() or merge_recursive() to conditionally call the new API, so this serves as another demonstration of what the new API looks and feels like. There are almost certainly some callsites that have not yet been modified to work with the new merge algorithm, but this represents the ones that I have been testing with thus far. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Nov 2, 2020 at 23:45 UTC 14c4586c2dfa94d86d71a60481dd20bc5b56e562

5 files changed +104 -15

builtin/merge.c

+23 -3

@@ -28,6 +28,7 @@

28 #include "rerere.h"

29 #include "help.h"

30 #include "merge-recursive.h"

31 +#include "merge-ort-wrappers.h"

32 #include "resolve-undo.h"

33 #include "remote.h"

34 #include "fmt-merge-msg.h"

@@ -88,6 +89,7 @@ static int no_verify;

89 static struct strategy all_strategy[] = {

90 { "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },

91 { "octopus", DEFAULT_OCTOPUS },

92 + { "ort", NO_TRIVIAL },

93 { "resolve", 0 },

94 { "ours", NO_FAST_FORWARD | NO_TRIVIAL },

95 { "subtree", NO_FAST_FORWARD | NO_TRIVIAL },

@@ -159,10 +161,17 @@ static struct strategy *get_strategy(const char *name)

161 struct strategy *ret;

162 static struct cmdnames main_cmds, other_cmds;

163 static int loaded;

164 + char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");

165

166 if (!name)

167 return NULL;

168

169 + if (default_strategy &&

170 + !strcmp(default_strategy, "ort") &&

171 + !strcmp(name, "recursive")) {

172 + name = "ort";

173 + }

174 +

175 for (i = 0; i < ARRAY_SIZE(all_strategy); i++)

176 if (!strcmp(name, all_strategy[i].name))

177 return &all_strategy[i];

@@ -701,7 +710,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,

710 if (refresh_and_write_cache(REFRESH_QUIET, SKIP_IF_UNCHANGED, 0) < 0)

711 return error(_("Unable to write index."));

712

704 - if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {

713 + if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree") ||

714 + !strcmp(strategy, "ort")) {

715 struct lock_file lock = LOCK_INIT;

716 int clean, x;

717 struct commit *result;

@@ -732,8 +742,12 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,

742 commit_list_insert(j->item, &reversed);

743

744 hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

735 - clean = merge_recursive(&o, head,

736 - remoteheads->item, reversed, &result);

745 + if (!strcmp(strategy, "ort"))

746 + clean = merge_ort_recursive(&o, head, remoteheads->item,

747 + reversed, &result);

748 + else

749 + clean = merge_recursive(&o, head, remoteheads->item,

750 + reversed, &result);

751 if (clean < 0)

752 exit(128);

753 if (write_locked_index(&the_index, &lock,

@@ -1264,6 +1278,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)

1278 if (branch)

1279 skip_prefix(branch, "refs/heads/", &branch);

1280

1281 + if (!pull_twohead) {

1282 + char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");

1283 + if (default_strategy && !strcmp(default_strategy, "ort"))

1284 + pull_twohead = "ort";

1285 + }

1286 +

1287 init_diff_ui_defaults();

1288 git_config(git_merge_config, NULL);

1289

builtin/rebase.c

+12 -1

@@ -119,6 +119,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)

119 struct replay_opts replay = REPLAY_OPTS_INIT;

120

121 replay.action = REPLAY_INTERACTIVE_REBASE;

122 + replay.strategy = NULL;

123 sequencer_init_config(&replay);

124

125 replay.signoff = opts->signoff;

@@ -136,7 +137,12 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)

137 opts->committer_date_is_author_date;

138 replay.ignore_date = opts->ignore_date;

139 replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);

139 - replay.strategy = opts->strategy;

140 + if (opts->strategy)

141 + replay.strategy = opts->strategy;

142 + else if (!replay.strategy && replay.default_strategy) {

143 + replay.strategy = replay.default_strategy;

144 + replay.default_strategy = NULL;

145 + }

146

147 if (opts->strategy_opts)

148 parse_strategy_opts(&replay, opts->strategy_opts);

@@ -1771,6 +1777,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)

1777 options.default_backend);

1778 }

1779

1780 + if (options.type == REBASE_MERGE &&

1781 + !options.strategy &&

1782 + getenv("GIT_TEST_MERGE_ALGORITHM"))

1783 + options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));

1784 +

1785 switch (options.type) {

1786 case REBASE_MERGE:

1787 case REBASE_PRESERVE_MERGES:

builtin/revert.c

@@ -172,6 +172,11 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)

172 NULL);

173 }

174

175 + if (!opts->strategy && opts->default_strategy) {

176 + opts->strategy = opts->default_strategy;

177 + opts->default_strategy = NULL;

178 + }

179 +

180 if (opts->allow_ff)

181 verify_opt_compatible(me, "--ff",

182 "--signoff", opts->signoff,

@@ -202,6 +207,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)

207 /* These option values will be free()d */

208 opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);

209 opts->strategy = xstrdup_or_null(opts->strategy);

210 + if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))

211 + opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));

212

213 if (cmd == 'q') {

214 int ret = sequencer_remove_state(opts);

sequencer.c

+61 -11

@@ -14,7 +14,8 @@

14 #include "diff.h"

15 #include "revision.h"

16 #include "rerere.h"

17 -#include "merge-recursive.h"

17 +#include "merge-ort.h"

18 +#include "merge-ort-wrappers.h"

19 #include "refs.h"

20 #include "strvec.h"

21 #include "quote.h"

@@ -204,6 +205,20 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)

205 return 0;

206 }

207

208 + if (!opts->default_strategy && !strcmp(k, "pull.twohead")) {

209 + int ret = git_config_string((const char**)&opts->default_strategy, k, v);

210 + if (ret == 0) {

211 + /*

212 + * pull.twohead is allowed to be multi-valued; we only

213 + * care about the first value.

214 + */

215 + char *tmp = strchr(opts->default_strategy, ' ');

216 + if (tmp)

217 + *tmp = '\0';

218 + }

219 + return ret;

220 + }

221 +

222 status = git_gpg_config(k, v, NULL);

223 if (status)

224 return status;

@@ -317,6 +332,7 @@ int sequencer_remove_state(struct replay_opts *opts)

332 free(opts->committer_name);

333 free(opts->committer_email);

334 free(opts->gpg_sign);

335 + free(opts->default_strategy);

336 free(opts->strategy);

337 for (i = 0; i < opts->xopts_nr; i++)

338 free(opts->xopts[i]);

@@ -595,8 +611,9 @@ static int do_recursive_merge(struct repository *r,

611 struct replay_opts *opts)

612 {

613 struct merge_options o;

614 + struct merge_result result;

615 struct tree *next_tree, *base_tree, *head_tree;

599 - int clean;

616 + int clean, show_output;

617 int i;

618 struct lock_file index_lock = LOCK_INIT;

619

@@ -620,12 +637,27 @@ static int do_recursive_merge(struct repository *r,

637 for (i = 0; i < opts->xopts_nr; i++)

638 parse_merge_opt(&o, opts->xopts[i]);

639

623 - clean = merge_trees(&o,

624 - head_tree,

625 - next_tree, base_tree);

626 - if (is_rebase_i(opts) && clean <= 0)

627 - fputs(o.obuf.buf, stdout);

628 - strbuf_release(&o.obuf);

640 + if (opts->strategy && !strcmp(opts->strategy, "ort")) {

641 + memset(&result, 0, sizeof(result));

642 + merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,

643 + &result);

644 + show_output = !is_rebase_i(opts) || !result.clean;

645 + /*

646 + * TODO: merge_switch_to_result will update index/working tree;

647 + * we only really want to do that if !result.clean || this is

648 + * the final patch to be picked. But determining this is the

649 + * final patch would take some work, and "head_tree" would need

650 + * to be replace with the tree the index matched before we

651 + * started doing any picks.

652 + */

653 + merge_switch_to_result(&o, head_tree, &result, 1, show_output);

654 + clean = result.clean;

655 + } else {

656 + clean = merge_trees(&o, head_tree, next_tree, base_tree);

657 + if (is_rebase_i(opts) && clean <= 0)

658 + fputs(o.obuf.buf, stdout);

659 + strbuf_release(&o.obuf);

660 + }

661 if (clean < 0) {

662 rollback_lock_file(&index_lock);

663 return clean;

@@ -1991,7 +2023,10 @@ static int do_pick_commit(struct repository *r,

2023

2024 if (is_rebase_i(opts) && write_author_script(msg.message) < 0)

2025 res = -1;

1994 - else if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {

2026 + else if (!opts->strategy ||

2027 + !strcmp(opts->strategy, "recursive") ||

2028 + !strcmp(opts->strategy, "ort") ||

2029 + command == TODO_REVERT) {

2030 res = do_recursive_merge(r, base, next, base_label, next_label,

2031 &head, &msgbuf, opts);

2032 if (res < 0)

@@ -3485,7 +3520,9 @@ static int do_merge(struct repository *r,

3520 struct commit_list *bases, *j, *reversed = NULL;

3521 struct commit_list *to_merge = NULL, **tail = &to_merge;

3522 const char *strategy = !opts->xopts_nr &&

3488 - (!opts->strategy || !strcmp(opts->strategy, "recursive")) ?

3523 + (!opts->strategy ||

3524 + !strcmp(opts->strategy, "recursive") ||

3525 + !strcmp(opts->strategy, "ort")) ?

3526 NULL : opts->strategy;

3527 struct merge_options o;

3528 int merge_arg_len, oneline_offset, can_fast_forward, ret, k;

@@ -3722,7 +3759,20 @@ static int do_merge(struct repository *r,

3759 o.branch2 = ref_name.buf;

3760 o.buffer_output = 2;

3761

3725 - ret = merge_recursive(&o, head_commit, merge_commit, reversed, &i);

3762 + if (opts->strategy && !strcmp(opts->strategy, "ort")) {

3763 + /*

3764 + * TODO: Should use merge_incore_recursive() and

3765 + * merge_switch_to_result(), skipping the call to

3766 + * merge_switch_to_result() when we don't actually need to

3767 + * update the index and working copy immediately.

3768 + */

3769 + ret = merge_ort_recursive(&o,

3770 + head_commit, merge_commit, reversed,

3771 + &i);

3772 + } else {

3773 + ret = merge_recursive(&o, head_commit, merge_commit, reversed,

3774 + &i);

3775 + }

3776 if (ret <= 0)

3777 fputs(o.obuf.buf, stdout);

3778 strbuf_release(&o.obuf);

sequencer.h

@@ -57,6 +57,7 @@ struct replay_opts {

57 int explicit_cleanup;

59 /* Merge strategy */

60 + char *default_strategy; /* from config options */

61 char *strategy;

62 char **xopts;

63 size_t xopts_nr, xopts_alloc;