Re: [PATCH v3 35/35] remote-curl: don't request v2 when pushing

[Brandon Williams]

On 02/22, Brandon Williams wrote:
> On 02/21, Jonathan Tan wrote:
> > On Tue,  6 Feb 2018 17:13:12 -0800
> > Brandon Williams  wrote:
> > 
> > > +test_expect_success 'push with http:// and a config of v2 does not 
> > > request v2' '
> > > + # Till v2 for push is designed, make sure that if a client has
> > > + # protocol.version configured to use v2, that the client instead falls
> > > + # back and uses v0.
> > > +
> > > + test_commit -C http_child three &&
> > > +
> > > + # Push to another branch, as the target repository has the
> > > + # master branch checked out and we cannot push into it.
> > > + GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
> > > + push origin HEAD:client_branch && 2>log &&
> > 
> > Should it be protocol.version=2? Also, two double ampersands?
> > 
> > Also, optionally, it might be better to do
> > GIT_TRACE_PACKET="$(pwd)/log", so that it does not get mixed with other
> > stderr output.
> 
> Wow thanks for catching that, let me fix that.

I like setting the log via "$(pwd)/log" but it turns out that this
appends to the file if it already exists, which means the previous tests
need to do some cleanup.  This is actually probably preferable anyway.

> 
> -- 
> Brandon Williams

-- 
Brandon Williams

Re: [PATCH v3 35/35] remote-curl: don't request v2 when pushing

[Brandon Williams]

On 02/21, Jonathan Tan wrote:
> On Tue,  6 Feb 2018 17:13:12 -0800
> Brandon Williams  wrote:
> 
> > +test_expect_success 'push with http:// and a config of v2 does not request 
> > v2' '
> > +   # Till v2 for push is designed, make sure that if a client has
> > +   # protocol.version configured to use v2, that the client instead falls
> > +   # back and uses v0.
> > +
> > +   test_commit -C http_child three &&
> > +
> > +   # Push to another branch, as the target repository has the
> > +   # master branch checked out and we cannot push into it.
> > +   GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
> > +   push origin HEAD:client_branch && 2>log &&
> 
> Should it be protocol.version=2? Also, two double ampersands?
> 
> Also, optionally, it might be better to do
> GIT_TRACE_PACKET="$(pwd)/log", so that it does not get mixed with other
> stderr output.

Wow thanks for catching that, let me fix that.

-- 
Brandon Williams

Re: [PATCH v3 35/35] remote-curl: don't request v2 when pushing

[Jonathan Tan]

On Tue,  6 Feb 2018 17:13:12 -0800
Brandon Williams  wrote:

> +test_expect_success 'push with http:// and a config of v2 does not request 
> v2' '
> + # Till v2 for push is designed, make sure that if a client has
> + # protocol.version configured to use v2, that the client instead falls
> + # back and uses v0.
> +
> + test_commit -C http_child three &&
> +
> + # Push to another branch, as the target repository has the
> + # master branch checked out and we cannot push into it.
> + GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
> + push origin HEAD:client_branch && 2>log &&

Should it be protocol.version=2? Also, two double ampersands?

Also, optionally, it might be better to do
GIT_TRACE_PACKET="$(pwd)/log", so that it does not get mixed with other
stderr output.

[PATCH v3 35/35] remote-curl: don't request v2 when pushing

[Brandon Williams]

In order to be able to ship protocol v2 with only supporting fetch, we
need clients to not issue a request to use protocol v2 when pushing
(since the client currently doesn't know how to push using protocol v2).
This allows a client to have protocol v2 configured in
`protocol.version` and take advantage of using v2 for fetch and falling
back to using v0 when pushing while v2 for push is being designed.

We could run into issues if we didn't fall back to protocol v2 when
pushing right now.  This is because currently a server will ignore a request to
use v2 when contacting the 'receive-pack' endpoint and fall back to
using v0, but when push v2 is rolled out to servers, the 'receive-pack'
endpoint will start responding using v2.  So we don't want to get into a
state where a client is requesting to push with v2 before they actually
know how to push using v2.

Signed-off-by: Brandon Williams 
---
 remote-curl.c  | 11 ++-
 t/t5702-protocol-v2.sh | 23 +++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/remote-curl.c b/remote-curl.c
index af431b658..c39b6ece6 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -320,6 +320,7 @@ static struct discovery *discover_refs(const char *service, 
int for_push)
struct discovery *last = last_discovery;
int http_ret, maybe_smart = 0;
struct http_get_options http_options;
+   enum protocol_version version = get_protocol_version_config();
 
if (last && !strcmp(service, last->service))
return last;
@@ -336,8 +337,16 @@ static struct discovery *discover_refs(const char 
*service, int for_push)
strbuf_addf(_url, "service=%s", service);
}
 
+   /*
+* NEEDSWORK: If we are trying to use protocol v2 and we are planning
+* to perform a push, then fallback to v0 since the client doesn't know
+* how to push yet using v2.
+*/
+   if (version == protocol_v2 && !strcmp("git-receive-pack", service))
+   version = protocol_v0;
+
/* Add the extra Git-Protocol header */
-   if (get_protocol_http_header(get_protocol_version_config(), 
_header))
+   if (get_protocol_http_header(version, _header))
string_list_append(_headers, protocol_header.buf);
 
memset(_options, 0, sizeof(http_options));
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index c2c39fe0c..14d589a7f 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -211,6 +211,29 @@ test_expect_success 'fetch with http:// using protocol v2' 
'
grep "git< version 2" log
 '
 
+test_expect_success 'push with http:// and a config of v2 does not request v2' 
'
+   # Till v2 for push is designed, make sure that if a client has
+   # protocol.version configured to use v2, that the client instead falls
+   # back and uses v0.
+
+   test_commit -C http_child three &&
+
+   # Push to another branch, as the target repository has the
+   # master branch checked out and we cannot push into it.
+   GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
+   push origin HEAD:client_branch && 2>log &&
+
+   git -C http_child log -1 --format=%s >actual &&
+   git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s 
client_branch >expect &&
+   test_cmp expect actual &&
+
+   # Client didnt request to use protocol v2
+   ! grep "Git-Protocol: version=2" log &&
+   # Server didnt respond using protocol v2
+   ! grep "git< version 2" log
+'
+
+
 stop_httpd
 
 test_done
-- 
2.16.0.rc1.238.g530d649a79-goog