Re: [RFC 03/14] upload-pack: test negotiation with changing repo
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 26, 2017, 22:33 UTC
- Message-ID
- <xmqq8tpx30zq.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <afe5d7d3f876893fdad318665805df1e056717c6.1485381677.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 14 quoted lines
> 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?
Show 11 quoted lines
> +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
OKShow 16 quoted lines
> 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?