ref_transaction_commit: bail out on failure to remove a ref

When removal of a loose or packed ref fails, bail out instead of trying to finish the transaction. This way, a single error message can be printed (instead of multiple messages being concatenated by mistake) and the operator can try to solve the underlying problem before there is a chance to muck things up even more. In particular, when git fails to remove a ref, git goes on to try to delete the reflog. Exiting early lets us keep the reflog. When git succeeds in deleting a ref A and fails to remove a ref B, it goes on to try to delete both reflogs. It would be better to just remove the reflog for A, but that would be a more invasive change. Failing early means we keep both reflogs, which puts the operator in a good position to understand the problem and recover. A long term goal is to avoid these problems altogether and roll back the transaction on failure. That kind of transactionality will have to wait for a later series (the plan for which is to make all destructive work happen in a single update of the packed-refs file). Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Aug 28, 2014 at 17:01 UTC 65732845e89eee69422b725bb495f4282065284f
1 file changed +6 -2
refs.c
+6 -2
@@ -3809,16 +3809,20 @@ int ref_transaction_commit(struct ref_transaction *transaction,
3809 struct ref_update *update = updates[i];
3810
3811 if (update->lock) {
3812 - if (delete_ref_loose(update->lock, update->type, err))
3812 + if (delete_ref_loose(update->lock, update->type, err)) {
3813 ret = TRANSACTION_GENERIC_ERROR;
3814 + goto cleanup;
3815 + }
3816
3817 if (!(update->flags & REF_ISPRUNING))
3818 delnames[delnum++] = update->lock->ref_name;
3819 }
3820 }
3821
3820 - if (repack_without_refs(delnames, delnum, err))
3822 + if (repack_without_refs(delnames, delnum, err)) {
3823 ret = TRANSACTION_GENERIC_ERROR;
3824 + goto cleanup;
3825 + }
3826 for (i = 0; i < delnum; i++)
3827 unlink_or_warn(git_path("logs/%s", delnames[i]));
3828 clear_loose_ref_cache(&ref_cache);