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

From: Junio C Hamano <[email-masked]>
Date: Thu, 20 Jul 2017 23:01:45 -0700

Jonathan Nieder <[email-masked]> writes:

>> Do we care about the ancient layout that used symlinks to emulate
>> the more modern worktree one?
>
> I think we do care.  In the context of people's changing workflows,
> "git worktree" is a relatively new tool.  Breaking the older
> git-new-workdir (and tools that emulate it) would affect a large
> number of users that don't necessarily know how to clean up the
> result.

Fair enough.  Even though the git-new-workdir is on its way out, and
we really do want to do certain updates under lock, we cannot just
allow the topic to graduate to 'master' in its current form without
some transition plan or mitigation strategy.

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.

Thoughts?  An even easier way out may be to just make pack-refs a
no-op when .git/refs/ is a symbolic link, but as a solution that is
far from satisfactory, while "locking the right file" smells right,
at least to me.