{"thread":{"id":"52824","subject":"[PATCH 1/2] git-receive-pack: document push options","startedAt":"2020-02-17T14:52:48Z","lastAt":"2020-02-18T17:44:55Z","messageCount":5,"participants":["Drew DeVault","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"391915","messageId":"20200217144432.43920-1-sir@cmpwn.com","threadId":"52824","inReplyTo":null,"subject":"[PATCH 1/2] git-receive-pack: document push options","fromName":"Drew DeVault","fromEmail":"sir@cmpwn.com","sentAt":"2020-02-17T14:44:31Z","receivedAt":"2020-02-17T14:52:48Z","isPatch":true,"sender":{"key":"sir@cmpwn.com","avatar":"https://avatars.githubusercontent.com/u/1310872?v=4"},"body":"This adds the missing documentation on how git push options are\npresented to the pre-receive hook.\n\nSigned-off-by: Drew DeVault \n---\n Documentation/git-receive-pack.txt | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 25702ed730..69b3e77776 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -109,6 +109,12 @@ the following environment variables:\n This hook is called before any refname is updated and before any\n fast-forward checks are performed.\n \n+If the user has specified any push options (see linkgit:git-push[1]),\n+`GIT_PUSH_OPTION_COUNT` is set to the number of options, and\n+`GIT_PUSH_OPTION_N` is set where N is an integer from 0 thru\n+`GIT_PUSH_OPTION_COUNT` - 1. In order for to receive push options,\n+`receive.advertisePushOptions` must be enabled on the server.\n+\n If the pre-receive hook exits with a non-zero exit status no updates\n will be performed, and the update, post-receive and post-update\n hooks will not be invoked either. This can be useful to quickly\n-- \n2.25.0\n\n"},{"id":"391916","messageId":"20200217144432.43920-2-sir@cmpwn.com","threadId":"52824","inReplyTo":"20200217144432.43920-1-sir@cmpwn.com","subject":"[PATCH 2/2] send-pack: downgrade push options error to warning","fromName":"Drew DeVault","fromEmail":"sir@cmpwn.com","sentAt":"2020-02-17T14:44:32Z","receivedAt":"2020-02-17T14:52:49Z","isPatch":true,"sender":{"key":"sir@cmpwn.com","avatar":"https://avatars.githubusercontent.com/u/1310872?v=4"},"body":"Because the receiving end has to explicitly enable\nreceive.advertisePushOptions, and many servers don't, it doesn't make\nsense to set push options globally when half of your pushes are just\ngoing to die.\n\nSigned-off-by: Drew DeVault \n---\n send-pack.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 0407841ae8..8c81825e7d 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -439,7 +439,7 @@ int send_pack(struct send_pack_args *args,\n \tuse_atomic = atomic_supported && args->atomic;\n \n \tif (args->push_options && !push_options_supported)\n-\t\tdie(_(\"the receiving end does not support push options\"));\n+\t\twarning(_(\"the receiving end does not support push options\"));\n \n \tuse_push_options = push_options_supported && args->push_options;\n \n-- \n2.25.0\n\n"},{"id":"391966","messageId":"20200218053040.GC1641086@coredump.intra.peff.net","threadId":"52824","inReplyTo":"20200217144432.43920-1-sir@cmpwn.com","subject":"Re: [PATCH 1/2] git-receive-pack: document push options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-18T05:30:40Z","receivedAt":"2020-02-18T05:30:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 17, 2020 at 09:44:31AM -0500, Drew DeVault wrote:\n\n> Documentation/git-receive-pack.txt | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\n> index 25702ed730..69b3e77776 100644\n> --- a/Documentation/git-receive-pack.txt\n> +++ b/Documentation/git-receive-pack.txt\n> @@ -109,6 +109,12 @@ the following environment variables:\n> This hook is called before any refname is updated and before any\n> fast-forward checks are performed.\n> \n> +If the user has specified any push options (see linkgit:git-push[1]),\n> +`GIT_PUSH_OPTION_COUNT` is set to the number of options, and\n> +`GIT_PUSH_OPTION_N` is set where N is an integer from 0 thru\n> +`GIT_PUSH_OPTION_COUNT` - 1. In order for to receive push options,\n> +`receive.advertisePushOptions` must be enabled on the server.\n\nHmm. This is covered already in the pre-receive sections of githooks(7).\nI wonder if it would be worth consolidating those and having one refer\nto the other.\n\nI'd be OK just duplicating the content in the meantime, but note that\nthe githooks version covers some more subtleties (like setting the count\nto 0 when push options are negotiated but none are sent).\n\nIf we do go with the text above, there's a typo: s/for to/to/.\n\n-Peff\n"},{"id":"391967","messageId":"20200218054009.GD1641086@coredump.intra.peff.net","threadId":"52824","inReplyTo":"20200217144432.43920-2-sir@cmpwn.com","subject":"Re: [PATCH 2/2] send-pack: downgrade push options error to warning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-18T05:40:09Z","receivedAt":"2020-02-18T05:40:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 17, 2020 at 09:44:32AM -0500, Drew DeVault wrote:\n\n> Because the receiving end has to explicitly enable\n> receive.advertisePushOptions, and many servers don't, it doesn't make\n> sense to set push options globally when half of your pushes are just\n> going to die.\n\nThis makes me a little nervous, because we don't know what those push\noptions were supposed to do. Yet we'll proceed with the push minus the\noptions, which might perform an action that it's hard for the user to\nundo. Imagine something as harmless as an option to suppress\nnotifications to your teammates about a push, or something as dangerous\nas one that changes how the push will do an auto-deploy to a production\nservice.\n\nThat latter is probably unlikely, but it feels like we ought to be\nerring on the conservative side here, especially since we've had the old\nbehavior for so many versions.\n\nI do agree that setting push.pushOptions in your global gitconfig is\nprobably going to be annoying. Even in the repo .git/config, you might\npush to multiple remotes, only some of which support the options.\n\nSo perhaps it would make sense to do one or both of:\n\n - allow remote.*.pushOptions for specific remotes\n\n - add a push.pushOptionIfAble key which behaves similarly to\n push.pushOption, but is quietly ignored if options aren't supported.\n Then you could put options there that you know are safe to be\n ignored.\n\nI'm not sure exactly what kinds of options you want be setting globally,\nso I'm not sure which of those would be more useful.\n\n-Peff\n"},{"id":"391996","messageId":"xmqqmu9fkcxd.fsf@gitster-ct.c.googlers.com","threadId":"52824","inReplyTo":"20200218054009.GD1641086@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] send-pack: downgrade push options error to warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-18T17:44:46Z","receivedAt":"2020-02-18T17:44:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This makes me a little nervous, because we don't know what those push\n> options were supposed to do.\n\nThanks for stopping this early. As you said, this die() is very\nmuch deliberate way for us to make sure that we do not damage an\nreceiving end that is not prepared.\n \n> So perhaps it would make sense to do one or both of:\n>\n> - allow remote.*.pushOptions for specific remotes\n>\n> - add a push.pushOptionIfAble key which behaves similarly to\n> push.pushOption, but is quietly ignored if options aren't supported.\n> Then you could put options there that you know are safe to be\n> ignored.\n\nSensible suggestions.\n"}]}