Re: [PATCH v2 2/2] connect, protocol: log negotiated protocol version

On 2021.08.04 18:28, Eric Sunshine wrote:

On Wed, Aug 4, 2021 at 6:17 PM Josh Steadmon [off-list ref] wrote:

quoted

It is useful for performance monitoring and debugging purposes to know
the wire protocol used for remote operations. This may differ from the
version set in local configuration due to differences in version and/or
configuration between the server and the client. Therefore, log the
negotiated wire protocol version via trace2, for both clients and
servers.

Signed-off-by: Josh Steadmon <redacted>
---

diff --git a/t/t5705-session-id-in-capabilities.sh b/t/t5705-session-id-in-capabilities.sh

@@ -73,6 +74,17 @@ do
+       test_expect_success "client & server log negotiated version (v${PROTO})" '
+               test_when_finished "rm -rf local tr2-client-events tr2-server-events" &&
+               cp -r "$LOCAL_PRISTINE" local &&
+               GIT_TRACE2_EVENT="$(pwd)/tr2-client-events" \
+               git -c protocol.version=$PROTO -C local fetch \
+                       --upload-pack "GIT_TRACE2_EVENT=\"$(pwd)/tr2-server-events\" git-upload-pack" \
+                       origin &&
+               test "$(grep \"key\":\"negotiated-version\",\"value\":\"$PROTO\" tr2-client-events)" &&
+               test "$(grep \"key\":\"negotiated-version\",\"value\":\"$PROTO\" tr2-server-events)"
+       '
 done

What are these `test` commands actually testing? Did you mean `test
-n`? Or, even better, just plain `grep` (not within a
command-substitution)?

Yes, sorry about that, just plain grep is best here. This was due to a
quick copy & paste modification, I should have spent a bit more time
thinking about the test case.

Will be fixed in V3.