[PATCH] Converted git-merge-ours.sh -> builtin-merge-ours.c

[PATCH] Converted git-merge-ours.sh -> builtin-merge-ours.c

[Thomas Harning]

Here's a simple patch to make git-merge-ours.sh into a builtin.

I figure this would be a simple way of getting in the git-development flow.

Signed-off-by: Thomas Harning Jr <harningt@gmail.com>
---
 Makefile             |    3 ++-
 builtin-merge-ours.c |   32 ++++++++++++++++++++++++++++++++
 builtin.h            |    1 +
 git-merge-ours.sh    |   14 --------------
 git.c                |    1 +
 5 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 builtin-merge-ours.c
 delete mode 100755 git-merge-ours.sh

diff --git a/Makefile b/Makefile
index cabde81..7a0ee78 100644
--- a/Makefile
+++ b/Makefile
@@ -221,7 +221,7 @@ SCRIPT_SH = \
 	git-sh-setup.sh \
 	git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
-	git-merge-resolve.sh git-merge-ours.sh \
+	git-merge-resolve.sh \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
 	git-stash.sh
@@ -353,6 +353,7 @@ BUILTIN_OBJS = \
 	builtin-mailsplit.o \
 	builtin-merge-base.o \
 	builtin-merge-file.o \
+	builtin-merge-ours.o \
 	builtin-mv.o \
 	builtin-name-rev.o \
 	builtin-pack-objects.o \
diff --git a/builtin-merge-ours.c b/builtin-merge-ours.c
new file mode 100644
index 0000000..fbfe183
--- /dev/null
+++ b/builtin-merge-ours.c
@@ -0,0 +1,32 @@
+/*
+ * Implementation of git-merge-ours.sh as builtin
+ * 
+ * Copyright (c) 2007 Thomas Harning Jr
+ * Original:
+ * Original Copyright (c) 2005 Junio C Hamano
+ *
+ * Pretend we resolved the heads, but declare our tree trumps everybody else.
+ */
+#include "git-compat-util.h"
+#include "builtin.h"
+
+int cmd_merge_ours(int argc, const char **argv, const char *prefix)
+{
+	const char *nargv[] = {
+		"diff-index",
+		"--quiet",
+		"--cached",
+		"HEAD",
+		NULL
+	};
+	int i;
+
+	int ret = cmd_diff_index(4, nargv, prefix);
+	printf("GOT: %i\n", ret);
+	/* We need to exit with 2 if the index does not match our HEAD tree,
+	 * because the current index is what we will be committing as the
+	 * merge result.
+	 */
+	if(ret) ret = 2;
+	return ret;
+}
diff --git a/builtin.h b/builtin.h
index 9a6213a..bcb54aa 100644
--- a/builtin.h
+++ b/builtin.h
@@ -51,6 +51,7 @@ extern int cmd_ls_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_mailinfo(int argc, const char **argv, const char *prefix);
 extern int cmd_mailsplit(int argc, const char **argv, const char *prefix);
 extern int cmd_merge_base(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_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
diff --git a/git-merge-ours.sh b/git-merge-ours.sh
deleted file mode 100755
index c81a790..0000000
--- a/git-merge-ours.sh
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2005 Junio C Hamano
-#
-# Pretend we resolved the heads, but declare our tree trumps everybody else.
-#
-
-# We need to exit with 2 if the index does not match our HEAD tree,
-# because the current index is what we will be committing as the
-# merge result.
-
-git diff-index --quiet --cached HEAD || exit 2
-
-exit 0
diff --git a/git.c b/git.c
index 7604319..80c2f14 100644
--- a/git.c
+++ b/git.c
@@ -325,6 +325,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "mailsplit", cmd_mailsplit },
 		{ "merge-base", cmd_merge_base, RUN_SETUP },
 		{ "merge-file", cmd_merge_file },
+		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 		{ "name-rev", cmd_name_rev, RUN_SETUP },
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-- 
1.5.3.6.861.gd794

Re: [PATCH] Converted git-merge-ours.sh -> builtin-merge-ours.c

[Junio C Hamano]

Thomas Harning <harningt@gmail.com> writes:

> Here's a simple patch to make git-merge-ours.sh into a builtin.
>
> I figure this would be a simple way of getting in the git-development flow.
>
> Signed-off-by: Thomas Harning Jr <harningt@gmail.com>

Have you tried to read this proposed commit log message in the
context of "git log", after applying it on top of 'master'?
"Here's a simple patch to" and "I figure ..." are of not much
use.

The patch looks good except for minor style nits.

> diff --git a/builtin-merge-ours.c b/builtin-merge-ours.c
> new file mode 100644
> index 0000000..fbfe183
> --- /dev/null
> +++ b/builtin-merge-ours.c
> @@ -0,0 +1,32 @@
> +/*
> + * Implementation of git-merge-ours.sh as builtin
> + * 

Traling whitespace.

> +int cmd_merge_ours(int argc, const char **argv, const char *prefix)
> +{
> +	const char *nargv[] = {
> +		"diff-index",
> +		"--quiet",
> +		"--cached",
> +		"HEAD",
> +		NULL
> +	};
> +	int i;
> +
> +	int ret = cmd_diff_index(4, nargv, prefix);
> +	printf("GOT: %i\n", ret);

Unused variable "i".

An unwanted blank line still in the sequence of variable
definitions, and a missing blank line after the definitions.

A leftover debug printf() is not very welcomed.

	[Not a nit but a comment]

        The call to cmd_diff_index() here raised my eyebrow a
	bit.  I would have skipped all the parameter parsing and
	arranged it to directly call into run_diff_index()
	instead.  

	As the result of literally translating the scripted
	version of git-merge-ours, the code inherits a
	corner-case bug.  Can you spot it?

> +	/* We need to exit with 2 if the index does not match our HEAD tree,
> +	 * because the current index is what we will be committing as the
> +	 * merge result.
> +	 */

We tend to format a multi-line comment block as:

	/*
         * We need to ...
         * ... merge result.
         */

> +	if(ret) ret = 2;

A SP between "if" and "("; put the body of the "if" on a
separate line.

Thanks.  No need to resend; all these minor nits can be fixed
here easily.

Re: [PATCH] Converted git-merge-ours.sh -> builtin-merge-ours.c

[Jakub Narebski]

Thomas Harning wrote:

> diff --git a/git-merge-ours.sh b/git-merge-ours.sh
> deleted file mode 100755
> index c81a790..0000000
> --- a/git-merge-ours.sh
> +++ /dev/null

Shouldn't script version be moved to contrib/examples/ as is git.git
policy, or is it too simple script?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git