From: Nguyen Thai Ngoc Duy Date: Sat, 11 Feb 2012 08:55:31 GMT Subject: Re: [PATCH 1/2] pack-refs: remove all empty directories under $GIT_DIR/refs Message-ID: In-Reply-To: <7vhayxn5cg.fsf@alter.siamese.dyndns.org> 2012/2/11 Junio C Hamano : > Nguyễn Thái Ngọc Duy   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. > 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