[PATCH] Re: git clean removes directories when not asked to

From: Joachim B Haga <jobh@broadpark.no>
Date: Apr 9, 2008, 17:04 UTC
Message-ID: <85fxtvj6y8.fsf_-_@lupus.strangled.net>
In-Reply-To: <85k5j8jioc.fsf@lupus.strangled.net>

Joachim B Haga <jobh@broadpark.no> writes:

Show 5 quoted lines

> Joachim B Haga <jobh@broadpark.no> writes:
>
>> When invoked from a subdirectory, git clean removes more than it
>> should. According to the documentation, it should not remove
>> directories unless "-d" is given. However:

I have tried to fix this, but I don't know the code. The previous logic was obviously (?) broken, as it had this (paraphrased):

if (remove_directories || matches)
	remove_dir_recursively(...);

which should have been &&. But with only this change, top-level directories were not removed even if "-d" was given. Looking at the (!ISDIR) branch, I guessed that it should instead trigger if pathspec is NULL; i.e, generally treat (!pathspec) as a match. It looks like the behaviour is correct now, but somebody who knows this code should check my guesses.

-j.

>From 73647e7bb73b6037b9d14535ec027da8ee7d6091 Mon Sep 17 00:00:00 2001

From: Joachim B Haga <jobh@broadpark.no>
Date: Wed, 9 Apr 2008 18:49:34 +0200
Subject: [PATCH] Stop builtin-clean from removing directories unless "-d" is given.

---
 builtin-clean.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/builtin-clean.c b/builtin-clean.c
index fefec30..15201d5 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -130,29 +130,32 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                        matches = match_pathspec(pathspec, ent->name, ent->len,
                                                 baselen, seen);
                } else {
-                       matches = 0;
+                       matches = 1;
                }
 
                if (S_ISDIR(st.st_mode)) {
                        strbuf_addstr(&directory, ent->name);
                        qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
-                       if (show_only && (remove_directories || matches)) {
-                               printf("Would remove %s\n", qname);
-                       } else if (remove_directories || matches) {
-                               if (!quiet)
-                                       printf("Removing %s\n", qname);
-                               if (remove_dir_recursively(&directory, 0) != 0) {
-                                       warning("failed to remove '%s'", qname);
-                                       errors++;
+                       if (remove_directories && matches) {
+                               if (show_only)
+                                       printf("Would remove %s\n", qname);
+                               else {
+                                       if (!quiet)
+                                               printf("Removing %s\n", qname);
+                                       if (remove_dir_recursively(&directory, 0) != 0) {
+                                               warning("failed to remove '%s'", qname);
+                                               errors++;
+                                       }
                                }
-                       } else if (show_only) {
-                               printf("Would not remove %s\n", qname);
                        } else {
-                               printf("Not removing %s\n", qname);
+                               if (show_only)
+                                       printf("Would not remove %s\n", qname);
+                               else
+                                       printf("Not removing %s\n", qname);
                        }
                        strbuf_reset(&directory);
                } else {
-                       if (pathspec && !matches)
+                       if (!matches)
                                continue;
                        qname = quote_path_relative(ent->name, -1, &buf, prefix);
                        if (show_only) {

-- 
1.5.4.4