Re: [PATCH] fetch --prune: exit with error if pruning fails

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Thomas & Junio,

On Fri, 28 Jan 2022, Thomas Gummerer wrote:

> Johannes Schindelin writes:
>
> > On Thu, 27 Jan 2022, Junio C Hamano wrote:
> >
> >> Thomas Gummerer <t.gummerer@gmail.com> writes:
> >>
> >> > +		if (retcode) {
> >> > +			free_refs(ref_map);
> >> > +			goto cleanup;
> >> >  		}
> >>
> >> This part is iffy.  We tried to prune refs, we may have removed some
> >> of the refs missing from the other side but we may still have some
> >> other refs that are missing from the other side due to the failure
> >> we noticed.
> >>
> >> Is it sensible to abort the fetching?  I am undecided, but without
> >> further input, my gut reaction is that it is safe and may even be
> >> better to treat this as a soft error and try to go closer to where
> >> the user wanted to go as much as possible by continuing to fetch
> >> from the other side, given that we have already paid for the cost of
> >> discovering the refs from the other side.
>
> > I am not so sure. When pruning failed, there may very well be directories
> > or files in the way of fetching the refs as desired. And it might be even
> > worse if pruning failed _without_ the fetch failing afterwards: the user
> > specifically asked for stale refs to be cleaned up, the command succeeded,
> > but did not do what the user asked for.
>
> I was thinking along similar lines here.  I was going back and forth
> between letting the fetch continue, and then exiting with a non-zero
> exit code, and just erroring out directly.

Oh, I think I misunderstood Junio. As long as the failed prune will cause
a non-zero exit code, I am fine with continuing to try to fetch.

Ciao,
Dscho