{"thread":{"id":"55207","subject":"[PATCH] githooks.txt: Clarify documentation on reference-transaction hook","startedAt":"2021-02-25T05:51:04Z","lastAt":"2021-03-01T16:58:57Z","messageCount":7,"participants":["Patrick Steinhardt","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"417788","messageId":"c30d41de55b8991a09e1d550e853f582b5394dee.1614232040.git.ps@pks.im","threadId":"55207","inReplyTo":null,"subject":"[PATCH] githooks.txt: Clarify documentation on reference-transaction hook","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-02-25T05:50:12Z","receivedAt":"2021-02-25T05:51:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The reference-transaction hook doesn't clearly document its scope and\nwhat values it receives as input. Document it to make it less surprising\nand clearly delimit its (current) scope.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n\nI've been postponing doing this simple doc update for far too long, but\nhere it finally is. It simply clarifies its current workings and\nlimitations without changing anything. This is not supposed to be a \"We\ndon't want it to ever cover symrefs\", but rather to avoid confusion.\n\nPatrick\n\n Documentation/githooks.txt | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 1f3b57d04d..b01de04702 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -473,7 +473,8 @@ reference-transaction\n \n This hook is invoked by any Git command that performs reference\n updates. It executes whenever a reference transaction is prepared,\n-committed or aborted and may thus get called multiple times.\n+committed or aborted and may thus get called multiple times. The hook\n+does not cover symbolic references.\n \n The hook takes exactly one argument, which is the current state the\n given reference transaction is in:\n@@ -492,6 +493,13 @@ receives on standard input a line of the format:\n \n   <old-value> SP <new-value> SP <ref-name> LF\n \n+where `<old-value>` is the old object name passed into the reference\n+transaction, `<new-value>` is the new object name to be stored in the\n+ref and `<ref-name>` is the full name of the ref. When force updating\n+the reference regardless of its current value or when the reference is\n+to be created anew, `<old-value>` is 40 `0`. To distinguish these cases,\n+you can inspect the current value of `<ref-name>` via `git rev-parse`.\n+\n The exit status of the hook is ignored for any state except for the\n \"prepared\" state. In the \"prepared\" state, a non-zero exit status will\n cause the transaction to be aborted. The hook will not be called with\n-- \n2.30.1\n\n"},{"id":"417869","messageId":"YDiPP3Ml73igpT7Q@coredump.intra.peff.net","threadId":"55207","inReplyTo":"c30d41de55b8991a09e1d550e853f582b5394dee.1614232040.git.ps@pks.im","subject":"Re: [PATCH] githooks.txt: Clarify documentation on reference-transaction hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-26T06:03:43Z","receivedAt":"2021-02-26T06:04:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 25, 2021 at 06:50:12AM +0100, Patrick Steinhardt wrote:\n\n> The reference-transaction hook doesn't clearly document its scope and\n> what values it receives as input. Document it to make it less surprising\n> and clearly delimit its (current) scope.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n> \n> I've been postponing doing this simple doc update for far too long, but\n> here it finally is. It simply clarifies its current workings and\n> limitations without changing anything. This is not supposed to be a \"We\n> don't want it to ever cover symrefs\", but rather to avoid confusion.\n\nI think that's a good step forward. We might want to say \"does not cover\nsymbolic references (but that may change in the future)\" to make it\nclear that nothing is definite.\n\nOTOH, I suspect adding them would require a change to the hook's stdin\nformat, so it is not like a hook could be written in a way to magically\nhandle them if things change in the future.\n\n> @@ -492,6 +493,13 @@ receives on standard input a line of the format:\n>  \n>    <old-value> SP <new-value> SP <ref-name> LF\n>  \n> +where `<old-value>` is the old object name passed into the reference\n> +transaction, `<new-value>` is the new object name to be stored in the\n> +ref and `<ref-name>` is the full name of the ref. When force updating\n> +the reference regardless of its current value or when the reference is\n> +to be created anew, `<old-value>` is 40 `0`. To distinguish these cases,\n> +you can inspect the current value of `<ref-name>` via `git rev-parse`.\n\nWe should probably avoid saying \"40\" here. Maybe \"all zeroes\" or\nsomething.\n\n-Peff\n"},{"id":"418018","messageId":"YDy05NMxhfSKbGtS@ncase","threadId":"55207","inReplyTo":"YDiPP3Ml73igpT7Q@coredump.intra.peff.net","subject":"Re: [PATCH] githooks.txt: Clarify documentation on reference-transaction hook","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-03-01T09:33:24Z","receivedAt":"2021-03-01T09:34:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Feb 26, 2021 at 01:03:43AM -0500, Jeff King wrote:\n> On Thu, Feb 25, 2021 at 06:50:12AM +0100, Patrick Steinhardt wrote:\n> \n> > The reference-transaction hook doesn't clearly document its scope and\n> > what values it receives as input. Document it to make it less surprising\n> > and clearly delimit its (current) scope.\n> > \n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> > \n> > I've been postponing doing this simple doc update for far too long, but\n> > here it finally is. It simply clarifies its current workings and\n> > limitations without changing anything. This is not supposed to be a \"We\n> > don't want it to ever cover symrefs\", but rather to avoid confusion.\n> \n> I think that's a good step forward. We might want to say \"does not cover\n> symbolic references (but that may change in the future)\" to make it\n> clear that nothing is definite.\n\nFair, I'll add that.\n\n> OTOH, I suspect adding them would require a change to the hook's stdin\n> format, so it is not like a hook could be written in a way to magically\n> handle them if things change in the future.\n\nYeah. Hindsight is 20/20, but this should've used some kind of prefix\nwith the explicitly stated hint that additional prefixed can be added in\nthe future. I've got myself to blame for that, I should've known better\nto make this more readily extensible.\n\n> > @@ -492,6 +493,13 @@ receives on standard input a line of the format:\n> >  \n> >    <old-value> SP <new-value> SP <ref-name> LF\n> >  \n> > +where `<old-value>` is the old object name passed into the reference\n> > +transaction, `<new-value>` is the new object name to be stored in the\n> > +ref and `<ref-name>` is the full name of the ref. When force updating\n> > +the reference regardless of its current value or when the reference is\n> > +to be created anew, `<old-value>` is 40 `0`. To distinguish these cases,\n> > +you can inspect the current value of `<ref-name>` via `git rev-parse`.\n> \n> We should probably avoid saying \"40\" here. Maybe \"all zeroes\" or\n> something.\n> \n> -Peff\n\nI wasn't completely sure how to word this and thus opted to do the same\nas the other hooks. Most notably, both pre-push and pre-receive hook\nexplicitly call out SHA-1 and the 40-character thingy.\n\nMaybe I'll just add a patch on top to also change those to instead say\n\"the all-zero object ID\" or something similar.\n\nPatrick\n"},{"id":"418020","messageId":"294fb32de2681dfeac836d2b64c7e0853721d1a3.1614591751.git.ps@pks.im","threadId":"55207","inReplyTo":"c30d41de55b8991a09e1d550e853f582b5394dee.1614232040.git.ps@pks.im","subject":"[PATCH v2 1/2] githooks.txt: Replace mentions of SHA-1 specific properties","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-03-01T09:43:47Z","receivedAt":"2021-03-01T09:44:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The githooks(5) documentation states in several places that the hook\nwill receive a SHA-1 or hashes of 40 characters length. Given that we're\ntransitioning to a world where both SHA-1 and SHA-256 are supported,\nthis is inaccurate.\n\nFix the issue by replacing mentions of SHA-1 with \"object name\" and not\nexplicitly mentioning the hash size.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/githooks.txt | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 1f3b57d04d..4dad80052e 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -138,7 +138,7 @@ given); `template` (if a `-t` option was given or the\n configuration option `commit.template` is set); `merge` (if the\n commit is a merge or a `.git/MERGE_MSG` file exists); `squash`\n (if a `.git/SQUASH_MSG` file exists); or `commit`, followed by\n-a commit SHA-1 (if a `-c`, `-C` or `--amend` option was given).\n+a commit object name (if a `-c`, `-C` or `--amend` option was given).\n \n If the exit status is non-zero, `git commit` will abort.\n \n@@ -231,19 +231,19 @@ named remote is not being used both values will be the same.\n Information about what is to be pushed is provided on the hook's standard\n input with lines of the form:\n \n-  <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF\n+  <local ref> SP <local object name> SP <remote ref> SP <remote object name> LF\n \n For instance, if the command +git push origin master:foreign+ were run the\n hook would receive a line like the following:\n \n   refs/heads/master 67890 refs/heads/foreign 12345\n \n-although the full, 40-character SHA-1s would be supplied.  If the foreign ref\n-does not yet exist the `<remote SHA-1>` will be 40 `0`.  If a ref is to be\n-deleted, the `<local ref>` will be supplied as `(delete)` and the `<local\n-SHA-1>` will be 40 `0`.  If the local commit was specified by something other\n-than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be\n-supplied as it was originally given.\n+although the full object name would be supplied.  If the foreign ref does not\n+yet exist the `<remote object name>` will be the all-zeroes object name.  If a\n+ref is to be deleted, the `<local ref>` will be supplied as `(delete)` and the\n+`<local object name>` will be the all-zeroes object name.  If the local commit\n+was specified by something other than a name which could be expanded (such as\n+`HEAD~`, or an object name) it will be supplied as it was originally given.\n \n If this hook exits with a non-zero status, `git push` will abort without\n pushing anything.  Information about why the push is rejected may be sent\n@@ -268,7 +268,7 @@ input a line of the format:\n where `<old-value>` is the old object name stored in the ref,\n `<new-value>` is the new object name to be stored in the ref and\n `<ref-name>` is the full name of the ref.\n-When creating a new ref, `<old-value>` is 40 `0`.\n+When creating a new ref, `<old-value>` is the all-zeroes object name.\n \n If the hook exits with non-zero status, none of the refs will be\n updated. If the hook exits with zero, updating of individual refs can\n@@ -550,7 +550,7 @@ command-dependent arguments may be passed in the future.\n The hook receives a list of the rewritten commits on stdin, in the\n format\n \n-  <old-sha1> SP <new-sha1> [ SP <extra-info> ] LF\n+  <old-object-name> SP <new-object-name> [ SP <extra-info> ] LF\n \n The 'extra-info' is again command-dependent.  If it is empty, the\n preceding SP is also omitted.  Currently, no commands pass any\n@@ -566,7 +566,7 @@ rebase::\n \tFor the 'squash' and 'fixup' operation, all commits that were\n \tsquashed are listed as being rewritten to the squashed commit.\n \tThis means that there will be several lines sharing the same\n-\t'new-sha1'.\n+\t'new-object-name'.\n +\n The commits are guaranteed to be listed in the order that they were\n processed by rebase.\n-- \n2.30.1\n\n"},{"id":"418021","messageId":"55905b8693dd49637d0516ee123405cbfb58b6c6.1614591751.git.ps@pks.im","threadId":"55207","inReplyTo":"c30d41de55b8991a09e1d550e853f582b5394dee.1614232040.git.ps@pks.im","subject":"[PATCH v2 2/2] githooks.txt: Clarify documentation on reference-transaction hook","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2021-03-01T09:43:55Z","receivedAt":"2021-03-01T09:45:04Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The reference-transaction hook doesn't clearly document its scope and\nwhat values it receives as input. Document it to make it less surprising\nand clearly delimit its (current) scope.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/githooks.txt | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 4dad80052e..b51959ff94 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -473,7 +473,8 @@ reference-transaction\n \n This hook is invoked by any Git command that performs reference\n updates. It executes whenever a reference transaction is prepared,\n-committed or aborted and may thus get called multiple times.\n+committed or aborted and may thus get called multiple times. The hook\n+does not cover symbolic references (but that may change in the future).\n \n The hook takes exactly one argument, which is the current state the\n given reference transaction is in:\n@@ -492,6 +493,14 @@ receives on standard input a line of the format:\n \n   <old-value> SP <new-value> SP <ref-name> LF\n \n+where `<old-value>` is the old object name passed into the reference\n+transaction, `<new-value>` is the new object name to be stored in the\n+ref and `<ref-name>` is the full name of the ref. When force updating\n+the reference regardless of its current value or when the reference is\n+to be created anew, `<old-value>` is the all-zeroes object name. To\n+distinguish these cases, you can inspect the current value of\n+`<ref-name>` via `git rev-parse`.\n+\n The exit status of the hook is ignored for any state except for the\n \"prepared\" state. In the \"prepared\" state, a non-zero exit status will\n cause the transaction to be aborted. The hook will not be called with\n-- \n2.30.1\n\n"},{"id":"418042","messageId":"xmqqsg5estvf.fsf@gitster.c.googlers.com","threadId":"55207","inReplyTo":"294fb32de2681dfeac836d2b64c7e0853721d1a3.1614591751.git.ps@pks.im","subject":"Re: [PATCH v2 1/2] githooks.txt: Replace mentions of SHA-1 specific properties","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-01T16:48:04Z","receivedAt":"2021-03-01T16:51:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Subject: Re: [PATCH v2 1/2] githooks.txt: Replace mentions of SHA-1 specific properties\n\nLooking at \"git shortlog --no-merges -200 master\" output, I'd\nsuggest to please downcase \"Replace\" to match.\n\n> The githooks(5) documentation states in several places that the hook\n> will receive a SHA-1 or hashes of 40 characters length. Given that we're\n> transitioning to a world where both SHA-1 and SHA-256 are supported,\n> this is inaccurate.\n>\n> Fix the issue by replacing mentions of SHA-1 with \"object name\" and not\n> explicitly mentioning the hash size.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/githooks.txt | 22 +++++++++++-----------\n>  1 file changed, 11 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 1f3b57d04d..4dad80052e 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -138,7 +138,7 @@ given); `template` (if a `-t` option was given or the\n>  configuration option `commit.template` is set); `merge` (if the\n>  commit is a merge or a `.git/MERGE_MSG` file exists); `squash`\n>  (if a `.git/SQUASH_MSG` file exists); or `commit`, followed by\n> -a commit SHA-1 (if a `-c`, `-C` or `--amend` option was given).\n> +a commit object name (if a `-c`, `-C` or `--amend` option was given).\n>  \n>  If the exit status is non-zero, `git commit` will abort.\n>  \n> @@ -231,19 +231,19 @@ named remote is not being used both values will be the same.\n>  Information about what is to be pushed is provided on the hook's standard\n>  input with lines of the form:\n>  \n> -  <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF\n> +  <local ref> SP <local object name> SP <remote ref> SP <remote object name> LF\n>  \n>  For instance, if the command +git push origin master:foreign+ were run the\n>  hook would receive a line like the following:\n>  \n>    refs/heads/master 67890 refs/heads/foreign 12345\n>  \n> -although the full, 40-character SHA-1s would be supplied.  If the foreign ref\n> -does not yet exist the `<remote SHA-1>` will be 40 `0`.  If a ref is to be\n> -deleted, the `<local ref>` will be supplied as `(delete)` and the `<local\n> -SHA-1>` will be 40 `0`.  If the local commit was specified by something other\n> -than a name which could be expanded (such as `HEAD~`, or a SHA-1) it will be\n> -supplied as it was originally given.\n> +although the full object name would be supplied.  If the foreign ref does not\n> +yet exist the `<remote object name>` will be the all-zeroes object name.  If a\n> +ref is to be deleted, the `<local ref>` will be supplied as `(delete)` and the\n> +`<local object name>` will be the all-zeroes object name.  If the local commit\n> +was specified by something other than a name which could be expanded (such as\n> +`HEAD~`, or an object name) it will be supplied as it was originally given.\n>  \n>  If this hook exits with a non-zero status, `git push` will abort without\n>  pushing anything.  Information about why the push is rejected may be sent\n> @@ -268,7 +268,7 @@ input a line of the format:\n>  where `<old-value>` is the old object name stored in the ref,\n>  `<new-value>` is the new object name to be stored in the ref and\n>  `<ref-name>` is the full name of the ref.\n> -When creating a new ref, `<old-value>` is 40 `0`.\n> +When creating a new ref, `<old-value>` is the all-zeroes object name.\n>  \n>  If the hook exits with non-zero status, none of the refs will be\n>  updated. If the hook exits with zero, updating of individual refs can\n> @@ -550,7 +550,7 @@ command-dependent arguments may be passed in the future.\n>  The hook receives a list of the rewritten commits on stdin, in the\n>  format\n>  \n> -  <old-sha1> SP <new-sha1> [ SP <extra-info> ] LF\n> +  <old-object-name> SP <new-object-name> [ SP <extra-info> ] LF\n\n<???-object-name> is a tad longer than the original as a\nplaceholder, but this line does not look so bad.\n\nThanks.\n\n>  The 'extra-info' is again command-dependent.  If it is empty, the\n>  preceding SP is also omitted.  Currently, no commands pass any\n> @@ -566,7 +566,7 @@ rebase::\n>  \tFor the 'squash' and 'fixup' operation, all commits that were\n>  \tsquashed are listed as being rewritten to the squashed commit.\n>  \tThis means that there will be several lines sharing the same\n> -\t'new-sha1'.\n> +\t'new-object-name'.\n>  +\n>  The commits are guaranteed to be listed in the order that they were\n>  processed by rebase.\n\n"},{"id":"418043","messageId":"xmqqo8g2stl4.fsf@gitster.c.googlers.com","threadId":"55207","inReplyTo":"55905b8693dd49637d0516ee123405cbfb58b6c6.1614591751.git.ps@pks.im","subject":"Re: [PATCH v2 2/2] githooks.txt: Clarify documentation on reference-transaction hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-01T16:54:15Z","receivedAt":"2021-03-01T16:58:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The reference-transaction hook doesn't clearly document its scope and\n> what values it receives as input. Document it to make it less surprising\n> and clearly delimit its (current) scope.\n\nSounds good (especially \"that may change in the future\" part is a\nnice touch).\n\nThanks.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/githooks.txt | 11 ++++++++++-\n>  1 file changed, 10 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 4dad80052e..b51959ff94 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -473,7 +473,8 @@ reference-transaction\n>  \n>  This hook is invoked by any Git command that performs reference\n>  updates. It executes whenever a reference transaction is prepared,\n> -committed or aborted and may thus get called multiple times.\n> +committed or aborted and may thus get called multiple times. The hook\n> +does not cover symbolic references (but that may change in the future).\n>  \n>  The hook takes exactly one argument, which is the current state the\n>  given reference transaction is in:\n> @@ -492,6 +493,14 @@ receives on standard input a line of the format:\n>  \n>    <old-value> SP <new-value> SP <ref-name> LF\n>  \n> +where `<old-value>` is the old object name passed into the reference\n> +transaction, `<new-value>` is the new object name to be stored in the\n> +ref and `<ref-name>` is the full name of the ref. When force updating\n> +the reference regardless of its current value or when the reference is\n> +to be created anew, `<old-value>` is the all-zeroes object name. To\n> +distinguish these cases, you can inspect the current value of\n> +`<ref-name>` via `git rev-parse`.\n> +\n>  The exit status of the hook is ignored for any state except for the\n>  \"prepared\" state. In the \"prepared\" state, a non-zero exit status will\n>  cause the transaction to be aborted. The hook will not be called with\n"}]}