Re: [RFC 03/14] upload-pack: test negotiation with changing repo

Jonathan Tan [off-list ref] writes:

CodingGuidelines?

+inconsistency() {
+	# Simulate that the server initially reports $2 as the ref
+	# corresponding to $1, and after that, $1 as the ref corresponding to
+	# $1. This corresponds to the real-life situation where the server's
+	# repository appears to change during negotiation, for example, when
+	# different servers in a load-balancing arrangement serve (stateless)
+	# RPCs during a single negotiation.
+	printf "s/%s/%s/" \
+	       $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
+	       $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
+	       >"$HTTPD_ROOT_PATH/one-time-sed"

I'd prefer for the printf'd result to have a final LF (i.e. not
leaving the resulting one-time-sed with a final incomplete line).
Also, do you really need the pipe to tr-d?  Doesn't the result of 
$(command substitution) omit the final LF anyway?

    $ printf '1 %s 2 %s 3\n' "$(echo foo)" "$(echo bar)"; echo OK
    1 foo 2 bar 3
    OK

This somehow looks like a good thing to do even in production.  Am I
mistaken?

On 01/26/2017 02:33 PM, Junio C Hamano wrote:

Jonathan Tan [off-list ref] writes:

quoted

diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
new file mode 100644
index 000000000..060ec0300
--- /dev/null
+++ b/t/lib-httpd/one-time-sed.sh

@@ -0,0 +1,8 @@
+#!/bin/sh
+
+if [ -e one-time-sed ]; then
+	"$GIT_EXEC_PATH/git-http-backend" | sed "$(cat one-time-sed)"
+	rm one-time-sed
+else
+	"$GIT_EXEC_PATH/git-http-backend"
+fi

CodingGuidelines?

Thanks - done locally and will send out in the next reroll.

quoted

+inconsistency() {
+	# Simulate that the server initially reports $2 as the ref
+	# corresponding to $1, and after that, $1 as the ref corresponding to
+	# $1. This corresponds to the real-life situation where the server's
+	# repository appears to change during negotiation, for example, when
+	# different servers in a load-balancing arrangement serve (stateless)
+	# RPCs during a single negotiation.
+	printf "s/%s/%s/" \
+	       $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
+	       $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
+	       >"$HTTPD_ROOT_PATH/one-time-sed"

I'd prefer for the printf'd result to have a final LF (i.e. not
leaving the resulting one-time-sed with a final incomplete line).
Also, do you really need the pipe to tr-d?  Doesn't the result of
$(command substitution) omit the final LF anyway?

    $ printf '1 %s 2 %s 3\n' "$(echo foo)" "$(echo bar)"; echo OK
    1 foo 2 bar 3
    OK

Done.

quoted

diff --git a/upload-pack.c b/upload-pack.c
index b88ed8e26..0678c53d6 100644
--- a/upload-pack.c
+++ b/upload-pack.c

@@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)
 		} else if (skip_prefix(line, "want ", &arg) &&
 			   !get_sha1_hex(arg, sha1_buf)) {
 			o = parse_object(sha1_buf);
-			if (!o)
+			if (!o) {
+				packet_write_fmt(1,
+						 "ERR upload-pack: not our ref %s",
+						 sha1_to_hex(sha1_buf));
 				die("git upload-pack: not our ref %s",
 				    sha1_to_hex(sha1_buf));
+			}

This somehow looks like a good thing to do even in production.  Am I
mistaken?

Yes, that's true. If this patch set stalls (for whatever reason), I'll 
spin this off into an independent patch.