Re: [PATCH v6 1/7] refs.c: add err arguments to reflog functions

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2015, 17:11 UTC
Message-ID: <xmqqr3oiy3pm.fsf@gitster.dls.corp.google.com>
In-Reply-To: <559D0281.6040908@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 5 quoted lines

> I think your v7 of this patch goes too far, by turning a failure to
> write to the reflog into a failure of the whole transaction. The problem
> is that this failure comes too late, in the commit phase of the
> transaction. Aborting at this late stage can leave some references
> changed and others rolled back, violating the promise of atomicity.

Yeah, that sounds problematic.

> The old code reported a failure to write the reflog to stderr but didn't
> fail the transaction. I think that behavior is more appropriate. The
> reflog is of lower importance than the references themselves. Junio, do
> you agree?

That is actually a loaded question.

Do I agree that the current (i.e. before this change) behaviour is more appropriate given the current choice of representation of refs and reflogs on the filesystem, treating a failure to update reflog as lower importance event and accept it as a limitation that it cannot abort the whole transaction atomically? Compared to leaving the repository in a half-updated state where some refs and their logs are updated already, other remaining proposed updates are ignored, and the transaction claims to have failed even though some things have already changed and we cannot rollback, I would say that is a better compromise to treat reflog update as a lower importance.

Do I agree that reflog writing should stay to be best-effort in the longer term? Not really. If we are moving the ref API in the direction where we can plug a backend that is different from the traditional filesystem based one for storing refs, I think the backend should also be responsible for storing logs for the refs it stores, and if the backend wants to promise atomicity, then we should be able to fail the whole transaction when updates to refs could proceed but updates to the log of one of these updated refs cannot. So I do not agree to cast in stone "the reflog is of lower importance" as a general rule.