[PATCH 1/5] Add test case for basic commit functionality.

[PATCH 1/5] Add test case for basic commit functionality.

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 t/t7501-commit.sh |  126 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 126 insertions(+), 0 deletions(-)
 create mode 100644 t/t7501-commit.sh

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
new file mode 100644
index 0000000..eba19da
--- /dev/null
+++ b/t/t7501-commit.sh
@@ -0,0 +1,126 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+#
+
+# FIXME: Test the various index usages, -i and -o, test reflog,
+# signoff, hooks
+
+test_description='git-commit'
+. ./test-lib.sh
+
+# Pick a date so we get consistent commits. 7/7/07 means good luck!
+export GIT_AUTHOR_DATE="July 7, 2007"
+export GIT_COMMITTER_DATE="July 7, 2007"
+
+echo "bongo bongo" >file
+test_expect_success \
+	"initial status" \
+	"git-add file && \
+	 git-status | grep 'Initial commit'"
+
+test_expect_failure \
+	"fail initial amend" \
+	"git-commit -m initial --amend"
+
+test_expect_success \
+	"initial commit" \
+	"git-commit -m initial"
+
+test_expect_failure \
+	"testing nothing to commit" \
+	"git-commit -m initial"
+
+echo "bongo bongo bongo" >file
+
+test_expect_success \
+	"next commit" \
+	"git-commit -m next -a"
+
+echo "more bongo: bongo bongo bongo bongo" >file
+
+test_expect_failure \
+	"commit message from non-existing file" \
+	"git-commit -F gah -a"
+
+cat >msg <<EOF
+		
+
+  
+Signed-off-by: hula
+EOF
+test_expect_failure \
+	"empty commit message" \
+	"git-commit -F msg -a"
+
+echo "this is the commit message, coming from a file" >msg
+test_expect_success \
+	"commit message from file" \
+	"git-commit -F msg -a"
+
+cat >editor <<\EOF
+#!/bin/sh
+sed -i -e "s/a file/an amend commit/g" $1
+EOF
+chmod 755 editor
+
+test_expect_success \
+	"amend commit" \
+	"VISUAL=./editor git-commit --amend"
+
+echo "enough with the bongos" >file
+test_expect_failure \
+	"passing --amend and -F" \
+	"git-commit -F msg --amend ."
+
+test_expect_success \
+	"using message from other commit" \
+	"git-commit -C HEAD^ ."
+
+cat >editor <<\EOF
+#!/bin/sh
+sed -i -e "s/amend/older/g" $1
+EOF
+chmod 755 editor
+
+echo "hula hula" >file
+test_expect_success \
+	"editing message from other commit" \
+	"VISUAL=./editor git-commit -c HEAD^ -a"
+
+echo "silly new contents" >file
+test_expect_success \
+	"message from stdin" \
+	"echo commit message from stdin | git-commit -F - -a"
+
+echo "gak" >file
+test_expect_success \
+	"overriding author from command line" \
+	"git-commit -m 'author' --author 'Rubber Duck <rduck@convoy.org>' -a"
+
+test_expect_success \
+	"interactive add" \
+	"echo 7 | git-commit --interactive | grep 'What now'"
+
+test_expect_success \
+	"showing committed revisions" \
+	"git-rev-list HEAD >current"
+
+# We could just check the head sha1, but checking each commit makes it
+# easier to isolate bugs.
+
+cat >expected <<\EOF
+b19013342b676054179a1685d62b07f56b354331
+763fd16cd476920986129b09672915be44847d90
+b97ce6debb52cbb541d798ce4c2cefa3c9f20332
+720cebf2e443885c6325a0a602ddb9922376452c
+19db513255cc049feee9c107e60297a48c6b0df4
+4cd324560cd9dcac18f17bac227b17238ad458f9
+821776fc6a0699927268feb5e157d245cdcd102a
+EOF
+
+test_expect_success \
+    'validate git-rev-list output.' \
+    'diff current expected'
+
+test_done
-- 
1.5.2.GIT

[PATCH 2/5] Enable wt-status output to a given FILE pointer.

[Kristian Høgsberg]

From: Kristian Høgsberg <krh@redhat.com>

Still defaults to stdout, but you can now override wt_status.fp after
calling wt_status_prepare().

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 color.c     |   18 ++++++------
 color.h     |    4 +-
 wt-status.c |   85 ++++++++++++++++++++++++++++++----------------------------
 wt-status.h |    3 ++
 4 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/color.c b/color.c
index 09d82ee..124ba33 100644
--- a/color.c
+++ b/color.c
@@ -135,39 +135,39 @@ int git_config_colorbool(const char *var, const char *value)
 	return git_config_bool(var, value);
 }
 
-static int color_vprintf(const char *color, const char *fmt,
+static int color_vfprintf(FILE *fp, const char *color, const char *fmt,
 		va_list args, const char *trail)
 {
 	int r = 0;
 
 	if (*color)
-		r += printf("%s", color);
-	r += vprintf(fmt, args);
+		r += fprintf(fp, "%s", color);
+	r += vfprintf(fp, fmt, args);
 	if (*color)
-		r += printf("%s", COLOR_RESET);
+		r += fprintf(fp, "%s", COLOR_RESET);
 	if (trail)
-		r += printf("%s", trail);
+		r += fprintf(fp, "%s", trail);
 	return r;
 }
 
 
 
-int color_printf(const char *color, const char *fmt, ...)
+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...)
 {
 	va_list args;
 	int r;
 	va_start(args, fmt);
-	r = color_vprintf(color, fmt, args, NULL);
+	r = color_vfprintf(fp, color, fmt, args, NULL);
 	va_end(args);
 	return r;
 }
 
-int color_printf_ln(const char *color, const char *fmt, ...)
+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
 {
 	va_list args;
 	int r;
 	va_start(args, fmt);
-	r = color_vprintf(color, fmt, args, "\n");
+	r = color_vfprintf(fp, color, fmt, args, "\n");
 	va_end(args);
 	return r;
 }
