Re: Re [bug] pull --prune could not delete references due to lock file already exists error

>
> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
>
> [snip]
>
> > The original implementation:
> > Starts a single transaction using ref_store_transaction_begin().
> > Adds all deletions to that transaction.
> > Commits the transaction.
> > If any deletion fails, the entire transaction is aborted.
> > On case-insensitive file systems, two refs like:
> > may conflict at the file system level (e.g. both mapped to the same file or directory).
> > If Git tries to delete both in one go, the transaction fails due to a lock file or unlink error.
> > (Above are my assumptions till now).
> >
> > What has changed is:
> > Deletes each reference in its own transaction
> > struct ref_transaction *transaction = ref_store_transaction_begin(...);
> > ref_transaction_delete(transaction, ...);
> > ref_transaction_commit(transaction, ...);
> > ref_transaction_free(transaction);
> > If one deletion fails due to a case conflict, the others still proceed.
> > It avoids rolling back the entire prune operation just because of a single failure.
> > Keeps failure count and returns appropriately
> > Signals that something went wrong, but Git can now give partial success feedback.
> >
> >
> > The question I have is
> > If this approach seems viable or perhaps any solution,
> > would it be possible to write a test case for this scenario?
> >
>
> You analysis is right. With 'kn/fetch-push-bulk-ref-update' in the works
> (possibly be merged to next soon), we will start using batched updates
> in git-fetch(1) too. Batched updates allow individual updates to fail,
> while allowing the transaction as a whole to succeed.
>
> Unfortunately, because our transaction mechanism doesn't handle
> conflicts, we separate out pruning as a pre-step. So this bug would
> still be present there.
>
> The issue with the fix you're suggesting is a huge performance drop,
> since creating individual transaction for each deletion has a lot of
> overhead and the reftable backend would perform a lot worse in such
> situations.
>
> I can see few solutions overall (including the one you suggested).
>
> One solution is to drop duplicates in case insensitive systems, this is
> the shortest and easiest fix for now.
>
> Perhaps something like (untested back of the hand code):
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index cc0a3deb61..bc79d74b82 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1352,10 +1352,16 @@ static int prune_refs(struct display_state
> *display_state,
>                                         goto cleanup;
>                         }
>                 } else {
> +                       const char *prev;
>                         struct string_list refnames = STRING_LIST_INIT_NODUP;
>
> -                       for (ref = stale_refs; ref; ref = ref->next)
> +                       for (ref = stale_refs; ref; ref = ref->next) {
> +                               if (ignore_case && prev && !strcasecmp(ref->next, prev))
> +                                       continue;
> +
>                                 string_list_append(&refnames, ref->name);
> +                               prev = ref->name;
> +                       }
>
>                         result = refs_delete_refs(get_main_ref_store(the_repository),
>                                                   "fetch: prune", &refnames,
>
>
> A bigger and eventual goal is to simply introduce conflict resolution in
> reference transactions. This would allow us to use batched transaction
> together for pruning and updating of refs, and using batched
> transactions would ensure that single reference changes can fail without
> failing the entire batch.
>
> - Karthik