On Mon, Jul 19, 2021 at 09:43:14AM -0700, Junio C Hamano wrote:
> >> + * such thing as a lock on the reflog, instead we always lock
> >> + * the "loose ref" (even if packet) above with
> >> + * lock_ref_oid_basic().
> >> + *
> >> + * If race happens we've got nothing more to do, we were asked
> >> + * to delete the reflog, and it's not there anymore. Great!
> >> + */
> >> if (!refs_reflog_exists(ref_store, refname)) {
> >> unlock_ref(lock);
> >> return 0;
> >
> > I think everything is accurate here, though I wouldn't have made the
> > distinction with "locking the loose ref". There is no such thing as
> > locking a packed ref; we always take the loose lock.
> >
> > s/packet/packed/, and maybe s/If race/If a race/.
>
> Meaning, there is no such thing as locking a packed ref or a loose
> ref, we just lock a "ref" and the way it is done in files backend is
> by creating a lockfile as if we are creating/updating a loose one?
Yes, exactly. We do take a lock on the packed-refs file sometimes, but I
generally think "lock the ref" always means to take refs/heads/foo.lock
in the filesystem. Probably not all that important, but if we need a
re-roll to clarify language anyway...:)
> I do agree that the second sentence needs rewriting to make it less
> confusing. lock_ref_oid_basic() being the way to lock "a ref" is
> not limited to cases where we want to do something to do to the
> reflog.
>
> Taking all together, perhaps:
>
> When refs are deleted, their reflog is deleted before the
> ref itself is deleted. This is because there is no separate
> lock for reflog---instead we take a lock on the ref with
> lock_ref_oid_basic().
>
> If a race happens we've got nothing more to do...
Yeah, that reads fine to me.
-Peff
