Skip to content

Commit

Permalink
merge,rebase,revert: select ort or recursive by config or environment
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
newren authored and gitster committed Nov 3, 2020
1 parent fe1a21d commit 14c4586
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 15 deletions.
26 changes: 23 additions & 3 deletions builtin/merge.c
Expand Up @@ -28,6 +28,7 @@
#include "rerere.h"
#include "help.h"
#include "merge-recursive.h"
#include "merge-ort-wrappers.h"
#include "resolve-undo.h"
#include "remote.h"
#include "fmt-merge-msg.h"
Expand Down Expand Up @@ -88,6 +89,7 @@ static int no_verify;
static struct strategy all_strategy[] = {
{ "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },
{ "octopus", DEFAULT_OCTOPUS },
{ "ort", NO_TRIVIAL },
{ "resolve", 0 },
{ "ours", NO_FAST_FORWARD | NO_TRIVIAL },
{ "subtree", NO_FAST_FORWARD | NO_TRIVIAL },
Expand Down Expand Up @@ -159,10 +161,17 @@ static struct strategy *get_strategy(const char *name)
struct strategy *ret;
static struct cmdnames main_cmds, other_cmds;
static int loaded;
char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");

if (!name)
return NULL;

if (default_strategy &&
!strcmp(default_strategy, "ort") &&
!strcmp(name, "recursive")) {
name = "ort";
}

for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
if (!strcmp(name, all_strategy[i].name))
return &all_strategy[i];
Expand Down Expand Up @@ -701,7 +710,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
if (refresh_and_write_cache(REFRESH_QUIET, SKIP_IF_UNCHANGED, 0) < 0)
return error(_("Unable to write index."));

if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree") ||
!strcmp(strategy, "ort")) {
struct lock_file lock = LOCK_INIT;
int clean, x;
struct commit *result;
Expand Down Expand Up @@ -732,8 +742,12 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
commit_list_insert(j->item, &reversed);

hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
clean = merge_recursive(&o, head,
remoteheads->item, reversed, &result);
if (!strcmp(strategy, "ort"))
clean = merge_ort_recursive(&o, head, remoteheads->item,
reversed, &result);
else
clean = merge_recursive(&o, head, remoteheads->item,
reversed, &result);
if (clean < 0)
exit(128);
if (write_locked_index(&the_index, &lock,
Expand Down Expand Up @@ -1264,6 +1278,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (branch)
skip_prefix(branch, "refs/heads/", &branch);

if (!pull_twohead) {
char *default_strategy = getenv("GIT_TEST_MERGE_ALGORITHM");
if (default_strategy && !strcmp(default_strategy, "ort"))
pull_twohead = "ort";
}

init_diff_ui_defaults();
git_config(git_merge_config, NULL);

Expand Down
13 changes: 12 additions & 1 deletion builtin/rebase.c
Expand Up @@ -119,6 +119,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
struct replay_opts replay = REPLAY_OPTS_INIT;

replay.action = REPLAY_INTERACTIVE_REBASE;
replay.strategy = NULL;
sequencer_init_config(&replay);

replay.signoff = opts->signoff;
Expand All @@ -136,7 +137,12 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
opts->committer_date_is_author_date;
replay.ignore_date = opts->ignore_date;
replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
replay.strategy = opts->strategy;
if (opts->strategy)
replay.strategy = opts->strategy;
else if (!replay.strategy && replay.default_strategy) {
replay.strategy = replay.default_strategy;
replay.default_strategy = NULL;
}

if (opts->strategy_opts)
parse_strategy_opts(&replay, opts->strategy_opts);
Expand Down Expand Up @@ -1771,6 +1777,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
options.default_backend);
}

if (options.type == REBASE_MERGE &&
!options.strategy &&
getenv("GIT_TEST_MERGE_ALGORITHM"))
options.strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));

switch (options.type) {
case REBASE_MERGE:
case REBASE_PRESERVE_MERGES:
Expand Down
7 changes: 7 additions & 0 deletions builtin/revert.c
Expand Up @@ -172,6 +172,11 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
NULL);
}

if (!opts->strategy && opts->default_strategy) {
opts->strategy = opts->default_strategy;
opts->default_strategy = NULL;
}

if (opts->allow_ff)
verify_opt_compatible(me, "--ff",
"--signoff", opts->signoff,
Expand Down Expand Up @@ -202,6 +207,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
/* These option values will be free()d */
opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
opts->strategy = xstrdup_or_null(opts->strategy);
if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));

if (cmd == 'q') {
int ret = sequencer_remove_state(opts);
Expand Down
72 changes: 61 additions & 11 deletions sequencer.c
Expand Up @@ -14,7 +14,8 @@
#include "diff.h"
#include "revision.h"
#include "rerere.h"
#include "merge-recursive.h"
#include "merge-ort.h"
#include "merge-ort-wrappers.h"
#include "refs.h"
#include "strvec.h"
#include "quote.h"
Expand Down Expand Up @@ -204,6 +205,20 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
return 0;
}

