Re: do people find t5504.8 flaky?

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> So really, what we want for this case is to just get the remote status,
> like so:
>
> diff --git a/send-pack.c b/send-pack.c
> index 34c77cbb1a..d4db965b96 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -565,19 +565,19 @@ int send_pack(struct send_pack_args *args,
>  
>  	if (need_pack_data && cmds_sent) {
>  		if (pack_objects(out, remote_refs, extra_have, args) < 0) {
> -			for (ref = remote_refs; ref; ref = ref->next)
> -				ref->status = REF_STATUS_NONE;
>  			if (args->stateless_rpc)
>  				close(out);
>  			if (git_connection_is_socket(conn))
>  				shutdown(fd[0], SHUT_WR);
>  
>  			/*
>  			 * Do not even bother with the return value; we know we
> -			 * are failing, and just want the error() side effects.
> +			 * are failing, and just want the error() side effects,
> +			 * as well as marking refs with the remote status (if
> +			 * we get one).
>  			 */
>  			if (status_report)
> -				receive_unpack_status(&reader);
> +				receive_status(&reader, remote_refs);
>  
>  			if (use_sideband) {
>  				close(demux.out);
>
> I was worried at first that we might make things worse for the case that
> the network has hung up completely (which would cause pack-objects to
> fail, but also cause us to not get anything from the remote). But this
> is really no worse. Even in the existing code, we'd complain to stderr
> about trying to read the unpack status. And then when we read the remote
> ref status, as soon as we see a bad packet we just quietly stop reading
> (thus leaving any unmentioned refs as EXPECTING_REPORT).
>
> So with that second patch above, the test failure goes away for me.

Nicely analyzed and patched.