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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.