We have a -s ours, but not a -s theirs. This is a WIP patch to implement that. It works, but I haven't dealt with this part of the internal API before, comments most welcome. The purpose of this is that I'm working with a rollout tool that is capable of doing hotfixes on top of old commits on "master". It does this by cherry-picking a commit from origin/master, and then merges it with origin/master & pushes it back, before finally reset --hard to the cherry-pick & rolling out. The reason it's doing this is to maintain the guarantee that all rolled out commits are reachable from "master", and to handle the more general case where original work is made during a hotfix, we don't want to then do a subsequent "normal" rollout and miss the fix. In cases where I detect (by looking at patch-id's) that the only commits that are being hotfixed are those cherry-picked from upstream, I want to fully resolve the merge in favor of @{u}, i.e. end up with the same tree SHA-1. This is the inverse of -s ours, but we have no -s theirs, this implements that. The `-s recursive -X theirs strategy` won't do, because that will just resolve conflicts in favor of @{u}, but will screw up the well-known cases where a merge produces bad results with no conflicts, due to edge cases where patches apply cleanly but produce broken programs. So this can be used as `-s theirs -X 2 @{u}` to implement that. The `-s ours` strategy is then equivalent ot `-s theirs -X 1` (1st commit), and we can do any value of `-X N` for octopus merges. As noted `-s theirs` should be the same as supplying it with `-X 2`, but I haven't implemented that yet. Questions: 1. Should I be calling read-tree here with run_command_v_opt(), or is there some internal API I should be using? 2. Currently merge-ours is just a no-op since we take the current HEAD, but maybe it would be cleaner to implement it in terms of this thing, also to get immediate test coverage for all the -s ours stuff. We'd be reading the tree redundantly into the index, but maybe it's worth it for the coverage... 3. Is there a better name for this than -s theirs? Maybe `-s nth -X N`? diff --git a/.gitignore b/.gitignore index 833ef3b0b7..65d62b191f 100644 --- a/.gitignore +++ b/.gitignore @@ -93,6 +93,7 @@ /git-merge-recursive /git-merge-resolve /git-merge-subtree +/git-merge-theirs /git-mergetool /git-mergetool--lib /git-mktag diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 4a58aad4b8..a84482aafc 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -117,6 +117,12 @@ ours:: branches. Note that this is different from the -Xours option to the 'recursive' merge strategy. +theirs:: + This resolves any number of heads, but the resulting tree of + the merge is always that of the Nth branch head specified via + `-X n`. If it's not specified it'll default ot `-X 2`, + supplying `-X 1` makes this equivalent to the `ours` strategy. + subtree:: This is a modified recursive strategy. When merging trees A and B, if B corresponds to a subtree of A, B is first adjusted to diff --git a/Makefile b/Makefile index f181687250..00ded0c37c 100644 --- a/Makefile +++ b/Makefile @@ -998,6 +998,7 @@ BUILTIN_OBJS += builtin/merge-file.o BUILTIN_OBJS += builtin/merge-index.o BUILTIN_OBJS += builtin/merge-ours.o BUILTIN_OBJS += builtin/merge-recursive.o +BUILTIN_OBJS += builtin/merge-theirs.o BUILTIN_OBJS += builtin/merge-tree.o BUILTIN_OBJS += builtin/mktag.o BUILTIN_OBJS += builtin/mktree.o @@ -2815,6 +2816,7 @@ check-docs:: case "$$v" in \ git-merge-octopus | git-merge-ours | git-merge-recursive | \ git-merge-resolve | git-merge-subtree | \ + git-merge-theirs \ git-fsck-objects | git-init-db | \ git-remote-* | git-stage | \ git-?*--?* ) continue ;; \ diff --git a/builtin.h b/builtin.h index 42378f3aa4..a48eaf3a67 100644 --- a/builtin.h +++ b/builtin.h @@ -187,6 +187,7 @@ extern int cmd_merge_index(int argc, const char **argv, const char *prefix); extern int cmd_merge_ours(int argc, const char **argv, const char *prefix); extern int cmd_merge_file(int argc, const char **argv, const char *prefix); extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix); +extern int cmd_merge_theirs(int argc, const char **argv, const char *prefix); extern int cmd_merge_tree(int argc, const char **argv, const char *prefix); extern int cmd_mktag(int argc, const char **argv, const char *prefix); extern int cmd_mktree(int argc, const char **argv, const char *prefix); diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c index c84c6e05e9..da5ba0b370 100644 --- a/builtin/merge-ours.c +++ b/builtin/merge-ours.c @@ -18,7 +18,6 @@ int cmd_merge_ours(int argc, const char **argv, const char *prefix) { if (argc == 2 && !strcmp(argv[1], "-h")) usage(builtin_merge_ours_usage); - /* * The contents of the current index becomes the tree we * commit. The index must match HEAD, or this merge cannot go diff --git a/builtin/merge-theirs.c b/builtin/merge-theirs.c new file mode 100644 index 0000000000..545680ebc6 --- /dev/null +++ b/builtin/merge-theirs.c @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2018 Ævar Arnfjörð Bjarmason + * + * Resolve the merge by picking the Nth (per -X) parent's tree is our + * new tree. + */ +#include "git-compat-util.h" +#include "builtin.h" +#include "run-command.h" +#include "diff.h" + +static const char builtin_merge_theirs_usage[] = + "git merge-theirs <base>... -- HEAD <remote>..."; + +static void read_tree_hex_oid(const char *hex_oid) +{ + int i = 0; + const char *args[4]; + + args[i++] = "read-tree"; + args[i++] = hex_oid; + args[i] = NULL; + + if (run_command_v_opt(args, RUN_GIT_CMD)) + die(_("read-tree failed")); +} + +int cmd_merge_theirs(int argc, const char **argv, const char *prefix) +{ + const char *mainline_str; + const int argc_offset = 3; + char *end; + int mainline; + const char *branch; + struct object_id commit; + + if (argc == 2 && !strcmp(argv[1], "-h")) + usage(builtin_merge_theirs_usage); + if (argc < 6) + usage(builtin_merge_theirs_usage); + + /* + * Parse the --N part of `git merge-theirs --N base -- HEAD + * other-branch [other-branch-2 ...]`. + * + * TODO: Handle no -X argument being equivalent to -X 2. + */ + mainline_str = argv[1]; + if (!mainline_str[2]) + usage(builtin_merge_theirs_usage); + mainline = strtol(mainline_str + 2, &end, 10); + if (*end || mainline <= 0) + die(_("'-s theirs -X N' expects a number greater than zero")); + if (mainline >= (argc - argc_offset)) + die(_("'-s theirs -X N' must come with a corresponding Nth commit to merge!")); + + /* Have the branch name */ + branch = argv[argc_offset + mainline]; + if (get_oid(branch, &commit)) + die(_("could not resolve ref '%s'"), branch); + + /* Final sanity checks, copied from merge-ours.c */ + if (read_cache() < 0) + die_errno("read_cache failed"); + if (index_differs_from("HEAD", NULL, 0)) + exit(2); + + /* Read the Nth tree */ + read_tree_hex_oid(oid_to_hex(&commit)); + + exit(0); +} diff --git a/builtin/merge.c b/builtin/merge.c index 9db5a2cf16..dd623530d8 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -81,6 +81,7 @@ static struct strategy all_strategy[] = { { "octopus", DEFAULT_OCTOPUS }, { "resolve", 0 }, { "ours", NO_FAST_FORWARD | NO_TRIVIAL }, + { "theirs", NO_FAST_FORWARD | NO_TRIVIAL }, { "subtree", NO_FAST_FORWARD | NO_TRIVIAL }, }; diff --git a/git.c b/git.c index 3a89893712..8e87accbb9 100644 --- a/git.c +++ b/git.c @@ -434,6 +434,7 @@ static struct cmd_struct commands[] = { { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT }, + { "merge-theirs", cmd_merge_theirs, RUN_SETUP | NO_PARSEOPT }, { "merge-tree", cmd_merge_tree, RUN_SETUP | NO_PARSEOPT }, { "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT }, { "mktree", cmd_mktree, RUN_SETUP },