{"thread":{"id":"42133","subject":"[PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","startedAt":"2016-04-24T20:20:27Z","lastAt":"2016-04-26T17:51:32Z","messageCount":17,"participants":["Ævar Arnfjörð Bjarmason","Jacob Keller","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"284362","messageId":"1461529229-15222-1-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":null,"subject":"[PATCH 1/3] githooks.txt: Improve the intro section","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-24T20:20:27Z","receivedAt":"2016-04-24T20:20:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the documentation so that:\n\n * We don't talk about \"little scripts\". Hooks can be as big as you\n want, and don't have to be scripts, just call them \"programs\".\n\n * We note what happens with chdir() before a hook is called, nothing\n documented this explicitly, but the current behavior is\n predictable. It helps a lot to know what directory these hooks will\n be executed from.\n\n * We don't make claims about the example hooks which may not be true\n depending on the configuration of 'init.templateDir'. Clarify that\n we're talking about the default settings of git-init in those cases,\n and move some of this documentation into git-init's documentation\n about the default templates.\n\n * We briefly note in the intro that hooks can get their arguments in\n various different ways, and that how exactly is described below for\n each hook.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-init.txt | 6 +++++-\n Documentation/githooks.txt | 32 ++++++++++++++++++++------------\n 2 files changed, 25 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\nindex 8174d27..cf37926 100644\n--- a/Documentation/git-init.txt\n+++ b/Documentation/git-init.txt\n@@ -130,7 +130,11 @@ The template directory will be one of the following (in order):\n - the default template directory: `/usr/share/git-core/templates`.\n \n The default template directory includes some directory structure, suggested\n-\"exclude patterns\" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).\n+\"exclude patterns\" (see linkgit:gitignore[5]), and example hook files.\n+\n+The example are all disabled by default. To enable a hook, rename it\n+by removing its `.sample` suffix. See linkgit:githooks[5] for more\n+info on hook execution.\n \n EXAMPLES\n --------\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a2f59b1..2f3caf7 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -13,18 +13,26 @@ $GIT_DIR/hooks/*\n DESCRIPTION\n -----------\n \n-Hooks are little scripts you can place in `$GIT_DIR/hooks`\n-directory to trigger action at certain points. When\n-'git init' is run, a handful of example hooks are copied into the\n-`hooks` directory of the new repository, but by default they are\n-all disabled. To enable a hook, rename it by removing its `.sample`\n-suffix.\n-\n-NOTE: It is also a requirement for a given hook to be executable.\n-However - in a freshly initialized repository - the `.sample` files are\n-executable by default.\n-\n-This document describes the currently defined hooks.\n+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to\n+trigger action at certain points. Hooks that don't have the executable\n+bit set are ignored.\n+\n+When a hook is called in a non-bare repository the working directory\n+is guaranteed to be the root of the working tree, in a bare repository\n+the working directory will be the path to the repository. I.e. hooks\n+don't need to worry about the user's current working directory.\n+\n+Hooks can get their arguments via the environment, command-line\n+arguments, and stdin. See the documentation for each below hook for\n+details.\n+\n+When 'git init' is run it may depending on its configuration copy\n+hooks to the new repository, see the the \"TEMPLATE DIRECTORY\" section\n+in linkgit:git-init[1] for details. When the rest of this document\n+refers to \"default hooks\" we're talking about the default template\n+shipped with Git.\n+\n+The currently supported hooks are described below.\n \n HOOKS\n -----\n-- \n2.1.3\n"},{"id":"284361","messageId":"1461529229-15222-2-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"1461529229-15222-1-git-send-email-avarab@gmail.com","subject":"[PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-24T20:20:28Z","receivedAt":"2016-04-24T20:20:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Any ACL you implement via an 'update' hook isn't actual access control\nif the user has login access to the machine running git, because they\ncan trivially just built their own git version which doesn't run the\nhook.\n\nChange the documentation to take this dangerous edge case into account,\nand remove the mention of the advice originating on the mailing list,\nthe users reading this don't care where the idea came up.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/githooks.txt | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 2f3caf7..e9d169e 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -275,9 +275,13 @@ does not know the entire set of branches, so it would end up\n firing one e-mail per ref when used naively, though. The\n <> hook is more suited to that.\n \n-Another use suggested on the mailing list is to use this hook to\n-implement access control which is finer grained than the one\n-based on filesystem group.\n+Another use for this hook to implement access control which is finer\n+grained than the one based on filesystem group. Note that if the user\n+pushing has a normal login shell on the machine receiving the push\n+implementing access control like this can be trivially bypassed by\n+just using not executing the hook. In those cases consider using\n+e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n+access.\n \n Both standard output and standard error output are forwarded to\n 'git send-pack' on the other end, so you can simply `echo` messages\n-- \n2.1.3\n"},{"id":"284363","messageId":"1461529229-15222-3-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"1461529229-15222-1-git-send-email-avarab@gmail.com","subject":"[PATCH 3/3] githooks.txt: Minor improvements to the grammar & phrasing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-24T20:20:29Z","receivedAt":"2016-04-24T20:20:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change:\n\n * Sentences that needed \"the\" or \"a\" to either add those or change them\n so they don't need them.\n\n * The little tangent about \"You can use this to do X (if your project\n wants to do X)\" can just be shortened to \"e.g. if you want to do X\".\n\n * s/parameter/parameters/ when the plural made more sense.\n\nMost of this goes all the way back to the initial introduction of\nhooks.txt in v0.99.5-76-g6d35cc7 by Junio.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/githooks.txt | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex e9d169e..d30492c 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -40,15 +40,15 @@ HOOKS\n applypatch-msg\n ~~~~~~~~~~~~~~\n \n-This hook is invoked by 'git am' script. It takes a single\n+This hook is invoked by 'git am'. It takes a single\n parameter, the name of the file that holds the proposed commit\n-log message. Exiting with non-zero status causes\n-'git am' to abort before applying the patch.\n+log message. Exiting with non-zero causes 'git am' to abort\n+before applying the patch.\n \n The hook is allowed to edit the message file in place, and can\n-be used to normalize the message into some project standard\n-format (if the project has one). It can also be used to refuse\n-the commit after inspecting the message file.\n+be used to e.g. normalize the message into some project standard\n+format. It can also be used to refuse the commit after inspecting\n+the message file.\n \n The default 'applypatch-msg' hook, when enabled, runs the\n 'commit-msg' hook, if the latter is enabled.\n@@ -81,10 +81,10 @@ pre-commit\n ~~~~~~~~~~\n \n This hook is invoked by 'git commit', and can be bypassed\n-with `--no-verify` option. It takes no parameter, and is\n+with the `--no-verify` option. It takes no parameters, 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+making a commit. Exiting with a non-zero status from this script\n+causes the 'git commit' command to abort before creating a commit.\n \n The default 'pre-commit' hook, when enabled, catches introduction\n of lines with trailing whitespaces and aborts the commit when\n@@ -123,15 +123,15 @@ commit-msg\n ~~~~~~~~~~\n \n This hook is invoked by 'git commit', and can be bypassed\n-with `--no-verify` option. It takes a single parameter, the\n+with the `--no-verify` option. It takes a single parameter, the\n name of the file that holds the proposed commit log message.\n-Exiting with non-zero status causes the 'git commit' to\n+Exiting with a non-zero status causes the 'git commit' to\n abort.\n \n-The hook is allowed to edit the message file in place, and can\n-be used to normalize the message into some project standard\n-format (if the project has one). It can also be used to refuse\n-the commit after inspecting the message file.\n+The hook is allowed to edit the message file in place, and can be used\n+to e.g. normalize the message into some project standard format. It\n+can also be used to refuse the commit after inspecting the message\n+file.\n \n The default 'commit-msg' hook, when enabled, detects duplicate\n \"Signed-off-by\" lines, and aborts the commit if one is found.\n@@ -139,8 +139,8 @@ The default 'commit-msg' hook, when enabled, detects duplicate\n post-commit\n ~~~~~~~~~~~\n \n-This hook is invoked by 'git commit'. It takes no\n-parameter, and is invoked after a commit is made.\n+This hook is invoked by 'git commit'. It takes no parameters, and is\n+invoked after a commit is made.\n \n This hook is meant primarily for notification, and cannot affect\n the outcome of 'git commit'.\n-- \n2.1.3\n"},{"id":"284364","messageId":"CA+P7+xqveyG7RTWb2k8UsFWRwjvM6JJkveRr3NV-bammqSkpkQ@mail.gmail.com","threadId":"42133","inReplyTo":"1461529229-15222-2-git-send-email-avarab@gmail.com","subject":"Re: [PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-24T20:32:39Z","receivedAt":"2016-04-24T20:32:39Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Sun, Apr 24, 2016 at 1:20 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> Any ACL you implement via an 'update' hook isn't actual access control\n> if the user has login access to the machine running git, because they\n> can trivially just built their own git version which doesn't run the\n> hook.\n>\n> Change the documentation to take this dangerous edge case into account,\n> and remove the mention of the advice originating on the mailing list,\n> the users reading this don't care where the idea came up.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Documentation/githooks.txt | 10 +++++++---\n> 1 file changed, 7 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 2f3caf7..e9d169e 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -275,9 +275,13 @@ does not know the entire set of branches, so it would end up\n> firing one e-mail per ref when used naively, though. The\n> <> hook is more suited to that.\n>\n> -Another use suggested on the mailing list is to use this hook to\n> -implement access control which is finer grained than the one\n> -based on filesystem group.\n> +Another use for this hook to implement access control which is finer\n> +grained than the one based on filesystem group. Note that if the user\n> +pushing has a normal login shell on the machine receiving the push\n> +implementing access control like this can be trivially bypassed by\n> +just using not executing the hook. In those cases consider using\n\n\"by just using not executing the hook.\"\n\nThis grammar doesn't make sense. It doesn't quite match what you said\nin the commit message either.\n\n> +e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n> +access.\n>\n> Both standard output and standard error output are forwarded to\n> 'git send-pack' on the other end, so you can simply `echo` messages\n> --\n> 2.1.3\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284368","messageId":"1461533202-10706-1-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"CA+P7+xqveyG7RTWb2k8UsFWRwjvM6JJkveRr3NV-bammqSkpkQ@mail.gmail.com","subject":"[PATCH v2 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-24T21:26:42Z","receivedAt":"2016-04-24T21:26:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Any ACL you implement via an 'update' hook isn't actual access control\nif the user has login access to the machine running git, because they\ncan trivially just built their own git version which doesn't run the\nhook.\n\nChange the documentation to take this dangerous edge case into account,\nand remove the mention of the advice originating on the mailing list,\nthe users reading this don't care where the idea came up.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n> \"by just using not executing the hook.\"\n\nYeah that made no sense. Fixed in this version.\n Documentation/githooks.txt | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 2f3caf7..86504ba 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -275,9 +275,13 @@ does not know the entire set of branches, so it would end up\n firing one e-mail per ref when used naively, though. The\n <> hook is more suited to that.\n \n-Another use suggested on the mailing list is to use this hook to\n-implement access control which is finer grained than the one\n-based on filesystem group.\n+Another use for this hook to implement access control which is finer\n+grained than the one based on filesystem group. Note that if the user\n+pushing has a normal login shell on the machine receiving the push\n+implementing access control like this can be trivially bypassed by\n+just not executing the hook. In those cases consider using\n+e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n+access.\n \n Both standard output and standard error output are forwarded to\n 'git send-pack' on the other end, so you can simply `echo` messages\n-- \n2.1.3\n"},{"id":"284373","messageId":"CAPig+cRYBChNVT2cixdzMCvO36Np-RdvTuccFshRoUDWnJv1Rw@mail.gmail.com","threadId":"42133","inReplyTo":"1461529229-15222-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH 1/3] githooks.txt: Improve the intro section","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-25T05:35:43Z","receivedAt":"2016-04-25T05:35:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 24, 2016 at 4:20 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> Change the documentation so that:\n> [...]\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\n> @@ -130,7 +130,11 @@ The template directory will be one of the following (in order):\n> The default template directory includes some directory structure, suggested\n> -\"exclude patterns\" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).\n> +\"exclude patterns\" (see linkgit:gitignore[5]), and example hook files.\n> +\n> +The example are all disabled by default. To enable a hook, rename it\n\ns/example/examples/\n\n> +by removing its `.sample` suffix. See linkgit:githooks[5] for more\n> +info on hook execution.\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*\n> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to\n> +trigger action at certain points. Hooks that don't have the executable\n> +bit set are ignored.\n> +\n> +When a hook is called in a non-bare repository the working directory\n> +is guaranteed to be the root of the working tree, in a bare repository\n> +the working directory will be the path to the repository. I.e. hooks\n> +don't need to worry about the user's current working directory.\n> +\n> +Hooks can get their arguments via the environment, command-line\n> +arguments, and stdin. See the documentation for each below hook for\n> +details.\n> +\n> +When 'git init' is run it may depending on its configuration copy\n\ns/may/&,/\ns/configuration/&,/\n\n> +hooks to the new repository, see the the \"TEMPLATE DIRECTORY\" section\n> +in linkgit:git-init[1] for details. When the rest of this document\n> +refers to \"default hooks\" we're talking about the default template\n> +shipped with Git.\n> +\n> +The currently supported hooks are described below.\n"},{"id":"284404","messageId":"1461593665-31395-1-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"CAPig+cRYBChNVT2cixdzMCvO36Np-RdvTuccFshRoUDWnJv1Rw@mail.gmail.com","subject":"[PATCH v3 0/3] Improvements to githooks.txt documentation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-25T14:14:22Z","receivedAt":"2016-04-25T14:14:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This includes minor grammar edits pointed out by Eric Sunshine + the\none v2 patch I sent out in response to comments by Jacob Keller.\n\nI thought it was less confusing to just send out a whole v3 series\nthan ask Junio to piece together v1..v3 of various patches.\n\nÆvar Arnfjörð Bjarmason (3):\n githooks.txt: Improve the intro section\n githooks.txt: Amend dangerous advice about 'update' hook ACL\n githooks.txt: Minor improvements to the grammar & phrasing\n\n Documentation/git-init.txt | 6 +++-\n Documentation/githooks.txt | 72 +++++++++++++++++++++++++++-------------------\n 2 files changed, 47 insertions(+), 31 deletions(-)\n\n-- \n2.1.3\n"},{"id":"284405","messageId":"1461593665-31395-2-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"1461593665-31395-1-git-send-email-avarab@gmail.com","subject":"[PATCH v3 1/3] githooks.txt: Improve the intro section","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-25T14:14:23Z","receivedAt":"2016-04-25T14:14:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the documentation so that:\n\n * We don't talk about \"little scripts\". Hooks can be as big as you\n want, and don't have to be scripts, just call them \"programs\".\n\n * We note what happens with chdir() before a hook is called, nothing\n documented this explicitly, but the current behavior is\n predictable. It helps a lot to know what directory these hooks will\n be executed from.\n\n * We don't make claims about the example hooks which may not be true\n depending on the configuration of 'init.templateDir'. Clarify that\n we're talking about the default settings of git-init in those cases,\n and move some of this documentation into git-init's documentation\n about the default templates.\n\n * We briefly note in the intro that hooks can get their arguments in\n various different ways, and that how exactly is described below for\n each hook.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-init.txt | 6 +++++-\n Documentation/githooks.txt | 32 ++++++++++++++++++++------------\n 2 files changed, 25 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\nindex 8174d27..cc3be7d 100644\n--- a/Documentation/git-init.txt\n+++ b/Documentation/git-init.txt\n@@ -130,7 +130,11 @@ The template directory will be one of the following (in order):\n - the default template directory: `/usr/share/git-core/templates`.\n \n The default template directory includes some directory structure, suggested\n-\"exclude patterns\" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).\n+\"exclude patterns\" (see linkgit:gitignore[5]), and example hook files.\n+\n+The example hooks are all disabled by default. To enable a hook,\n+rename it by removing its `.sample` suffix. See linkgit:githooks[5]\n+for more info on hook execution.\n \n EXAMPLES\n --------\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex a2f59b1..6db515e 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -13,18 +13,26 @@ $GIT_DIR/hooks/*\n DESCRIPTION\n -----------\n \n-Hooks are little scripts you can place in `$GIT_DIR/hooks`\n-directory to trigger action at certain points. When\n-'git init' is run, a handful of example hooks are copied into the\n-`hooks` directory of the new repository, but by default they are\n-all disabled. To enable a hook, rename it by removing its `.sample`\n-suffix.\n-\n-NOTE: It is also a requirement for a given hook to be executable.\n-However - in a freshly initialized repository - the `.sample` files are\n-executable by default.\n-\n-This document describes the currently defined hooks.\n+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to\n+trigger action at certain points. Hooks that don't have the executable\n+bit set are ignored.\n+\n+When a hook is called in a non-bare repository the working directory\n+is guaranteed to be the root of the working tree, in a bare repository\n+the working directory will be the path to the repository. I.e. hooks\n+don't need to worry about the user's current working directory.\n+\n+Hooks can get their arguments via the environment, command-line\n+arguments, and stdin. See the documentation for each below hook for\n+details.\n+\n+When 'git init' is run it may, depending on its configuration, copy\n+hooks to the new repository, see the the \"TEMPLATE DIRECTORY\" section\n+in linkgit:git-init[1] for details. When the rest of this document\n+refers to \"default hooks\" we're talking about the default template\n+shipped with Git.\n+\n+The currently supported hooks are described below.\n \n HOOKS\n -----\n-- \n2.1.3\n"},{"id":"284406","messageId":"1461593665-31395-3-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"1461593665-31395-1-git-send-email-avarab@gmail.com","subject":"[PATCH v3 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-25T14:14:24Z","receivedAt":"2016-04-25T14:14:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Any ACL you implement via an 'update' hook isn't actual access control\nif the user has login access to the machine running git, because they\ncan trivially just built their own git version which doesn't run the\nhook.\n\nChange the documentation to take this dangerous edge case into account,\nand remove the mention of the advice originating on the mailing list,\nthe users reading this don't care where the idea came up.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/githooks.txt | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 6db515e..38bea7d 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -275,9 +275,13 @@ does not know the entire set of branches, so it would end up\n firing one e-mail per ref when used naively, though. The\n <> hook is more suited to that.\n \n-Another use suggested on the mailing list is to use this hook to\n-implement access control which is finer grained than the one\n-based on filesystem group.\n+Another use for this hook to implement access control which is finer\n+grained than the one based on filesystem group. Note that if the user\n+pushing has a normal login shell on the machine receiving the push\n+implementing access control like this can be trivially bypassed by\n+just not executing the hook. In those cases consider using\n+e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n+access.\n \n Both standard output and standard error output are forwarded to\n 'git send-pack' on the other end, so you can simply `echo` messages\n-- \n2.1.3\n"},{"id":"284407","messageId":"1461593665-31395-4-git-send-email-avarab@gmail.com","threadId":"42133","inReplyTo":"1461593665-31395-1-git-send-email-avarab@gmail.com","subject":"[PATCH v3 3/3] githooks.txt: Minor improvements to the grammar & phrasing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-25T14:14:25Z","receivedAt":"2016-04-25T14:14:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change:\n\n * Sentences that needed \"the\" or \"a\" to either add those or change them\n so they don't need them.\n\n * The little tangent about \"You can use this to do X (if your project\n wants to do X)\" can just be shortened to \"e.g. if you want to do X\".\n\n * s/parameter/parameters/ when the plural made more sense.\n\nMost of this goes all the way back to the initial introduction of\nhooks.txt in v0.99.5-76-g6d35cc7 by Junio.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/githooks.txt | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 38bea7d..339e9ca 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -40,15 +40,15 @@ HOOKS\n applypatch-msg\n ~~~~~~~~~~~~~~\n \n-This hook is invoked by 'git am' script. It takes a single\n+This hook is invoked by 'git am'. It takes a single\n parameter, the name of the file that holds the proposed commit\n-log message. Exiting with non-zero status causes\n-'git am' to abort before applying the patch.\n+log message. Exiting with non-zero causes 'git am' to abort\n+before applying the patch.\n \n The hook is allowed to edit the message file in place, and can\n-be used to normalize the message into some project standard\n-format (if the project has one). It can also be used to refuse\n-the commit after inspecting the message file.\n+be used to e.g. normalize the message into some project standard\n+format. It can also be used to refuse the commit after inspecting\n+the message file.\n \n The default 'applypatch-msg' hook, when enabled, runs the\n 'commit-msg' hook, if the latter is enabled.\n@@ -81,10 +81,10 @@ pre-commit\n ~~~~~~~~~~\n \n This hook is invoked by 'git commit', and can be bypassed\n-with `--no-verify` option. It takes no parameter, and is\n+with the `--no-verify` option. It takes no parameters, 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+making a commit. Exiting with a non-zero status from this script\n+causes the 'git commit' command to abort before creating a commit.\n \n The default 'pre-commit' hook, when enabled, catches introduction\n of lines with trailing whitespaces and aborts the commit when\n@@ -123,15 +123,15 @@ commit-msg\n ~~~~~~~~~~\n \n This hook is invoked by 'git commit', and can be bypassed\n-with `--no-verify` option. It takes a single parameter, the\n+with the `--no-verify` option. It takes a single parameter, the\n name of the file that holds the proposed commit log message.\n-Exiting with non-zero status causes the 'git commit' to\n+Exiting with a non-zero status causes the 'git commit' to\n abort.\n \n-The hook is allowed to edit the message file in place, and can\n-be used to normalize the message into some project standard\n-format (if the project has one). It can also be used to refuse\n-the commit after inspecting the message file.\n+The hook is allowed to edit the message file in place, and can be used\n+to e.g. normalize the message into some project standard format. It\n+can also be used to refuse the commit after inspecting the message\n+file.\n \n The default 'commit-msg' hook, when enabled, detects duplicate\n \"Signed-off-by\" lines, and aborts the commit if one is found.\n@@ -139,8 +139,8 @@ The default 'commit-msg' hook, when enabled, detects duplicate\n post-commit\n ~~~~~~~~~~~\n \n-This hook is invoked by 'git commit'. It takes no\n-parameter, and is invoked after a commit is made.\n+This hook is invoked by 'git commit'. It takes no parameters, and is\n+invoked after a commit is made.\n \n This hook is meant primarily for notification, and cannot affect\n the outcome of 'git commit'.\n-- \n2.1.3\n"},{"id":"284460","messageId":"xmqqr3dtlf7n.fsf@gitster.mtv.corp.google.com","threadId":"42133","inReplyTo":"1461529229-15222-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH 1/3] githooks.txt: Improve the intro section","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-25T18:23:56Z","receivedAt":"2016-04-25T18:23:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change the documentation so that:\n>\n> * We don't talk about \"little scripts\". Hooks can be as big as you\n> want, and don't have to be scripts, just call them \"programs\".\n>\n> * We note what happens with chdir() before a hook is called, nothing\n> documented this explicitly, but the current behavior is\n> predictable. It helps a lot to know what directory these hooks will\n> be executed from.\n>\n> * We don't make claims about the example hooks which may not be true\n> depending on the configuration of 'init.templateDir'. Clarify that\n> we're talking about the default settings of git-init in those cases,\n> and move some of this documentation into git-init's documentation\n> about the default templates.\n>\n> * We briefly note in the intro that hooks can get their arguments in\n> various different ways, and that how exactly is described below for\n> each hook.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Documentation/git-init.txt | 6 +++++-\n> Documentation/githooks.txt | 32 ++++++++++++++++++++------------\n> 2 files changed, 25 insertions(+), 13 deletions(-)\n>\n> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\n> index 8174d27..cf37926 100644\n> --- a/Documentation/git-init.txt\n> +++ b/Documentation/git-init.txt\n> @@ -130,7 +130,11 @@ The template directory will be one of the following (in order):\n> - the default template directory: `/usr/share/git-core/templates`.\n> \n> The default template directory includes some directory structure, suggested\n> -\"exclude patterns\" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).\n> +\"exclude patterns\" (see linkgit:gitignore[5]), and example hook files.\n> +\n> +The example are all disabled by default. To enable a hook, rename it\n\n\"sample hooks are all disabled\" would be better; if for some unknown\nreason you are trying to avoid \"sample hooks\", \"examples are all\ndisabled\" may be acceptable.\n\n> +by removing its `.sample` suffix. See linkgit:githooks[5] for more\n> +info on hook execution.\n\nMakes a first-time reader wonder if I am allowed to ignore the\nsample and write my own from scratch, or if the only thing that is\nallowed is to enable what is shipped with .sample suffix.\n\nI wonder it would become less confusing if we placed even _less_\nstress on these sample hooks; I find that saying we ship sample\nhooks that are disabled is probably more confusing.\n\nWe do not ship any hook (hence nothing is enabled by definition);\nthere are a handful of sample files that you can use when adding\nyour own hook by either referencing them or taking them as-is, and\nthe latter can be done by removing .sample suffix, which is merely a\nspecial case convenience.\n\n\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index a2f59b1..2f3caf7 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*\n> DESCRIPTION\n> -----------\n> \n> -Hooks are little scripts you can place in `$GIT_DIR/hooks`\n> -directory to trigger action at certain points. When\n> -'git init' is run, a handful of example hooks are copied into the\n> -`hooks` directory of the new repository, but by default they are\n> -all disabled. To enable a hook, rename it by removing its `.sample`\n> -suffix.\n> -\n> -NOTE: It is also a requirement for a given hook to be executable.\n> -However - in a freshly initialized repository - the `.sample` files are\n> -executable by default.\n> -\n> -This document describes the currently defined hooks.\n> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to\n> +trigger action at certain points. Hooks that don't have the executable\n> +bit set are ignored.\n\nThe last sentence is POSIXPERM only, though.\n\n> +When a hook is called in a non-bare repository the working directory\n> +is guaranteed to be the root of the working tree, in a bare repository\n> +the working directory will be the path to the repository. I.e. hooks\n> +don't need to worry about the user's current working directory.\n\nThis sentence took me two reads until I mentally substituted \"the\nworking directory\" with \"its working diretory\", to realize that you\nare talking about the cwd of the process that runs the hook.\n\nWhile \"is guaranteed\" may be technically correct and we have no\nintention to change it, it sounds unnecessarily strong.\n\n When a hook is invoked, it is run at the root of the working\n tree in a non-bare repository, or in the $GIT_DIR in a bare\n repository.\n\nperhaps?\n\n> +Hooks can get their arguments via the environment, command-line\n> +arguments, and stdin. See the documentation for each below hook for\n> +details.\n\n\"each below hook\" sounds somewhat ungrammatical.\n"},{"id":"284461","messageId":"xmqqmvohlexx.fsf@gitster.mtv.corp.google.com","threadId":"42133","inReplyTo":"CA+P7+xqveyG7RTWb2k8UsFWRwjvM6JJkveRr3NV-bammqSkpkQ@mail.gmail.com","subject":"Re: [PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-25T18:29:46Z","receivedAt":"2016-04-25T18:29:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n>> -Another use suggested on the mailing list is to use this hook to\n>> -implement access control which is finer grained than the one\n>> -based on filesystem group.\n>> +Another use for this hook to implement access control which is finer\n>> +grained than the one based on filesystem group. Note that if the user\n>> +pushing has a normal login shell on the machine receiving the push\n>> +implementing access control like this can be trivially bypassed by\n>> +just using not executing the hook. In those cases consider using\n>\n> \"by just using not executing the hook.\"\n>\n> This grammar doesn't make sense. It doesn't quite match what you said\n> in the commit message either.\n>\n>> +e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n>> +access.\n\nWhile there is nothing technically wrong in what it says, I wonder\nif it is worth to state the obvious. If one can bypass update hook,\none can bypass any other hook, so the information does not even\nbelong here.\n\nInstead of saying \"acl can be implemented on top of update hook, but\nnot quite because you can bypass it\", it may be more useful to say\n\"in an environment that restricts the users' access only to git\ncommands over the wire, this hook can be used to access control\nwithout relying on filesystem ownership and group membership\",\nperhaps?\n"},{"id":"284463","messageId":"xmqqinz5les6.fsf@gitster.mtv.corp.google.com","threadId":"42133","inReplyTo":"1461529229-15222-3-git-send-email-avarab@gmail.com","subject":"Re: [PATCH 3/3] githooks.txt: Minor improvements to the grammar & phrasing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-25T18:33:13Z","receivedAt":"2016-04-25T18:33:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> -This hook is invoked by 'git am' script. It takes a single\n> +This hook is invoked by 'git am'. It takes a single\n\nGood, as it does not matter to the readers that \"am\" happens to be\nimplemented as a script.\n\n> parameter, the name of the file that holds the proposed commit\n> -log message. Exiting with non-zero status causes\n> -'git am' to abort before applying the patch.\n> +log message. Exiting with non-zero causes 'git am' to abort\n> +before applying the patch.\n\nI am not sure why you dropped \"status\" from here. The result is\nharder to grok, at least to me. Perhaps \"with a non-zero status\"?\n\n> The hook is allowed to edit the message file in place, and can\n> -be used to normalize the message into some project standard\n> -format (if the project has one). It can also be used to refuse\n> -the commit after inspecting the message file.\n> +be used to e.g. normalize the message into some project standard\n> +format. It can also be used to refuse the commit after inspecting\n> +the message file.\n\nOK. Or we can even drop \"e.g.\" and the result still reads perfectly\nfine.\n\n> This hook is invoked by 'git commit', and can be bypassed\n> -with `--no-verify` option. It takes no parameter, and is\n> +with the `--no-verify` option. It takes no parameters, 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> +making a commit. Exiting with a non-zero status from this script\n> +causes the 'git commit' command to abort before creating a commit.\n\nGood. And you kept \"status\" here, which is doubly good.\n"},{"id":"284464","messageId":"xmqqeg9tlepp.fsf@gitster.mtv.corp.google.com","threadId":"42133","inReplyTo":"1461593665-31395-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v3 0/3] Improvements to githooks.txt documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-25T18:34:42Z","receivedAt":"2016-04-25T18:34:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This includes minor grammar edits pointed out by Eric Sunshine + the\n> one v2 patch I sent out in response to comments by Jacob Keller.\n>\n> I thought it was less confusing to just send out a whole v3 series\n> than ask Junio to piece together v1..v3 of various patches.\n\nYup, that greatly reduces the chance of screw-up on my end and is\nvery much appreciated.\n\n>\n> Ævar Arnfjörð Bjarmason (3):\n> githooks.txt: Improve the intro section\n> githooks.txt: Amend dangerous advice about 'update' hook ACL\n> githooks.txt: Minor improvements to the grammar & phrasing\n>\n> Documentation/git-init.txt | 6 +++-\n> Documentation/githooks.txt | 72 +++++++++++++++++++++++++++-------------------\n> 2 files changed, 47 insertions(+), 31 deletions(-)\n"},{"id":"284539","messageId":"CACBZZX4R99fJXoBjO_Ca7b+uAPf=7pH=gW8mq5Ae_BSA3v=G=g@mail.gmail.com","threadId":"42133","inReplyTo":"xmqqinz5les6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] githooks.txt: Minor improvements to the grammar & phrasing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-26T16:55:35Z","receivedAt":"2016-04-26T16:55:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Apr 25, 2016 at 8:33 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> -This hook is invoked by 'git am' script. It takes a single\n>> +This hook is invoked by 'git am'. It takes a single\n>\n> Good, as it does not matter to the readers that \"am\" happens to be\n> implemented as a script.\n\nYeah, no need to mention it being a script. But aside from that I was\nmainly trying to fix grammar errors:\n\n* Incorrect: \"This is invoked by 'git am' script\"\n* Correct: \"This is invoked by the 'git am' script\"\n* Correct: \"This is invoked by a 'git am' script\"\n\nI.e. a few things in these docs were missing the definite/indefinite article.\n\n>> parameter, the name of the file that holds the proposed commit\n>> -log message. Exiting with non-zero status causes\n>> -'git am' to abort before applying the patch.\n>> +log message. Exiting with non-zero causes 'git am' to abort\n>> +before applying the patch.\n>\n> I am not sure why you dropped \"status\" from here. The result is\n> harder to grok, at least to me. Perhaps \"with a non-zero status\"?\n\nI'm not 100% sure about this actually, but I thought:\n\n* Incorrect: \"Exiting with non-zero status\"\n* Correct: \"Exiting with non-zero\"\n* Correct: \"Exiting with a non-zero status\".\n\nI.e. the first one is missing an \"a\", I thought I'd just make it more\nbrief, but sure, I'll make it \"a non-zero status\".\n\n>> The hook is allowed to edit the message file in place, and can\n>> -be used to normalize the message into some project standard\n>> -format (if the project has one). It can also be used to refuse\n>> -the commit after inspecting the message file.\n>> +be used to e.g. normalize the message into some project standard\n>> +format. It can also be used to refuse the commit after inspecting\n>> +the message file.\n>\n> OK. Or we can even drop \"e.g.\" and the result still reads perfectly\n> fine.\n\nDone, and willdo too for the other occurrence.\n\n>> This hook is invoked by 'git commit', and can be bypassed\n>> -with `--no-verify` option. It takes no parameter, and is\n>> +with the `--no-verify` option. It takes no parameters, 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>> +making a commit. Exiting with a non-zero status from this script\n>> +causes the 'git commit' command to abort before creating a commit.\n>\n> Good. And you kept \"status\" here, which is doubly good.\n>\n"},{"id":"284547","messageId":"CACBZZX4-0OwiSf6wVQJ-ZVZtEOcuRNs_1YkjHvYJzpyx7Q-X9A@mail.gmail.com","threadId":"42133","inReplyTo":"xmqqmvohlexx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-26T17:39:38Z","receivedAt":"2016-04-26T17:39:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Apr 25, 2016 at 8:29 PM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>\n>>> -Another use suggested on the mailing list is to use this hook to\n>>> -implement access control which is finer grained than the one\n>>> -based on filesystem group.\n>>> +Another use for this hook to implement access control which is finer\n>>> +grained than the one based on filesystem group. Note that if the user\n>>> +pushing has a normal login shell on the machine receiving the push\n>>> +implementing access control like this can be trivially bypassed by\n>>> +just using not executing the hook. In those cases consider using\n>>\n>> \"by just using not executing the hook.\"\n>>\n>> This grammar doesn't make sense. It doesn't quite match what you said\n>> in the commit message either.\n>>\n>>> +e.g. linkgit:git-shell[1] as the login shell to restrict the user's\n>>> +access.\n>\n> While there is nothing technically wrong in what it says, I wonder\n> if it is worth to state the obvious. If one can bypass update hook,\n> one can bypass any other hook, so the information does not even\n> belong here.\n>\n> Instead of saying \"acl can be implemented on top of update hook, but\n> not quite because you can bypass it\", it may be more useful to say\n> \"in an environment that restricts the users' access only to git\n> commands over the wire, this hook can be used to access control\n> without relying on filesystem ownership and group membership\",\n> perhaps?\n\nWill fix to use something closer to that phrasing.\n"},{"id":"284553","messageId":"CACBZZX7_C9Xsd65FrB2cUAN+UQ5Fxc7MiBdw=HtAGre=Ppxjpw@mail.gmail.com","threadId":"42133","inReplyTo":"xmqqr3dtlf7n.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] githooks.txt: Improve the intro section","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2016-04-26T17:51:32Z","receivedAt":"2016-04-26T17:51:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Apr 25, 2016 at 8:23 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change the documentation so that:\n>>\n>> * We don't talk about \"little scripts\". Hooks can be as big as you\n>> want, and don't have to be scripts, just call them \"programs\".\n>>\n>> * We note what happens with chdir() before a hook is called, nothing\n>> documented this explicitly, but the current behavior is\n>> predictable. It helps a lot to know what directory these hooks will\n>> be executed from.\n>>\n>> * We don't make claims about the example hooks which may not be true\n>> depending on the configuration of 'init.templateDir'. Clarify that\n>> we're talking about the default settings of git-init in those cases,\n>> and move some of this documentation into git-init's documentation\n>> about the default templates.\n>>\n>> * We briefly note in the intro that hooks can get their arguments in\n>> various different ways, and that how exactly is described below for\n>> each hook.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> Documentation/git-init.txt | 6 +++++-\n>> Documentation/githooks.txt | 32 ++++++++++++++++++++------------\n>> 2 files changed, 25 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\n>> index 8174d27..cf37926 100644\n>> --- a/Documentation/git-init.txt\n>> +++ b/Documentation/git-init.txt\n>> @@ -130,7 +130,11 @@ The template directory will be one of the following (in order):\n>> - the default template directory: `/usr/share/git-core/templates`.\n>>\n>> The default template directory includes some directory structure, suggested\n>> -\"exclude patterns\" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).\n>> +\"exclude patterns\" (see linkgit:gitignore[5]), and example hook files.\n>> +\n>> +The example are all disabled by default. To enable a hook, rename it\n>\n> \"sample hooks are all disabled\" would be better; if for some unknown\n> reason you are trying to avoid \"sample hooks\", \"examples are all\n> disabled\" may be acceptable.\n>\n>> +by removing its `.sample` suffix. See linkgit:githooks[5] for more\n>> +info on hook execution.\n>\n> Makes a first-time reader wonder if I am allowed to ignore the\n> sample and write my own from scratch, or if the only thing that is\n> allowed is to enable what is shipped with .sample suffix.\n>\n> I wonder it would become less confusing if we placed even _less_\n> stress on these sample hooks; I find that saying we ship sample\n> hooks that are disabled is probably more confusing.\n>\n> We do not ship any hook (hence nothing is enabled by definition);\n> there are a handful of sample files that you can use when adding\n> your own hook by either referencing them or taking them as-is, and\n> the latter can be done by removing .sample suffix, which is merely a\n> special case convenience.\n\nWill fix this confusion.\n\n>\n>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n>> index a2f59b1..2f3caf7 100644\n>> --- a/Documentation/githooks.txt\n>> +++ b/Documentation/githooks.txt\n>> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*\n>> DESCRIPTION\n>> -----------\n>>\n>> -Hooks are little scripts you can place in `$GIT_DIR/hooks`\n>> -directory to trigger action at certain points. When\n>> -'git init' is run, a handful of example hooks are copied into the\n>> -`hooks` directory of the new repository, but by default they are\n>> -all disabled. To enable a hook, rename it by removing its `.sample`\n>> -suffix.\n>> -\n>> -NOTE: It is also a requirement for a given hook to be executable.\n>> -However - in a freshly initialized repository - the `.sample` files are\n>> -executable by default.\n>> -\n>> -This document describes the currently defined hooks.\n>> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to\n>> +trigger action at certain points. Hooks that don't have the executable\n>> +bit set are ignored.\n>\n> The last sentence is POSIXPERM only, though.\n\nSo what does this do on non-POSIX systems?:\n\n const char *find_hook(const char *name)\n [...]\n strbuf_git_path(&path, \"hooks/%s\", name);\n if (access(path.buf, X_OK) < 0)\n return NULL;\n\nJust always return true I guess.\n\nI'm not going to change this bit, I think that if we have special\nsystems that don't have +x it makes sense to just document how +x\nworks on those systems rather than the entirety of the rest of the git\ndocumentation adding caveats every time the executable bit concept\ncomes up.\n\n>> +When a hook is called in a non-bare repository the working directory\n>> +is guaranteed to be the root of the working tree, in a bare repository\n>> +the working directory will be the path to the repository. I.e. hooks\n>> +don't need to worry about the user's current working directory.\n>\n> This sentence took me two reads until I mentally substituted \"the\n> working directory\" with \"its working diretory\", to realize that you\n> are talking about the cwd of the process that runs the hook.\n>\n> While \"is guaranteed\" may be technically correct and we have no\n> intention to change it, it sounds unnecessarily strong.\n>\n> When a hook is invoked, it is run at the root of the working\n> tree in a non-bare repository, or in the $GIT_DIR in a bare\n> repository.\n>\n> perhaps?\n\nFixed.\n\n>> +Hooks can get their arguments via the environment, command-line\n>> +arguments, and stdin. See the documentation for each below hook for\n>> +details.\n>\n> \"each below hook\" sounds somewhat ungrammatical.\nYeah. Now \"each hook below\".\n"}]}