[PATCH 2/2] commit: keep the commit on a no-op amend

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git commit --amend --no-edit" reset the committer date to "now" and
rewrote the commit even when nothing else changed, moving the branch tip
to a new hash for an effective no-op.

Build the amended commit reusing the existing committer date: if that
reproduces the current commit, leave the branch alone, report "nothing
to amend", and skip the reflog entry and the post-commit and post-rewrite
hooks.

Signing always rewrites the commit, since its signature cannot reproduce
the original, so it skips this detection.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-commit.adoc         |   6 ++
 builtin/commit.c                      |  56 ++++++++++++
 t/t7501-commit-basic-functionality.sh | 119 ++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..c433c60929 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -282,6 +282,12 @@ variable (see linkgit:git-config[1]).
 	parents and author as the current one (the `--reset-author`
 	option can countermand this).
 +
+If the amended commit would be identical to the original (its tree,
+message, author, parents, and committer are all unchanged), the original
+committer date is kept so that the commit, and thus the branch tip, is
+left untouched. A commit that is being signed (`-S`, or `commit.gpgsign`)
+is always rewritten, since its signature cannot reproduce the original.
++
 --
 It is a rough equivalent for:
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 1a51450660..e330a53d5c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -17,6 +17,7 @@
 #include "dir.h"
 #include "editor.h"
 #include "environment.h"
+#include "ident.h"
 #include "diff.h"
 #include "commit.h"
 #include "add-interactive.h"
@@ -760,6 +761,49 @@ static void prepare_amend_commit(struct commit *commit, struct strbuf *sb,
 	repo_unuse_commit_buffer(the_repository, commit, buffer);
 }
 