diff --git a/color.h b/color.h
index 88bb8ff..6809800 100644
--- a/color.h
+++ b/color.h
@@ -6,7 +6,7 @@
 
 int git_config_colorbool(const char *var, const char *value);
 void color_parse(const char *var, const char *value, char *dst);
-int color_printf(const char *color, const char *fmt, ...);
-int color_printf_ln(const char *color, const char *fmt, ...);
+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
 
 #endif /* COLOR_H */
diff --git a/wt-status.c b/wt-status.c
index 5205420..65a7259 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -52,31 +52,33 @@ void wt_status_prepare(struct wt_status *s)
 	head = resolve_ref("HEAD", sha1, 0, NULL);
 	s->branch = head ? xstrdup(head) : NULL;
 	s->reference = "HEAD";
+	s->fp = stdout;
 }
 
-static void wt_status_print_cached_header(const char *reference)
+static void wt_status_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER);
-	color_printf_ln(c, "# Changes to be committed:");
-	if (reference) {
-		color_printf_ln(c, "#   (use \"git reset %s <file>...\" to unstage)", reference);
+	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
+	if (s->reference) {
+		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
 	} else {
-		color_printf_ln(c, "#   (use \"git rm --cached <file>...\" to unstage)");
+		color_fprintf_ln(s->fp, c, "#   (use \"git rm --cached <file>...\" to unstage)");
 	}
-	color_printf_ln(c, "#");
+	color_fprintf_ln(s->fp, c, "#");
 }
 
-static void wt_status_print_header(const char *main, const char *sub)
+static void wt_status_print_header(struct wt_status *s,
+				   const char *main, const char *sub)
 {
 	const char *c = color(WT_STATUS_HEADER);
-	color_printf_ln(c, "# %s:", main);
-	color_printf_ln(c, "#   (%s)", sub);
-	color_printf_ln(c, "#");
+	color_fprintf_ln(s->fp, c, "# %s:", main);
+	color_fprintf_ln(s->fp, c, "#   (%s)", sub);
+	color_fprintf_ln(s->fp, c, "#");
 }
 
-static void wt_status_print_trailer(void)
+static void wt_status_print_trailer(struct wt_status *s)
 {
-	color_printf_ln(color(WT_STATUS_HEADER), "#");
+	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
 }
 
 static const char *quote_crlf(const char *in, char *buf, size_t sz)
@@ -108,7 +110,8 @@ static const char *quote_crlf(const char *in, char *buf, size_t sz)
 	return ret;
 }
 
-static void wt_status_print_filepair(int t, struct diff_filepair *p)
+static void wt_status_print_filepair(struct wt_status *s,
+				     int t, struct diff_filepair *p)
 {
 	const char *c = color(t);
 	const char *one, *two;
@@ -117,36 +120,36 @@ static void wt_status_print_filepair(int t, struct diff_filepair *p)
 	one = quote_crlf(p->one->path, onebuf, sizeof(onebuf));
 	two = quote_crlf(p->two->path, twobuf, sizeof(twobuf));
 
-	color_printf(color(WT_STATUS_HEADER), "#\t");
+	color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
 	switch (p->status) {
 	case DIFF_STATUS_ADDED:
-		color_printf(c, "new file:   %s", one);
+		color_fprintf(s->fp, c, "new file:   %s", one);
 		break;
 	case DIFF_STATUS_COPIED:
-		color_printf(c, "copied:     %s -> %s", one, two);
+		color_fprintf(s->fp, c, "copied:     %s -> %s", one, two);
 		break;
 	case DIFF_STATUS_DELETED:
-		color_printf(c, "deleted:    %s", one);
+		color_fprintf(s->fp, c, "deleted:    %s", one);
 		break;
 	case DIFF_STATUS_MODIFIED:
-		color_printf(c, "modified:   %s", one);
+		color_fprintf(s->fp, c, "modified:   %s", one);
 		break;
 	case DIFF_STATUS_RENAMED:
-		color_printf(c, "renamed:    %s -> %s", one, two);
+		color_fprintf(s->fp, c, "renamed:    %s -> %s", one, two);
 		break;
 	case DIFF_STATUS_TYPE_CHANGED:
-		color_printf(c, "typechange: %s", one);
+		color_fprintf(s->fp, c, "typechange: %s", one);
 		break;
 	case DIFF_STATUS_UNKNOWN:
-		color_printf(c, "unknown:    %s", one);
+		color_fprintf(s->fp, c, "unknown:    %s", one);
 		break;
 	case DIFF_STATUS_UNMERGED:
-		color_printf(c, "unmerged:   %s", one);
+		color_fprintf(s->fp, c, "unmerged:   %s", one);
 		break;
 	default:
 		die("bug: unhandled diff status %c", p->status);
 	}
-	printf("\n");
+	fprintf(s->fp, "\n");
 }
 
 static void wt_status_print_updated_cb(struct diff_queue_struct *q,
@@ -160,14 +163,14 @@ static void wt_status_print_updated_cb(struct diff_queue_struct *q,
 		if (q->queue[i]->status == 'U')
 			continue;
 		if (!shown_header) {
-			wt_status_print_cached_header(s->reference);
+			wt_status_print_cached_header(s);
 			s->commitable = 1;
 			shown_header = 1;
 		}
-		wt_status_print_filepair(WT_STATUS_UPDATED, q->queue[i]);
+		wt_status_print_filepair(s, WT_STATUS_UPDATED, q->queue[i]);
 	}
 	if (shown_header)
-		wt_status_print_trailer();
+		wt_status_print_trailer(s);
 }
 
 static void wt_status_print_changed_cb(struct diff_queue_struct *q,
@@ -184,12 +187,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
 				msg = use_add_rm_msg;
 				break;
 			}
-		wt_status_print_header("Changed but not updated", msg);
+		wt_status_print_header(s, "Changed but not updated", msg);
 	}
 	for (i = 0; i < q->nr; i++)
