{"thread":{"id":"29891","subject":"Stop a git commit by a specific author using pre-commit hook","startedAt":"2012-03-09T01:15:37Z","lastAt":"2012-03-12T11:29:21Z","messageCount":13,"participants":["Adrian Cornish","Neal Kreitzinger","Junio C Hamano","Johannes Sixt","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"186515","messageId":"CAGc=MuCQoQ711uXnB8J80RqsY+pqbaeCOXHjy0-mjq7a_ESdyA@mail.gmail.com","threadId":"29891","inReplyTo":null,"subject":"Stop a git commit by a specific author using pre-commit hook","fromName":"Adrian Cornish","fromEmail":"git@bluedreamer.com","sentAt":"2012-03-09T01:15:37Z","receivedAt":"2012-03-09T01:15:37Z","isPatch":false,"sender":{"key":"git@bluedreamer.com","avatar":null},"body":"Hi All,\n\nMy plan is to use git to keep track of changes in /etc but when\ncommitting I want to have the person making the change specify\nthemselves as author by adding the --author option on the commandline.\n\nSo I would like to stop accidental commits as root.\n\nI tried creating this pre-commit hook but it is not working - git var\nis still returning root even if I specify the author on commit line.\n\nTIA\nAdrian\n\nAUTHOR=`git var GIT_AUTHOR_IDENT | sed -n 's/^\\(.*>\\).*$/\\1/p'`\nif [ \"$AUTHOR\" == \"root <root@localhost>\" ];\nthen\n   echo \"Please commit under your own user name instead of \\\"$AUTHOR\\\":\"\n   echo 'git commit --author=\"Adrian\"'\n   echo \"or if your name is not already in logs use full ident\"\n   echo 'git commit --author=\"Adrian Cornish <a@localhost>\"'\n   exit 1\nfi\nexit 0\n"},{"id":"186628","messageId":"4F5BAE5E.5030407@gmail.com","threadId":"29891","inReplyTo":"CAGc=MuCQoQ711uXnB8J80RqsY+pqbaeCOXHjy0-mjq7a_ESdyA@mail.gmail.com","subject":"Re: Stop a git commit by a specific author using pre-commit hook","fromName":"Neal Kreitzinger","fromEmail":"nkreitzinger@gmail.com","sentAt":"2012-03-10T19:41:18Z","receivedAt":"2012-03-10T19:41:18Z","isPatch":false,"sender":{"key":"nkreitzinger@gmail.com","avatar":null},"body":"On 3/8/2012 7:15 PM, Adrian Cornish wrote:\n> My plan is to use git to keep track of changes in /etc but when\n> committing I want to have the person making the change specify\n> themselves as author by adding the --author option on the commandline.\n>\n> So I would like to stop accidental commits as root.\n>\n> I tried creating this pre-commit hook but it is not working - git var\n> is still returning root even if I specify the author on commit line.\n>\n> AUTHOR=`git var GIT_AUTHOR_IDENT | sed -n 's/^\\(.*>\\).*$/\\1/p'`\n> if [ \"$AUTHOR\" == \"root<root@localhost>\" ];\n> then\n>     echo \"Please commit under your own user name instead of \\\"$AUTHOR\\\":\"\n>     echo 'git commit --author=\"Adrian\"'\n>     echo \"or if your name is not already in logs use full ident\"\n>     echo 'git commit --author=\"Adrian Cornish<a@localhost>\"'\n>     exit 1\n> fi\n> exit 0\n\nWe use whoami in our pre-commit hook to see who the user is that is \ndoing the commit.  I think you could also use GIT_COMMITTER_NAME or \nlinux $USER environment variables.  Either way, the --author seems like \nan unnecessary and unreliable way to get the username.  You're relying \non the user to remember or obey that step.  By doing --author, you also \nseem to be implying that the git commit author value may not be the same \nas the user doing the commit so that would also ruin the original commit \nauthor data audit trail.  I haven't used these particular git \nenvironment variables, but I suspect they are dependent upon the user \nhaving their gitconfig filled out properly, so the linux whoami and \n$USER are probably more reliable.  If people can su to root then $USER \nwill not work because it will still be set to their original user name \n(before they did su to root).  Therefore, \"whoami\" seems like your best \nsolution.\n\nv/r,\nneal\n"},{"id":"186633","messageId":"7vboo415rn.fsf@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"4F5BAE5E.5030407@gmail.com","subject":"Re: Stop a git commit by a specific author using pre-commit hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-10T21:54:20Z","receivedAt":"2012-03-10T21:54:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neal Kreitzinger <nkreitzinger@gmail.com> writes:\n\n> On 3/8/2012 7:15 PM, Adrian Cornish wrote:\n>> My plan is to use git to keep track of changes in /etc but when\n>> committing I want to have the person making the change specify\n>> themselves as author by adding the --author option on the commandline.\n>\n>> So I would like to stop accidental commits as root.\n> ...\n> We use whoami in our pre-commit hook to see who the user is that is\n> doing the commit.  I think you could also use GIT_COMMITTER_NAME or\n> linux $USER environment variables.  Either way, the --author seems\n> like an unnecessary and unreliable way to get the username.\n> ...  If people can su to root\n> then $USER will not work because it will still be set to their\n> original user name (before they did su to root).  Therefore, \"whoami\"\n> seems like your best solution.\n\nWhen people want to raise a red flag against a commit made by root,\nthey are coming from two different schools.\n\nOne is \"do not run 'git' or any development tool for that matter\nwhile being root\". It is a good discipline to follow in general to\nlimit what you do with escalated privilege to the minimum.\n\nThe other is \"record who actually did the work, not 'root' that\npeople cannot later track down who it actually was\".  People from\nthis school do not mind running development tools as root.\n\nAnd your advice is a good one for the former, but not very relevant\nfor the latter.\n\nAnd I think Adrian is asking for the latter.\n"},{"id":"186637","messageId":"4F5BDDAC.4050700@gmail.com","threadId":"29891","inReplyTo":"7vboo415rn.fsf@alter.siamese.dyndns.org","subject":"Re: Stop a git commit by a specific author using pre-commit hook","fromName":"Neal Kreitzinger","fromEmail":"nkreitzinger@gmail.com","sentAt":"2012-03-10T23:03:08Z","receivedAt":"2012-03-10T23:03:08Z","isPatch":false,"sender":{"key":"nkreitzinger@gmail.com","avatar":null},"body":"On 3/10/2012 3:54 PM, Junio C Hamano wrote:\n> Neal Kreitzinger<nkreitzinger@gmail.com>  writes:\n>\n>> On 3/8/2012 7:15 PM, Adrian Cornish wrote:\n>>> My plan is to use git to keep track of changes in /etc but when\n>>> committing I want to have the person making the change specify\n>>> themselves as author by adding the --author option on the commandline.\n>>> So I would like to stop accidental commits as root.\n>> ...\n>> We use whoami in our pre-commit hook to see who the user is that is\n>> doing the commit.  I think you could also use GIT_COMMITTER_NAME or\n>> linux $USER environment variables.  Either way, the --author seems\n>> like an unnecessary and unreliable way to get the username.\n>> ...  If people can su to root\n>> then $USER will not work because it will still be set to their\n>> original user name (before they did su to root).  Therefore, \"whoami\"\n>> seems like your best solution.\n> When people want to raise a red flag against a commit made by root,\n> they are coming from two different schools.\n>\n> One is \"do not run 'git' or any development tool for that matter\n> while being root\". It is a good discipline to follow in general to\n> limit what you do with escalated privilege to the minimum.\n>\n> The other is \"record who actually did the work, not 'root' that\n> people cannot later track down who it actually was\".  People from\n> this school do not mind running development tools as root.\n>\n> And your advice is a good one for the former, but not very relevant\n> for the latter.\n>\n> And I think Adrian is asking for the latter.\n>\nNow I see.  In that case, at the point pre-commit is run git has not \noverriden GIT_AUTHOR_IDENT with your --author value, yet.  I don't know \nif that is a bug or not.  The prepare-commit-msg hook is the same way.  \nHowever, by the time the commit-msg hook runs git has placed your \n--author override into GIT_AUTHOR_IDENT so if you check it there it will \nwork and you can abort the commit.  Of course, at that point the user \nhas already typed their commit message and may lose it.  You could \ncreate a git alias like \"git root-commit\" that prompts them to enter \ntheir authorname and then runs git-commit --author with that user \nprovided value.  Pre-commit hook could detect root user and error out \ntelling them to run git-root-commit instead.\n\nv/r,\nneal\n"},{"id":"186645","messageId":"7v62ebz9c0.fsf@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"4F5BDDAC.4050700@gmail.com","subject":"Re: Stop a git commit by a specific author using pre-commit hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:05:35Z","receivedAt":"2012-03-11T11:05:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neal Kreitzinger <nkreitzinger@gmail.com> writes:\n\n> ... at the point pre-commit is run git has not\n> overriden GIT_AUTHOR_IDENT with your --author value, yet.  I don't\n> know if that is a bug or not.  The prepare-commit-msg hook is the same\n> way.\n\nI'd call it a bug, as the command line --author should have the same\neffect as GIT_AUTHOR_NAME and friends observable by the outside\nworld.\n\nThere are two slightly different approaches to solve this, that\nshare the first two preparatory commits.  I'll send out the\npreparatory ones first and then show both of these approaches.\n"},{"id":"186646","messageId":"7v1uozz97f.fsf_-_@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"7v62ebz9c0.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/(3/4)] test: does pre-commit-hook learn authorship?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:08:20Z","receivedAt":"2012-03-11T11:08:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When \"--author\" option is used to lie the authorship to \"git commit\"\ncommand, hooks should learn the author name and email just like when\nGIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL environment variables are used\nto lie the authorship.  Test this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * This is the first one of two patches that are shread by two\n   approaches. What it tests should be obvious---the current code\n   fails with the last piece that use --author.\n\n t/t7503-pre-commit-hook.sh |   18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex ee7f0cd..fc6de5b 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -118,4 +118,22 @@ test_expect_success 'with failing hook requiring GIT_PREFIX' '\n \tgit checkout -- file\n '\n \n+test_expect_failure 'check the author in hook' '\n+\tcat >\"$HOOK\" <<-\\EOF &&\n+\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n+\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\tEOF\n+\ttest_must_fail git commit --allow-empty -m \"by a.u.thor\" &&\n+\t(\n+\t\tGIT_AUTHOR_NAME=\"New Author\" &&\n+\t\tGIT_AUTHOR_EMAIL=\"newauthor@example.com\" &&\n+\t\texport GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&\n+\t\tgit commit --allow-empty -m \"by new.author via env\" &&\n+\t\tgit show -s\n+\t) &&\n+\tgit commit --author=\"New Author <newauthor@example.com>\" \\\n+\t\t--allow-empty -m \"by new.author via command line\" &&\n+\tgit show -s\n+'\n+\n test_done\n-- \n1.7.10.rc0.33.g8866af\n"},{"id":"186647","messageId":"7vwr6rxul6.fsf_-_@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"7v62ebz9c0.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/(3/4)] ident.c: add split_ident_line() to parse formatted ident line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:09:25Z","receivedAt":"2012-03-11T11:09:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The commit formatting logic format_person_part() in pretty.c\nimplements the logic to split an author/committer ident line into\nits parts, intermixed with logic to compute its output using these\npiece it computes.\n\nSeparate the former out to a helper function split_ident_line() so\nthat other codepath can use the same logic, and rewrite the function\nusing the helper function.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * The second shared change between the two approaches.\n\n cache.h  |   16 +++++++++++++++\n ident.c  |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n pretty.c |   64 ++++++++++++++++++----------------------------------------\n 3 files changed, 104 insertions(+), 44 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 3a8e125..7c42eca 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -928,6 +928,22 @@ extern const char *fmt_name(const char *name, const char *email);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n \n+struct ident_split {\n+\tconst char *name_begin;\n+\tconst char *name_end;\n+\tconst char *mail_begin;\n+\tconst char *mail_end;\n+\tconst char *date_begin;\n+\tconst char *date_end;\n+\tconst char *tz_begin;\n+\tconst char *tz_end;\n+};\n+/*\n+ * Signals an success with 0, but time part of the result may be NULL\n+ * if the input lacks timestamp and zone\n+ */\n+extern int split_ident_line(struct ident_split *, const char *, int);\n+\n struct checkout {\n \tconst char *base_dir;\n \tint base_dir_len;\ndiff --git a/ident.c b/ident.c\nindex f619619..87c697c 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -220,6 +220,74 @@ static int copy(char *buf, size_t size, int offset, const char *src)\n \treturn offset;\n }\n \n+/*\n+ * Reverse of fmt_ident(); given an ident line, split the fields\n+ * to allow the caller to parse it.\n+ * Signal a success by returning 0, but date/tz fields of the result\n+ * can still be NULL if the input line only has the name/email part\n+ * (e.g. reading from a reflog entry).\n+ */\n+int split_ident_line(struct ident_split *split, const char *line, int len)\n+{\n+\tconst char *cp;\n+\tsize_t span;\n+\tint status = -1;\n+\n+\tmemset(split, 0, sizeof(*split));\n+\n+\tsplit->name_begin = line;\n+\tfor (cp = line; *cp && cp < line + len; cp++)\n+\t\tif (*cp == '<') {\n+\t\t\tsplit->mail_begin = cp + 1;\n+\t\t\tbreak;\n+\t\t}\n+\tif (!split->mail_begin)\n+\t\treturn status;\n+\n+\tfor (cp = split->mail_begin - 2; line < cp; cp--)\n+\t\tif (!isspace(*cp)) {\n+\t\t\tsplit->name_end = cp + 1;\n+\t\t\tbreak;\n+\t\t}\n+\tif (!split->name_end)\n+\t\treturn status;\n+\n+\tfor (cp = split->mail_begin; cp < line + len; cp++)\n+\t\tif (*cp == '>') {\n+\t\t\tsplit->mail_end = cp;\n+\t\t\tbreak;\n+\t\t}\n+\tif (!split->mail_end)\n+\t\treturn status;\n+\n+\tfor (cp = split->mail_end + 1; cp < line + len && isspace(*cp); cp++)\n+\t\t;\n+\tif (line + len <= cp)\n+\t\tgoto person_only;\n+\tsplit->date_begin = cp;\n+\tspan = strspn(cp, \"0123456789\");\n+\tif (!span)\n+\t\tgoto person_only;\n+\tsplit->date_end = split->date_begin + span;\n+\tfor (cp = split->date_end; cp < line + len && isspace(*cp); cp++)\n+\t\t;\n+\tif (line + len <= cp || (*cp != '+' && *cp != '-'))\n+\t\tgoto person_only;\n+\tsplit->tz_begin = cp;\n+\tspan = strspn(cp + 1, \"0123456789\");\n+\tif (!span)\n+\t\tgoto person_only;\n+\tsplit->tz_end = split->tz_begin + 1 + span;\n+\treturn 0;\n+\n+person_only:\n+\tsplit->date_begin = NULL;\n+\tsplit->date_end = NULL;\n+\tsplit->tz_begin = NULL;\n+\tsplit->tz_end = NULL;\n+\treturn 0;\n+}\n+\n static const char *env_hint =\n \"\\n\"\n \"*** Please tell me who you are.\\n\"\ndiff --git a/pretty.c b/pretty.c\nindex 8688b8f..f2dee30 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -531,41 +531,24 @@ static size_t format_person_part(struct strbuf *sb, char part,\n {\n \t/* currently all placeholders have same length */\n \tconst int placeholder_len = 2;\n-\tint start, end, tz = 0;\n+\tint tz;\n \tunsigned long date = 0;\n-\tchar *ep;\n-\tconst char *name_start, *name_end, *mail_start, *mail_end, *msg_end = msg+len;\n \tchar person_name[1024];\n \tchar person_mail[1024];\n+\tstruct ident_split s;\n+\tconst char *name_start, *name_end, *mail_start, *mail_end;\n \n-\t/* advance 'end' to point to email start delimiter */\n-\tfor (end = 0; end < len && msg[end] != '<'; end++)\n-\t\t; /* do nothing */\n-\n-\t/*\n-\t * When end points at the '<' that we found, it should have\n-\t * matching '>' later, which means 'end' must be strictly\n-\t * below len - 1.\n-\t */\n-\tif (end >= len - 2)\n+\tif (split_ident_line(&s, msg, len) < 0)\n \t\tgoto skip;\n \n-\t/* Seek for both name and email part */\n-\tname_start = msg;\n-\tname_end = msg+end;\n-\twhile (name_end > name_start && isspace(*(name_end-1)))\n-\t\tname_end--;\n-\tmail_start = msg+end+1;\n-\tmail_end = mail_start;\n-\twhile (mail_end < msg_end && *mail_end != '>')\n-\t\tmail_end++;\n-\tif (mail_end == msg_end)\n-\t\tgoto skip;\n-\tend = mail_end-msg;\n+\tname_start = s.name_begin;\n+\tname_end = s.name_end;\n+\tmail_start = s.mail_begin;\n+\tmail_end = s.mail_end;\n \n \tif (part == 'N' || part == 'E') { /* mailmap lookup */\n-\t\tstrlcpy(person_name, name_start, name_end-name_start+1);\n-\t\tstrlcpy(person_mail, mail_start, mail_end-mail_start+1);\n+\t\tstrlcpy(person_name, name_start, name_end - name_start + 1);\n+\t\tstrlcpy(person_mail, mail_start, mail_end - mail_start + 1);\n \t\tmailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));\n \t\tname_start = person_name;\n \t\tname_end = name_start + strlen(person_name);\n@@ -581,28 +564,20 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \t\treturn placeholder_len;\n \t}\n \n-\t/* advance 'start' to point to date start delimiter */\n-\tfor (start = end + 1; start < len && isspace(msg[start]); start++)\n-\t\t; /* do nothing */\n-\tif (start >= len)\n-\t\tgoto skip;\n-\tdate = strtoul(msg + start, &ep, 10);\n-\tif (msg + start == ep)\n+\tif (!s.date_begin)\n \t\tgoto skip;\n \n+\tdate = strtoul(s.date_begin, NULL, 10);\n+\n \tif (part == 't') {\t/* date, UNIX timestamp */\n-\t\tstrbuf_add(sb, msg + start, ep - (msg + start));\n+\t\tstrbuf_add(sb, s.date_begin, s.date_end - s.date_begin);\n \t\treturn placeholder_len;\n \t}\n \n \t/* parse tz */\n-\tfor (start = ep - msg + 1; start < len && isspace(msg[start]); start++)\n-\t\t; /* do nothing */\n-\tif (start + 1 < len) {\n-\t\ttz = strtoul(msg + start + 1, NULL, 10);\n-\t\tif (msg[start] == '-')\n-\t\t\ttz = -tz;\n-\t}\n+\ttz = strtoul(s.tz_begin + 1, NULL, 10);\n+\tif (*s.tz_begin == '-')\n+\t\ttz = -tz;\n \n \tswitch (part) {\n \tcase 'd':\t/* date */\n@@ -621,8 +596,9 @@ static size_t format_person_part(struct strbuf *sb, char part,\n \n skip:\n \t/*\n-\t * bogus commit, 'sb' cannot be updated, but we still need to\n-\t * compute a valid return value.\n+\t * reading from either a bogus commit, or a reflog entry with\n+\t * %gn, %ge, etc.; 'sb' cannot be updated, but we still need\n+\t * to compute a valid return value.\n \t */\n \tif (part == 'n' || part == 'e' || part == 't' || part == 'd'\n \t    || part == 'D' || part == 'r' || part == 'i')\n-- \n1.7.10.rc0.33.g8866af\n"},{"id":"186648","messageId":"7vsjhfxuhh.fsf_-_@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"7vwr6rxul6.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH 3/3] commit: pass author/committer info to hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:11:38Z","receivedAt":"2012-03-11T11:11:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When lying the author name via GIT_AUTHOR_NAME environment variable\nto \"git commit\", the hooks run by the command saw it and could act\non the name that will be recorded in the final commit. When the user\nuses the \"--author\" option from the command line, the command should\ngive the same information to the hook, and back when \"git command\"\nwas a scripted Porcelain, it did set the environment variable and\nhooks can learn the author name from it.\n\nHowever, when the command was reimplemented in C, the rewritten code\nwas not very faithful to the original, and hooks stopped getting the\nauthorship information given with \"--author\".  Fix this by exporting\nthe necessary environment variables.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is the last patch of the simpler of the two approaches, that\n   builds on top of the two common preparatory patches.  It uses setenv()\n   to directly affect the execution environment of \"git commit\" process,\n   which is closer to the original scripted Porcelain implementation.\n\n builtin/commit.c           |   22 +++++++++++++++++++---\n t/t7503-pre-commit-hook.sh |    2 +-\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eae5a29..57a60f9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -533,9 +533,20 @@ static int is_a_merge(const struct commit *current_head)\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n \n+static void export_one(const char *var, const char *s, const char *e, int hack)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tif (hack)\n+\t\tstrbuf_addch(&buf, hack);\n+\tstrbuf_addf(&buf, \"%.*s\", (int)(e - s), s);\n+\tsetenv(var, buf.buf, 1);\n+\tstrbuf_release(&buf);\n+}\n+\n static void determine_author_info(struct strbuf *author_ident)\n {\n \tchar *name, *email, *date;\n+\tstruct ident_split author;\n \n \tname = getenv(\"GIT_AUTHOR_NAME\");\n \temail = getenv(\"GIT_AUTHOR_EMAIL\");\n@@ -585,6 +596,11 @@ static void determine_author_info(struct strbuf *author_ident)\n \t\tdate = force_date;\n \tstrbuf_addstr(author_ident, fmt_ident(name, email, date,\n \t\t\t\t\t      IDENT_ERROR_ON_NO_NAME));\n+\tif (!split_ident_line(&author, author_ident->buf, author_ident->len)) {\n+\t\texport_one(\"GIT_AUTHOR_NAME\", author.name_begin, author.name_end, 0);\n+\t\texport_one(\"GIT_AUTHOR_EMAIL\", author.mail_begin, author.mail_end, 0);\n+\t\texport_one(\"GIT_AUTHOR_DATE\", author.date_begin, author.tz_end, '@');\n+\t}\n }\n \n static int ends_rfc2822_footer(struct strbuf *sb)\n@@ -652,6 +668,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tint ident_shown = 0;\n \tint clean_message_contents = (cleanup_mode != CLEANUP_NONE);\n \n+\t/* This checks and barfs if author is badly specified */\n+\tdetermine_author_info(author_ident);\n+\n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n \n@@ -771,9 +790,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \n \tstrbuf_release(&sb);\n \n-\t/* This checks and barfs if author is badly specified */\n-\tdetermine_author_info(author_ident);\n-\n \t/* This checks if committer ident is explicitly given */\n \tstrbuf_addstr(&committer_ident, git_committer_info(0));\n \tif (use_editor && include_status) {\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex fc6de5b..9301a0c 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -118,7 +118,7 @@ test_expect_success 'with failing hook requiring GIT_PREFIX' '\n \tgit checkout -- file\n '\n \n-test_expect_failure 'check the author in hook' '\n+test_expect_success 'check the author in hook' '\n \tcat >\"$HOOK\" <<-\\EOF &&\n \ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n \ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n-- \n1.7.10.rc0.33.g8866af\n"},{"id":"186649","messageId":"7vlin7xub7.fsf_-_@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"7vwr6rxul6.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH 3/4] run_hook(): enhance the interface to pass arbitrary environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:15:24Z","receivedAt":"2012-03-11T11:15:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Originally \"run_hook()\" that used to live in builtin/commit.c had a\nperfectly appropriate API and name for a private function to spawn a\nhook from \"git commit\" command. The only necessary tweak in the\ncontext was to optionally specify which file to use as the index\nfile.\n\nBut then we stupidly moved this private API to run-command.c without\nmaking the interface suitable for general consumption, and there is\nno way to tweak environment variables other than GIT_INDEX_FILE when\nrunning a hook.\n\nCorrect this mistake by adding run_hook_e() that takes an array of\nenvironment variables.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is the third patch of the more complex of the two approaches.\n   Instead of the simpler \"setenv()\" one, the final one tries to\n   limit the environment change only to the hook scripts, without\n   affecting the \"git commit\" process itself. It probably should not\n   make much difference in the short run, but later when we want to\n   distinguish where the authorship came from, it may be easier to\n   tell with this approach as it does not contaminate the environment\n   of \"git commit\" itself with what we obtained from \"--author\" option.\n\n   Also this is independently a useful refactoring.\n\n run-command.c |   49 ++++++++++++++++++++++++++++++++++++-------------\n run-command.h |    1 +\n 2 files changed, 37 insertions(+), 13 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1db8abf..a51d8e5 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -672,36 +672,59 @@ int finish_async(struct async *async)\n #endif\n }\n \n-int run_hook(const char *index_file, const char *name, ...)\n+static int run_hook_le(const char *const *env,\n+\t\t       const char *name,\n+\t\t       va_list args)\n {\n \tstruct child_process hook;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tconst char *p, *env[2];\n-\tchar index[PATH_MAX];\n-\tva_list args;\n+\tconst char *p;\n \tint ret;\n \n \tif (access(git_path(\"hooks/%s\", name), X_OK) < 0)\n \t\treturn 0;\n \n-\tva_start(args, name);\n \targv_array_push(&argv, git_path(\"hooks/%s\", name));\n \twhile ((p = va_arg(args, const char *)))\n \t\targv_array_push(&argv, p);\n-\tva_end(args);\n \n \tmemset(&hook, 0, sizeof(hook));\n \thook.argv = argv.argv;\n \thook.no_stdin = 1;\n \thook.stdout_to_stderr = 1;\n-\tif (index_file) {\n-\t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tenv[0] = index;\n-\t\tenv[1] = NULL;\n-\t\thook.env = env;\n-\t}\n-\n+\thook.env = env;\n \tret = run_command(&hook);\n \targv_array_clear(&argv);\n \treturn ret;\n }\n+\n+int run_hook(const char *index_file, const char *name, ...)\n+{\n+\tconst char *const *env = NULL, *env_buf[2];\n+\tstruct strbuf index_buf = STRBUF_INIT;\n+\tint status;\n+\tva_list args;\n+\n+\tva_start(args, name);\n+\tif (index_file) {\n+\t\tstrbuf_addf(&index_buf, \"GIT_INDEX_FILE=%s\", index_file);\n+\t\tenv_buf[0] = index_buf.buf;\n+\t\tenv_buf[1] = NULL;\n+\t\tenv = env_buf;\n+\t}\n+\tstatus = run_hook_le(env, name, args);\n+\tva_end(args);\n+\tstrbuf_release(&index_buf);\n+\treturn status;\n+}\n+\n+int run_hook_e(const char *const *env, const char *name, ...)\n+{\n+\tint status;\n+\tva_list args;\n+\n+\tva_start(args, name);\n+\tstatus = run_hook_le(env, name, args);\n+\tva_end(args);\n+\treturn status;\n+}\ndiff --git a/run-command.h b/run-command.h\nindex 44f7d2b..87207b9 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -47,6 +47,7 @@ int finish_command(struct child_process *);\n int run_command(struct child_process *);\n \n extern int run_hook(const char *index_file, const char *name, ...);\n+extern int run_hook_e(const char *const *, const char *name, ...);\n \n #define RUN_COMMAND_NO_STDIN 1\n #define RUN_GIT_CMD\t     2\t/*If this is to be git sub-command */\n-- \n1.7.10.rc0.33.g8866af\n"},{"id":"186650","messageId":"7vhaxvxu9y.fsf_-_@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"7vwr6rxul6.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH 4/4] commit: pass author/committer info to hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T11:16:09Z","receivedAt":"2012-03-11T11:16:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When lying the author name via GIT_AUTHOR_NAME environment variable\nto \"git commit\", the hooks run by the command saw it and could act\non the name that will be recorded in the final commit. When the user\nuses the \"--author\" option from the command line, the command should\ngive the same information to the hook, and back when \"git command\"\nwas a scripted Porcelain, it did set the environment variable and\nhooks can learn the author name from it.\n\nHowever, when the command was reimplemented in C, the rewritten code\nwas not very faithful to the original, and hooks stopped getting the\nauthorship information given with \"--author\".  Fix this by exporting\nthe necessary environment variables.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * And this is the final patch in the more complex of the two\n   approaches.\n\n builtin/commit.c           |  132 ++++++++++++++++++++++++++++++--------------\n t/t7503-pre-commit-hook.sh |    2 +-\n 2 files changed, 93 insertions(+), 41 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eae5a29..4a47e39 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -27,6 +27,7 @@\n #include \"quote.h\"\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n+#include \"argv-array.h\"\n \n static const char * const builtin_commit_usage[] = {\n \t\"git commit [options] [--] <filepattern>...\",\n@@ -637,13 +638,63 @@ static char *cut_ident_timestamp_part(char *string)\n \treturn ket;\n }\n \n-static int prepare_to_commit(const char *index_file, const char *prefix,\n+struct commit_context {\n+\tconst char *index_file;\n+\tstruct strbuf *author_ident;\n+\tstruct strbuf *committer_ident;\n+\tstruct argv_array env;\n+};\n+\n+static void setup_commit_context(struct commit_context *ctx,\n+\t\t\t\t const char *index_file,\n+\t\t\t\t struct strbuf *author_ident,\n+\t\t\t\t struct strbuf *committer_ident)\n+{\n+\tstruct ident_split s;\n+\tstruct argv_array *env = &ctx->env;\n+\n+\tctx->index_file = index_file;\n+\tctx->author_ident = author_ident;\n+\tctx->committer_ident = committer_ident;\n+\targv_array_init(env);\n+\tif (index_file)\n+\t\targv_array_pushf(env, \"GIT_INDEX_FILE=%s\", index_file);\n+\n+\tif (!split_ident_line(&s, author_ident->buf, author_ident->len)) {\n+\t\targv_array_pushf(env, \"GIT_AUTHOR_NAME=%.*s\",\n+\t\t\t\t (int)(s.name_end - s.name_begin),\n+\t\t\t\t s.name_begin);\n+\t\targv_array_pushf(env, \"GIT_AUTHOR_EMAIL=%.*s\",\n+\t\t\t\t (int)(s.mail_end - s.mail_begin),\n+\t\t\t\t s.mail_begin);\n+\t\targv_array_pushf(env, \"GIT_AUTHOR_DATE=@%.*s %.*s\",\n+\t\t\t\t (int)(s.date_end - s.date_begin),\n+\t\t\t\t s.date_begin,\n+\t\t\t\t (int)(s.tz_end - s.tz_begin),\n+\t\t\t\t s.tz_begin);\n+\t}\n+\n+\tif (!split_ident_line(&s, committer_ident->buf, committer_ident->len)) {\n+\t\targv_array_pushf(env, \"GIT_COMMITTER_NAME=%.*s\",\n+\t\t\t\t (int)(s.name_end - s.name_begin),\n+\t\t\t\t s.name_begin);\n+\t\targv_array_pushf(env, \"GIT_COMMITTER_EMAIL=%.*s\",\n+\t\t\t\t (int)(s.mail_end - s.mail_begin),\n+\t\t\t\t s.mail_begin);\n+\t\targv_array_pushf(env, \"GIT_COMMITTER_DATE=@%.*s %.*s\",\n+\t\t\t\t (int)(s.date_end - s.date_begin),\n+\t\t\t\t s.date_begin,\n+\t\t\t\t (int)(s.tz_end - s.tz_begin),\n+\t\t\t\t s.tz_begin);\n+\t}\n+}\n+\n+static int prepare_to_commit(struct commit_context *ctx,\n+\t\t\t     const char *prefix,\n \t\t\t     struct commit *current_head,\n-\t\t\t     struct wt_status *s,\n-\t\t\t     struct strbuf *author_ident)\n+\t\t\t     struct wt_status *s)\n {\n \tstruct stat statbuf;\n-\tstruct strbuf committer_ident = STRBUF_INIT;\n \tint commitable, saved_color_setting;\n \tstruct strbuf sb = STRBUF_INIT;\n \tchar *buffer;\n@@ -652,7 +703,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tint ident_shown = 0;\n \tint clean_message_contents = (cleanup_mode != CLEANUP_NONE);\n \n-\tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n+\tif (!no_verify && run_hook_e(ctx->env.argv, \"pre-commit\", NULL))\n \t\treturn 0;\n \n \tif (squash_message) {\n@@ -771,11 +822,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \n \tstrbuf_release(&sb);\n \n-\t/* This checks and barfs if author is badly specified */\n-\tdetermine_author_info(author_ident);\n-\n-\t/* This checks if committer ident is explicitly given */\n-\tstrbuf_addstr(&committer_ident, git_committer_info(0));\n \tif (use_editor && include_status) {\n \t\tchar *ai_tmp, *ci_tmp;\n \t\tif (whence != FROM_COMMIT)\n@@ -809,28 +855,28 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n \t\t\t\t\t\"%s\", only_include_assumed);\n \n-\t\tai_tmp = cut_ident_timestamp_part(author_ident->buf);\n-\t\tci_tmp = cut_ident_timestamp_part(committer_ident.buf);\n-\t\tif (strcmp(author_ident->buf, committer_ident.buf))\n+\t\tai_tmp = cut_ident_timestamp_part(ctx->author_ident->buf);\n+\t\tci_tmp = cut_ident_timestamp_part(ctx->committer_ident->buf);\n+\t\tif (strcmp(ctx->author_ident->buf, ctx->committer_ident->buf))\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t_(\"%s\"\n-\t\t\t\t\"Author:    %s\"),\n-\t\t\t\tident_shown++ ? \"\" : \"\\n\",\n-\t\t\t\tauthor_ident->buf);\n+\t\t\t\t\t _(\"%s\"\n+\t\t\t\t\t   \"Author:    %s\"),\n+\t\t\t\t\t ident_shown++ ? \"\" : \"\\n\",\n+\t\t\t\t\t ctx->author_ident->buf);\n \n \t\tif (!user_ident_sufficiently_given())\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n-\t\t\t\t_(\"%s\"\n-\t\t\t\t\"Committer: %s\"),\n-\t\t\t\tident_shown++ ? \"\" : \"\\n\",\n-\t\t\t\tcommitter_ident.buf);\n+\t\t\t\t\t _(\"%s\"\n+\t\t\t\t\t   \"Committer: %s\"),\n+\t\t\t\t\t ident_shown++ ? \"\" : \"\\n\",\n+\t\t\t\t\t ctx->committer_ident->buf);\n \n \t\tif (ident_shown)\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL, \"\");\n \n \t\tsaved_color_setting = s->use_color;\n \t\ts->use_color = 0;\n-\t\tcommitable = run_status(s->fp, index_file, prefix, 1, s);\n+\t\tcommitable = run_status(s->fp, ctx->index_file, prefix, 1, s);\n \t\ts->use_color = saved_color_setting;\n \n \t\t*ai_tmp = ' ';\n@@ -850,8 +896,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\telse\n \t\t\tcommitable = index_differs_from(parent, 0);\n \t}\n-\tstrbuf_release(&committer_ident);\n-\n \tfclose(s->fp);\n \n \t/*\n@@ -861,7 +905,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t */\n \tif (!commitable && whence != FROM_MERGE && !allow_empty &&\n \t    !(amend && is_a_merge(current_head))) {\n-\t\trun_status(stdout, index_file, prefix, 0, s);\n+\t\trun_status(stdout, ctx->index_file, prefix, 0, s);\n \t\tif (amend)\n \t\t\tfputs(_(empty_amend_advice), stderr);\n \t\telse if (whence == FROM_CHERRY_PICK)\n@@ -875,22 +919,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t * the editor and after we invoke run_status above.\n \t */\n \tdiscard_cache();\n-\tread_cache_from(index_file);\n+\tread_cache_from(ctx->index_file);\n \tif (update_main_cache_tree(0)) {\n \t\terror(_(\"Error building trees\"));\n \t\treturn 0;\n \t}\n \n-\tif (run_hook(index_file, \"prepare-commit-msg\",\n-\t\t     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))\n+\tif (run_hook_e(ctx->env.argv, \"prepare-commit-msg\",\n+\t\t       git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))\n \t\treturn 0;\n \n \tif (use_editor) {\n-\t\tchar index[PATH_MAX];\n-\t\tconst char *env[2] = { NULL };\n-\t\tenv[0] =  index;\n-\t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tif (launch_editor(git_path(commit_editmsg), NULL, env)) {\n+\t\tif (launch_editor(git_path(commit_editmsg), NULL, ctx->env.argv)) {\n \t\t\tfprintf(stderr,\n \t\t\t_(\"Please supply the message using either -m or -F option.\\n\"));\n \t\t\texit(1);\n@@ -898,7 +938,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t}\n \n \tif (!no_verify &&\n-\t    run_hook(index_file, \"commit-msg\", git_path(commit_editmsg), NULL)) {\n+\t    run_hook_e(ctx->env.argv, \"commit-msg\", git_path(commit_editmsg), NULL)) {\n \t\treturn 0;\n \t}\n \n@@ -1389,6 +1429,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf author_ident = STRBUF_INIT;\n+\tstruct strbuf committer_ident = STRBUF_INIT;\n \tconst char *index_file, *reflog_msg;\n \tchar *nl, *p;\n \tunsigned char sha1[20];\n@@ -1399,6 +1440,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tstruct wt_status s;\n \tstruct commit *current_head = NULL;\n \tstruct commit_extra_header *extra = NULL;\n+\tstruct commit_context commit_ctx;\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_commit_usage, builtin_commit_options);\n@@ -1420,10 +1462,16 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n \tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\n \n-\t/* Set up everything for writing the commit object.  This includes\n-\t   running hooks, writing the trees, and interacting with the user.  */\n-\tif (!prepare_to_commit(index_file, prefix,\n-\t\t\t       current_head, &s, &author_ident)) {\n+\t/*\n+\t * Set up everything for writing the commit object.  This includes\n+\t * running hooks, writing the trees, and interacting with the user.\n+\t */\n+\tdetermine_author_info(&author_ident);\n+\tstrbuf_addstr(&committer_ident, git_committer_info(0));\n+\tsetup_commit_context(&commit_ctx,\n+\t\t\t     index_file, &author_ident, &committer_ident);\n+\n+\tif (!prepare_to_commit(&commit_ctx, prefix, current_head, &s)) {\n \t\trollback_index_files();\n \t\treturn 1;\n \t}\n@@ -1513,7 +1561,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\trollback_index_files();\n \t\tdie(_(\"failed to write commit object\"));\n \t}\n-\tstrbuf_release(&author_ident);\n \tfree_commit_extra_headers(extra);\n \n \tref_lock = lock_any_ref_for_update(\"HEAD\",\n@@ -1552,7 +1599,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n \trerere(0);\n-\trun_hook(get_index_file(), \"post-commit\", NULL);\n+\n+\targv_array_clear(&commit_ctx.env);\n+\tsetup_commit_context(&commit_ctx,\n+\t\t\t     get_index_file(),\n+\t\t\t     &author_ident, &committer_ident);\n+\trun_hook_e(commit_ctx.env.argv, \"post-commit\", NULL);\n \tif (amend && !no_post_rewrite) {\n \t\tstruct notes_rewrite_cfg *cfg;\n \t\tcfg = init_copy_notes_for_rewrite(\"amend\");\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex fc6de5b..9301a0c 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -118,7 +118,7 @@ test_expect_success 'with failing hook requiring GIT_PREFIX' '\n \tgit checkout -- file\n '\n \n-test_expect_failure 'check the author in hook' '\n+test_expect_success 'check the author in hook' '\n \tcat >\"$HOOK\" <<-\\EOF &&\n \ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n \ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n-- \n1.7.10.rc0.33.g8866af\n"},{"id":"186666","messageId":"4F5D0B50.20803@kdbg.org","threadId":"29891","inReplyTo":"7v1uozz97f.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/(3/4)] test: does pre-commit-hook learn authorship?","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-03-11T20:30:08Z","receivedAt":"2012-03-11T20:30:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.03.2012 12:08, schrieb Junio C Hamano:\n> +test_expect_failure 'check the author in hook' '\n> +\tcat >\"$HOOK\" <<-\\EOF &&\n> +\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n> +\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n> +\tEOF\n\nPlease insert #!/bin/sh as the first line of the script. We'll need it\non Windows.\n\n-- Hannes\n"},{"id":"186669","messageId":"7vehsyx2sx.fsf@alter.siamese.dyndns.org","threadId":"29891","inReplyTo":"4F5D0B50.20803@kdbg.org","subject":"Re: [PATCH 1/(3/4)] test: does pre-commit-hook learn authorship?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-11T21:09:34Z","receivedAt":"2012-03-11T21:09:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; I should have just used write_script() for this.\n"},{"id":"186693","messageId":"20120312112921.GB12921@sigill.intra.peff.net","threadId":"29891","inReplyTo":"7vsjhfxuhh.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: pass author/committer info to hooks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-12T11:29:21Z","receivedAt":"2012-03-12T11:29:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 11, 2012 at 04:11:38AM -0700, Junio C Hamano wrote:\n\n> However, when the command was reimplemented in C, the rewritten code\n> was not very faithful to the original, and hooks stopped getting the\n> authorship information given with \"--author\".  Fix this by exporting\n> the necessary environment variables.\n> [...]\n>  * This is the last patch of the simpler of the two approaches, that\n>    builds on top of the two common preparatory patches.  It uses setenv()\n>    to directly affect the execution environment of \"git commit\" process,\n>    which is closer to the original scripted Porcelain implementation.\n\nFWIW, I like this approach better. It's simpler, and I think it is\nperfectly sane to define \"--author\" as \"act as if the contents were in\nGIT_AUTHOR_*\"[1]. We do the same thing already with \"git --git-dir\",\n\"git --work-tree\", and other variables, and it keeps the code simple by\nmaking the environment variable the definitive source.\n\n-Peff\n\n[1] Your approach is slightly more complex, in that it handles \"-c\"\n    (which is a good thing). But I think the principle is the same.\n"}]}