[RFC/PATCH] commit: update the index after running the pre-commit hook

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Change git-commit to update the index after running the pre-commit
hook, but before it constructs the comments that'll accompany the
commit message displayed in the $EDITOR.

The use case for this is e.g. a pre-commit hook that looks like this:

    #!/bin/sh
    echo unf >>hlagh
    git add hlagh

In that case the $EDITOR will display "hlagh" under "Untracked files",
but it should be under "Changes to be committed:". Refreshing before
we construct the message fixes this bug.

Reported-by: Hinrik Örn Sigurðsson <hinrik.sig@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

2009/5/3 Hinrik Örn Sigurðsson <hinrik.sig@gmail.com>:
> I have a pre-commit hook which extracts documentation from file $foo
> if it has pending changes to be committed. The hook creates/updates
> the documentation file and calls "git add" on it.
>
> When I do "git commit", the COMMIT_EDITMSG delivered to my editor
> notes that this documentation file has been created/updated, but not
> that its changes have been added to the index. However, if I go ahead
> with the commit, I can see that the doc file changes were indeed
> committed.
>
> Here is a simplified test case:
>
> $ cat .git/hooks/pre-commit
> #!/usr/bin/env perl
> use strict;
> use warnings;
>
> my $old = qx"git rev-parse HEAD:foo 2>/dev/null";
> my $new = qx"git rev-parse :foo 2>/dev/null";
>
> if (($? >> 8) != 0 || $old ne $new) {
>    system "cat source > dest";
>    system "git add dest";
> }
>
> $ ls
> source
>
> $ echo 123 >> source
>
> $ git status
> # On branch master
> # Changed but not updated:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   source
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> $ git commit -a
> Test commit
> # Please enter the commit message for your changes. Lines starting
> # with '#' will be ignored, and an empty message aborts the commit.
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       modified:   source
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       dest
> [master 535ed7f] Test commit
>  2 files changed, 3 insertions(+), 0 deletions(-)
>  create mode 100644 dest
>
> $ git status
> # On branch master
> nothing to commit (working directory clean)

Thanks for the report. Here's a proposed fix for this. I'm not
familiar with the commit code so this may be the wrong way to go, but
it fixes the bug and passes all tests.

The reporter has promised offlist to write a test for this.

 builtin/commit.c |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..dbb4ff5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -550,6 +550,20 @@ static int ends_rfc2822_footer(struct strbuf *sb)
 	return 1;
 }
 
+static int update_index(const char *index_file) {
+	discard_cache();
+	read_cache_from(index_file);
+	if (!active_cache_tree)
+		active_cache_tree = cache_tree();
+	if (cache_tree_update(active_cache_tree,
+			      active_cache, active_nr, 0, 0) < 0) {
+		error("Error building trees");
+		return 0;
+	}
+
+	return 1;
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct wt_status *s)
 {
@@ -565,6 +579,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
 
+	/* Update the index after we run the pre-commit hook, but before
+	 * we construct the message we're sending to the editor. The
+	 * pre-commit hook may e.g. create a new file and add it to the
+	 * index.
+	 *
+	 * Having that file show up as modified but not staged is confusing.
+	 */
+	if (!update_index(index_file))
+		return 0;
+
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
@@ -728,15 +752,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 * and write it out as a tree.  We must do this before we invoke
 	 * the editor and after we invoke run_status above.
 	 */
-	discard_cache();
-	read_cache_from(index_file);
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
-	if (cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0) < 0) {
-		error("Error building trees");
+	if (!update_index(index_file))
 		return 0;
-	}
 
 	if (run_hook(index_file, "prepare-commit-msg",
 		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
-- 
1.7.2.1.414.g9bf49