Re: [PATCHv2] push options: pass push options to the transport helper

On Thu, Feb 9, 2017 at 5:56 PM, Jonathan Nieder [off-list ref] wrote:

Private reply because using HTML mail on phone:

So I presume putting it back on the list is fine.

Does this need to be a remote helper capability?

No, as all other capabilities of the git-protocol are mapped 1:1 to
the transport helper protocol for support, e.g. each transport helper
has to handle this on its own, c.f. remote-curl.c:set_option
(line 39 ff and 1065),

What happens with remote
helpers that don't understand the option?

The helper ought to print "unsupported"
(remote-curl.c:1065 for the http helper) and then the transport helper
will take care of it (transport-helper.c: 265 and e.g. 820)

How do remote helpers communicate whether the server has said it accepts
push options?

I guess the remote helper would communicate it the same way as communicating
if the push succeeded? (i.e. reject non fast forward.)

On Wed, Feb 8, 2017, 14:04 Stefan Beller [off-list ref] wrote:

quoted

When using non-builtin protocols relying on a transport helper
(such as http), push options are not propagated to the helper.

The user could ask for push options and a push would seemingly succeed,
but the push options would never be transported to the server,
misleading the users expectation.

Fix this by propagating the push options to the transport helper.

This is only addressing the first issue of
   (1) the helper protocol does not propagate push-option
   (2) the http helper is not prepared to handle push-option

Once we fix (2), the http transport helper can make use of push options
as well, but that happens as a follow up. (1) is a bug fix, whereas (2)
is a feature, which is why we only do (1) here.

Signed-off-by: Stefan Beller <redacted>
---
 Documentation/gitremote-helpers.txt |  4 ++++
 t/t5545-push-options.sh             | 15 +++++++++++++++
 transport-helper.c                  |  7 +++++++
 3 files changed, 26 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt

b/Documentation/gitremote-helpers.txt
index 9e8681f9e1..23474b1eab 100644

--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt

@@ -462,6 +462,10 @@ set by Git if the remote helper has the 'option'

capability.
 'option pushcert {'true'|'false'}::
        GPG sign pushes.

+'option push-option <string>::
+       Transmit <string> as a push option. As the a push option
+       must not contain LF or NUL characters, the string is not encoded.
+
 SEE ALSO
 --------
 linkgit:git-remote[1]

diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
index ea813b9383..9a57a7d8f2 100755
--- a/t/t5545-push-options.sh
+++ b/t/t5545-push-options.sh

@@ -3,6 +3,8 @@
 test_description='pushing to a repository using push options'

 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd

 mk_repo_pair () {
        rm -rf workbench upstream &&

@@ -100,4 +102,17 @@ test_expect_success 'two push options work' '
        test_cmp expect upstream/.git/hooks/post-receive.push_options
 '

+test_expect_success 'push option denied properly by http remote helper'

'\
+       mk_repo_pair &&
+       git -C upstream config receive.advertisePushOptions false &&
+       git -C upstream config http.receivepack true &&
+       cp -R upstream/.git "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git &&
+       git clone "$HTTPD_URL"/smart/upstream test_http_clone &&
+       test_commit -C test_http_clone one &&
+       test_must_fail git -C test_http_clone push --push-option=asdf
origin master &&
+       git -C test_http_clone push origin master
+'
+
+stop_httpd
+
 test_done

diff --git a/transport-helper.c b/transport-helper.c
index 91aed35ebb..1258d6aedd 100644
--- a/transport-helper.c
+++ b/transport-helper.c

@@ -826,6 +826,13 @@ static void set_common_push_options(struct transport

*transport,
                if (set_helper_option(transport, TRANS_OPT_PUSH_CERT,
"if-asked") != 0)
                        die("helper %s does not support
--signed=if-asked", name);
        }
+
+       if (flags & TRANSPORT_PUSH_OPTIONS) {
+               struct string_list_item *item;
+               for_each_string_list_item(item, transport->push_options)
+                       if (set_helper_option(transport, "push-option",
item->string) != 0)
+                               die("helper %s does not support
'push-option'", name);
+       }
 }

 static int push_refs_with_push(struct transport *transport,
--
2.12.0.rc0.1.g018cb5e6f4