Re: [PATCH 1/2] pack-refs: remove all empty directories under $GIT_DIR/refs

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Feb 11, 2012, 08:55 UTC
Message-ID: <CACsJy8Bh=FZ6kNN5hERK5_H7XnZ83BZ_EfsZ5XmJbrnn+CfgcQ@mail.gmail.com>
In-Reply-To: <7vhayxn5cg.fsf@alter.siamese.dyndns.org>

2012/2/11 Junio C Hamano <gitster@pobox.com>:

Show 12 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Deleting refs does not remove parent directories if they are empty.
>> Empty directories add extra overhead to startup time of most of git
>> commands because they have to traverse $GIT_DIR/refs.
>
> Perhaps drop the first line and replace with the description of what you
> do differently from the first round?
>
>    "git pack-refs" tries to remove directory that becomes empty but it
>    does not try to do so hard enough, leaving a parent directory full of
>    empty children directories without removing.

Sure.

Show 5 quoted lines

> While I agree with Peff that people would expect doing other things while
> pack-refs is running would be much "riskier" and doing this inside
> pack-refs is far more preferable than doing so during normal read-only
> operation, I wonder why we would want a completely separate pass that
> scans the entire hierarchy

Less complex code. Doing it in one pass, I think get_ref_dir() needs to learn read-only vs read-write mode and I haven't figured out a non-ugly way to do it.

> Would it make more sense to note the
> directory for which rmdir() fails in try_remove_empty_parents(), and
> revisit only these directories, at least?

That would leave empty directories not sharing the ref's path until the failed rmdir() unexamined, I think.

> Wouldn't we want to rmdir() the corresponding logs/ hierarchy while at it
> to be consistent?

Good idea.

>> +     subpath = xmalloc(pathlen + 257);
>
> What is this 257 about?

This function is a ripoff from get_ref_dir(). I think 257 is 255 below plus '/' and NIL.

>> +             if (namelen > 255)
>> +                     continue;

-- 
Duy