-		wt_status_print_filepair(WT_STATUS_CHANGED, q->queue[i]);
+		wt_status_print_filepair(s, WT_STATUS_CHANGED, q->queue[i]);
 	if (q->nr)
-		wt_status_print_trailer();
+		wt_status_print_trailer(s);
 }
 
 static void wt_read_cache(struct wt_status *s)
@@ -206,16 +209,16 @@ static void wt_status_print_initial(struct wt_status *s)
 	wt_read_cache(s);
 	if (active_nr) {
 		s->commitable = 1;
-		wt_status_print_cached_header(NULL);
+		wt_status_print_cached_header(s);
 	}
 	for (i = 0; i < active_nr; i++) {
-		color_printf(color(WT_STATUS_HEADER), "#\t");
-		color_printf_ln(color(WT_STATUS_UPDATED), "new file: %s",
+		color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
+		color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
 				quote_crlf(active_cache[i]->name,
 					   buf, sizeof(buf)));
 	}
 	if (active_nr)
-		wt_status_print_trailer();
+		wt_status_print_trailer(s);
 }
 
 static void wt_status_print_updated(struct wt_status *s)
@@ -281,12 +284,12 @@ static void wt_status_print_untracked(struct wt_status *s)
 		}
 		if (!shown_header) {
 			s->workdir_untracked = 1;
-			wt_status_print_header("Untracked files",
+			wt_status_print_header(s, "Untracked files",
 					       use_add_to_include_msg);
 			shown_header = 1;
 		}
-		color_printf(color(WT_STATUS_HEADER), "#\t");
-		color_printf_ln(color(WT_STATUS_UNTRACKED), "%.*s",
+		color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
+		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), "%.*s",
 				ent->len, ent->name);
 	}
 }
@@ -316,14 +319,14 @@ void wt_status_print(struct wt_status *s)
 			branch_name = "";
 			on_what = "Not currently on any branch.";
 		}