if (!opts->default_strategy && !strcmp(k, "pull.twohead")) {
int ret = git_config_string((const char**)&opts->default_strategy, k, v);
if (ret == 0) {
/*
* pull.twohead is allowed to be multi-valued; we only
* care about the first value.
*/
char *tmp = strchr(opts->default_strategy, ' ');
if (tmp)
*tmp = '\0';
}
return ret;
}

status = git_gpg_config(k, v, NULL);
if (status)
return status;
Expand Down Expand Up @@ -317,6 +332,7 @@ int sequencer_remove_state(struct replay_opts *opts)
free(opts->committer_name);
free(opts->committer_email);
free(opts->gpg_sign);
free(opts->default_strategy);
free(opts->strategy);
for (i = 0; i < opts->xopts_nr; i++)
free(opts->xopts[i]);
Expand Down Expand Up @@ -595,8 +611,9 @@ static int do_recursive_merge(struct repository *r,
struct replay_opts *opts)
{
struct merge_options o;
struct merge_result result;
struct tree *next_tree, *base_tree, *head_tree;
int clean;
int clean, show_output;
int i;
struct lock_file index_lock = LOCK_INIT;

Expand All @@ -620,12 +637,27 @@ static int do_recursive_merge(struct repository *r,
for (i = 0; i < opts->xopts_nr; i++)
parse_merge_opt(&o, opts->xopts[i]);

clean = merge_trees(&o,
head_tree,
next_tree, base_tree);
if (is_rebase_i(opts) && clean <= 0)
fputs(o.obuf.buf, stdout);
strbuf_release(&o.obuf);
if (opts->strategy && !strcmp(opts->strategy, "ort")) {
memset(&result, 0, sizeof(result));
merge_incore_nonrecursive(&o, base_tree, head_tree, next_tree,
&result);
show_output = !is_rebase_i(opts) || !result.clean;
/*
* TODO: merge_switch_to_result will update index/working tree;
* we only really want to do that if !result.clean || this is
* the final patch to be picked. But determining this is the
* final patch would take some work, and "head_tree" would need
* to be replace with the tree the index matched before we
* started doing any picks.
*/
merge_switch_to_result(&o, head_tree, &result, 1, show_output);
clean = result.clean;
} else {
clean = merge_trees(&o, head_tree, next_tree, base_tree);
if (is_rebase_i(opts) && clean <= 0)
fputs(o.obuf.buf, stdout);
strbuf_release(&o.obuf);
}
if (clean < 0) {
rollback_lock_file(&index_lock);
return clean;
Expand Down Expand Up @@ -1991,7 +2023,10 @@ static int do_pick_commit(struct repository *r,

if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
res = -1;
else if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
else if (!opts->strategy ||
!strcmp(opts->strategy, "recursive") ||
!strcmp(opts->strategy, "ort") ||
command == TODO_REVERT) {
res = do_recursive_merge(r, base, next, base_label, next_label,
&head, &msgbuf, opts);
if (res < 0)
Expand Down Expand Up @@ -3485,7 +3520,9 @@ static int do_merge(struct repository *r,
struct commit_list *bases, *j, *reversed = NULL;
struct commit_list *to_merge = NULL, **tail = &to_merge;
const char *strategy = !opts->xopts_nr &&
(!opts->strategy || !strcmp(opts->strategy, "recursive")) ?
(!opts->strategy ||
!strcmp(opts->strategy, "recursive") ||
!strcmp(opts->strategy, "ort")) ?
NULL : opts->strategy;
struct merge_options o;
int merge_arg_len, oneline_offset, can_fast_forward, ret, k;
Expand Down Expand Up @@ -3722,7 +3759,20 @@ static int do_merge(struct repository *r,
o.branch2 = ref_name.buf;
o.buffer_output = 2;

ret = merge_recursive(&o, head_commit, merge_commit, reversed, &i);
if (opts->strategy && !strcmp(opts->strategy, "ort")) {
/*
* TODO: Should use merge_incore_recursive() and
* merge_switch_to_result(), skipping the call to
* merge_switch_to_result() when we don't actually need to
* update the index and working copy immediately.
*/
ret = merge_ort_recursive(&o,
head_commit, merge_commit, reversed,
&i);
} else {
ret = merge_recursive(&o, head_commit, merge_commit, reversed,
&i);
}
if (ret <= 0)
fputs(o.obuf.buf, stdout);
strbuf_release(&o.obuf);
Expand Down
1 change: 1 addition & 0 deletions sequencer.h
Expand Up @@ -57,6 +57,7 @@ struct replay_opts {
int explicit_cleanup;

/* Merge strategy */
char *default_strategy; /* from config options */
char *strategy;
char **xopts;
size_t xopts_nr, xopts_alloc;
Expand Down

0 comments on commit 14c4586

Please sign in to comment.