Re: [PATCH 05/15] refs.c: update rename_ref to use a transaction
- From
Ronnie Sahlberg <sahlberg@google.com>
- Date
- Oct 30, 2014, 18:46 UTC
- Message-ID
- <CAL=YDW=jk1wO8KB1=Wpiw-AKa9xzLJYr36zF6HxMDPxUHoP_6Q@mail.gmail.com>
- In-Reply-To
- <xmqqtx2myawy.fsf@gitster.dls.corp.google.com>
On Wed, Oct 29, 2014 at 11:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Ronnie Sahlberg <sahlberg@google.com> writes: > >> 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.
Right. I have changed it to use an optimized function to read the whole existing reflog as a blob into a strbuf and then a new transaction function transaction_replace_reflog(... the-blob ...) to write the whole blob back to the new location.
Show 9 quoted lines
> > 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.