-		color_printf_ln(color(WT_STATUS_HEADER),
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER),
 			"# %s%s", on_what, branch_name);
 	}
 
 	if (s->is_initial) {
-		color_printf_ln(color(WT_STATUS_HEADER), "#");
-		color_printf_ln(color(WT_STATUS_HEADER), "# Initial commit");
-		color_printf_ln(color(WT_STATUS_HEADER), "#");
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "# Initial commit");
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
 		wt_status_print_initial(s);
 	}
 	else {
@@ -337,7 +340,7 @@ void wt_status_print(struct wt_status *s)
 		wt_status_print_verbose(s);
 	if (!s->commitable) {
 		if (s->amend)
-			printf("# No changes\n");
+			fprintf(s->fp, "# No changes\n");
 		else if (s->workdir_dirty)
 			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
 		else if (s->workdir_untracked)
diff --git a/wt-status.h b/wt-status.h
index cfea4ae..4f3a615 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -1,6 +1,8 @@
 #ifndef STATUS_H
 #define STATUS_H
 
+#include <stdio.h>
+
 enum color_wt_status {
 	WT_STATUS_HEADER,
 	WT_STATUS_UPDATED,
@@ -19,6 +21,7 @@ struct wt_status {
 	int commitable;
 	int workdir_dirty;
 	int workdir_untracked;
+	FILE *fp;
 };
 
 int git_status_config(const char *var, const char *value);
-- 
1.5.2.GIT

[PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Kristian Høgsberg]

From: Kristian Høgsberg <krh@redhat.com>

Also, expose strbuf_add() and strbuf_add_char() to add raw data to the buffer.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 strbuf.c |   37 +++++++++++++++++++++++++++++++------
 strbuf.h |    3 +++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index e33d06b..a0539db 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -11,16 +11,28 @@ static void strbuf_begin(struct strbuf *sb) {
 	strbuf_init(sb);
 }
 
-static void inline strbuf_add(struct strbuf *sb, int ch) {
-	if (sb->alloc <= sb->len) {
+static void inline strbuf_grow(struct strbuf *sb, size_t extra)
+{
+	while (sb->alloc < sb->len + extra)
 		sb->alloc = sb->alloc * 3 / 2 + 16;
-		sb->buf = xrealloc(sb->buf, sb->alloc);
-	}
+	sb->buf = xrealloc(sb->buf, sb->alloc);
+}
+
+void strbuf_add(struct strbuf *sb, const char *data, size_t len)
+{
+	strbuf_grow(sb, len);
+	memcpy(sb->buf + sb->len, data, len);
+	sb->len += len;
+}
+
+void strbuf_add_char(struct strbuf *sb, int ch)
+{
+	strbuf_grow(sb, 1);
 	sb->buf[sb->len++] = ch;
 }
 
 static void strbuf_end(struct strbuf *sb) {
-	strbuf_add(sb, 0);
+	strbuf_add_char(sb, 0);
 }
 
 void read_line(struct strbuf *sb, FILE *fp, int term) {
@@ -33,9 +45,22 @@ void read_line(struct strbuf *sb, FILE *fp, int term) {
 	while ((ch = fgetc(fp)) != EOF) {
 		if (ch == term)
 			break;
-		strbuf_add(sb, ch);
+		strbuf_add_char(sb, ch);
 	}
 	if (ch == EOF && sb->len == 0)
 		sb->eof = 1;
 	strbuf_end(sb);
 }
+
+void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
+{
+	char one_line[2048];
+	va_list args;
+	int len;
+
+	va_start(args, fmt);
+	len = vsnprintf(one_line, sizeof(one_line), fmt, args);
+	va_end(args);
+	strbuf_add(sb, one_line, len);
+}
+
diff --git a/strbuf.h b/strbuf.h
index 74cc012..1e5d09e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -9,5 +9,8 @@ struct strbuf {
 
 extern void strbuf_init(struct strbuf *);
 extern void read_line(struct strbuf *, FILE *, int);
+extern void strbuf_add(struct strbuf *sb, const char *data, size_t len);
+extern void strbuf_add_char(struct strbuf *sb, int ch);
+extern void strbuf_printf(struct strbuf *sb, const char *fmt, ...);
 
 #endif /* STRBUF_H */
-- 
1.5.2.GIT

[PATCH 4/5] Make builtin-commit-tree use a strbuf instead of hand-rolled realloc buffer.

[Kristian Høgsberg]

From: Kristian Høgsberg <krh@redhat.com>

With the new strbuf_printf() function we can now just a strbuf for
building up the commit object.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 builtin-commit-tree.c |   57 +++++++++++-------------------------------------
 1 files changed, 13 insertions(+), 44 deletions(-)

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index ccbcbe3..72884eb 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -8,42 +8,13 @@
 #include "tree.h"
 #include "builtin.h"
 #include "utf8.h"
+#include "strbuf.h"
 
 #define BLOCKING (1ul << 14)
 
 /*
  * FIXME! Share the code with "write-tree.c"
  */
-static void init_buffer(char **bufp, unsigned int *sizep)
-{
-	*bufp = xmalloc(BLOCKING);
-	*sizep = 0;
-}
-
-static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
-{
-	char one_line[2048];
-	va_list args;
-	int len;
-	unsigned long alloc, size, newsize;
-	char *buf;
-
-	va_start(args, fmt);
-	len = vsnprintf(one_line, sizeof(one_line), fmt, args);
-	va_end(args);
-	size = *sizep;
-	newsize = size + len + 1;
-	alloc = (size + 32767) & ~32767;
-	buf = *bufp;
-	if (newsize > alloc) {
-		alloc = (newsize + 32767) & ~32767;
-		buf = xrealloc(buf, alloc);
-		*bufp = buf;
-	}
-	*sizep = newsize - 1;
-	memcpy(buf + size, one_line, len);
-}
-
 static void check_valid(unsigned char *sha1, enum object_type expect)
 {
 	enum object_type type = sha1_object_info(sha1, NULL);
@@ -88,8 +59,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	unsigned char tree_sha1[20];
 	unsigned char commit_sha1[20];
 	char comment[1000];
-	char *buffer;
-	unsigned int size;
+	struct strbuf sb;
 	int encoding_is_utf8;
 
 	git_config(git_default_config);
@@ -118,8 +88,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	/* Not having i18n.commitencoding is the same as having utf-8 */
 	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
 
-	init_buffer(&buffer, &size);
-	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
+	strbuf_init(&sb);
+	strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
 
 	/*
 	 * NOTE! This ordering means that the same exact tree merged with a
@@ -127,26 +97,25 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	 * if everything else stays the same.
 	 */
 	for (i = 0; i < parents; i++)
-		add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));
+		strbuf_printf(&sb, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
 	/* Person/date information */
-	add_buffer(&buffer, &size, "author %s\n", git_author_info(1));
-	add_buffer(&buffer, &size, "committer %s\n", git_committer_info(1));
+	strbuf_printf(&sb, "author %s\n", git_author_info(1));
+	strbuf_printf(&sb, "committer %s\n", git_committer_info(1));
 	if (!encoding_is_utf8)
-		add_buffer(&buffer, &size,
-				"encoding %s\n", git_commit_encoding);
-	add_buffer(&buffer, &size, "\n");
+		strbuf_printf(&sb, "encoding %s\n", git_commit_encoding);
+	strbuf_printf(&sb, "\n");
 
 	/* And add the comment */
 	while (fgets(comment, sizeof(comment), stdin) != NULL)
-		add_buffer(&buffer, &size, "%s", comment);
+		strbuf_printf(&sb, "%s", comment);
 
 	/* And check the encoding */
-	buffer[size] = '\0';
-	if (encoding_is_utf8 && !is_utf8(buffer))
+	strbuf_add_char(&sb, '\0');
+	if (encoding_is_utf8 && !is_utf8(sb.buf))
 		fprintf(stderr, commit_utf8_warn);
 
-	if (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {
+	if (!write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1)) {
 		printf("%s\n", sha1_to_hex(commit_sha1));
 		return 0;
 	}
-- 
1.5.2.GIT

[PATCH 5/5] Split out the actual commit creation from the option parsing etc.

[Kristian Høgsberg]

From: Kristian Høgsberg <krh@redhat.com>

The functionality is available inside git through the function
create_commit().

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 builtin-commit-tree.c |   96 ++++++++++++++++++++++++++++++------------------
 commit.h              |    7 ++++
 2 files changed, 67 insertions(+), 36 deletions(-)

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 72884eb..e20f0b4 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -52,15 +52,59 @@ static const char commit_utf8_warn[] =
 "You may want to amend it after fixing the message, or set the config\n"
 "variable i18n.commitencoding to the encoding your project uses.\n";
 
+const unsigned char *
+create_commit(const unsigned char *tree_sha1,
+	      unsigned char parent_sha1[][20], int parents,
+	      const char *author_info, const char *committer_info,
+	      const char *message, int length)
+{
+	static unsigned char commit_sha1[20];
+	int encoding_is_utf8, i;
+	struct strbuf sb;
+
+	/* Not having i18n.commitencoding is the same as having utf-8 */
+	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
+
+	strbuf_init(&sb);
+	strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
+
+	/*
+	 * NOTE! This ordering means that the same exact tree merged with a
+	 * different order of parents will be a _different_ changeset even
+	 * if everything else stays the same.
+	 */
+	for (i = 0; i < parents; i++)
+		strbuf_printf(&sb, "parent %s\n", sha1_to_hex(parent_sha1[i]));
+
+	/* Person/date information */
+	strbuf_printf(&sb, "author %s\n", author_info);
+	strbuf_printf(&sb, "committer %s\n", committer_info);
+	if (!encoding_is_utf8)
+		strbuf_printf(&sb, "encoding %s\n", git_commit_encoding);
+	strbuf_printf(&sb, "\n");
+
+	/* And add the comment */
+	strbuf_add(&sb, message, length);
+
+	/* And check the encoding */
+	strbuf_add_char(&sb, '\0');
+	if (encoding_is_utf8 && !is_utf8(sb.buf))
+		fprintf(stderr, commit_utf8_warn);
+
+	if (!write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1))
+		return commit_sha1;
+
+	return NULL;
+}
+
 int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	int parents = 0;
 	unsigned char tree_sha1[20];
-	unsigned char commit_sha1[20];
-	char comment[1000];
-	struct strbuf sb;
-	int encoding_is_utf8;
+	char *buffer;
+	unsigned long len;
+	const unsigned char *commit_sha1;
 
 	git_config(git_default_config);
 
@@ -85,40 +129,20 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 			parents++;
 	}
 
-	/* Not having i18n.commitencoding is the same as having utf-8 */
-	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
+	buffer = NULL;
+	if (read_fd(0, &buffer, &len))
+		die("Could not read commit message from standard input");
 
-	strbuf_init(&sb);
-	strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
+	commit_sha1 = create_commit(tree_sha1,
+				    parent_sha1, parents,
+				    xstrdup(git_author_info(1)),
+				    xstrdup(git_committer_info(1)),
+				    buffer, len);
 
-	/*
-	 * NOTE! This ordering means that the same exact tree merged with a
-	 * different order of parents will be a _different_ changeset even
-	 * if everything else stays the same.
-	 */
-	for (i = 0; i < parents; i++)
-		strbuf_printf(&sb, "parent %s\n", sha1_to_hex(parent_sha1[i]));
-
-	/* Person/date information */
-	strbuf_printf(&sb, "author %s\n", git_author_info(1));
-	strbuf_printf(&sb, "committer %s\n", git_committer_info(1));
-	if (!encoding_is_utf8)
-		strbuf_printf(&sb, "encoding %s\n", git_commit_encoding);
-	strbuf_printf(&sb, "\n");
-
-	/* And add the comment */
-	while (fgets(comment, sizeof(comment), stdin) != NULL)
-		strbuf_printf(&sb, "%s", comment);
+	if (!commit_sha1)
+		return 1;
 
-	/* And check the encoding */
-	strbuf_add_char(&sb, '\0');
-	if (encoding_is_utf8 && !is_utf8(sb.buf))
-		fprintf(stderr, commit_utf8_warn);
+	printf("%s\n", sha1_to_hex(commit_sha1));
 
-	if (!write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1)) {
-		printf("%s\n", sha1_to_hex(commit_sha1));
-		return 0;
-	}
-	else
-		return 1;
+	return 0;
 }
diff --git a/commit.h b/commit.h
index 467872e..9f640ba 100644
--- a/commit.h
+++ b/commit.h
@@ -122,4 +122,11 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 		int depth, int shallow_flag, int not_shallow_flag);
 
 int in_merge_bases(struct commit *, struct commit **, int);
+
+extern const unsigned char *
+create_commit(const unsigned char *tree_sha1,
+	      unsigned char parent_sha1[][20], int parents,
+	      const char *author_info, const char *committer_info,
+	      const char *message, int length);
+
 #endif /* COMMIT_H */
-- 
1.5.2.GIT

Re: [PATCH 5/5] Split out the actual commit creation from the option parsing etc.

[Junio C Hamano]

Kristian Høgsberg <krh@redhat.com> writes:

> @@ -85,40 +129,20 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
>  			parents++;
>  	}
>  
> -	/* Not having i18n.commitencoding is the same as having utf-8 */
> -	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
> +	buffer = NULL;
> +	if (read_fd(0, &buffer, &len))
> +		die("Could not read commit message from standard input");
>  
> -	strbuf_init(&sb);
> -	strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
> +	commit_sha1 = create_commit(tree_sha1,
> +				    parent_sha1, parents,
> +				    xstrdup(git_author_info(1)),
> +				    xstrdup(git_committer_info(1)),
> +				    buffer, len);

Hmph, the series was so nice so far, but here we have a few new
leak, presumably so small per process invocation that we do not
care about?

Re: [PATCH 5/5] Split out the actual commit creation from the option parsing etc.

[Kristian Høgsberg]

On Mon, 2007-07-30 at 21:43 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > @@ -85,40 +129,20 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
> >  			parents++;
> >  	}
> >  
> > -	/* Not having i18n.commitencoding is the same as having utf-8 */
> > -	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
> > +	buffer = NULL;
> > +	if (read_fd(0, &buffer, &len))
> > +		die("Could not read commit message from standard input");
> >  
> > -	strbuf_init(&sb);
> > -	strbuf_printf(&sb, "tree %s\n", sha1_to_hex(tree_sha1));
> > +	commit_sha1 = create_commit(tree_sha1,
> > +				    parent_sha1, parents,
> > +				    xstrdup(git_author_info(1)),
> > +				    xstrdup(git_committer_info(1)),
> > +				    buffer, len);
> 
> Hmph, the series was so nice so far, but here we have a few new
> leak, presumably so small per process invocation that we do not
> care about?

