Re: [PATCH 00/20] Delete directories left empty after ref deletion

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 17, 2016, 00:08 UTC
Message-ID: <xmqqfuwskxka.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <cover.1455626201.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 8 quoted lines

> Previously, we were pretty sloppy about leaving empty directories
> behind (under both $GIT_DIR/refs and $GIT_DIR/logs) when deleting
> references. Such directories could accumulate essentially forever.
> It's true that `pack-refs` deletes directories that it empties, but if
> a directory is *already* empty, then `pack-refs` doesn't remove it. It
> is also true that if an empty directory gets in the way of the
> creation of a *new* reference, then it is deleted. But otherwise there
> is no systematic cleanup of empty directories.

I had an impression that all of the above are deliberate design decision, perhaps except "pack-refs do not go into an already empty one", as the races between creation into an directory that is about to become empty by ongoing deletion are unpleasant to deal with.

Show 5 quoted lines

> This patch series makes the reference update machinery more aggressive
> about deleting empty directories under $GIT_DIR/refs and under
> $GIT_DIR/logs when deleting references and/or reflogs. This doesn't
> eliminate all situations where empty directories can be left behind
> [3], but it covers the worst offenders.

Good!!  As long as it is done in a correctly non-racy way, that is.

Show 7 quoted lines

> As prelude to the main change, there are a number of patches that make
> the *creation* of reflog directories more robust against races. Since
> we want to delete such directories more aggressively, we have to worry
> more about a race between a process that is creating a new reflog, and
> another process that might be deleting the containing directory at the
> same time. (We already had protection against this sort of race for
> reference creation, but not for reflog creation.)

Yup.

> And since I got tired of writing the same code over and over, I
> abstracted the code for retrying directory creation into a new
> function, raceproof_create_file().

Nice.

I do not think I can get to the meat of the series tonight, but am looking forward to reading it over.

Thanks.