Jonathan Tan <jonathanta...@google.com> writes:

> 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?

> +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

> 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?

Reply via email to