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

From

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

Date

Aug 18, 2010, 13:54 UTC

Message-ID

<1282139649-21097-1-git-send-email-avarab@gmail.com>

In-Reply-To

<6e4c323b0905031620m48769dbdp52684d6b470ebea3@mail.gmail.com>

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

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

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)

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