{"thread":{"id":"24290","subject":"Make author and committer available to pre-commit hook","startedAt":"2010-07-05T11:30:46Z","lastAt":"2010-07-06T15:47:37Z","messageCount":8,"participants":["Gisle Aas","Jeff King","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"144810","messageId":"AANLkTinpCDRf_Yhuj2-tdZwmvHk8yna1Xjdtbrmx35CB@mail.gmail.com","threadId":"24290","inReplyTo":null,"subject":"Make author and committer available to pre-commit hook","fromName":"Gisle Aas","fromEmail":"gisle@aas.no","sentAt":"2010-07-05T11:30:46Z","receivedAt":"2010-07-05T11:30:46Z","isPatch":false,"sender":{"key":"gisle@aas.no","avatar":"https://avatars.githubusercontent.com/u/4326?v=4"},"body":"I would like to implement a pre-commit hook that validates the\n--author set for the commit.  Our use case is a shared repository of\nconfiguration info where different persons all commit as root; but we\nwant to make sure they override the --author to something sensible.\n\nWhat would be the preferred mechanism to pass on this information?  It\ncould for instance be arguments to the hook script or via environment\nvariables.\n\nI created the following patch to explore what it would take to pass on\nthis information as command line arguments.  It seems to do the trick\nfor me.  Any drawback with this approach?\n\nRegards,\nGisle\n\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c101f00..acb8dc2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -557,8 +557,20 @@ static int prepare_to_commit(const char\n*index_file, const char *prefix,\n        const char *hook_arg2 = NULL;\n        int ident_shown = 0;\n\n-       if (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n+       determine_author_info();\n+       if (!no_verify) {\n+           int rc;\n+           hook_arg1 = xstrdup(fmt_name(author_name, author_email));\n+           hook_arg2 = xstrdup(fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+                                        getenv(\"GIT_COMMITTER_EMAIL\")));\n+           rc = run_hook(index_file, \"pre-commit\", hook_arg1, hook_arg2, NULL);\n+           free((char*)hook_arg1);\n+           free((char*)hook_arg2);\n+           if (rc)\n                return 0;\n+           hook_arg1 = NULL;\n+           hook_arg2 = NULL;\n+       }\n\n        if (message.len) {\n                strbuf_addbuf(&sb, &message);\n@@ -632,8 +644,6 @@ static int prepare_to_commit(const char\n*index_file, const char *prefix,\n\n        strbuf_release(&sb);\n\n-       determine_author_info();\n-\n        /* This checks if committer ident is explicitly given */\n        git_committer_info(0);\n        if (use_editor && include_status) {\n"},{"id":"144811","messageId":"20100705114619.GA21146@sigill.intra.peff.net","threadId":"24290","inReplyTo":"AANLkTinpCDRf_Yhuj2-tdZwmvHk8yna1Xjdtbrmx35CB@mail.gmail.com","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-05T11:46:19Z","receivedAt":"2010-07-05T11:46:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 05, 2010 at 01:30:46PM +0200, Gisle Aas wrote:\n\n> I would like to implement a pre-commit hook that validates the\n> --author set for the commit.  Our use case is a shared repository of\n> configuration info where different persons all commit as root; but we\n> want to make sure they override the --author to something sensible.\n> \n> What would be the preferred mechanism to pass on this information?  It\n> could for instance be arguments to the hook script or via environment\n> variables.\n\nIt would make sense to me for it to be passed in through the environment\nusing GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL}. You could even pass\nGIT_AUTHOR_DATE to detect if somebody is using \"git commit -c\" to get\nthe date from a previous commit.\n\n-Peff\n"},{"id":"144834","messageId":"AANLkTinlGBt3nBa7Ge5ytjisTeDu3As4wCkF8M1iz5JV@mail.gmail.com","threadId":"24290","inReplyTo":"20100705114619.GA21146@sigill.intra.peff.net","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Gisle Aas","fromEmail":"gisle@aas.no","sentAt":"2010-07-05T18:04:11Z","receivedAt":"2010-07-05T18:04:11Z","isPatch":false,"sender":{"key":"gisle@aas.no","avatar":"https://avatars.githubusercontent.com/u/4326?v=4"},"body":"On Mon, Jul 5, 2010 at 13:46, Jeff King <peff@peff.net> wrote:\n> On Mon, Jul 05, 2010 at 01:30:46PM +0200, Gisle Aas wrote:\n>\n>> I would like to implement a pre-commit hook that validates the\n>> --author set for the commit.  Our use case is a shared repository of\n>> configuration info where different persons all commit as root; but we\n>> want to make sure they override the --author to something sensible.\n>>\n>> What would be the preferred mechanism to pass on this information?  It\n>> could for instance be arguments to the hook script or via environment\n>> variables.\n>\n> It would make sense to me for it to be passed in through the environment\n> using GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL}. You could even pass\n> GIT_AUTHOR_DATE to detect if somebody is using \"git commit -c\" to get\n> the date from a previous commit.\n\nI agree that would be a more natural interface.  Attached is a patch\nthat sets these environment variables before the hooks are invoked.\nThe patch also updates the documentation and adds some tests.\n\nRegards,\nGisle\n\n\nFrom c352cfdda7dbd3f9185d4c0b1b4472c69cd64399 Mon Sep 17 00:00:00 2001\nFrom: Gisle Aas <gisle@aas.no>\nDate: Mon, 5 Jul 2010 17:45:30 +0200\nSubject: [PATCH] Set ident environment variables for the commit hooks to observe\n\nThe GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} environment variables are set to\nreflect the values about to be committed.\n---\n Documentation/githooks.txt |    4 ++-\n builtin/commit.c           |    9 ++++---\n cache.h                    |    2 +\n ident.c                    |   46 ++++++++++++++++++++++++++++++-------------\n t/t7503-pre-commit-hook.sh |   26 ++++++++++++++++++++++++\n 5 files changed, 68 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 7183aa9..a330a67 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -84,7 +84,9 @@ 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+to modify the commit message.  The 'GIT_AUTHOR_NAME', 'GIT_AUTHOR_EMAIL',\n+'GIT_COMMITTER_NAME', 'GIT_COMMITTER_EMAIL' environment variables\n+will also be set.\n \n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c101f00..06e9342 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -557,6 +557,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tconst char *hook_arg2 = NULL;\n \tint ident_shown = 0;\n \n+\tdetermine_author_info();\n+\tgit_committer_info(0); /* This checks if committer ident is explicitly given */\n+\tgit_setenv_ident(author_name, author_email,\n+\t\t\t getenv(\"GIT_COMMITTER_NAME\"), getenv(\"GIT_COMMITTER_EMAIL\"));\n+\n \tif (!no_verify && run_hook(index_file, \"pre-commit\", NULL))\n \t\treturn 0;\n \n@@ -632,10 +637,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \n \tstrbuf_release(&sb);\n \n-\tdetermine_author_info();\n-\n-\t/* This checks if committer ident is explicitly given */\n-\tgit_committer_info(0);\n \tif (use_editor && include_status) {\n \t\tchar *author_ident;\n \t\tconst char *committer_ident;\ndiff --git a/cache.h b/cache.h\nindex c9fa3df..2e70ff7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -822,6 +822,8 @@ enum date_mode parse_date_format(const char *format);\n #define IDENT_NO_DATE\t       4\n extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n+extern void git_setenv_ident(const char *author_name, const char *author_email,\n+\t\t\t     const char *committer_name, const char *committer_email);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\n extern const char *fmt_name(const char *name, const char *email);\n extern const char *git_editor(void);\ndiff --git a/ident.c b/ident.c\nindex 9e24388..190e4a0 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -182,39 +182,45 @@ static const char *env_hint =\n \"Omit --global to set the identity only in this repository.\\n\"\n \"\\n\";\n \n-const char *fmt_ident(const char *name, const char *email,\n-\t\t      const char *date_str, int flag)\n+static void get_ident(const char **name, const char **email, int flag)\n {\n-\tstatic char buffer[1000];\n-\tchar date[50];\n-\tint i;\n \tint error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);\n \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n-\tint name_addr_only = (flag & IDENT_NO_DATE);\n \n \tsetup_ident();\n-\tif (!name)\n-\t\tname = git_default_name;\n-\tif (!email)\n-\t\temail = git_default_email;\n+\tif (!*name)\n+\t\t*name = git_default_name;\n+\tif (!*email)\n+\t\t*email = git_default_email;\n \n-\tif (!*name) {\n+\tif (!**name) {\n \t\tstruct passwd *pw;\n \n \t\tif ((warn_on_no_name || error_on_no_name) &&\n-\t\t    name == git_default_name && env_hint) {\n+\t\t    *name == git_default_name && env_hint) {\n \t\t\tfputs(env_hint, stderr);\n \t\t\tenv_hint = NULL; /* warn only once */\n \t\t}\n \t\tif (error_on_no_name)\n-\t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n+\t\t\tdie(\"empty ident %s <%s> not allowed\", *name, *email);\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n \t\tstrlcpy(git_default_name, pw->pw_name,\n \t\t\tsizeof(git_default_name));\n-\t\tname = git_default_name;\n+\t\t*name = git_default_name;\n \t}\n+}\n+\n+const char *fmt_ident(const char *name, const char *email,\n+\t\t      const char *date_str, int flag)\n+{\n+\tstatic char buffer[1000];\n+\tchar date[50];\n+\tint i;\n+\tint name_addr_only = (flag & IDENT_NO_DATE);\n+\n+\tget_ident(&name, &email, flag);\n \n \tstrcpy(date, git_default_date);\n \tif (!name_addr_only && date_str)\n@@ -240,6 +246,18 @@ const char *fmt_name(const char *name, const char *email)\n \treturn fmt_ident(name, email, NULL, IDENT_ERROR_ON_NO_NAME | IDENT_NO_DATE);\n }\n \n+void git_setenv_ident(const char *author_name, const char *author_email,\n+\t\t      const char *committer_name, const char *committer_email)\n+{\n+\tget_ident(&author_name, &author_email, IDENT_ERROR_ON_NO_NAME);\n+\tsetenv(\"GIT_AUTHOR_NAME\", author_name, 1);\n+\tsetenv(\"GIT_AUTHOR_EMAIL\", author_email, 1);\n+\n+\tget_ident(&committer_name, &committer_email, IDENT_ERROR_ON_NO_NAME);\n+\tsetenv(\"GIT_COMMITTER_NAME\", committer_name, 1);\n+\tsetenv(\"GIT_COMMITTER_EMAIL\", committer_email, 1);\n+}\n+\n const char *git_author_info(int flag)\n {\n \treturn fmt_ident(getenv(\"GIT_AUTHOR_NAME\"),\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex 8528f64..21ac57d 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -85,4 +85,30 @@ test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n \n '\n \n+# a hook that test author\n+cat > \"$HOOK\" <<EOT\n+#!/bin/sh\n+case \"\\$GIT_AUTHOR_NAME\" in Superman) exit 0;; esac\n+echo \"Only patches from Superman is accepted now\"\n+exit 1\n+EOT\n+cat \"$HOOK\"\n+chmod +x \"$HOOK\"\n+\n+test_expect_success 'with failing hook' '\n+\n+\techo \"another\" >> file &&\n+\tgit add file &&\n+\ttest_must_fail git commit -m \"another\"\n+\n+'\n+\n+test_expect_success 'with non-failing hook' '\n+\n+\techo \"another\" >> file &&\n+\tgit add file &&\n+\tgit commit -m \"another\" --author \"Superman <root@example.com>\"\n+\n+'\n+\n test_done\n-- \n1.6.6.rc1.31.g1a56b\n\n"},{"id":"144850","messageId":"AANLkTim-TXX4uVyUN9loOI276wnKp22m4F7g5c-js61U@mail.gmail.com","threadId":"24290","inReplyTo":"AANLkTinlGBt3nBa7Ge5ytjisTeDu3As4wCkF8M1iz5JV@mail.gmail.com","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-05T21:31:13Z","receivedAt":"2010-07-05T21:31:13Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jul 5, 2010 at 18:04, Gisle Aas <gisle@aas.no> wrote:\n> I agree that would be a more natural interface.  Attached is a patch\n> that sets these environment variables before the hooks are invoked.\n> The patch also updates the documentation and adds some tests.\n\nHi, just a note that patches submitted for inclusion in Git should\nhave [PATCH] in the subject, be submitted inline and you should CC\nJunio. See Documentation/SubmittingPatches for the instructions.\n"},{"id":"144871","messageId":"7vocel5n08.fsf@alter.siamese.dyndns.org","threadId":"24290","inReplyTo":"AANLkTim-TXX4uVyUN9loOI276wnKp22m4F7g5c-js61U@mail.gmail.com","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-06T03:04:39Z","receivedAt":"2010-07-06T03:04:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Hi, just a note that patches submitted for inclusion in Git should\n> have [PATCH] in the subject, be submitted inline and you should CC\n> Junio.\n\nThanks.  Please add \"... after a list concensus that it is a good change\nis reached\" at the end.\n\nI wonder if we should also make author/committer dates available via the\nsame mechanism.\n"},{"id":"144885","messageId":"20100706072947.GB3567@sigill.intra.peff.net","threadId":"24290","inReplyTo":"7vocel5n08.fsf@alter.siamese.dyndns.org","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-07-06T07:29:47Z","receivedAt":"2010-07-06T07:29:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 05, 2010 at 08:04:39PM -0700, Junio C Hamano wrote:\n\n> I wonder if we should also make author/committer dates available via the\n> same mechanism.\n\nI don't see why not. It is a trivial amount of code, and I can see at\nleast author date being useful.\n\n-Peff\n"},{"id":"144892","messageId":"AANLkTilsdbJLHTY7cx2GeZg9ai_OZhmnEt0KdFjF81Pd@mail.gmail.com","threadId":"24290","inReplyTo":"20100706072947.GB3567@sigill.intra.peff.net","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Gisle Aas","fromEmail":"gisle@aas.no","sentAt":"2010-07-06T09:22:07Z","receivedAt":"2010-07-06T09:22:07Z","isPatch":false,"sender":{"key":"gisle@aas.no","avatar":"https://avatars.githubusercontent.com/u/4326?v=4"},"body":"On Tue, Jul 6, 2010 at 09:29, Jeff King <peff@peff.net> wrote:\n> On Mon, Jul 05, 2010 at 08:04:39PM -0700, Junio C Hamano wrote:\n>\n>> I wonder if we should also make author/committer dates available via the\n>> same mechanism.\n>\n> I don't see why not. It is a trivial amount of code, and I can see at\n> least author date being useful.\n\nOne question that arise is what format the date variables should have.\n Is the raw format (that is \"1278400946 +0200\") sensible?\n\nI've created a patch for this locally, but it has some side effects in\nmessing up the dates stored by 'git commit -C...' which I have not\nfigured out yet; and now I need to get some other stuff done.  It\nmight take some days until a revised patch is ready.  Feel free to\ncontribute your \"trivial amount of code\" before I get around to it :-)\n\nRegards,\nGisle\n"},{"id":"144918","messageId":"7veifg4nom.fsf@alter.siamese.dyndns.org","threadId":"24290","inReplyTo":"AANLkTilsdbJLHTY7cx2GeZg9ai_OZhmnEt0KdFjF81Pd@mail.gmail.com","subject":"Re: Make author and committer available to pre-commit hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-06T15:47:37Z","receivedAt":"2010-07-06T15:47:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Gisle Aas <gisle@aas.no> writes:\n\n> One question that arise is what format the date variables should have.\n> Is the raw format (that is \"1278400946 +0200\") sensible?\n\nI'd say so.  We are doing this for scripts (hooks) and there is not much\npoint doing machine-readable-to-human conversion they could do if they so\nchoose.\n\n> I've created a patch for this locally, but it has some side effects in\n> messing up the dates stored by 'git commit -C...' which I have not\n> figured out yet; and now I need to get some other stuff done.\n\nI imagine that you would be sending the date information just the same way\nyou send the author information, so if your addition messes up the dates\nfor \"commit -C\", wouldn't your original patch mess up the authorship\ninformation the same way?\n\nIn any case, this new feature won't be in 'master' nor 'next' while we are\nin pre-release freeze, so no need to rush.\n\nThanks.\n"}]}