commit/tag: ensure the message is cleaned up

'git commit' and 'git tag -a' enforce some conventions, like cleaning up excess whitespace and making sure that the last line ends with a '\n'. This fix replicates this behavior.

Fix libgit2/libgit2sharp#117

8 files changed, 272 insertions, 7 deletions

diff --git a/include/git2/commit.h b/include/git2/commit.h
index c274b6b95..e519bc128 100644
--- a/include/git2/commit.h
+++ b/include/git2/commit.h
@@ -182,6 +182,9 @@ GIT_EXTERN(const git_oid *) git_commit_parent_oid(git_commit *commit, unsigned i
  * Create a new commit in the repository using `git_object`
  * instances as parameters.
  *
+ * The message will be cleaned up from excess whitespace
+ * it will be made sure that the last line ends with a '\n'.
+ *
  * @param oid Pointer where to store the OID of the
  *	newly created commit
  *
@@ -238,6 +241,9 @@ GIT_EXTERN(int) git_commit_create(
  * Create a new commit in the repository using a variable
  * argument list.
  *
+ * The message will be cleaned up from excess whitespace
+ * it will be made sure that the last line ends with a '\n'.
+ *
  * The parents for the commit are specified as a variable
  * list of pointers to `const git_commit *`. Note that this
  * is a convenience method which may not be safe to export
diff --git a/include/git2/tag.h b/include/git2/tag.h
index 9ab4b7b9e..8224edfea 100644
--- a/include/git2/tag.h
+++ b/include/git2/tag.h
@@ -137,6 +137,9 @@ GIT_EXTERN(const char *) git_tag_message(git_tag *tag);
  * this tag object. If `force` is true and a reference
  * already exists with the given name, it'll be replaced.
  *
+ * The message will be cleaned up from excess whitespace
+ * it will be made sure that the last line ends with a '\n'.
+ *
  * @param oid Pointer where to store the OID of the
  * newly created tag. If the tag already exists, this parameter
  * will be the oid of the existing tag, and the function will
diff --git a/src/commit.c b/src/commit.c
index 04f37fe16..2bf12f3a5 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -14,6 +14,7 @@
 #include "odb.h"
 #include "commit.h"
 #include "signature.h"
+#include "message.h"
 
 #include <stdarg.h>
 
@@ -161,7 +162,7 @@ int git_commit_create(
 		int parent_count,
 		const git_commit *parents[])
 {
-	git_buf commit = GIT_BUF_INIT;
+	git_buf commit = GIT_BUF_INIT, cleaned_message = GIT_BUF_INIT;
 	int i;
 	git_odb *odb;
 
@@ -181,11 +182,16 @@ int git_commit_create(
 		git_buf_printf(&commit, "encoding %s\n", message_encoding);
 
 	git_buf_putc(&commit, '\n');
-	git_buf_puts(&commit, message);
 
-	if (git_buf_oom(&commit))
+	/* Remove comments by default */
+	if (git_message_prettify(&cleaned_message, message, 1) < 0)
 		goto on_error;
 
+	if (git_buf_puts(&commit, git_buf_cstr(&cleaned_message)) < 0)
+		goto on_error;
+
+	git_buf_free(&cleaned_message);
+
 	if (git_repository_odb__weakptr(&odb, repo) < 0)
 		goto on_error;
 
@@ -201,6 +207,8 @@ int git_commit_create(
 
 on_error:
 	git_buf_free(&commit);
+	git_buf_free(&cleaned_message);
+	giterr_set(GITERR_OBJECT, "Failed to create commit.");
 	return -1;
 }
 
diff --git a/src/message.c b/src/message.c
new file mode 100644
index 000000000..94745ea81
--- /dev/null
+++ b/src/message.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2009-2012 the libgit2 contributors
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#include "message.h"
+
+static size_t line_length_without_trailing_spaces(const char *line, size_t len)
+{
+	while (len) {
+		unsigned char c = line[len - 1];
+		if (!isspace(c))
+			break;
+		len--;
+	}
+
+	return len;
+}
+
+/* Greatly inspired from git.git "stripspace" */
+/* see https://github.com/git/git/blob/497215d8811ac7b8955693ceaad0899ecd894ed2/builtin/stripspace.c#L4-67 */
+int git_message_prettify(git_buf *message_out, const char *message, int strip_comments)
+{
+	int consecutive_empty_lines = 0;
+	size_t i, line_length, rtrimmed_line_length;
+	char *next_newline;
+
+	for (i = 0; i < strlen(message); i += line_length) {
+		next_newline = memchr(message + i, '\n', strlen(message) - i);
+		line_length = next_newline ? next_newline - (message + i) + 1 : strlen(message) - i;
+
+		if (strip_comments && line_length && message[i] == '#')
+			continue;
+
+		rtrimmed_line_length = line_length_without_trailing_spaces(message + i, line_length);
+
+		if (!rtrimmed_line_length) {
+			consecutive_empty_lines++;
+			continue;
+		}
+
+		if (consecutive_empty_lines > 0 && message_out->size > 0)
+			if (git_buf_putc(message_out, '\n') < 0)
+				return -1;
+
+		consecutive_empty_lines = 0;
+		git_buf_put(message_out, message + i, rtrimmed_line_length);
+		git_buf_putc(message_out, '\n');
+	}
+
+	return 0;
+}
diff --git a/src/message.h b/src/message.h
new file mode 100644
index 000000000..ddfa13e18
--- /dev/null
+++ b/src/message.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2009-2012 the libgit2 contributors
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+#ifndef INCLUDE_message_h__
+#define INCLUDE_message_h__
+
+#include "buffer.h"
+
+int git_message_prettify(git_buf *message_out, const char *message, int strip_comments);
+
+#endif /* INCLUDE_message_h__ */
diff --git a/src/tag.c b/src/tag.c
index aa549fdd0..13481c2a6 100644
--- a/src/tag.c
+++ b/src/tag.c
@@ -9,6 +9,7 @@
 #include "commit.h"
 #include "tag.h"
 #include "signature.h"
+#include "message.h"
 #include "git2/object.h"
 #include "git2/repository.h"
 #include "git2/signature.h"
@@ -195,7 +196,7 @@ static int write_tag_annotation(
 		const git_signature *tagger,
 		const char *message)
 {
-	git_buf tag = GIT_BUF_INIT;
+	git_buf tag = GIT_BUF_INIT, cleaned_message = GIT_BUF_INIT;
 	git_odb *odb;
 
 	git_oid__writebuf(&tag, "object ", git_object_id(target));
@@ -203,11 +204,16 @@ static int write_tag_annotation(
 	git_buf_printf(&tag, "tag %s\n", tag_name);
 	git_signature__writebuf(&tag, "tagger ", tagger);
 	git_buf_putc(&tag, '\n');
-	git_buf_puts(&tag, message);
 
-	if (git_buf_oom(&tag))
+	/* Remove comments by default */
+	if (git_message_prettify(&cleaned_message, message, 1) < 0)
 		goto on_error;
 
+	if (git_buf_puts(&tag, git_buf_cstr(&cleaned_message)) < 0)
+		goto on_error;
+
+	git_buf_free(&cleaned_message);
+
 	if (git_repository_odb__weakptr(&odb, repo) < 0)
 		goto on_error;
 
@@ -216,8 +222,11 @@ static int write_tag_annotation(
 
 	git_buf_free(&tag);
 	return 0;
+
 on_error:
 	git_buf_free(&tag);
+	git_buf_free(&cleaned_message);
+	giterr_set(GITERR_OBJECT, "Failed to create tag annotation.");
 	return -1;
 }
 
diff --git a/tests-clar/object/commit/commitstagedfile.c b/tests-clar/object/commit/commitstagedfile.c
index cd04e96d4..a852458f4 100644
--- a/tests-clar/object/commit/commitstagedfile.c
+++ b/tests-clar/object/commit/commitstagedfile.c
@@ -114,7 +114,7 @@ void test_object_commit_commitstagedfile__generate_predictable_object_ids(void)
 		signature,
 		signature,
 		NULL,
-		"Initial commit\n",		// Note: the trailing linefeed is mandatory to replicate git behavior
+		"Initial commit",
 		tree,
 		0));
 
diff --git a/tests-clar/object/message.c b/tests-clar/object/message.c
new file mode 100644
index 000000000..cbdc80a64
--- /dev/null
+++ b/tests-clar/object/message.c
@@ -0,0 +1,171 @@
+#include "clar_libgit2.h"
+#include "buffer.h"
+#include "message.h"
+
+static void assert_message_prettifying(char *expected_output, char *input, int strip_comments)
+{
+	git_buf prettified_message = GIT_BUF_INIT;
+
+	git_message_prettify(&prettified_message, input, strip_comments);
+	cl_assert_equal_s(expected_output, git_buf_cstr(&prettified_message));
+
+	git_buf_free(&prettified_message);
+}
+
+#define t40 "A quick brown fox jumps over the lazy do"
+#define s40 "                                        "
+#define sss s40 s40 s40 s40 s40 s40 s40 s40 s40 s40 // # 400
+#define ttt t40 t40 t40 t40 t40 t40 t40 t40 t40 t40 // # 400
+
+/* Ported from git.git */
+/* see https://github.com/git/git/blob/master/t/t0030-stripspace.sh */
+void test_object_message__long_lines_without_spaces_should_be_unchanged(void)
+{
+	assert_message_prettifying(ttt "\n", ttt, 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt, 0);
+	assert_message_prettifying(ttt ttt ttt "\n", ttt ttt ttt, 0);
+	assert_message_prettifying(ttt ttt ttt ttt "\n", ttt ttt ttt ttt, 0);
+}
+
+void test_object_message__lines_with_spaces_at_the_beginning_should_be_unchanged(void)
+{
+	assert_message_prettifying(sss ttt "\n", sss ttt, 0);
+	assert_message_prettifying(sss sss ttt "\n", sss sss ttt, 0);
+	assert_message_prettifying(sss sss sss ttt "\n", sss sss sss ttt, 0);
+}
+
+void test_object_message__lines_with_intermediate_spaces_should_be_unchanged(void)
+{
+	assert_message_prettifying(ttt sss ttt "\n", ttt sss ttt, 0);
+	assert_message_prettifying(ttt sss sss ttt "\n", ttt sss sss ttt, 0);
+}
+
+void test_object_message__consecutive_blank_lines_should_be_unified(void)
+{
+	assert_message_prettifying(ttt "\n\n" ttt "\n", ttt "\n\n\n\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt "\n\n" ttt "\n", ttt ttt "\n\n\n\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt ttt "\n\n" ttt "\n", ttt ttt ttt "\n\n\n\n\n" ttt "\n", 0);
+
+	assert_message_prettifying(ttt "\n\n" ttt ttt "\n", ttt "\n\n\n\n\n" ttt ttt "\n", 0);
+	assert_message_prettifying(ttt "\n\n" ttt ttt ttt "\n", ttt "\n\n\n\n\n" ttt ttt ttt "\n", 0);
+
+	assert_message_prettifying(ttt "\n\n" ttt "\n", ttt "\n\t\n \n\n  \t\t\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt "\n\n" ttt "\n", ttt ttt "\n\t\n \n\n  \t\t\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt ttt "\n\n" ttt "\n", ttt ttt ttt "\n\t\n \n\n  \t\t\n" ttt "\n", 0);
+
+	assert_message_prettifying(ttt "\n\n" ttt ttt "\n", ttt "\n\t\n \n\n  \t\t\n" ttt ttt "\n", 0);
+	assert_message_prettifying(ttt "\n\n" ttt ttt ttt "\n", ttt "\n\t\n \n\n  \t\t\n" ttt ttt ttt "\n", 0);
+}
+
+void test_object_message__only_consecutive_blank_lines_should_be_completely_removed(void)
+{
+	assert_message_prettifying("", "\n", 0);
+	assert_message_prettifying("", "\n\n\n", 0);
+	assert_message_prettifying("", sss "\n" sss "\n" sss "\n", 0);
+	assert_message_prettifying("", sss sss "\n" sss "\n\n", 0);
+}
+
+void test_object_message__consecutive_blank_lines_at_the_beginning_should_be_removed(void)
+{
+	assert_message_prettifying(ttt "\n", "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", "\n\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt "\n", "\n\n\n" ttt ttt "\n", 0);
+	assert_message_prettifying(ttt ttt ttt "\n", "\n\n\n" ttt ttt ttt "\n", 0);
+	assert_message_prettifying(ttt ttt ttt ttt "\n", "\n\n\n" ttt ttt ttt ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", sss "\n" sss "\n" sss "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", "\n" sss "\n" sss sss "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", sss sss "\n" sss "\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", sss sss sss "\n\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", "\n" sss sss sss "\n\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n", "\n\n" sss sss sss "\n" ttt "\n", 0);
+}
+
+void test_object_message__consecutive_blank_lines_at_the_end_should_be_removed(void)
+{
+	assert_message_prettifying(ttt "\n", ttt "\n\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n\n\n\n", 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt "\n\n\n\n", 0);
+	assert_message_prettifying(ttt ttt ttt "\n", ttt ttt ttt "\n\n\n\n", 0);
+	assert_message_prettifying(ttt ttt ttt ttt "\n", ttt ttt ttt ttt "\n\n\n\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n" sss "\n" sss "\n" sss "\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n\n" sss "\n" sss sss "\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n" sss sss "\n" sss "\n\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n" sss sss sss "\n\n\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n\n" sss sss sss "\n\n", 0);
+	assert_message_prettifying(ttt "\n", ttt "\n\n\n" sss sss sss "\n\n", 0);
+}
+
+void test_object_message__text_without_newline_at_end_should_end_with_newline(void)
+{
+	assert_message_prettifying(ttt "\n", ttt, 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt, 0);
+	assert_message_prettifying(ttt ttt ttt "\n", ttt ttt ttt, 0);
+	assert_message_prettifying(ttt ttt ttt ttt "\n", ttt ttt ttt ttt, 0);
+}
+
+void test_object_message__text_plus_spaces_without_newline_should_not_show_spaces_and_end_with_newline(void)
+{
+	assert_message_prettifying(ttt "\n", ttt sss, 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt sss, 0);
+	assert_message_prettifying(ttt ttt ttt "\n", ttt ttt ttt sss, 0);
+	assert_message_prettifying(ttt "\n", ttt sss sss, 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt sss sss, 0);
+	assert_message_prettifying(ttt "\n", ttt sss sss sss, 0);
+}
+
+void test_object_message__text_plus_spaces_ending_with_newline_should_be_cleaned_and_newline_must_remain(void){
+	assert_message_prettifying(ttt "\n", ttt sss "\n", 0);
+	assert_message_prettifying(ttt "\n", ttt sss sss "\n", 0);
+	assert_message_prettifying(ttt "\n", ttt sss sss sss "\n", 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt sss "\n", 0);
+	assert_message_prettifying(ttt ttt "\n", ttt ttt sss sss "\n", 0);
+	assert_message_prettifying(ttt ttt ttt "\n", ttt ttt ttt sss "\n", 0);
+}
+
+void test_object_message__spaces_with_newline_at_end_should_be_replaced_with_empty_string(void)
+{
+	assert_message_prettifying("", sss "\n", 0);
+	assert_message_prettifying("", sss sss "\n", 0);
+	assert_message_prettifying("", sss sss sss "\n", 0);
+	assert_message_prettifying("", sss sss sss sss "\n", 0);
+}
+
+void test_object_message__spaces_without_newline_at_end_should_be_replaced_with_empty_string(void)
+{
+	assert_message_prettifying("", "", 0);
+	assert_message_prettifying("", sss sss, 0);
+	assert_message_prettifying("", sss sss sss, 0);
+	assert_message_prettifying("", sss sss sss sss, 0);
+}
+
+void test_object_message__consecutive_text_lines_should_be_unchanged(void)
+{
+	assert_message_prettifying(ttt ttt "\n" ttt "\n", ttt ttt "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n" ttt ttt "\n" ttt "\n", ttt "\n" ttt ttt "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n" ttt "\n" ttt "\n" ttt ttt "\n", ttt "\n" ttt "\n" ttt "\n" ttt ttt "\n", 0);
+	assert_message_prettifying(ttt "\n" ttt "\n\n" ttt ttt "\n" ttt "\n", ttt "\n" ttt "\n\n" ttt ttt "\n" ttt "\n", 0);
+	assert_message_prettifying(ttt ttt "\n\n" ttt "\n" ttt ttt "\n", ttt ttt "\n\n" ttt "\n" ttt ttt "\n", 0);
+	assert_message_prettifying(ttt "\n" ttt ttt "\n\n" ttt "\n", ttt "\n" ttt ttt "\n\n" ttt "\n", 0);
+}
+
+void test_object_message__strip_comments(void)
+{
+	assert_message_prettifying("", "# comment", 1);
+	assert_message_prettifying("", "# comment\n", 1);
+	assert_message_prettifying("", "# comment    \n", 1);
+
+	assert_message_prettifying(ttt "\n", ttt "\n" "# comment\n", 1);
+	assert_message_prettifying(ttt "\n", "# comment\n" ttt "\n", 1);
+	assert_message_prettifying(ttt "\n" ttt "\n", ttt "\n" "# comment\n" ttt "\n", 1);
+}
+
+void test_object_message__keep_comments(void)
+{
+	assert_message_prettifying("# comment\n", "# comment", 0);
+	assert_message_prettifying("# comment\n", "# comment\n", 0);
+	assert_message_prettifying("# comment\n", "# comment    \n", 0);
+
+	assert_message_prettifying(ttt "\n" "# comment\n", ttt "\n" "# comment\n", 0);
+	assert_message_prettifying("# comment\n" ttt "\n", "# comment\n" ttt "\n", 0);
+	assert_message_prettifying(ttt "\n" "# comment\n" ttt "\n", ttt "\n" "# comment\n" ttt "\n", 0);
+}