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

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

Hi Thomas,

On Fri, 28 Jan 2022, Thomas Gummerer wrote:

> Junio C Hamano writes:
>
> > Thomas Gummerer <t.gummerer@gmail.com> writes:
> >
> >> +test_expect_success REFFILES 'fetch --prune fails to delete branches' '
> >> +	cd "$D" &&
> >> +	git clone . prune-fail &&
> >> +	cd prune-fail &&
> >> +	git update-ref refs/remotes/origin/extrabranch main &&
> >> +	>.git/packed-refs.new &&
> >> +	test_must_fail git fetch --prune origin
> >
> > Is it because the lockfile ".new" on packed-refs prevents deletion
> > of refs but does not block creation and updates to existing refs
> > that it is an effective test for the "--prune" issue?  If we somehow
> > "locked" the whole ref updates, then the fetch would fail even
> > without "--prune", so it may be "effective", but smells like knowing
> > too much implementation detail.  Yuck, but I do not offhand think of
> > any better way (it is easier to think of worse ways), so without
> > further input, I would say that this is the best (or "least bad") we
> > can do.
>
> Yes, that's correct.  New refs will be created as loose refs, so they
> don't care about packed-refs.  However deletions can potentially be
> happening in packed-refs, and that's why it fails when 'packed-refs.new'
> exists.
>
> I don't love the test either, but I also can't think of a better way to
> do this.

Maybe add a code comment about it? Something like:

	[...]
	: this will prevent --prune from locking packed-refs &&
	>.git/packed-refs.new &&
	[...]

Ciao,
Dscho