On 05/07/2013 04:38 AM, Jeff King wrote:
> [...]
> This patch solves it by factoring out the fallback code from
> the lstat() case and calling it from both places. We do not
> need to do the same thing for the symlink/readlink code
> path, even though it might receive ENOENT, too, because
> symlinks cannot be packed (so if it disappears after the
> lstat, it is truly being deleted).
To be really pedantic, we should handle all kinds of changes that
another process might make simultaneously:
* symlink -> deleted
Was already OK, as you explained above.
* regular file -> deleted
Handled by your patch
* symlink -> regular file
Could come from
update-ref --no-deref
if the old version of the reference were a symlink-based symbolic
reference. Oops, wait, that's broken anyway:
$ ln -sf master .git/refs/heads/foo
$ git for-each-ref
4204906e2ee75f9b7224860c40df712d112c004b commit refs/heads/foo
4204906e2ee75f9b7224860c40df712d112c004b commit refs/heads/master
$ git update-ref --no-deref refs/heads/foo master
$ dir .git/refs/heads/foo
lrwxrwxrwx 1 mhagger mhagger 6 May 13 01:07 .git/refs/heads/foo ->
master
But supposing that were fixed and it occurred between the call to
lstat() and the call to readlink(), then readlink() would fail with
errno == EINVAL and we should respond by repeating the code starting
with the lstat().
* regular file -> symlink
Could come from overwriting a reference with a symlink-based symbolic
reference. But this could only happen if the parallel process were
an old version of Git
I think it's been quite a while since Git has used symlinks for symbolic
references, so I doubt that it is worth fixing the second two cases.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html