Re: Bug Report ( including test script ): Non-Fastforward merges misses directory deletion
- From
Jeff King <peff@peff.net>
- Date
- Feb 19, 2010, 05:57 UTC
- Message-ID
- <20100219055721.GC22645@coredump.intra.peff.net>
- In-Reply-To
- <loom.20100218T113103-602@post.gmane.org>
On Thu, Feb 18, 2010 at 11:43:23AM +0000, Sebastian Thiel wrote:
> To me it appears as if merge, once it detects a file deletion, > internally uses git-rm to delete the affected files from the working > tree. Git-rm will only delete the file's immediate parent directory, > but does not consider other empty parent directories.
Hmm. It seems to be a bug.
-- >8 -- Subject: [PATCH] rm: fix bug in recursive subdirectory removal
If we remove a path in a/deep/subdirectory, we should try to remove as many trailing components as possible (i.e., subdirectory, then deep, then a). However, the test for the return value of rmdir was reversed, so we only ever deleted at most one level.
The fix is in remove_path, so "apply" and "merge-recursive" also are fixed.
Signed-off-by: Jeff King <peff@peff.net> --- This was introduced by Alex's 4a92d1b (Add remove_path: a function to remove as much as possible of a path, 2008-09-27), which ironically complained about bugs in the code it was replacing. :)
As an added bonus, we used to see a failed rmdir as success and keep walking backwards. So now we are avoiding some useless rmdir calls on the parent directories (think of the microseconds we must be saving!).
dir.c | 2 +- t/t3600-rm.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/dir.c b/dir.c index 67c3af6..133c333 100644 --- a/dir.c +++ b/dir.c @@ -1044,7 +1044,7 @@ int remove_path(const char *name) slash = dirs + (slash - name); do { *slash = '\0'; - } while (rmdir(dirs) && (slash = strrchr(dirs, '/'))); + } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } return 0; diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 76b1bb4..0aaf0ad 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -271,4 +271,12 @@ test_expect_success 'choking "git rm" should not let it die with cruft' ' test "$status" != 0 ' +test_expect_success 'rm removes subdirectories recursively' ' + mkdir -p dir/subdir/subsubdir && + echo content >dir/subdir/subsubdir/file && + git add dir/subdir/subsubdir/file && + git rm -f dir/subdir/subsubdir/file && + ! test -d dir +' + test_done
-- 1.7.0.77.gb5742