There's number of buffers that don't get freed: the strbuf, the commit
message buffer, and the strdup'ed author and committer info.  All the
leaks are not critical since the process exits immediately.  As for the
strbuf leak, I was thinking about renaming strbuf_begin to strbuf_reset
and making it public[1], which will then be used for freeing up strbuf
memory.  The message buffer leak should be fixed by adding a
strbuf_read_fd() that just reads it straight into the strbuf.  The
xstrdup's are necessary because fmt_ident uses a static buffer (thanks,
test case :).  We could add rotating static buffers for fmt_ident like
git_path and avoid the strdups, but again, the leaks are not critical.

Kristian

[1] strbuf_begin() is a good name the way it's used in strbuf.c where
it's balanced by strbuf_end(), but as a general purpose reset function
it's better name strbuf_reset(), I think

Re: [PATCH 4/5] Make builtin-commit-tree use a strbuf instead of hand-rolled realloc buffer.

[Junio C Hamano]

Very nice.

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Junio C Hamano]

Kristian Høgsberg <krh@redhat.com> writes:

> +static void inline strbuf_grow(struct strbuf *sb, size_t extra)
> +{
> +	while (sb->alloc < sb->len + extra)
>  		sb->alloc = sb->alloc * 3 / 2 + 16;
> +	sb->buf = xrealloc(sb->buf, sb->alloc);
> +}

