[RFC WIP PATCH] merge: implement -s theirs -X N

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 },



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux