{"thread":{"id":"11867","subject":"[PATCH 1/4] git-commit: support variable number of hook arguments","startedAt":"2008-01-21T14:02:48Z","lastAt":"2008-02-05T06:07:33Z","messageCount":13,"participants":["Paolo Bonzini","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"67352","messageId":"2885c7896ba72adee6bc5c9e2210a8881904f601.1200933409.git.bonzini@gnu.org","threadId":"11867","inReplyTo":"2885c7896ba72adee6bc5c9e2210a8881904f601.head.git.bonzini@gnu.org","subject":"[PATCH 1/4] git-commit: support variable number of hook arguments","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-01-21T14:02:48Z","receivedAt":"2008-01-21T14:02:48Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"This is a preparatory patch to allow using run_hook for the\nprepare-commit-msg hook.\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n\n---\n builtin-commit.c |   59 ++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 33 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0227936..fc59660 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -339,6 +339,38 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \treturn s.commitable;\n }\n \n+static int run_hook(const char *index_file, const char *name, ...)\n+{\n+\tstruct child_process hook;\n+\tconst char *argv[10], *env[2];\n+\tchar index[PATH_MAX];\n+\tva_list args;\n+\tint i;\n+\n+\tva_start(args, name);\n+\targv[0] = git_path(\"hooks/%s\", name);\n+\ti = 0;\n+\tdo {\n+\t\targv[++i] = va_arg(args, const char *);\n+\t} while (argv[i]);\n+\tva_end(args);\n+\n+\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n+\tenv[0] = index;\n+\tenv[1] = NULL;\n+\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn 0;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.no_stdin = 1;\n+\thook.stdout_to_stderr = 1;\n+\thook.env = env;\n+\n+\treturn run_command(&hook);\n+}\n+\n static const char sign_off_header[] = \"Signed-off-by: \";\n \n static int prepare_log_message(const char *index_file, const char *prefix)\n@@ -673,31 +705,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \treturn commitable ? 0 : 1;\n }\n \n-static int run_hook(const char *index_file, const char *name, const char *arg)\n-{\n-\tstruct child_process hook;\n-\tconst char *argv[3], *env[2];\n-\tchar index[PATH_MAX];\n-\n-\targv[0] = git_path(\"hooks/%s\", name);\n-\targv[1] = arg;\n-\targv[2] = NULL;\n-\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\tenv[0] = index;\n-\tenv[1] = NULL;\n-\n-\tif (access(argv[0], X_OK) < 0)\n-\t\treturn 0;\n-\n-\tmemset(&hook, 0, sizeof(hook));\n-\thook.argv = argv;\n-\thook.no_stdin = 1;\n-\thook.stdout_to_stderr = 1;\n-\thook.env = env;\n-\n-\treturn run_command(&hook);\n-}\n-\n static void print_summary(const char *prefix, const unsigned char *sha1)\n {\n \tstruct rev_info rev;\n@@ -872,7 +879,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n \t}\n \tif (!no_verify &&\n-\t    run_hook(index_file, \"commit-msg\", git_path(commit_editmsg))) {\n+\t    run_hook(index_file, \"commit-msg\", git_path(commit_editmsg), NULL)) {\n \t\trollback_index_files();\n \t\texit(1);\n \t}\n"},{"id":"67354","messageId":"3e99e048c3f249bb0440ec6ab221ef622a2db825.1200933409.git.bonzini@gnu.org","threadId":"11867","inReplyTo":"2885c7896ba72adee6bc5c9e2210a8881904f601.head.git.bonzini@gnu.org","subject":"[PATCH 2/4] git-commit: set GIT_EDITOR=: if editor will not be launched","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-01-21T14:06:45Z","receivedAt":"2008-01-21T14:06:45Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"This is a preparatory patch that provides a simple way for the future\nprepare-commit-msg hook to discover if the editor will be launched.\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n\n---\n Documentation/hooks.txt |    4 ++++\n builtin-commit.c        |    2 ++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/Documentation/hooks.txt b/Documentation/hooks.txt\nindex f110162..e8d80cf 100644\n--- a/Documentation/hooks.txt\n+++ b/Documentation/hooks.txt\n@@ -61,6 +61,10 @@ The default 'pre-commit' hook, when enabled, catches introduction\n of lines with trailing whitespaces and aborts the commit when\n such a line is found.\n \n+All the `git-commit` hooks are invoked with the environment\n+variable `GIT_EDITOR=:` if the command will not bring up an editor\n+to modify the commit message.\n+\n commit-msg\n ----------\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex fc59660..955cc84 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -593,6 +593,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tuse_editor = 0;\n \tif (edit_flag)\n \t\tuse_editor = 1;\n+\tif (!use_editor)\n+\t\tsetenv(\"GIT_EDITOR\", \":\", 1);\n \n \tif (get_sha1(\"HEAD\", head_sha1))\n \t\tinitial_commit = 1;\n"},{"id":"67353","messageId":"2885c7896ba72adee6bc5c9e2210a8881904f601.head.git.bonzini@gnu.org","threadId":"11867","inReplyTo":null,"subject":"[PATCH] git-commit: add a prepare-commit-msg hook","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-01-21T14:27:35Z","receivedAt":"2008-01-21T14:27:35Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"This series of patches adds a prepare-commit-msg hook.\nThe prepare-commit-msg hook is run whenever a \"fresh\" commit message\nis prepared, just before it is shown in the editor (if it is).\nIt can modify the commit message in-place and/or abort the commit.\nI implemented Alex Riesen's suggestion to tell the hook where the\nmessage came from, and now run the hook even if the editor is not\nrun.\n\nPatches 1 and 2 are small changes.  Patch 1 changes run_hook to\naccept a variable-length NULL-terminated list of arguments.  Patch 2\nforces GIT_EDITOR to : if editor will not be launched; this is the\nsimplest way I found to tell the prepare-commit-msg hook whether\nthe editor will be launched.\n\nPatch 3 is bigger; it refactors parts of git-commit to do all the\nlog message processing at the same time.  Currently the message\nis prepared soon, but only edited after the first part of the commit\nobject is prepared.  This simplifies a little the code for part 4.\n\nPart 4 actually adds the hook, including documentation and testcases.\nThe hook takes two parameters.  The first is the source of the commit\nmessage (detailed more in the commit message and in the docs), which\nis either an English word or a commit SHA1.  The second\nparameter if the name of the file that the commit log message.\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n"},{"id":"67356","messageId":"5840dd34338023e3af8446241a63927d18772f62.1200933409.git.bonzini@gnu.org","threadId":"11867","inReplyTo":"2885c7896ba72adee6bc5c9e2210a8881904f601.head.git.bonzini@gnu.org","subject":"[PATCH 4/4] git-commit: add a prepare-commit-msg hook","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-01-21T14:27:35Z","receivedAt":"2008-01-21T14:27:35Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"The prepare-commit-msg hook is run whenever a \"fresh\" commit message\nis prepared, just before it is shown in the editor (if it is).\nIt can modify the commit message in-place and/or abort the commit.\n\nIt takes two parameters.  The first is the source of the commit\nmessage, and can be: `message` (if a `\\-m` or `\\-F` option was\ngiven); `template` (if a `\\-t` option was given or the\nconfiguration option `commit.template` is set); `merge` (if the\ncommit is a merge or a `.git/MERGE_MSG file exists); `squash`\n(if a `.git/SQUASH_MSG file exists); or a commit id (if a\n`\\-c`, `\\-C` or `\\--amend` option was given).  The second\nparameter if the name of the file that the commit log message.\n\nThe hook is not suppressed by the `\\--no-verify` option.  However,\nexiting with non-zero status only aborts the commit if said option\nis not given to `git-commit`.\n\nWhile the default hook just adds a Signed-Off-By line at the bottom\nof the commit messsage, the hook is more intended to build a template\nfor the commit message following project standards, that the user\ncan then edit or discard altogether.\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n\n---\n Documentation/git-commit.txt        |    4 \n Documentation/hooks.txt             |   33 +++\n builtin-commit.c                    |   21 ++\n t/t7505-prepare-commit-msg-hook.sh  |  303 ++++++++++++++++++++++++++++++++++++\n templates/hooks--commit-msg         |    3 \n templates/hooks--prepare-commit-msg |   14 +\n 6 files changed, 375 insertions(+), 3 deletions(-)\n\n\tThe whole patch series is against the `next' branch.\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex c3725b2..b4ae61f 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -280,8 +280,8 @@ order).\n \n HOOKS\n -----\n-This command can run `commit-msg`, `pre-commit`, and\n-`post-commit` hooks.  See link:hooks.html[hooks] for more\n+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,\n+and `post-commit` hooks.  See link:hooks.html[hooks] for more\n information.\n \n \ndiff --git a/Documentation/hooks.txt b/Documentation/hooks.txt\nindex e8d80cf..f25bcd5 100644\n--- a/Documentation/hooks.txt\n+++ b/Documentation/hooks.txt\n@@ -55,7 +55,8 @@ This hook is invoked by `git-commit`, and can be bypassed\n with `\\--no-verify` option.  It takes no parameter, and is\n invoked before obtaining the proposed commit log message and\n making a commit.  Exiting with non-zero status from this script\n-causes the `git-commit` to abort.\n+causes the `git-commit` to abort.  This hook can also modify\n+the index.\n \n The default 'pre-commit' hook, when enabled, catches introduction\n of lines with trailing whitespaces and aborts the commit when\n@@ -65,6 +66,36 @@ All the `git-commit` hooks are invoked with the environment\n variable `GIT_EDITOR=:` if the command will not bring up an editor\n to modify the commit message.\n \n+prepare-commit-msg\n+------------------\n+\n+This hook is invoked by `git-commit` right after preparing the\n+default log message, and before the editor is started.\n+\n+It takes two parameters.  The first is the source of the commit\n+message, and can be: `message` (if a `\\-m` or `\\-F` option was\n+given); `template` (if a `\\-t` option was given or the\n+configuration option `commit.template` is set); `merge` (if the\n+commit is a merge or a `.git/MERGE_MSG file exists); `squash`\n+(if a `.git/SQUASH_MSG file exists); or a commit id (if a\n+`\\-c`, `\\-C` or `\\--amend` option was given).  The second\n+parameter if the name of the file that the commit log message.\n+\n+The hook is not suppressed by the `\\--no-verify` option.  However,\n+exiting with non-zero status only aborts the commit if said option\n+is not given to `git-commit`.\n+\n+The hook is allowed to edit the message file in place, and\n+can be used to augment the default commit message with some\n+project standard information.  It can also be used for the same\n+purpose as the pre-commit message, if the verification has\n+to be skipped for automatic commits (e.g. during rebasing).\n+\n+The default 'prepare-commit-msg' hook adds a Signed-Off-By line\n+(doing it with a hook is not necessarily a good idea, but doing\n+it in 'commit-msg' is worse because you are not reminded in\n+the editor).\n+\n commit-msg\n ----------\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex fed549e..1a083f7 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -388,36 +388,49 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tstruct strbuf sb;\n \tchar *buffer;\n \tFILE *fp;\n+\tconst char *hook_arg = NULL;\n \n \tstrbuf_init(&sb, 0);\n \tif (message.len) {\n \t\tstrbuf_addbuf(&sb, &message);\n+\t\thook_arg = \"message\";\n \t} else if (logfile && !strcmp(logfile, \"-\")) {\n \t\tif (isatty(0))\n \t\t\tfprintf(stderr, \"(reading log message from standard input)\\n\");\n \t\tif (strbuf_read(&sb, 0, 0) < 0)\n \t\t\tdie(\"could not read log from standard input\");\n+\t\thook_arg = \"message\";\n \t} else if (logfile) {\n \t\tif (strbuf_read_file(&sb, logfile, 0) < 0)\n \t\t\tdie(\"could not read log file '%s': %s\",\n \t\t\t    logfile, strerror(errno));\n+\t\thook_arg = \"message\";\n \t} else if (use_message) {\n \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n \t\tif (!buffer || buffer[2] == '\\0')\n \t\t\tdie(\"commit has empty message\");\n \t\tstrbuf_add(&sb, buffer + 2, strlen(buffer + 2));\n+\t\thook_arg = use_message;\n \t} else if (!stat(git_path(\"MERGE_MSG\"), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path(\"MERGE_MSG\"), 0) < 0)\n \t\t\tdie(\"could not read MERGE_MSG: %s\", strerror(errno));\n+\t\thook_arg = \"merge\";\n \t} else if (!stat(git_path(\"SQUASH_MSG\"), &statbuf)) {\n \t\tif (strbuf_read_file(&sb, git_path(\"SQUASH_MSG\"), 0) < 0)\n \t\t\tdie(\"could not read SQUASH_MSG: %s\", strerror(errno));\n+\t\thook_arg = \"squash\";\n \t} else if (template_file && !stat(template_file, &statbuf)) {\n \t\tif (strbuf_read_file(&sb, template_file, 0) < 0)\n \t\t\tdie(\"could not read %s: %s\",\n \t\t\t    template_file, strerror(errno));\n+\t\thook_arg = \"template\";\n \t}\n \n+\t/* This final case does not modify the template message, it just sets\n+\t   the argument to the prepare-commit-msg hook.  */\n+\telse if (in_merge)\n+\t\thook_arg = \"merge\";\n+\n \tfp = fopen(git_path(commit_editmsg), \"w\");\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n@@ -511,6 +524,14 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\treturn 0;\n \t}\n \n+\t/* Note that we always run the hook, even if no_verify! */\n+\tif (run_hook(index_file, \"prepare-commit-msg\",\n+\t\t     git_path(commit_editmsg), hook_arg, NULL) &&\n+\t    !no_verify) {\n+\t\trollback_index_files();\n+\t\treturn 0;\n+\t}\n+\n \tif (use_editor) {\n \t\tchar index[PATH_MAX];\n \t\tconst char *env[2] = { index, NULL };\ndiff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh\nnew file mode 100755\nindex 0000000..e632bbe\n--- /dev/null\n+++ b/t/t7505-prepare-commit-msg-hook.sh\n@@ -0,0 +1,303 @@\n+#!/bin/sh\n+\n+test_description='prepare-commit-msg hook'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'with no hook' '\n+\n+\techo \"foo\" > file &&\n+\tgit add file &&\n+\tgit commit -m \"first\"\n+\n+'\n+\n+# set up fake editor for interactive editing\n+cat > fake-editor <<'EOF'\n+#!/bin/sh\n+exit 0\n+EOF\n+chmod +x fake-editor\n+FAKE_EDITOR=\"$(pwd)/fake-editor\"\n+export FAKE_EDITOR\n+\n+# now install hook that always succeeds and adds a message\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+HOOK=\"$HOOKDIR/prepare-commit-msg\"\n+mkdir -p \"$HOOKDIR\"\n+cat > \"$HOOK\" <<'EOF'\n+#!/bin/sh\n+if test \"$2\" = HEAD; then\n+  set \"$1\" $(git-rev-parse HEAD)\n+fi\n+if test \"$GIT_EDITOR\" = :; then\n+  sed -e \"1s/.*/${2-default} (no editor)/\" \"$1\" > msg.tmp\n+else\n+  sed -e \"1s/.*/${2-default}/\" \"$1\" > msg.tmp\n+fi\n+mv msg.tmp \"$1\"\n+exit 0\n+EOF\n+chmod +x \"$HOOK\"\n+\n+echo dummy template > \"$(git rev-parse --git-dir)/template\"\n+\n+test_expect_success 'with succeeding hook (-m)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -m \"more\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"message (no editor)\"\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-m editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit -e -m \"more more\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = message\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-t)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -t \"$(git rev-parse --git-dir)/template\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = template\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-F)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more | git commit -F -) &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"message (no editor)\"\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-F editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more more | GIT_EDITOR=\"$FAKE_EDITOR\" git commit -e -F -) &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = message\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-C)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -C $head &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head (no editor)\"\n+\n+'\n+\n+test_expect_success 'with succeeding hook (editor)' '\n+\n+\techo \"more more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = default\n+\n+'\n+\n+test_expect_success 'with succeeding hook (--amend)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --amend &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head\"\n+\n+'\n+\n+test_expect_success 'with succeeding hook (-c)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit -c $head &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head\"\n+\n+'\n+\n+# now a hook that fails\n+cat > \"$HOOK\" << 'EOF'\n+#!/bin/sh\n+if test \"$2\" = HEAD; then\n+  set \"$1\" $(git-rev-parse HEAD)\n+fi\n+if test \"$GIT_EDITOR\" = :; then\n+  sed -e \"1s/.*/${2-default} (no editor)/\" \"$1\" > msg.tmp\n+else\n+  sed -e \"1s/.*/${2-default}/\" \"$1\" > msg.tmp\n+fi\n+mv msg.tmp \"$1\"\n+exit 1\n+EOF\n+\n+test_expect_success 'with failing hook and --no-verify (-m)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit --no-verify -m \"more\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"message (no editor)\"\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-m editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --no-verify -e -m \"more more\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = message\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-t)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit --no-verify -t \"$(git rev-parse --git-dir)/template\" &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = template\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-F)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more | git commit --no-verify -F -) &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"message (no editor)\"\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-F editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more more | GIT_EDITOR=\"$FAKE_EDITOR\" git commit --no-verify -e -F -) &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = message\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-C)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit --no-verify -C $head &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head (no editor)\"\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (editor)' '\n+\n+\techo \"more more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --no-verify &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = default\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (--amend)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --no-verify --amend &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head\"\n+\n+'\n+\n+test_expect_success 'with failing hook and --no-verify (-c)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --no-verify -c $head &&\n+\ttest \"`git log -1 --pretty=format:%s`\" = \"$head\"\n+\n+'\n+\n+test_expect_failure 'with failing hook (-m)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -m \"more\"\n+\n+'\n+\n+test_expect_failure 'with failing hook (-m editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit -e -m \"more more\"\n+\n+'\n+\n+test_expect_failure 'with failing hook (-t)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -t \"$(git rev-parse --git-dir)/template\"\n+\n+'\n+\n+test_expect_failure 'with failing hook (-F)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more | git commit -F -)\n+\n+'\n+\n+test_expect_failure 'with failing hook (-F editor)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\t(echo more more | GIT_EDITOR=\"$FAKE_EDITOR\" git commit -e -F -)\n+\n+'\n+\n+test_expect_failure 'with failing hook (-C)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tgit commit -C $head\n+\n+'\n+\n+test_expect_failure 'with failing hook (editor)' '\n+\n+\techo \"more more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit\n+\n+'\n+\n+test_expect_failure 'with failing hook (--amend)' '\n+\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit --amend\n+\n+'\n+\n+test_expect_failure 'with failing hook (-c)' '\n+\n+\thead=`git rev-parse HEAD` &&\n+\techo \"more\" >> file &&\n+\tgit add file &&\n+\tGIT_EDITOR=\"$FAKE_EDITOR\" git commit -c $head\n+\n+'\n+\n+\n+test_done\ndiff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg\nindex c5cdb9d..4ef86eb 100644\n--- a/templates/hooks--commit-msg\n+++ b/templates/hooks--commit-msg\n@@ -9,6 +9,9 @@\n # To enable this hook, make this file executable.\n \n # Uncomment the below to add a Signed-off-by line to the message.\n+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg\n+# hook is more suited to it.\n+#\n # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\\(.*>\\).*$/Signed-off-by: \\1/p')\n # grep -qs \"^$SOB\" \"$1\" || echo \"$SOB\" >> \"$1\"\n \ndiff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg\nnew file mode 100644\nindex 0000000..176283b\n--- /dev/null\n+++ b/templates/hooks--prepare-commit-msg\n@@ -0,0 +1,14 @@\n+#!/bin/sh\n+#\n+# An example hook script to prepare the commit log message.\n+# Called by git-commit with one argument, the name of the file\n+# that has the commit message.  The hook should exit with non-zero\n+# status after issuing an appropriate message if it wants to stop the\n+# commit.  The hook is allowed to edit the commit message file.\n+#\n+# To enable this hook, make this file executable.\n+\n+# This example adds a Signed-off-by line to the message, that can\n+# still be edited.\n+SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\\(.*>\\).*$/Signed-off-by: \\1/p')\n+grep -qs \"^$SOB\" \"$1\" || echo \"$SOB\" >> \"$1\"\n"},{"id":"67355","messageId":"14fc1ba3f94a773ab2e5b8cdf0c230962f32180d.1200933409.git.bonzini@gnu.org","threadId":"11867","inReplyTo":"2885c7896ba72adee6bc5c9e2210a8881904f601.head.git.bonzini@gnu.org","subject":"[PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-01-21T14:33:51Z","receivedAt":"2008-01-21T14:33:51Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"This patch moves around some code from run_commit to prepare_log_message.\nThis simplifies a little the code for the next patch.  The biggest change\nis that the editor and the commit-msg hook are invoked before building\nthe prolog of the commit object.\n\nThis means that: 1) the commit may be aborted after editing the message\nif there is a problem writing out the tree object (slight disadvantage);\n2) no tree will be written out if the commit-msg hook fails (slight\nadvantage).\n\nSigned-off-by: Paolo Bonzini <bonzini@gnu.org>\n\n---\n builtin-commit.c |  114 ++++++++++++++++++++++++++++---------------------------\n 1 files changed, 60 insertions(+), 54 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 955cc84..fed549e 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -371,6 +371,14 @@ static int run_hook(const char *index_file, const char *name, ...)\n \treturn run_command(&hook);\n }\n \n+static int is_a_merge(const unsigned char *sha1)\n+{\n+\tstruct commit *commit = lookup_commit(sha1);\n+\tif (!commit || parse_commit(commit))\n+\t\tdie(\"could not parse HEAD commit\");\n+\treturn !!(commit->parents && commit->parents->next);\n+}\n+\n static const char sign_off_header[] = \"Signed-off-by: \";\n \n static int prepare_log_message(const char *index_file, const char *prefix)\n@@ -441,13 +449,38 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \n \tstrbuf_release(&sb);\n \n-\tif (!use_editor) {\n+\tif (use_editor) {\n+\t\tif (in_merge)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"#\\n\"\n+\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\t\"#\t%s\\n\"\n+\t\t\t\t\"# and try again.\\n\"\n+\t\t\t\t\"#\\n\",\n+\t\t\t\tgit_path(\"MERGE_HEAD\"));\n+\n+\t\tfprintf(fp,\n+\t\t\t\"\\n\"\n+\t\t\t\"# Please enter the commit message for your changes.\\n\"\n+\t\t\t\"# (Comment lines starting with '#' will \");\n+\t\tif (cleanup_mode == CLEANUP_ALL)\n+\t\t\tfprintf(fp, \"not be included)\\n\");\n+\t\telse /* CLEANUP_SPACE, that is. */\n+\t\t\tfprintf(fp, \"be kept.\\n\"\n+\t\t\t\t\"# You can remove them yourself if you want to)\\n\");\n+\t\tif (only_include_assumed)\n+\t\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n+\n+\t\tsaved_color_setting = wt_status_use_color;\n+\t\twt_status_use_color = 0;\n+\t\tcommitable = run_status(fp, index_file, prefix, 1);\n+\t\twt_status_use_color = saved_color_setting;\n+\t} else {\n \t\tstruct rev_info rev;\n \t\tunsigned char sha1[20];\n \t\tconst char *parent = \"HEAD\";\n \n-\t\tfclose(fp);\n-\n \t\tif (!active_nr && read_cache() < 0)\n \t\t\tdie(\"Cannot read index\");\n \n@@ -464,39 +499,31 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \t\trun_diff_index(&rev, 1 /* cached */);\n \n-\t\treturn !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n+\t\tcommitable = !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n \t}\n \n-\tif (in_merge)\n-\t\tfprintf(fp,\n-\t\t\t\"#\\n\"\n-\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n-\t\t\t\"# If this is not correct, please remove the file\\n\"\n-\t\t\t\"#\t%s\\n\"\n-\t\t\t\"# and try again.\\n\"\n-\t\t\t\"#\\n\",\n-\t\t\tgit_path(\"MERGE_HEAD\"));\n-\n-\tfprintf(fp,\n-\t\t\"\\n\"\n-\t\t\"# Please enter the commit message for your changes.\\n\"\n-\t\t\"# (Comment lines starting with '#' will \");\n-\tif (cleanup_mode == CLEANUP_ALL)\n-\t\tfprintf(fp, \"not be included)\\n\");\n-\telse /* CLEANUP_SPACE, that is. */\n-\t\tfprintf(fp, \"be kept.\\n\"\n-\t\t\t\"# You can remove them yourself if you want to)\\n\");\n-\tif (only_include_assumed)\n-\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n-\n-\tsaved_color_setting = wt_status_use_color;\n-\twt_status_use_color = 0;\n-\tcommitable = run_status(fp, index_file, prefix, 1);\n-\twt_status_use_color = saved_color_setting;\n-\n \tfclose(fp);\n \n-\treturn commitable;\n+\tif (!commitable && !in_merge && !allow_empty &&\n+\t    !(amend && is_a_merge(head_sha1))) {\n+\t\trun_status(stdout, index_file, prefix, 0);\n+\t\tunlink(commit_editmsg);\n+\t\treturn 0;\n+\t}\n+\n+\tif (use_editor) {\n+\t\tchar index[PATH_MAX];\n+\t\tconst char *env[2] = { index, NULL };\n+\t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n+\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n+\t}\n+\n+\tif (!no_verify &&\n+\t    run_hook(index_file, \"commit-msg\", git_path(commit_editmsg), NULL)) {\n+\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n }\n \n /*\n@@ -755,14 +782,6 @@ int git_commit_config(const char *k, const char *v)\n \treturn git_status_config(k, v);\n }\n \n-static int is_a_merge(const unsigned char *sha1)\n-{\n-\tstruct commit *commit = lookup_commit(sha1);\n-\tif (!commit || parse_commit(commit))\n-\t\tdie(\"could not parse HEAD commit\");\n-\treturn !!(commit->parents && commit->parents->next);\n-}\n-\n static const char commit_utf8_warn[] =\n \"Warning: commit message does not conform to UTF-8.\\n\"\n \"You may want to amend it after fixing the message, or set the config\\n\"\n@@ -799,11 +818,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \t}\n \n-\tif (!prepare_log_message(index_file, prefix) && !in_merge &&\n-\t    !allow_empty && !(amend && is_a_merge(head_sha1))) {\n-\t\trun_status(stdout, index_file, prefix, 0);\n+\t/* Prepare, let the user edit, and validate the log message.  */\n+\tif (!prepare_log_message(index_file, prefix)) {\n \t\trollback_index_files();\n-\t\tunlink(commit_editmsg);\n \t\treturn 1;\n \t}\n \n@@ -872,19 +889,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_addf(&sb, \"encoding %s\\n\", git_commit_encoding);\n \tstrbuf_addch(&sb, '\\n');\n \n-\t/* Get the commit message and validate it */\n+\t/* Finally, get the commit message */\n \theader_len = sb.len;\n-\tif (use_editor) {\n-\t\tchar index[PATH_MAX];\n-\t\tconst char *env[2] = { index, NULL };\n-\t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n-\t}\n-\tif (!no_verify &&\n-\t    run_hook(index_file, \"commit-msg\", git_path(commit_editmsg), NULL)) {\n-\t\trollback_index_files();\n-\t\texit(1);\n-\t}\n \tif (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {\n \t\trollback_index_files();\n \t\tdie(\"could not read commit message\");\n"},{"id":"67379","messageId":"alpine.LSU.1.00.0802041637460.7372@racer.site","threadId":"11867","inReplyTo":"2885c7896ba72adee6bc5c9e2210a8881904f601.1200933409.git.bonzini@gnu.org","subject":"Re: [PATCH 1/4] git-commit: support variable number of hook arguments","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-04T16:43:27Z","receivedAt":"2008-02-04T16:43:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 21 Jan 2008, Paolo Bonzini wrote:\n\n> +static int run_hook(const char *index_file, const char *name, ...)\n> +{\n> +\tstruct child_process hook;\n> +\tconst char *argv[10], *env[2];\n> +\tchar index[PATH_MAX];\n> +\tva_list args;\n> +\tint i;\n> +\n> +\tva_start(args, name);\n> +\targv[0] = git_path(\"hooks/%s\", name);\n> +\ti = 0;\n> +\tdo {\n\nHere, an\n\n\t\tif (++i >= ARRAY_SIZE(argv))\n\t\t\tdie (\"run_hook(): too many arguments\");\n\nis missing.  Even nicer, you could have\n\n\tint argc = 1;\n\tchar **argv = malloc(sizeof(*argv) * 2);\n\nand\n\n\t\tif (++i >= argc)\n\t\t\targc = ALLOC_GROW(argv, i + 1, argc);\n\nin the loop.  Of course, you would have to change the \"++i\" to \"i\" in this \nline:\n\n> +\t\targv[++i] = va_arg(args, const char *);\n\nCiao,\nDscho\n"},{"id":"67380","messageId":"alpine.LSU.1.00.0802041646461.7372@racer.site","threadId":"11867","inReplyTo":"14fc1ba3f94a773ab2e5b8cdf0c230962f32180d.1200933409.git.bonzini@gnu.org","subject":"Re: [PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-04T16:48:55Z","receivedAt":"2008-02-04T16:48:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 21 Jan 2008, Paolo Bonzini wrote:\n\n> This means that: 1) the commit may be aborted after editing the message\n> if there is a problem writing out the tree object (slight disadvantage);\n\nI consider this more than a slight disadvantage.  I regularly take ages \ncoming up with a good commit message, because I think that the overall \ntime balance is better with me spending more time on the message, but \nevery reader spending less time to guess what I meant.\n\nSo I would be quite annoyed to edit a message, only to find out that for \nwhatever reason the commit was not successful.\n\nAre you _sure_ you need 3/4 for 4/4?\n\nCiao,\nDscho\n"},{"id":"67383","messageId":"47A747EB.9020909@gnu.org","threadId":"11867","inReplyTo":"alpine.LSU.1.00.0802041646461.7372@racer.site","subject":"Re: [PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-02-04T17:14:19Z","receivedAt":"2008-02-04T17:14:19Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 21 Jan 2008, Paolo Bonzini wrote:\n> \n>> This means that: 1) the commit may be aborted after editing the message\n>> if there is a problem writing out the tree object (slight disadvantage);\n> \n> I consider this more than a slight disadvantage.  I regularly take ages \n> coming up with a good commit message, because I think that the overall \n> time balance is better with me spending more time on the message, but \n> every reader spending less time to guess what I meant.\n> \n> So I would be quite annoyed to edit a message, only to find out that for \n> whatever reason the commit was not successful.\n\nJust to make it clearer, the piece of code that would have to fail, for \nthe behavior to change, is this:\n\n         discard_cache();\n         read_cache_from(index_file);\n         if (!active_cache_tree)\n                 active_cache_tree = cache_tree();\n         if (cache_tree_update(active_cache_tree,\n                               active_cache, active_nr, 0, 0) < 0) {\n                 rollback_index_files();\n                 die(\"Error building trees\");\n         }\n\nThere are a couple of failure modes hidden in update_one (in cache-tree.c):\n\n         sub = find_subtree(it, path + baselen, entlen, 0);\n         if (!sub)\n                 die(\"cache-tree.c: '%.*s' in '%s' not found\",\n                     entlen, path + baselen, path);\n\n         ...\n\n         if (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1))\n                 return error(\"invalid object %s\", sha1_to_hex(sha1));\n\nbut I don't really understand how they could happen.  If you do, I \nappreciate being taught.  :-)\n\nAlso note that some problems writing the tree object (I cannot think of \nanything but running out of diskspace -- permission errors could be \ncaught creating the index and logmessage too) could also happen writing \nthe commit object.  I don't think in practice the disadvantage I \nmentioned can happen.\n\n> Are you _sure_ you need 3/4 for 4/4?\n\nProbably no, but it makes it much easier to avoid duplicated code (all \nthe cases are already enumerated in prepare_log_message).  I tried it \nfirst and did not finish because the code was really really ugly.\n\nThanks for the review.\n\nPaolo\n"},{"id":"67468","messageId":"7vzluggp0q.fsf@gitster.siamese.dyndns.org","threadId":"11867","inReplyTo":"47A747EB.9020909@gnu.org","subject":"Re: [PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T01:39:33Z","receivedAt":"2008-02-05T01:39:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini <bonzini@gnu.org> writes:\n\n> Johannes Schindelin wrote:\n>>\n>> On Mon, 21 Jan 2008, Paolo Bonzini wrote:\n>>\n>>> This means that: 1) the commit may be aborted after editing the message\n>>> if there is a problem writing out the tree object (slight disadvantage);\n>>\n>> I consider this more than a slight disadvantage.  I regularly take\n>> ages coming up with a good commit message, because I think that the\n>> overall time balance is better with me spending more time on the\n>> message, but every reader spending less time to guess what I meant.\n>>\n>> So I would be quite annoyed to edit a message, only to find out that\n>> for whatever reason the commit was not successful.\n>\n> Just to make it clearer, the piece of code that would have to fail,\n> for the behavior to change, is this:\n\nI suspect Dscho was worried about the case where he says \"git\ncommit\", types message and then write-tree finds out that the\nindex is still unmerged and the tree cannot be written out.\n\nAnd I'd be majorly annoyed if the \"slight disadvange\" was about\nthat.\n\n>         discard_cache();\n>         read_cache_from(index_file);\n>         if (!active_cache_tree)\n>                 active_cache_tree = cache_tree();\n>         if (cache_tree_update(active_cache_tree,\n>                               active_cache, active_nr, 0, 0) < 0) {\n>                 rollback_index_files();\n>                 die(\"Error building trees\");\n>         }\n\nI think this _could_ error out if your index is unmerged.\n\nHowever, if you have other code to error out early upon unmeregd\nindex before you collect the message from the editor, I think\nyou are Ok.\n"},{"id":"67472","messageId":"7vk5lkgkw2.fsf@gitster.siamese.dyndns.org","threadId":"11867","inReplyTo":"5840dd34338023e3af8446241a63927d18772f62.1200933409.git.bonzini@gnu.org","subject":"Re: [PATCH 4/4] git-commit: add a prepare-commit-msg hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T03:08:45Z","receivedAt":"2008-02-05T03:08:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini <bonzini@gnu.org> writes:\n\n> It takes two parameters.  The first is the source of the commit\n> message, and can be: `message` (if a `\\-m` or `\\-F` option was\n> given); `template` (if a `\\-t` option was given or the\n> configuration option `commit.template` is set); `merge` (if the\n> commit is a merge or a `.git/MERGE_MSG file exists); `squash`\n> (if a `.git/SQUASH_MSG file exists); or a commit id (if a\n> `\\-c`, `\\-C` or `\\--amend` option was given).  The second\n> parameter if the name of the file that the commit log message.\n\nPlease do without funny mark-ups.  The commit log is not\nAsciiDoc.\n\n> The hook is not suppressed by the `\\--no-verify` option.  However,\n> exiting with non-zero status only aborts the commit if said option\n> is not given to `git-commit`.\n\nI do not understand why.  I do understand that you do not want\nto bypass prepare-commit-msg with or without --no-verify,\nbecause this hook is not about input validation but about input\npreparation.  But I do not understand why a failure exit from it\nshould be treated any differently with or without --no-verify?\n\nIf you want to be strict and be safe catching a breakage in the\nprepare-commit-msg hook, you should always abort.  You could\nalso choose to ignore.  In either case, shouldn't the validation\nbe left to pre-commit hook (for the tree) and commit-msg hook\n(for the message, that is left after this hook)?\n\n> While the default hook just adds a Signed-Off-By line at the bottom\n\nIt's \"s/[OB]/ob/;\", and ...\n\n> of the commit messsage, the hook is more intended to build a template\n> for the commit message following project standards, that the user\n> can then edit or discard altogether.\n>\n> Signed-off-by: Paolo Bonzini <bonzini@gnu.org>\n\n... you know it ;-)\n"},{"id":"67473","messageId":"7vejbsgkup.fsf@gitster.siamese.dyndns.org","threadId":"11867","inReplyTo":"alpine.LSU.1.00.0802041637460.7372@racer.site","subject":"Re: [PATCH 1/4] git-commit: support variable number of hook arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T03:09:34Z","receivedAt":"2008-02-05T03:09:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Mon, 21 Jan 2008, Paolo Bonzini wrote:\n>\n>> +static int run_hook(const char *index_file, const char *name, ...)\n>> +{\n>> +\tstruct child_process hook;\n>> +\tconst char *argv[10], *env[2];\n>> +\tchar index[PATH_MAX];\n>> +\tva_list args;\n>> +\tint i;\n>> +\n>> +\tva_start(args, name);\n>> +\targv[0] = git_path(\"hooks/%s\", name);\n>> +\ti = 0;\n>> +\tdo {\n>\n> Here, an\n>\n> \t\tif (++i >= ARRAY_SIZE(argv))\n> \t\t\tdie (\"run_hook(): too many arguments\");\n>\n> is missing.  Even nicer, you could have\n>\n> \tint argc = 1;\n> \tchar **argv = malloc(sizeof(*argv) * 2);\n>\n> and\n>\n> \t\tif (++i >= argc)\n> \t\t\targc = ALLOC_GROW(argv, i + 1, argc);\n\nThe sanity check to make sure we do not feed too many is\ndefinitely needed.  For this application I think ALLOC_GROW() is\noverkill, as we do not pass unbound number of arguments to the\nhook.\n"},{"id":"67477","messageId":"7vzlugf3la.fsf@gitster.siamese.dyndns.org","threadId":"11867","inReplyTo":"7vzluggp0q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T04:07:45Z","receivedAt":"2008-02-05T04:07:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I suspect Dscho was worried about the case where he says \"git\n> commit\", types message and then write-tree finds out that the\n> index is still unmerged and the tree cannot be written out.\n>\n> And I'd be majorly annoyed if the \"slight disadvange\" was about\n> that.\n>\n>>         discard_cache();\n>>         read_cache_from(index_file);\n>>         if (!active_cache_tree)\n>>                 active_cache_tree = cache_tree();\n>>         if (cache_tree_update(active_cache_tree,\n>>                               active_cache, active_nr, 0, 0) < 0) {\n>>                 rollback_index_files();\n>>                 die(\"Error building trees\");\n>>         }\n>\n> I think this _could_ error out if your index is unmerged.\n\nI just tried.  This would annoy me.\n\nIt is common to do something like this:\n\n\t$ git cherry-pick -n somethingelse\n        $ git add this-and-that-path.c\n        $ git commit\n\nto get partial changes from somethingelse for paths you only\ncare about, and commit the result.  And you often get conflicts\nto paths that do not matter (think of backporting trivial part\nof fixes).  You need to reset the paths you do not care about\nthat conflicted, but you can forget that before running \"git\ncommit\".\n\nWith our \"git commit\", you first get \"foo: unmerged\" and you do\nnot see the editor.  With this change, you edit the message and\nthen see \"foo: ummerged\".\n"},{"id":"67484","messageId":"47A7FD25.7040401@gnu.org","threadId":"11867","inReplyTo":"7vzlugf3la.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/4] git-commit: Refactor creation of log message.","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2008-02-05T06:07:33Z","receivedAt":"2008-02-05T06:07:33Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"\n>> I think this _could_ error out if your index is unmerged.\n> \n> I just tried.  This would annoy me.\n\nRight, you beat me to it.  It would annoy me too.\n\nPaolo\n"}]}