Re: [PATCH v8 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- May 23, 2014, 18:24 UTC
- Message-ID
- <20140523182429.GD12314@google.com>
- In-Reply-To
- <CAL=YDWnRkwX3dCv9dyZmBdh2ZV7aSUCS=F6w67gJuBmCzGGBwQ@mail.gmail.com>
Ronnie Sahlberg wrote:
> On Thu, May 22, 2014 at 4:53 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 10 quoted lines
>> ... or is the problem that the reflogs conflict? >> >> How does rename_ref handle propagating the reflog from the old >> name to the new name, by the way? > > I haven't touched that yet, but we can fix it after the next series > when we have transaction support for reflogs. > > It still renames the reflog via the magic name > #define TMP_RENAMED_LOG "logs/refs/.tmp-renamed-log"
Okay, after looking at what "git branch -m" currently does (deletes the old ref and creates the new one in separate transactions), I'm convinced that this take-locks-for-a-shorter-period-than-is-necessary trick is an improvement relative to the status quo. It gets rid of the window with objects unreferenced between the ref deletion and ref creation, which is a nice improvement.
I still haven't looked closely at the details of the code change but the idea looks sane as a first step.
Later we can try to get the semantics right for this kind of delete/create pair in general transactions, if someone is interested. :) No need for the perfect to be the enemy of the good in the meantime.
Thanks, Jonathan