Re: [PATCH v19 03/10] receive-pack: add new proc-receive hook

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 5, 2020, 18:39 UTC
Message-ID: <xmqqo8kbk5r5.fsf@gitster.c.googlers.com>
In-Reply-To: <CANYiYbFcXN_ARmZ70GQkrQ=uFaf7YPObYcopEOFseON6TQzNpQ@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

> I think this is the right way to fix this issue, even though I cannot
> reproduce this issue in my laptop.

Perhaps it depends on the size of the pipe buffer and timing of the processes getting scheduled?

This is a tangent but I very much like the way comments on a set of changes below is given interspersed, explaining why the change in each hunk is done next to it.

Show 57 quoted lines

> 1. In the `run_proc_receive_hook()` function of "receive-pack", should
> close the input (proc.in) before reading result from "proc-receive":
>
> --  snip --
> @@ -1196,11 +1197,12 @@ static int run_proc_receive_hook(struct
> command *commands,
>                 packet_flush(proc.in);
>         }
>
> +       close(proc.in);
> +
>         /* Read result from proc-receive */
>         code = read_proc_receive_report(&reader, commands, &errmsg);
>
>  cleanup:
> -       close(proc.in);
>         close(proc.out);
>         if (use_sideband)
>                 finish_async(&muxer);
> -- snap --
>
> 2. test helper for proc-receive should consume the input stream before
> die earlier:
>
> -- snip --
> @@ -79,9 +79,11 @@ static void proc_receive_read_commands(struct
> packet_reader *reader,
>                     *p++ != ' ' ||
>                     parse_oid_hex(p, &new_oid, &p) ||
>                     *p++ != ' ' ||
> -                   die_readline)
> +                   die_readline) {
> +                       while (packet_reader_read(reader) != PACKET_READ_EOF);
>                         die("protocol error: expected 'old new ref', got '%s'",
> -                           reader->line);
> +                               die_readline? "<call with
> --die-readline>": reader->line);
> +               }
>                 refname = p;
>                 FLEX_ALLOC_STR(cmd, ref_name, refname);
>                 oidcpy(&cmd->old_oid, &old_oid);
> @@ -136,7 +138,7 @@ int cmd__proc_receive(int argc, const char **argv)
>                 usage_msg_opt("Too many arguments.",
> proc_receive_usage, options);
>         packet_reader_init(&reader, 0, NULL, 0,
>                            PACKET_READ_CHOMP_NEWLINE |
> -                          PACKET_READ_DIE_ON_ERR_PACKET);
> +                          PACKET_READ_GENTLE_ON_EOF);
>
>         sigchain_push(SIGPIPE, SIG_IGN);
>         proc_receive_verison(&reader);
> -- snap --
>
> I will send a standalone patch using git-send-email command line later.
>
> --
> Jiang Xin