Re: [PATCH 21/28] commit_packed_refs(): use a staging file separate from the lockfile

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 21, 2017, 07:14 UTC
Message-ID: <xmqqlgnii7mj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqpocuib06.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 28 quoted lines

> The general strategy we take for these atomic update of a file
> entity at $path is to:
>
>  (1) try to create $path.lock exclusively; if we cannot, somebody
>      else holds the lock so fail (or backoff and retry, which
>      amounts to the same thing in the larger picture).
>
>  (2) update $path.lock and close the fd.
>
>  (3) rename $path.lock to $path atomically to unlock.
>
> Would it be sufficent to tweak the above procedure with a new,
> zeroth step?
>
>  (0) see $path is a symlink; if so, readlink it and assign the
>      result to $path.  Then go on to step (1) above.
>
> I do not think such an enhancement would break atomicity guarantee
> we want from the locking.  When updating .git/packed-refs in an
> ancient new-workdir'ed directory, we would end up locking the
> corresponding file in the real repository, which is exactly what we
> want anyway.  As the basic assumption of having a symbolic link in
> the new-workdir'ed directory is that a symlink can stay the same
> forever while the linked target will be updated, I think this would
> be a reasonable short-term "fix".
>
> It would be ideal if we can do this at somewhat a low level, i.e. in
> the lockfile subsystem.

One thing I forgot to mention. For the above to work safely, we should think through the possible interaction this may have with the core.preferSymlinkRefs configuration. If the above is implemented naively, an update of a symref (e.g. "HEAD") that points at something to point at something else would end up taking a lock on the underlying ref and updating it, which is not what we want at all.

Perhaps it is about time we stopped supporting the configuration. We may be able to come up with a solution while keeping it alive, but dropping it would mean we would have one less thing we need to worry about.