Somehow this while () loop to compute the growth factor bothers
me but that is probably a minor detail.

> +void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> +{
> +	char one_line[2048];
> +	va_list args;
> +	int len;

Such a nice abstraction so far, and then at the highest level of
callchain we have this hardcoded limit?

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Kristian Høgsberg]

On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > +static void inline strbuf_grow(struct strbuf *sb, size_t extra)
> > +{
> > +	while (sb->alloc < sb->len + extra)
> >  		sb->alloc = sb->alloc * 3 / 2 + 16;
> > +	sb->buf = xrealloc(sb->buf, sb->alloc);
> > +}
> 
> Somehow this while () loop to compute the growth factor bothers
> me but that is probably a minor detail.

Think of it as a more efficient way of adding one character at a time :)
And it's logarithmic in the number of extra bytes.  By the way, I
normally just double the size in these cases, which gives you amortized
linear performance for adding to the buffer.  What's behind the * 3 / 2
idea?

> > +void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > +{
> > +	char one_line[2048];
> > +	va_list args;
> > +	int len;
> 
> Such a nice abstraction so far, and then at the highest level of
> callchain we have this hardcoded limit?

Yeah, I know, it sucks.  I'd like to just run vsnprintf with a 0-sized
buffer to get the length, and then grow the buffer by that much, but
that's not very portable as far as I know.  Another approach is to just
vsnprintf into the one_line buffer and copy it into the strbuf if it
doesn't overflow.  If it does overflow, grow the buffer with that amount
and vsnprintf into the buffer.  I don't think that's portable either,
since vsnprintf can't be relied upon to return the number of characters
it would have printed.  One thing that would work, but is much more
involved is to lift the vsnprintf implementation from something like
glib.

So, I dunno... given the options, I opted for a simple solution with
some limitations.  Given how many other functions in git work on a fixed
sized static buffer (for example, the fmt_ident case mentioned in
another email), I wouldn't think this is a problem

Kristian

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 882 bytes --]

Hi,

On Tue, 31 Jul 2007, Kristian H?gsberg wrote:

> On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
> > Kristian Høgsberg <krh@redhat.com> writes:
> > 
> > > +static void inline strbuf_grow(struct strbuf *sb, size_t extra)
> > > +{
> > > +	while (sb->alloc < sb->len + extra)
> > >  		sb->alloc = sb->alloc * 3 / 2 + 16;
> > > +	sb->buf = xrealloc(sb->buf, sb->alloc);
> > > +}
> > 
> > Somehow this while () loop to compute the growth factor bothers
> > me but that is probably a minor detail.
> 
> Think of it as a more efficient way of adding one character at a time :)
> And it's logarithmic in the number of extra bytes.  By the way, I
> normally just double the size in these cases, which gives you amortized
> linear performance for adding to the buffer.  What's behind the * 3 / 2
> idea?

But why not

	sb->alloc = alloc_nr(sb->len + extra);

Hmm?

Ciao,
Dscho

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Kristian Høgsberg]

On Tue, 2007-07-31 at 15:55 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 31 Jul 2007, Kristian H?gsberg wrote:
> 
> > On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
> > > Kristian Høgsberg <krh@redhat.com> writes:
> > > 
> > > > +static void inline strbuf_grow(struct strbuf *sb, size_t extra)
> > > > +{
> > > > +	while (sb->alloc < sb->len + extra)
> > > >  		sb->alloc = sb->alloc * 3 / 2 + 16;
> > > > +	sb->buf = xrealloc(sb->buf, sb->alloc);
> > > > +}
> > > 
> > > Somehow this while () loop to compute the growth factor bothers
> > > me but that is probably a minor detail.
> > 
> > Think of it as a more efficient way of adding one character at a time :)
> > And it's logarithmic in the number of extra bytes.  By the way, I
> > normally just double the size in these cases, which gives you amortized
> > linear performance for adding to the buffer.  What's behind the * 3 / 2
> > idea?
> 
> But why not
> 
> 	sb->alloc = alloc_nr(sb->len + extra);

That should work, or even ALLOC_GROW, I guess.  Didn't know about those
macros.  But still, why not just double the size?

Kristian

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 660 bytes --]

Hi,

On Tue, 31 Jul 2007, Kristian H?gsberg wrote:

