Re: [PATCH] clone: clean up directory after transport_fetch_refs() failure

[Taylor Blau <me@ttaylorr.com>]

On Wed, May 19, 2021 at 07:17:15AM -0400, Jeff King wrote:
> One obvious option to fix this is to reorder the end of the function to
> set the flag first, before cleanup code, and put the label between them.
>
> But we can observe another small bug: the error return from
> transport_fetch_refs() is generally "-1", and we propagate that to the
> return value of cmd_clone(), which ultimately becomes the exit code of
> the process. And we try to avoid transmitting negative values via exit
> codes (only the low 8 bits are passed along as an unsigned value, though
> in practice for "-1" this at least retains the property that it's
> non-zero).
>
> Instead, let's just die(). That makes us consistent with rest of the
> code in the function. It does add a new "fatal:" line to the output, but
> I'd argue that's a good thing:

This reasoning makes sense to me, and thanks for cleaning this up.

    Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor