On Fri, Feb 10, 2012 at 11:25:27PM +0700, Nguyen Thai Ngoc Duy wrote:
> Empty directories in $GIT_DIR/refs increases overhead at startup.
> Removing a ref does not remove its parent directories even if it's the
> only file left so empty directories will be hanging around.
> [...]
> This patch removes empty directories as we see while traversing
> $GIT_DIR/refs and reverts be7c6d4 because it's no longer needed.
It feels wrong to me to be writing to the repository during what would
otherwise be a read-only operation. Especially without locking. Doesn't
this create a race condition with:
git update-ref refs/foo/bar $sha1 & (a)
git for-each-ref (b)
if you have this sequence of events:
1. (a) wants to create the ref, so it must first mkdir
".git/refs/foo".
2. (b) is reading refs and notices the empty "foo" directory. It
rmdirs it.
3. (a) now attempts to create "bar" inside the newly created "foo"
directory. This fails, because the directory does not exist.
A similar race already can happen with:
git update-ref refs/foo/bar $sha1 &
git update-ref refs/foo $sha1
since the latter will remove a stale "foo" directory before it can
create the new ref file. But that race is OK, I think. Those are both
write operations, and one of them _must_ fail, because they are in
conflict (and I think even with the race they fail gracefully, with the
latter one "winning").
> pack-refs was taught of cleaning up empty directories in be7c6d4
> (pack-refs: remove newly empty directories - 2010-07-06), but it only
> checks parent directories of packed refs only. Already empty dirs are
> left untouched.
I'd much rather have pack-refs simply learn to remove all stale
directories. We at least know that "gc" is a slightly riskier operation.
-Peff
--
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