+/*
+ * Rebuild the amended commit reusing the existing committer date and report
+ * whether it reproduces the current commit. Because the committer date is the
+ * only field that an amend would otherwise replace with "now", an exact match
+ * means everything else (tree, message, author, parents, committer identity)
+ * is unchanged too.
+ */
+static int amend_is_noop(struct commit *current_head,
+				 const struct strbuf *message,
+				 const struct commit_list *parents,
+				 const char *author,
+				 const struct commit_extra_header *extra,
+				 struct object_id *oid)
+{
+	const char *buffer, *committer_line;
+	size_t len;
+	struct ident_split ident;
+	struct strbuf date = STRBUF_INIT;
+	int unchanged = 0;
+
+	buffer = repo_get_commit_buffer(the_repository, current_head, NULL);
+	committer_line = find_commit_header(buffer, "committer", &len);
+	if (committer_line && !split_ident_line(&ident, committer_line, len) &&
+	    ident.date_begin) {
+		const char *committer;
+
+		strbuf_add(&date, ident.date_begin,
+			   ident.tz_end - ident.date_begin);
+		committer = fmt_ident(getenv("GIT_COMMITTER_NAME"),
+				      getenv("GIT_COMMITTER_EMAIL"),
+				      WANT_COMMITTER_IDENT, date.buf,
+				      IDENT_STRICT);
+		if (!commit_tree_extended(message->buf, message->len,
+					  &the_repository->index->cache_tree->oid,
+					  parents, oid, author, committer, NULL,
+					  extra))
+			unchanged = oideq(oid, &current_head->object.oid);
+	}
+	repo_unuse_commit_buffer(the_repository, current_head, buffer);
+	strbuf_release(&date);
+	return unchanged;
+}
+
 static void change_data_free(void *util, const char *str UNUSED)
 {
 	struct wt_status_change_data *d = util;
@@ -1943,6 +1987,18 @@ int cmd_commit(int argc,
 		append_merge_tag_headers(parents, &tail);
 	}
 
+	if (amend && current_head && !sign_commit &&
+	    amend_is_noop(current_head, &sb, parents, author_ident.buf,
+			  extra, &oid)) {
+		commit_index_files_or_die();
+		if (!quiet)
+			fprintf(stderr,
+				_("nothing to amend; %s left unchanged\n"),
+				repo_find_unique_abbrev(the_repository, &oid,
+							DEFAULT_ABBREV));
+		goto cleanup;
+	}
+
 	if (commit_tree_extended(sb.buf, sb.len, &the_repository->index->cache_tree->oid,
 				 parents, &oid, author_ident.buf, NULL,
 				 sign_commit, extra)) {
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index a37509f004..160edb9c0a 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -11,6 +11,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-diff.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
 
 author='The Real Author <someguy@his.email.org>'
 
@@ -654,6 +655,124 @@ test_expect_success 'amend commit to fix author' '
 
 '
 
+test_expect_success 'amend --no-edit that changes nothing keeps the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	git commit --amend --no-edit 2>err &&
+	test_cmp_rev $old HEAD &&
+	test_grep "nothing to amend" err
+'
+
+test_expect_success 'amend --no-edit keeps the commit out of the reflog' '
+	git reset --hard &&
+	git rev-parse HEAD@{0} >before &&
+	test_tick &&
+	git commit --amend --no-edit &&
+	git rev-parse HEAD@{0} >after &&
+	test_cmp before after
+'
+
+test_expect_success 'amend --signoff is idempotent once signed off' '
+	git reset --hard &&
+	test_tick &&
+	git commit --amend --no-edit --signoff &&
+	signed=$(git rev-parse HEAD) &&
+	git log -1 --format=%B | grep "^Signed-off-by:" &&
+	test_tick &&
+	git commit --amend --no-edit --signoff &&
+	test_cmp_rev $signed HEAD
+'
+
+test_expect_success 'amend that changes the tree still rewrites the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	echo changed >>file &&
+	git add file &&
+	test_tick &&
+	git commit --amend --no-edit &&
+	test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes the committer still rewrites the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	GIT_COMMITTER_EMAIL=other@example.com \
+		git commit --amend --no-edit &&
+	test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes only the message still rewrites the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	git commit --amend -m "new message" &&
+	test_cmp_rev ! $old HEAD &&
+	echo "new message" >expect &&
+	git log -1 --format=%s >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'amend --allow-empty of an empty commit that changes nothing keeps it' '
+	test_when_finished "git reset --hard parent && git tag -d parent" &&
+	git tag parent &&
+	git commit --allow-empty -m "empty" &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	git commit --amend --no-edit --allow-empty 2>err &&
+	test_cmp_rev $old HEAD &&
+	test_grep "nothing to amend" err
+'
+
+test_expect_success GPG 'amend --no-edit of a signed commit is not a no-op' '
+	git reset --hard &&
+	test_tick &&
+	git commit --amend --no-edit -S &&
+	signed=$(git rev-parse HEAD) &&
+	git verify-commit HEAD &&
+	test_tick &&
+	git commit --amend --no-edit -S &&
+	test_cmp_rev ! $signed HEAD &&
+	git verify-commit HEAD
+'
+
+test_expect_success GPG 'amend --no-edit with commit.gpgsign is not a no-op' '
+	git reset --hard &&
+	test_tick &&
+	old=$(git rev-parse HEAD) &&
+	git -c commit.gpgsign=true commit --amend --no-edit &&
+	test_cmp_rev ! $old HEAD &&
+	git verify-commit HEAD
+'
+
+test_expect_success 'amend --reset-author rewrites the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	git commit --amend --no-edit --reset-author &&
+	test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend --date rewrites the commit' '
+	git reset --hard &&
+	old=$(git rev-parse HEAD) &&
+	test_tick &&
+	git commit --amend --no-edit --date="@1234567890 +0000" &&
+	test_cmp_rev ! $old HEAD
+'
+
+test_expect_success 'amend that changes nothing skips the post-commit hook' '
+	test_when_finished "rm -f post-commit.ran" &&
+	test_hook post-commit <<-\EOF &&
+	>post-commit.ran
+	EOF
+	git reset --hard &&
+	test_tick &&
+	git commit --amend --no-edit &&
+	test_path_is_missing post-commit.ran
+'
+
 test_expect_success 'git commit <file> with dirty index' '
 	echo tacocat >elif &&
 	echo tehlulz >chz &&
-- 
gitgitgadget