Re: [PATCH 05/15] refs.c: update rename_ref to use a transaction
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 29, 2014, 18:43 UTC
- Message-ID
- <xmqqtx2myawy.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <CAL=YDWkOZ29+ikXJUzhZqW8-Mk91Z_E1QCiXxT1HZ1oj04pk0w@mail.gmail.com>
Ronnie Sahlberg <sahlberg@google.com> writes:
Show 15 quoted lines
> On Tue, Oct 28, 2014 at 2:12 PM, Junio C Hamano <gitster@pobox.com> wrote: > >> More importantly, when you know that the end result you want to see >> is that the old and new log files are bit-for-bit identical, and if >> not there is some bug in either parsing or formatting, why parse the >> old and reformat into the new? What would happen when there were >> malformed entries in the old that makes your parsing fail? >> > > Fair enough. I will change it to ONLY use a transaction for the actual > ref update and keep using rename() for the reflog handling. > Only real change I will do for the reflog handling is to change the > temporary file name used to be less collission prone if there are two > renames happening at the same time > so that they don't destroy each others reflogs.
I think it is a good idea to make renaming the entire reflog a logical element of transaction (as you mentioned in our private discussion) to allow different backends implement in their best efficient & robust way.
And for filesystem-backed backends, I actually think "keep the original until we know we do not have to roll back", that follows the same pattern for the other transactional updates, is a good implementation of that "best efficient & robust way", compared to the original "just rename it". It frees us from having to be worried about what happens if we cannot rename it back.
Thanks.