files_transaction_commit(): clean up empty directories

When deleting/pruning references, remove any directories that are made
empty by the deletion of loose references or of reflogs. Otherwise such
empty directories can survive forever and accumulate over time. (Even
'pack-refs', which is smart enough to remove the parent directories of
loose references that it prunes, leaves directories that were already
empty.)

And now that files_transaction_commit() takes care of deleting the
parent directories of loose references that it prunes, we don't have to
do that in prune_ref() anymore.

This change would be unwise if the *creation* of these directories could
race with our deletion of them. But the earlier changes in this patch
series made the creation paths robust against races, so now it is safe
to tidy them up more aggressively.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Michael Haggerty 2017-01-06 17:22:43 +01:00 committed by Junio C Hamano
parent a8f0db2d99
commit 446397774a
3 changed files with 64 additions and 8 deletions

View file

@ -2346,7 +2346,6 @@ static void prune_ref(struct ref_to_prune *r)
}
ref_transaction_free(transaction);
strbuf_release(&err);
try_remove_empty_parents(r->name, REMOVE_EMPTY_PARENTS_REF);
}
static void prune_refs(struct ref_to_prune *r)
@ -3794,6 +3793,7 @@ static int files_transaction_commit(struct ref_store *ref_store,
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
update->flags |= REF_DELETED_LOOSE;
}
if (!(update->flags & REF_ISPRUNING))
@ -3806,16 +3806,38 @@ static int files_transaction_commit(struct ref_store *ref_store,
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
for_each_string_list_item(ref_to_delete, &refs_to_delete)
unlink_or_warn(git_path("logs/%s", ref_to_delete->string));
/* Delete the reflogs of any references that were deleted: */
for_each_string_list_item(ref_to_delete, &refs_to_delete) {
if (!unlink_or_warn(git_path("logs/%s", ref_to_delete->string)))
try_remove_empty_parents(ref_to_delete->string,
REMOVE_EMPTY_PARENTS_REFLOG);
}
clear_loose_ref_cache(refs);
cleanup:
transaction->state = REF_TRANSACTION_CLOSED;
for (i = 0; i < transaction->nr; i++)
if (transaction->updates[i]->backend_data)
unlock_ref(transaction->updates[i]->backend_data);
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
struct ref_lock *lock = update->backend_data;
if (lock)
unlock_ref(lock);
if (update->flags & REF_DELETED_LOOSE) {
/*
* The loose reference was deleted. Delete any
* empty parent directories. (Note that this
* can only work because we have already
* removed the lockfile.)
*/
try_remove_empty_parents(update->refname,
REMOVE_EMPTY_PARENTS_REF);
}
}
string_list_clear(&refs_to_delete, 0);
free(head_ref);
string_list_clear(&affected_refnames, 0);

View file

@ -55,6 +55,12 @@
*/
#define REF_UPDATE_VIA_HEAD 0x100
/*
* Used as a flag in ref_update::flags when the loose reference has
* been deleted.
*/
#define REF_DELETED_LOOSE 0x200
/*
* Return true iff refname is minimally safe. "Safe" here means that
* deleting a loose reference by this name will not do any damage, for
@ -158,8 +164,9 @@ struct ref_update {
/*
* One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
* REF_DELETING, REF_ISPRUNING, REF_LOG_ONLY, and
* REF_UPDATE_VIA_HEAD:
* REF_DELETING, REF_ISPRUNING, REF_LOG_ONLY,
* REF_UPDATE_VIA_HEAD, REF_NEEDS_COMMIT, and
* REF_DELETED_LOOSE:
*/
unsigned int flags;

View file

@ -191,6 +191,33 @@ test_expect_success \
git update-ref HEAD'" $A &&
test $A"' = $(cat .git/'"$m"')'
test_expect_success "empty directory removal" '
git branch d1/d2/r1 HEAD &&
git branch d1/r2 HEAD &&
test -f .git/refs/heads/d1/d2/r1 &&
test -f .git/logs/refs/heads/d1/d2/r1 &&
git branch -d d1/d2/r1 &&
! test -e .git/refs/heads/d1/d2 &&
! test -e .git/logs/refs/heads/d1/d2 &&
test -f .git/refs/heads/d1/r2 &&
test -f .git/logs/refs/heads/d1/r2
'
test_expect_success "symref empty directory removal" '
git branch e1/e2/r1 HEAD &&
git branch e1/r2 HEAD &&
git checkout e1/e2/r1 &&
test_when_finished "git checkout master" &&
test -f .git/refs/heads/e1/e2/r1 &&
test -f .git/logs/refs/heads/e1/e2/r1 &&
git update-ref -d HEAD &&
! test -e .git/refs/heads/e1/e2 &&
! test -e .git/logs/refs/heads/e1/e2 &&
test -f .git/refs/heads/e1/r2 &&
test -f .git/logs/refs/heads/e1/r2 &&
test -f .git/logs/HEAD
'
cat >expect <<EOF
$Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 Initial Creation
$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000 Switch