> On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
> > Kristian Høgsberg <krh@redhat.com> writes:
> > 
> > > +void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > > +{
> > > +	char one_line[2048];
> > > +	va_list args;
> > > +	int len;
> > 
> > Such a nice abstraction so far, and then at the highest level of
> > callchain we have this hardcoded limit?
> 
> Yeah, I know, it sucks.  I'd like to just run vsnprintf with a 0-sized
> buffer to get the length, and then grow the buffer by that much, but
> that's not very portable as far as I know.

We do have nfvasprintf()...

Ciao,
Dscho

Re: [PATCH 3/5] Add strbuf_printf() to do formatted printing to a strbuf.

[Kristian Høgsberg]

On Tue, 2007-07-31 at 15:57 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 31 Jul 2007, Kristian H?gsberg wrote:
> 
> > On Mon, 2007-07-30 at 21:36 -0700, Junio C Hamano wrote:
> > > Kristian Høgsberg <krh@redhat.com> writes:
> > > 
> > > > +void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > > > +{
> > > > +	char one_line[2048];
> > > > +	va_list args;
> > > > +	int len;
> > > 
> > > Such a nice abstraction so far, and then at the highest level of
> > > callchain we have this hardcoded limit?
> > 
> > Yeah, I know, it sucks.  I'd like to just run vsnprintf with a 0-sized
> > buffer to get the length, and then grow the buffer by that much, but
> > that's not very portable as far as I know.
> 
> We do have nfvasprintf()...

Ah, ok... it looks like it's using the second approach I described:
printing into a temp buffer, and realloc'ing and printing again if it
didn't fit.  But it dies with 'out of memory' if vsnprintf returns < 0,
is that reasonable?  I'm not familiar with what will make vsnprintf
return a negative value...  Also, that implementation potentially uses
the va_list twice, which isn't universally supported.  C99 introduces
va_copy for this case, but that's also not available everywhere.

However, if the nfvasprintf implementation in trace.c is fine, I'll just
use that approach in strbuf_printf too.  I'd just like to know that the
above issues aren't a problem on win32, since part of my motivation for
doing this is to make git available on windows so it will be easier for
people to choose git.

Kristian

[PATCH] Add strbuf_printf() to do formatted printing to a strbuf.

[Kristian Høgsberg]

Also, expose strbuf_add() and strbuf_add_char() to add raw data to the buffer.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---

Ok, this gets uglier as we try to work around different versions of
vsnprintf.  On windows, vsnprintf returns -1 if the output doesn't fit in
the given buffer.  What we do is to keep doubling the buffer size until it
fits.  Not sure this is the best idea.

The old hardcoded limitation of just 2048 bytes wasn't too bad, considering
that it's just the limit for one strbuf_printf invocation, not the total
size of the strbuf contents.

I dunno...
Kristian

 strbuf.c |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 strbuf.h |    3 ++
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index e33d06b..2805c11 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -11,16 +11,26 @@ static void strbuf_begin(struct strbuf *sb) {
 	strbuf_init(sb);
 }
 
-static void inline strbuf_add(struct strbuf *sb, int ch) {
-	if (sb->alloc <= sb->len) {
-		sb->alloc = sb->alloc * 3 / 2 + 16;
-		sb->buf = xrealloc(sb->buf, sb->alloc);
-	}
+static void inline strbuf_grow(struct strbuf *sb, size_t extra)
+{
+	ALLOC_GROW(sb->buf, sb->len + extra, sb->alloc);
+}
+
+void strbuf_add(struct strbuf *sb, const char *data, size_t len)
+{
+	strbuf_grow(sb, len);
+	memcpy(sb->buf + sb->len, data, len);
+	sb->len += len;
+}
+
+void strbuf_add_char(struct strbuf *sb, int ch)
+{
+	strbuf_grow(sb, 1);
 	sb->buf[sb->len++] = ch;
 }
 
 static void strbuf_end(struct strbuf *sb) {
-	strbuf_add(sb, 0);
+	strbuf_add_char(sb, 0);
 }
 
 void read_line(struct strbuf *sb, FILE *fp, int term) {
@@ -33,9 +43,54 @@ void read_line(struct strbuf *sb, FILE *fp, int term) {
 	while ((ch = fgetc(fp)) != EOF) {
 		if (ch == term)
 			break;
-		strbuf_add(sb, ch);
+		strbuf_add_char(sb, ch);
 	}
 	if (ch == EOF && sb->len == 0)
 		sb->eof = 1;
 	strbuf_end(sb);
 }
+
+void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
+{
+	char buffer[2048];
+	va_list args;
+	int len, size = 2 * sizeof buffer;
+
+	va_start(args, fmt);
+	len = vsnprintf(buffer, sizeof(buffer), fmt, args);
+	va_end(args);
+
+	if (len > sizeof(buffer)) {
+		/*
+		 * Didn't fit in the buffer, but this vsnprintf at
+		 * least gives us the required length back.  Grow the
+		 * buffer acccordingly and try again.
+		 */
+		strbuf_grow(sb, len);
+		va_start(args, fmt);
+		len = vsnprintf(sb->buf + sb->len,
+				sb->alloc - sb->len, fmt, args);
+		va_end(args);
+	} else if (len >= 0) {
+		/*
+		 * The initial vsnprintf fit in the temp buffer, just
+		 * copy it to the strbuf.
+		 */
+		strbuf_add(sb, buffer, len);
+	} else {
+		/*
+		 * This vnsprintf sucks and just returns -1 when the
+		 * buffer is too small.  Keep doubling the size until
+		 * it fits.
+		 */
+		while (len < 0) {
+			strbuf_grow(sb, size);
+			va_start(args, fmt);
+			len = vsnprintf(sb->buf + sb->len,
+					sb->alloc - sb->len, fmt, args);
+			va_end(args);
+			size *= 2;
+		}
+	}
+}
+
diff --git a/strbuf.h b/strbuf.h
index 74cc012..1e5d09e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -9,5 +9,8 @@ struct strbuf {
 
 extern void strbuf_init(struct strbuf *);
 extern void read_line(struct strbuf *, FILE *, int);
+extern void strbuf_add(struct strbuf *sb, const char *data, size_t len);
+extern void strbuf_add_char(struct strbuf *sb, int ch);
+extern void strbuf_printf(struct strbuf *sb, const char *fmt, ...);
 
 #endif /* STRBUF_H */
-- 
1.5.2.GIT

Re: [PATCH] Add strbuf_printf() to do formatted printing to a strbuf.

[Junio C Hamano]

Kristian Høgsberg <krh@redhat.com> writes:

> The old hardcoded limitation of just 2048 bytes wasn't too bad, considering
> that it's just the limit for one strbuf_printf invocation, not the total
> size of the strbuf contents.

I tend to agree with you now.  Sorry for the noise to make you
go to wild goose chase.  This does not look worth it.

Re: [PATCH 1/5] Add test case for basic commit functionality.

[Junio C Hamano]

Kristian Høgsberg <krh@redhat.com> writes:

> +# Pick a date so we get consistent commits. 7/7/07 means good luck!
> +export GIT_AUTHOR_DATE="July 7, 2007"
> +export GIT_COMMITTER_DATE="July 7, 2007"

Other scripts use test_tick for consistent commits; just to let
you know, the above is as good if you intend to use only one
timestamp.

> +echo "bongo bongo" >file
> +test_expect_success \
> +	"initial status" \
> +	"git-add file && \
> +	 git-status | grep 'Initial commit'"

We tend to prefer to have the "create testfile" part also in the
test.

> +test_expect_failure \
> +	"fail initial amend" \
> +	"git-commit -m initial --amend"

Does it fail because it is initial, or because these two
options, "-m <msg>" and "--amend", are incompatible?

> +echo "bongo bongo bongo" >file

Ditto.

> +cat >msg <<EOF
> +		
> +
> +  
> +Signed-off-by: hula
> +EOF

... except <<here text are left outside of test_expect_success
because there were reports that some shells cannot grok here
text in eval correctly.

Re: [PATCH 1/5] Add test case for basic commit functionality.

[Kristian Høgsberg]

On Mon, 2007-07-30 at 21:18 -0700, Junio C Hamano wrote:
> Kristian Høgsberg <krh@redhat.com> writes:
> 
> > +# Pick a date so we get consistent commits. 7/7/07 means good luck!
> > +export GIT_AUTHOR_DATE="July 7, 2007"
> > +export GIT_COMMITTER_DATE="July 7, 2007"
> 
> Other scripts use test_tick for consistent commits; just to let
> you know, the above is as good if you intend to use only one
> timestamp.

Ok, cool, I'll send out an updated test case.

Kristian

[PATCH] Add test case for basic commit functionality.

[Kristian Høgsberg]

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---

Update: using test_tick now, create test files as part of tests, split
	amend failure and bad options failure into separate test cases.

 t/t7501-commit.sh |  133 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 133 insertions(+), 0 deletions(-)
 create mode 100644 t/t7501-commit.sh

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
new file mode 100644
index 0000000..ef0aab5
--- /dev/null
+++ b/t/t7501-commit.sh
@@ -0,0 +1,133 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+#
+
+# FIXME: Test the various index usages, -i and -o, test reflog,
+# signoff, hooks
+
+test_description='git-commit'
+. ./test-lib.sh
+
+test_tick
+
+test_expect_success \
+	"initial status" \
+	"echo 'bongo bongo' >file &&
+	 git-add file && \
+	 git-status | grep 'Initial commit'"
+
+test_expect_failure \
+	"fail initial amend" \
+	"git-commit --amend"
+
+test_expect_success \
+	"initial commit" \
+	"git-commit -m initial"
+
+test_expect_failure \
+	"invalid options 1" \
+	"git-commit --amend -F file"
+
+test_expect_failure \
+	"invalid options 2" \
+	"git-commit -C HEAD -m illegal"
+
+test_expect_failure \
+	"using invalid commit with -C" \
+	"git-commit -C bogus"
+
+test_expect_failure \
+	"testing nothing to commit" \
+	"git-commit -m initial"
+
+test_expect_success \
+	"next commit" \
+	"echo 'bongo bongo bongo' >file \
+	 git-commit -m next -a"
+
+test_expect_failure \
+	"commit message from non-existing file" \
+	"echo 'more bongo: bongo bongo bongo bongo' >file && \
+	 git-commit -F gah -a"
+
+cat >msg <<EOF
+		
+
+  
+Signed-off-by: hula
+EOF
+test_expect_failure \
+	"empty commit message" \
+	"git-commit -F msg -a"
+
+test_expect_success \
+	"commit message from file" \
+	"echo 'this is the commit message, coming from a file' >msg && \
+	 git-commit -F msg -a"
+
+cat >editor <<\EOF
+#!/bin/sh
+sed -i -e "s/a file/an amend commit/g" $1
+EOF
+chmod 755 editor
+
+test_expect_success \
+	"amend commit" \
+	"VISUAL=./editor git-commit --amend"
+
+test_expect_failure \
+	"passing --amend and -F" \
+	"echo 'enough with the bongos' >file && \
+	 git-commit -F msg --amend ."
+
+test_expect_success \
+	"using message from other commit" \
+	"git-commit -C HEAD^ ."
+
+cat >editor <<\EOF
+#!/bin/sh
+sed -i -e "s/amend/older/g" $1
+EOF
+chmod 755 editor
+
+test_expect_success \
+	"editing message from other commit" \
+	"echo 'hula hula' >file && \
+	 VISUAL=./editor git-commit -c HEAD^ -a"
+
+test_expect_success \
+	"message from stdin" \
+	"echo 'silly new contents' >file && \
+	 echo commit message from stdin | git-commit -F - -a"
+
+test_expect_success \
+	"overriding author from command line" \
+	"echo 'gak' >file && \
+	 git-commit -m 'author' --author 'Rubber Duck <rduck@convoy.org>' -a"
+
+test_expect_success \
+	"interactive add" \
+	"echo 7 | git-commit --interactive | grep 'What now'"
+
+test_expect_success \
+	"showing committed revisions" \
+	"git-rev-list HEAD >current"
+
+# We could just check the head sha1, but checking each commit makes it
+# easier to isolate bugs.
+
+cat >expected <<\EOF
+72c0dc9855b0c9dadcbfd5a31cab072e0cb774ca
+9b88fc14ce6b32e3d9ee021531a54f18a5cf38a2
+3536bbb352c3a1ef9a420f5b4242d48578b92aa7
+d381ac431806e53f3dd7ac2f1ae0534f36d738b9
+4fd44095ad6334f3ef72e4c5ec8ddf108174b54a
+402702b49136e7587daa9280e91e4bb7cb2179f7
+EOF
+
+test_expect_success \
+    'validate git-rev-list output.' \
+    'diff current expected'
+
+test_done
-- 
1.5.2.GIT