[PATCH] fix deletion of .git/objects sub-directories in git-prune/repack

On some systems (e.g. Windows XP), directories cannot be deleted while
they're open. Both git-prune and git-repack (and thus, git-gc) try to
rmdir while holding a DIR* handle on the directory, leaving dangling
empty directories in the .git/objects store.

Fix it by swapping the rmdir / closedir calls.

Reported-by: John Chen <redacted>
Reported-by: Stefan Naewe <redacted>
Signed-off-by: Karsten Blees <redacted>
---

Erik Faye-Lund [off-list ref] wrote on 05.03.2012 11:26:08:

On Mon, Mar 5, 2012 at 10:57 AM,  [off-list ref] wrote:

[...]

quoted

I thought the delete / rename problems were a Windows illness (at
least the retry-ask-user-yes-no logic is exclusive to mingw.c)?

POSIX allows for this behavior:

http://pubs.opengroup.org/onlinepubs/009695399/functions/rmdir.html

(EBUSY: "The directory to be removed is currently in use by the system
or some process and the implementation considers this to be an
error.")

I've found traces of similar issues on exotic unices, so I don't think
it's completely hypothetical either...

quoted

And as all mingw stuff
is eventually sent upstream (I hope :-)), I don't see a particular
reason to rush this one.

There's no automatic system for this. I tend to send all my patches
that cleanly fix problems present in git.git directly upstream, so
people on Windows who doesn't use our branch can benefit from them as
well.

So what about this one? It applies cleanly to git master, and when
merged / cherry-picked to msysgit devel, the prune-packed.c fix we
already have in b4cb824d is silently ignored.

Note: the is_dir_empty() function in mingw.c is broken in upstream git
as well, but I'm reluctant to backport this as it will clash with the
Unicode patches.

The upstream patch can be pulled from here:
https://github.com/kblees/git/commit/56e1bc62

And cherry-picked to msysgit devel from here:
https://github.com/kblees/git/commit/b07bfd51

Regards,
Karsten


 builtin/prune-packed.c |    2 +-
 builtin/prune.c        |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index f9463de..a834417 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c

@@ -36,7 +36,6 @@ static void prune_dir(int i, DIR *dir, char *pathname, 

int len, int opts)
                display_progress(progress, i + 1);
        }
        pathname[len] = 0;
-       rmdir(pathname);
 }
 
 void prune_packed_objects(int opts)

@@ -65,6 +64,7 @@ void prune_packed_objects(int opts)
                        continue;
                prune_dir(i, d, pathname, len + 3, opts);
                closedir(d);
+               rmdir(pathname);
        }
        stop_progress(&progress);
 }

diff --git a/builtin/prune.c b/builtin/prune.c
index 58d7cb8..b99b635 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c

@@ -85,9 +85,9 @@ static int prune_dir(int i, char *path)
                }
                fprintf(stderr, "bad sha1 file: %s/%s\n", path, 

de->d_name);
        }
+       closedir(dir);
        if (!show_only)
                rmdir(path);
-       closedir(dir);
        return 0;
 }
 
-- 
1.7.9.msysgit.1.364.g3e2096

Am 06.03.2012 10:18, schrieb karsten.blees@dcon.de:

On some systems (e.g. Windows XP), directories cannot be deleted while
they're open. Both git-prune and git-repack (and thus, git-gc) try to
rmdir while holding a DIR* handle on the directory, leaving dangling
empty directories in the .git/objects store.

Fix it by swapping the rmdir / closedir calls.

The reasoning makes a lot of sense. I wonder why object directories are
pruned nevertheless when I run git gc --prune (I run git master plus a
few topics from pu).

After moving the rmdir() away from prune_dir(), the truncation of the
pathname does not logically belong here anymore. It should be moved with
the rmdir(). Looks good otherwise.

Johannes Sixt [off-list ref] wrote on 06.03.2012 19:32:41:

Am 06.03.2012 10:18, schrieb karsten.blees@dcon.de:

quoted

On some systems (e.g. Windows XP), directories cannot be deleted while
they're open. Both git-prune and git-repack (and thus, git-gc) try to
rmdir while holding a DIR* handle on the directory, leaving dangling
empty directories in the .git/objects store.

Fix it by swapping the rmdir / closedir calls.

The reasoning makes a lot of sense. I wonder why object directories are
pruned nevertheless when I run git gc --prune (I run git master plus a
few topics from pu).

You're right...in upstream git, closedir() is essentially a NOOP. The OS
handle is closed in readdir() on reading the last entry. The dirent.c
'improvements' that move FindFirstFile() to opendir() and FindClose() to
closedir() are in the msysgit fork only [1] (and entirely my fault, I
guess :-)).

So this patch actually isn't currently required for Windows XP (probably
other platforms, I don't know).

[1] https://github.com/msysgit/git/commit/e2b3f70b

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***

*** Please avoid top-posting. ***

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en