git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Phil Hord <phil.hord@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	Michael J Gruber <git@drmicha.warpmail.net>,
	Marc Branchaud <marcnarc@xiplink.com>,
	"W. Trevor King" <wking@tremily.us>
Subject: [PATCH] submodule deinit: don't output "Cleared directory" when directory is empty
Date: Tue, 16 Apr 2013 22:47:10 +0200	[thread overview]
Message-ID: <516DB8CE.6050109@web.de> (raw)
In-Reply-To: <CABURp0rkeYc7K0COhc1+96Q2Ox2TaNRpMSmQYOiwBpgPJbsafA@mail.gmail.com>

Currently a "submodule deinit" run on a non-populated submodule will still
print the "Cleared directory" message even though the directory is already
empty. While that is technically correct (as the directory is removed and
created again), it is rather surprising to see this message for an empty
submodule directory where nothing is to be cleared.

Fix that by using 'test ! -d "$(find "$sm_path" -maxdepth 0 -empty)"' to
test for the directory being not empty before removing and recreating it.

Thanks-to: Phil Hord <phil.hord@gmail.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 16.04.2013 15:32, schrieb Phil Hord:
> On Mon, Apr 1, 2013 at 3:02 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Okay, so here is the patch for that. If someone could point out
>> a portable and efficient way to check if a directory is already
>> empty I would be happy to use that to silence the "Cleaned
>> directory" message currently printed also when deinit is run on
>> an already empty directory.
> 
>    isemptydir() {
>         test -d "$(find $1 -maxdepth 0 -empty)"
>    }
> 
> Sorry for the late reply.  I see this patch is already in master
> (which is fine with me).

Thanks, I managed to miss that solution when googling for it.


 git-submodule.sh           | 35 +++++++++++++++++++----------------
 t/t7400-submodule-basic.sh |  8 ++++----
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..52ecbf1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -594,27 +594,30 @@ cmd_deinit()
 		die_if_unmatched "$mode"
 		name=$(module_name "$sm_path") || exit

-		# Remove the submodule work tree (unless the user already did it)
-		if test -d "$sm_path"
+		# Remove the submodule work tree (unless the user already did it or it is empty)
+		if test ! -d "$(find "$sm_path" -maxdepth 0 -empty)"
 		then
-			# Protect submodules containing a .git directory
-			if test -d "$sm_path/.git"
+			if test -d "$sm_path"
 			then
-				echo >&2 "$(eval_gettext "Submodule work tree '\$sm_path' contains a .git directory")"
-				die "$(eval_gettext "(use 'rm -rf' if you really want to remove it including all of its history)")"
-			fi
+				# Protect submodules containing a .git directory
+				if test -d "$sm_path/.git"
+				then
+					echo >&2 "$(eval_gettext "Submodule work tree '\$sm_path' contains a .git directory")"
+					die "$(eval_gettext "(use 'rm -rf' if you really want to remove it including all of its history)")"
+				fi

-			if test -z "$force"
-			then
-				git rm -qn "$sm_path" ||
-				die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"
+				if test -z "$force"
+				then
+					git rm -qn "$sm_path" ||
+					die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"
+				fi
+				rm -rf "$sm_path" &&
+				say "$(eval_gettext "Cleared directory '\$sm_path'")" ||
+				say "$(eval_gettext "Could not remove submodule work tree '\$sm_path'")"
 			fi
-			rm -rf "$sm_path" &&
-			say "$(eval_gettext "Cleared directory '\$sm_path'")" ||
-			say "$(eval_gettext "Could not remove submodule work tree '\$sm_path'")"
-		fi

-		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+			mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+		fi

 		# Remove the .git/config entries (unless the user already did it)
 		if test -n "$(git config --get-regexp submodule."$name\.")"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index ff26535..b56e4a5 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -792,7 +792,7 @@ test_expect_success 'submodule deinit deinits a submodule when its work tree is
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
 	test_i18ngrep ! "Cleared directory .init" actual &&
-	test_i18ngrep "Cleared directory .example2" actual &&
+	test_i18ngrep ! "Cleared directory .example2" actual &&
 	rmdir init
 '

@@ -842,15 +842,15 @@ test_expect_success 'submodule deinit is silent when used on an uninitialized su
 	test_i18ngrep "Cleared directory .init" actual &&
 	git submodule deinit init >actual &&
 	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
-	test_i18ngrep "Cleared directory .init" actual &&
+	test_i18ngrep ! "Cleared directory .init" actual &&
 	git submodule deinit . >actual &&
 	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
 	test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual &&
-	test_i18ngrep "Cleared directory .init" actual &&
+	test_i18ngrep ! "Cleared directory .init" actual &&
 	git submodule deinit . >actual &&
 	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
 	test_i18ngrep ! "Submodule .example2. (.*) unregistered for path .example2" actual &&
-	test_i18ngrep "Cleared directory .init" actual &&
+	test_i18ngrep ! "Cleared directory .init" actual &&
 	rmdir init example2
 '

-- 
1.8.2.1.419.g33f67ba

  reply	other threads:[~2013-04-16 20:50 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-06 21:11 [PATCH v4] submodule: add 'deinit' command Jens Lehmann
2013-02-12 17:11 ` Phil Hord
2013-02-12 18:03   ` Junio C Hamano
2013-02-13 19:33   ` Jens Lehmann
2013-02-13 19:56     ` Junio C Hamano
2013-02-17 20:06       ` [PATCH v5] " Jens Lehmann
2013-02-17 22:32         ` Junio C Hamano
2013-02-18 19:20           ` Jens Lehmann
2013-03-04 21:20             ` [PATCH v6] " Jens Lehmann
2013-03-05  7:29               ` Heiko Voigt
2013-03-05 15:45                 ` Junio C Hamano
2013-03-06  6:52                   ` Heiko Voigt
2013-03-12 15:39 ` [PATCH v4] " Phil Hord
2013-03-12 16:22   ` Junio C Hamano
2013-03-18 20:54     ` Jens Lehmann
2013-03-19  1:45       ` Junio C Hamano
2013-04-01 19:02         ` [PATCH] submodule deinit: clarify work tree removal message Jens Lehmann
2013-04-16 13:32           ` Phil Hord
2013-04-16 20:47             ` Jens Lehmann [this message]
2013-04-17  5:16             ` Junio C Hamano
2013-04-17 20:30               ` Jens Lehmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=516DB8CE.6050109@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=marcnarc@xiplink.com \
    --cc=phil.hord@gmail.com \
    --cc=wking@tremily.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).