[PATCH v4] submodule: add 'deinit' command

[PATCH v4] submodule: add 'deinit' command

[Jens Lehmann]

With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config together with the work tree
himself).

Help those users by providing a 'deinit' command. This removes the whole
submodule.<name> section from .git/config either for the given
submodule(s) or for all those which have been initialized if '.' is
given. Fail if the current work tree contains modifications unless
forced. Complain when for a submodule given on the command line the url
setting can't be found in .git/config, but nonetheless don't fail.

Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Changes since v3:
- Add deinit to the --force documentation of "git submodule"
- Never remove submodules containing a .git dir, even when forced
- Diagnostic output when "rm -rf" or "mkdir" fails
- More test cases


 Documentation/git-rm.txt        |   4 ++
 Documentation/git-submodule.txt |  18 +++++++-
 git-submodule.sh                |  78 ++++++++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh      | 100 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 198 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 92bac27..1d876c2 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing the removal,
+use linkgit:git-submodule[1] `deinit` instead.
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a0c9df8..bc06159 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
@@ -134,6 +135,19 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.

+deinit::
+	Unregister the given submodules, i.e. remove the whole
+	`submodule.$name` section from .git/config together with their work
+	tree. Further calls to `git submodule update`, `git submodule foreach`
+	and `git submodule sync` will skip any unregistered submodules until
+	they are initialized again, so use this command if you don't want to
+	have a local checkout of the submodule in your work tree anymore. If
+	you really want to remove a submodule from the repository and commit
+	that use linkgit:git-rm[1] instead.
++
+If `--force` is specified, the submodule's work tree will be removed even if
+it contains local modifications.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.
@@ -213,8 +227,10 @@ OPTIONS

 -f::
 --force::
-	This option is only valid for add and update commands.
+	This option is only valid for add, deinit and update commands.
 	When running add, allow adding an otherwise ignored submodule path.
+	When running deinit the submodule work trees will be removed even if
+	they contain local changes.
 	When running update, throw away local changes in submodules when
 	switching to a different commit; and always run a checkout operation
 	in the submodule, even if the commit listed in the index of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..f1b552f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
@@ -547,6 +548,81 @@ cmd_init()
 }

 #
+# Unregister submodules from .git/config and remove their work tree
+#
+# $@ = requested paths (use '.' to deinit all submodules)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... init".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-f|--force)
+			force=$1
+			;;
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# = 0
+	then
+		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
+	fi
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		die_if_unmatched "$mode"
+		name=$(module_name "$sm_path") || exit
+		url=$(git config submodule."$name".url)
+		if test -z "$url"
+		then
+			say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+			continue
+		fi
+
+		# Remove the submodule work tree (unless the user already did it)
+		if test -d "$sm_path"
+		then
+			# 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 -n "$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 "Could not remove submodule work tree '\$sm_path'")"
+		fi
+
+		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+
+		# Remove the whole section so we have a clean state when the
+		# user later decides to init this submodule again
+		git config --remove-section submodule."$name" &&
+		say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$sm_path'")"
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)
@@ -1157,7 +1233,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2683cba..f54a40d 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,4 +757,104 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '

+test_expect_success 'set up a second submodule' '
+	git submodule add ./init2 example2 &&
+	git commit -m "submodle example2 added"
+'
+
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	git submodule deinit init &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)" &&
+	test -n "$(git config submodule.example2.url)" &&
+	test -n "$(git config submodule.example2.frotz)" &&
+	test -f example2/.git &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit . deinits all initialized submodules' '
+	git submodule update --init &&
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	test_must_fail git submodule deinit &&
+	git submodule deinit . &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)" &&
+	test -z "$(git config submodule.example2.url)" &&
+	test -z "$(git config submodule.example2.frotz)" &&
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
+	git submodule update --init &&
+	rm -rf init example2/* example2/.git &&
+	git submodule deinit init example2 &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example2.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains modifications unless forced' '
+	git submodule update --init &&
+	echo X >>init/s &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains untracked files unless forced' '
+	git submodule update --init &&
+	echo X >>init/untracked &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule HEAD does not match unless forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		git checkout HEAD^
+	) &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit complains but does not fail when used on an uninitialized submodule' '
+	git submodule update --init &&
+	git submodule deinit init >actual &&
+	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual
+	git submodule deinit init >actual &&
+	test_i18ngrep "No url found for submodule path .init. in .git/config" actual &&
+	git submodule deinit . >actual &&
+	test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		rm .git &&
+		cp -R ../.git/modules/example .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git submodule deinit init &&
+	test_must_fail git submodule deinit -f init &&
+	test -d init/.git &&
+	test -n "$(git config submodule.example.url)"
+'
+
 test_done
-- 
1.8.1.2.546.gea155c0

Re: [PATCH v4] submodule: add 'deinit' command

[Phil Hord]

I haven't tried it yet, but I have some comments.

On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> With "git submodule init" the user is able to tell git he cares about one
> or more submodules and wants to have it populated on the next call to "git
> submodule update". But currently there is no easy way he could tell git he
> does not care about a submodule anymore and wants to get rid of his local
> work tree (except he knows a lot about submodule internals and removes the
> "submodule.$name.url" setting from .git/config together with the work tree
> himself).
>
> Help those users by providing a 'deinit' command. This removes the whole
> submodule.<name> section from .git/config either for the given
> submodule(s) or for all those which have been initialized if '.' is
> given. Fail if the current work tree contains modifications unless
> forced. Complain when for a submodule given on the command line the url
> setting can't be found in .git/config, but nonetheless don't fail.
>
> Add tests and link the man pages of "git submodule deinit" and "git rm"
> to assist the user in deciding whether removing or unregistering the
> submodule is the right thing to do for him.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> Changes since v3:
> - Add deinit to the --force documentation of "git submodule"
> - Never remove submodules containing a .git dir, even when forced
> - Diagnostic output when "rm -rf" or "mkdir" fails
> - More test cases
>
>
>  Documentation/git-rm.txt        |   4 ++
>  Documentation/git-submodule.txt |  18 +++++++-
>  git-submodule.sh                |  78 ++++++++++++++++++++++++++++++-
>  t/t7400-submodule-basic.sh      | 100 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 198 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
> index 92bac27..1d876c2 100644
> --- a/Documentation/git-rm.txt
> +++ b/Documentation/git-rm.txt
> @@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
>  Ignored files are deemed expendable and won't stop a submodule's work
>  tree from being removed.
>
> +If you only want to remove the local checkout of a submodule from your
> +work tree without committing the removal,
> +use linkgit:git-submodule[1] `deinit` instead.
> +
>  EXAMPLES
>  --------
>  `git rm Documentation/\*.txt`::
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index a0c9df8..bc06159 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -13,6 +13,7 @@ SYNOPSIS
>               [--reference <repository>] [--] <repository> [<path>]
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
> +'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
>  'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase]
>               [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
> @@ -134,6 +135,19 @@ init::
>         the explicit 'init' step if you do not intend to customize
>         any submodule locations.
>
> +deinit::
> +       Unregister the given submodules, i.e. remove the whole
> +       `submodule.$name` section from .git/config together with their work
> +       tree. Further calls to `git submodule update`, `git submodule foreach`
> +       and `git submodule sync` will skip any unregistered submodules until
> +       they are initialized again, so use this command if you don't want to
> +       have a local checkout of the submodule in your work tree anymore. If
> +       you really want to remove a submodule from the repository and commit
> +       that use linkgit:git-rm[1] instead.
> ++
> +If `--force` is specified, the submodule's work tree will be removed even if
> +it contains local modifications.
> +
>  update::
>         Update the registered submodules, i.e. clone missing submodules and
>         checkout the commit specified in the index of the containing repository.
> @@ -213,8 +227,10 @@ OPTIONS
>
>  -f::
>  --force::
> -       This option is only valid for add and update commands.
> +       This option is only valid for add, deinit and update commands.
>         When running add, allow adding an otherwise ignored submodule path.
> +       When running deinit the submodule work trees will be removed even if
> +       they contain local changes.
>         When running update, throw away local changes in submodules when
>         switching to a different commit; and always run a checkout operation
>         in the submodule, even if the commit listed in the index of the
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 004c034..f1b552f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
>  USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
>     or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] init [--] [<path>...]
> +   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
>     or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
>     or: $dashless [--quiet] foreach [--recursive] <command>
> @@ -547,6 +548,81 @@ cmd_init()
>  }
>
>  #
> +# Unregister submodules from .git/config and remove their work tree
> +#
> +# $@ = requested paths (use '.' to deinit all submodules)
> +#
> +cmd_deinit()
> +{
> +       # parse $args after "submodule ... init".
> +       while test $# -ne 0
> +       do
> +               case "$1" in
> +               -f|--force)
> +                       force=$1
> +                       ;;
> +               -q|--quiet)
> +                       GIT_QUIET=1
> +                       ;;
> +               --)
> +                       shift
> +                       break
> +                       ;;
> +               -*)
> +                       usage
> +                       ;;
> +               *)
> +                       break
> +                       ;;
> +               esac
> +               shift
> +       done
> +
> +       if test $# = 0
> +       then
> +               die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
> +       fi
> +
> +       module_list "$@" |
> +       while read mode sha1 stage sm_path
> +       do
> +               die_if_unmatched "$mode"
> +               name=$(module_name "$sm_path") || exit
> +               url=$(git config submodule."$name".url)
> +               if test -z "$url"
> +               then
> +                       say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"

Is it safe to shelter the user a little bit more from the git
internals here and say instead:

   Submodule '\$sm_path' is not initialized.


Also, I think this code will show this message for each submodule on
'git submodule deinit .'  But I think I would prefer to suppress it in
that case.  If I have not explicitly stated which submodules to
deinit, then I do not think git should complain that some of them are
not initialized.

> +                       continue
> +               fi
> +
> +               # Remove the submodule work tree (unless the user already did it)
> +               if test -d "$sm_path"
> +               then
> +                       # 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)")"

I expect this is the right thing to do for now.  But I wonder if we
can also move $sm_path/.git to $GIT_DIR/modules/$sm_path in this case
(though I think I am not spelling this path correctly).  Would that be
ok?  What extra work is needed to relocate the .git dir like this?

> +                       fi
> +
> +                       if test -z "$force"
> +                       then
> +                               git rm -n "$sm_path" ||
> +                               die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"

Nit, grammar: use a semicolon instead of a comma.

> +                       fi
> +                       rm -rf "$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'")"
> +
> +               # Remove the whole section so we have a clean state when the
> +               # user later decides to init this submodule again
> +               git config --remove-section submodule."$name" &&
> +               say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$sm_path'")"
> +       done
> +}
> +
> +#
>  # Update each submodule path to correct revision, using clone and checkout as needed
>  #
>  # $@ = requested paths (default to all)
> @@ -1157,7 +1233,7 @@ cmd_sync()
>  while test $# != 0 && test -z "$command"
>  do
>         case "$1" in
> -       add | foreach | init | update | status | summary | sync)
> +       add | foreach | init | deinit | update | status | summary | sync)
>                 command=$1
>                 ;;
>         -q|--quiet)
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 2683cba..f54a40d 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -757,4 +757,104 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
>         )
>  '
>
> +test_expect_success 'set up a second submodule' '
> +       git submodule add ./init2 example2 &&
> +       git commit -m "submodle example2 added"

Nit: submodule is misspelled

> +'
> +
> +test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
> +       git config submodule.example.foo bar &&
> +       git config submodule.example2.frotz nitfol &&
> +       git submodule deinit init &&
> +       test -z "$(git config submodule.example.url)" &&
> +       test -z "$(git config submodule.example.foo)" &&
> +       test -n "$(git config submodule.example2.url)" &&
> +       test -n "$(git config submodule.example2.frotz)" &&
> +       test -f example2/.git &&
> +       rmdir init
> +'
> +
> +test_expect_success 'submodule deinit . deinits all initialized submodules' '
> +       git submodule update --init &&
> +       git config submodule.example.foo bar &&
> +       git config submodule.example2.frotz nitfol &&
> +       test_must_fail git submodule deinit &&
> +       git submodule deinit . &&
> +       test -z "$(git config submodule.example.url)" &&
> +       test -z "$(git config submodule.example.foo)" &&
> +       test -z "$(git config submodule.example2.url)" &&
> +       test -z "$(git config submodule.example2.frotz)" &&
> +       rmdir init example2
> +'
> +
> +test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
> +       git submodule update --init &&
> +       rm -rf init example2/* example2/.git &&
> +       git submodule deinit init example2 &&
> +       test -z "$(git config submodule.example.url)" &&
> +       test -z "$(git config submodule.example2.url)" &&
> +       rmdir init
> +'
> +
> +test_expect_success 'submodule deinit fails when the submodule contains modifications unless forced' '
> +       git submodule update --init &&
> +       echo X >>init/s &&
> +       test_must_fail git submodule deinit init &&
> +       test -n "$(git config submodule.example.url)" &&
> +       test -f example2/.git &&
> +       git submodule deinit -f init &&
> +       test -z "$(git config submodule.example.url)" &&
> +       rmdir init
> +'
> +
> +test_expect_success 'submodule deinit fails when the submodule contains untracked files unless forced' '
> +       git submodule update --init &&
> +       echo X >>init/untracked &&
> +       test_must_fail git submodule deinit init &&
> +       test -n "$(git config submodule.example.url)" &&
> +       test -f example2/.git &&
> +       git submodule deinit -f init &&
> +       test -z "$(git config submodule.example.url)" &&
> +       rmdir init
> +'
> +
> +test_expect_success 'submodule deinit fails when the submodule HEAD does not match unless forced' '
> +       git submodule update --init &&
> +       (
> +               cd init &&
> +               git checkout HEAD^
> +       ) &&
> +       test_must_fail git submodule deinit init &&
> +       test -n "$(git config submodule.example.url)" &&
> +       test -f example2/.git &&
> +       git submodule deinit -f init &&
> +       test -z "$(git config submodule.example.url)" &&
> +       rmdir init
> +'
> +
> +test_expect_success 'submodule deinit complains but does not fail when used on an uninitialized submodule' '
> +       git submodule update --init &&
> +       git submodule deinit init >actual &&
> +       test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual
> +       git submodule deinit init >actual &&
> +       test_i18ngrep "No url found for submodule path .init. in .git/config" actual &&
> +       git submodule deinit . >actual &&
> +       test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual
> +       rmdir init example2
> +'
> +
> +test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
> +       git submodule update --init &&
> +       (
> +               cd init &&
> +               rm .git &&
> +               cp -R ../.git/modules/example .git &&
> +               GIT_WORK_TREE=. git config --unset core.worktree
> +       ) &&
> +       test_must_fail git submodule deinit init &&
> +       test_must_fail git submodule deinit -f init &&
> +       test -d init/.git &&
> +       test -n "$(git config submodule.example.url)"
> +'
> +
>  test_done
> --
> 1.8.1.2.546.gea155c0
>
>

Re: [PATCH v4] submodule: add 'deinit' command

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

>> +       if test $# = 0
>> +       then
>> +               die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
>> +       fi
>> +
>> +       module_list "$@" |
>> +       while read mode sha1 stage sm_path
>> +       do
>> +               die_if_unmatched "$mode"
>> +               name=$(module_name "$sm_path") || exit
>> +               url=$(git config submodule."$name".url)
>> +               if test -z "$url"
>> +               then
>> +                       say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
>
> Is it safe to shelter the user a little bit more from the git
> internals here and say instead:
>
>    Submodule '\$sm_path' is not initialized.

Sounds like a sensible suggestion.

> Also, I think this code will show this message for each submodule on
> 'git submodule deinit .'  But I think I would prefer to suppress it in
> that case.  If I have not explicitly stated which submodules to
> deinit,...

But isn't it the way to explicitly say "everything under the sun"?
After all, what does the message say to "git submodule deinit"?

Re: [PATCH v4] submodule: add 'deinit' command

[Jens Lehmann]

Am 12.02.2013 18:11, schrieb Phil Hord:
> On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> +               die_if_unmatched "$mode"
>> +               name=$(module_name "$sm_path") || exit
>> +               url=$(git config submodule."$name".url)
>> +               if test -z "$url"
>> +               then
>> +                       say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
> 
> Is it safe to shelter the user a little bit more from the git
> internals here and say instead:
> 
>    Submodule '\$sm_path' is not initialized.

Yeah, that makes much more sense. But I'd rather use the name too:

   Submodule '\$name' is not initialized for path '\$sm_path'

> Also, I think this code will show this message for each submodule on
> 'git submodule deinit .'  But I think I would prefer to suppress it in
> that case.  If I have not explicitly stated which submodules to
> deinit, then I do not think git should complain that some of them are
> not initialized.

Yes, that message gets printed for each uninitialized submodule. We
could easily suppress that for '.', but it would be really hard to
get that right for other wildcards like 'foo*'. (And e.g. running a
"submodule update" also lists all submodules it skips because of an
"update=none" setting, so I'm not sure if it's that important here)

But if people really want that, I'd suppress that for the '.' case.
Further opinions?

>> +                       continue
>> +               fi
>> +
>> +               # Remove the submodule work tree (unless the user already did it)
>> +               if test -d "$sm_path"
>> +               then
>> +                       # 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)")"
> 
> I expect this is the right thing to do for now.  But I wonder if we
> can also move $sm_path/.git to $GIT_DIR/modules/$sm_path in this case
> (though I think I am not spelling this path correctly).  Would that be
> ok?  What extra work is needed to relocate the .git dir like this?

Hmm, I'm a bit torn about automagically moving the repo somewhere
else. While I think it is a sane solution for most users I suspect
some users might hate us for doing that without asking (and with no
option to turn that off). What about adding a separate "git submodule
to-gitfile" command which does that and hinting that here? However I
do have the feeling that this should be done in another commit.

>> +                               die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"
> 
> Nit, grammar: use a semicolon instead of a comma.

Ok. And while looking at it I noticed that "$sm_path" should be
"'\$sm_path'" here, same a few lines above ... sigh

>> +test_expect_success 'set up a second submodule' '
>> +       git submodule add ./init2 example2 &&
>> +       git commit -m "submodle example2 added"
> 
> Nit: submodule is misspelled

Thanks.

Junio, this looks like a we have v5 as soon as we decide what to do
with the "not initialized" messages when '.' is used, right?

My current diff to v4 looks like this:

-------------8<-------------
diff --git a/git-submodule.sh b/git-submodule.sh
index f1b552f..27f8e12 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -591,7 +591,7 @@ cmd_deinit()
 		url=$(git config submodule."$name".url)
 		if test -z "$url"
 		then
-			say "$(eval_gettext "No url found for submodule path '\$sm_path' in .git/config")"
+			say "$(eval_gettext "Submodule '\$name' is not initialized for path '\$sm_path'")"
 			continue
 		fi

@@ -601,14 +601,14 @@ cmd_deinit()
 			# 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")"
+				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 -n "$sm_path" ||
-				die "$(eval_gettext "Submodule work tree $sm_path contains local modifications, use '-f' to discard them")"
+				die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"
 			fi
 			rm -rf "$sm_path" || say "$(eval_gettext "Could not remove submodule work tree '\$sm_path'")"
 		fi
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index f54a40d..e4b0a59 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -759,7 +759,7 @@ test_expect_success 'submodule add with an existing name fails unless forced' '

 test_expect_success 'set up a second submodule' '
 	git submodule add ./init2 example2 &&
-	git commit -m "submodle example2 added"
+	git commit -m "submodule example2 added"
 '

 test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
@@ -837,7 +837,7 @@ test_expect_success 'submodule deinit complains but does not fail when used on a
 	git submodule deinit init >actual &&
 	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual
 	git submodule deinit init >actual &&
-	test_i18ngrep "No url found for submodule path .init. in .git/config" actual &&
+	test_i18ngrep "Submodule .example. is not initialized for path .init" actual &&
 	git submodule deinit . >actual &&
 	test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual
 	rmdir init example2

Re: [PATCH v4] submodule: add 'deinit' command

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Junio, this looks like a we have v5 as soon as we decide what to do
> with the "not initialized" messages when '.' is used, right?

OK.  I myself do not deeply care if we end up special casing "." or
not; I'll leave it up to you and other submodule folks.

Thanks.

[PATCH v5] submodule: add 'deinit' command

[Jens Lehmann]

With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config together with the work tree
himself).

Help those users by providing a 'deinit' command. This removes the whole
submodule.<name> section from .git/config either for the given
submodule(s) or for all those which have been initialized if '.' is
given. Fail if the current work tree contains modifications unless
forced. Complain when for a submodule given on the command line the url
setting can't be found in .git/config, but nonetheless don't fail.

Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him. Also add the deinit subcommand
to the completion list.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 13.02.2013 20:56, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Junio, this looks like a we have v5 as soon as we decide what to do
>> with the "not initialized" messages when '.' is used, right?
> 
> OK.  I myself do not deeply care if we end up special casing "." or
> not; I'll leave it up to you and other submodule folks.

Here we go, changes to v4 are:

- I decided to do the proposed special casing for "."; no messages
  about uninitialized submodules will show up anymore (this is also
  tested)
- The spelling fixes and better 'uninitialized' message Phil proposed
- Added the missing quotation for $sm_path in output strings
- "deinit" is added to the submodule completion list
- Added two missing "&&" in t7400



 Documentation/git-rm.txt               |   4 ++
 Documentation/git-submodule.txt        |  18 +++++-
 contrib/completion/git-completion.bash |   2 +-
 git-submodule.sh                       |  79 +++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh             | 101 +++++++++++++++++++++++++++++++++
 5 files changed, 201 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 92bac27..1d876c2 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing the removal,
+use linkgit:git-submodule[1] `deinit` instead.
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a0c9df8..bc06159 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase]
 	      [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
@@ -134,6 +135,19 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.

+deinit::
+	Unregister the given submodules, i.e. remove the whole
+	`submodule.$name` section from .git/config together with their work
+	tree. Further calls to `git submodule update`, `git submodule foreach`
+	and `git submodule sync` will skip any unregistered submodules until
+	they are initialized again, so use this command if you don't want to
+	have a local checkout of the submodule in your work tree anymore. If
+	you really want to remove a submodule from the repository and commit
+	that use linkgit:git-rm[1] instead.
++
+If `--force` is specified, the submodule's work tree will be removed even if
+it contains local modifications.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.
@@ -213,8 +227,10 @@ OPTIONS

 -f::
 --force::
-	This option is only valid for add and update commands.
+	This option is only valid for add, deinit and update commands.
 	When running add, allow adding an otherwise ignored submodule path.
+	When running deinit the submodule work trees will be removed even if
+	they contain local changes.
 	When running update, throw away local changes in submodules when
 	switching to a different commit; and always run a checkout operation
 	in the submodule, even if the commit listed in the index of the
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 059ba9d..7cee9bd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2401,7 +2401,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return

-	local subcommands="add status init update summary foreach sync"
+	local subcommands="add status init deinit update summary foreach sync"
 	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
 		case "$cur" in
 		--*)
diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..0fb6ee0 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
@@ -547,6 +548,82 @@ cmd_init()
 }

 #
+# Unregister submodules from .git/config and remove their work tree
+#
+# $@ = requested paths (use '.' to deinit all submodules)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... init".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-f|--force)
+			force=$1
+			;;
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# = 0
+	then
+		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
+	fi
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		die_if_unmatched "$mode"
+		name=$(module_name "$sm_path") || exit
+		url=$(git config submodule."$name".url)
+		if test -z "$url"
+		then
+			test $# -ne 1 || test "$@" = "." ||
+			say "$(eval_gettext "Submodule '\$name' is not initialized for path '\$sm_path'")"
+			continue
+		fi
+
+		# Remove the submodule work tree (unless the user already did it)
+		if test -d "$sm_path"
+		then
+			# 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 -n "$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 "Could not remove submodule work tree '\$sm_path'")"
+		fi
+
+		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+
+		# Remove the whole section so we have a clean state when the
+		# user later decides to init this submodule again
+		git config --remove-section submodule."$name" &&
+		say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$sm_path'")"
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)
@@ -1157,7 +1234,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2683cba..e32b62b 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,4 +757,105 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '

+test_expect_success 'set up a second submodule' '
+	git submodule add ./init2 example2 &&
+	git commit -m "submodule example2 added"
+'
+
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	git submodule deinit init &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)" &&
+	test -n "$(git config submodule.example2.url)" &&
+	test -n "$(git config submodule.example2.frotz)" &&
+	test -f example2/.git &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit . deinits all initialized submodules' '
+	git submodule update --init &&
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	test_must_fail git submodule deinit &&
+	git submodule deinit . &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example.foo)" &&
+	test -z "$(git config submodule.example2.url)" &&
+	test -z "$(git config submodule.example2.frotz)" &&
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
+	git submodule update --init &&
+	rm -rf init example2/* example2/.git &&
+	git submodule deinit init example2 &&
+	test -z "$(git config submodule.example.url)" &&
+	test -z "$(git config submodule.example2.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains modifications unless forced' '
+	git submodule update --init &&
+	echo X >>init/s &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains untracked files unless forced' '
+	git submodule update --init &&
+	echo X >>init/untracked &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule HEAD does not match unless forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		git checkout HEAD^
+	) &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config submodule.example.url)" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config submodule.example.url)" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit complains but does not fail when used on an uninitialized submodule' '
+	git submodule update --init &&
+	git submodule deinit init >actual &&
+	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual &&
+	git submodule deinit init >actual &&
+	test_i18ngrep "Submodule .example. is not initialized for path .init" actual &&
+	git submodule deinit . >actual &&
+	test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual &&
+	test_i18ngrep ! "Submodule .example. is not initialized for path .init" actual &&
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		rm .git &&
+		cp -R ../.git/modules/example .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git submodule deinit init &&
+	test_must_fail git submodule deinit -f init &&
+	test -d init/.git &&
+	test -n "$(git config submodule.example.url)"
+'
+
 test_done
-- 
1.8.1.2.677.ga1bd48d

Re: [PATCH v5] submodule: add 'deinit' command

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Here we go, changes to v4 are:
>
> - I decided to do the proposed special casing for "."; no messages
>   about uninitialized submodules will show up anymore (this is also
>   tested)
> - The spelling fixes and better 'uninitialized' message Phil proposed
> - Added the missing quotation for $sm_path in output strings
> - "deinit" is added to the submodule completion list
> - Added two missing "&&" in t7400

Thanks for being thorough. I honestly did not expect this topic to
take this many cycles before becoming 'next-ready'.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 004c034..0fb6ee0 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -547,6 +548,82 @@ cmd_init()
>  }
>
>  #
> +# Unregister submodules from .git/config and remove their work tree
> +#
> +# $@ = requested paths (use '.' to deinit all submodules)
> +#
> +cmd_deinit()
> +{
> +	# parse $args after "submodule ... init".
> +	while test $# -ne 0
> +	do
> ..
> +	done
> +
> +	if test $# = 0
> +	then
> +		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"

I do not think I saw anybody mentioned this so far, but how is
"deinit" supposed to work inside a subdirectory of a superproject?
If the answer is to work on submodules appear in that subdirectory,
'.' should probably not mean "all in the superproject" I think?

> +	module_list "$@" |
> +	while read mode sha1 stage sm_path
> +	do
> +		die_if_unmatched "$mode"
> +		name=$(module_name "$sm_path") || exit
> +		url=$(git config submodule."$name".url)
> +		if test -z "$url"
> +		then
> +			test $# -ne 1 || test "$@" = "." ||
> +			say "$(eval_gettext "Submodule '\$name' is not initialized for path '\$sm_path'")"
> +			continue
> +		fi

This 'test "$@" = "."' makes readers feel uneasy.  This particular
invocation happens to be safe only because it is protected with
"test $# -ne 1 ||", but for all other values of $# this will result
in a syntax error.  'test "$1" = "."' would make the intention of
the check more clear.

But stepping back a bit, is the condition this test is trying to
warn against really worth warning?

It seems that this "warn if user told us to deinitialize a submodule
that hasn't been initialized" is from the very initial round of this
series, and not something other people asked for during the review.
If somebody did

	git submodule deinit foo bar

and then later said:

	git submodule deinit foo

would it a mistake that the user wants to be warned about?

Perhaps the user did not mean to deinitialize foo (e.g. wanted to
*initialize* foo instead, or wanted to deinitialize *foz* instead)
and that is worth warning about?  I am not sure, but I have a
feeling that we can do without this check.

Also the value of submodule.$name.url is not used in the later
codepath to ensure that the named submodule is in the pristine state
in the superproject's working tree (i.e. no submodule.$name section
in the local configuration, no working tree for that submodule), so
it may be even a good change to remove the "does submodule.$name.url
exist" check and always do the "deinitialize" process.  That would
give the users a way to recover from a state where a submodule is
only half initialized for some reason safely, no?

Re: [PATCH v5] submodule: add 'deinit' command

[Jens Lehmann]

Am 17.02.2013 23:32, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 004c034..0fb6ee0 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -547,6 +548,82 @@ cmd_init()
>>  }
>>
>>  #
>> +# Unregister submodules from .git/config and remove their work tree
>> +#
>> +# $@ = requested paths (use '.' to deinit all submodules)
>> +#
>> +cmd_deinit()
>> +{
>> +	# parse $args after "submodule ... init".
>> +	while test $# -ne 0
>> +	do
>> ..
>> +	done
>> +
>> +	if test $# = 0
>> +	then
>> +		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
> 
> I do not think I saw anybody mentioned this so far, but how is
> "deinit" supposed to work inside a subdirectory of a superproject?
> If the answer is to work on submodules appear in that subdirectory,
> '.' should probably not mean "all in the superproject" I think?

"git submodule" fails when not run in the top level directory.

>> +	module_list "$@" |
>> +	while read mode sha1 stage sm_path
>> +	do
>> +		die_if_unmatched "$mode"
>> +		name=$(module_name "$sm_path") || exit
>> +		url=$(git config submodule."$name".url)
>> +		if test -z "$url"
>> +		then
>> +			test $# -ne 1 || test "$@" = "." ||
>> +			say "$(eval_gettext "Submodule '\$name' is not initialized for path '\$sm_path'")"
>> +			continue
>> +		fi
> 
> This 'test "$@" = "."' makes readers feel uneasy.  This particular
> invocation happens to be safe only because it is protected with
> "test $# -ne 1 ||", but for all other values of $# this will result
> in a syntax error.  'test "$1" = "."' would make the intention of
> the check more clear.

I used $@ here because it makes the code more robust against
someone accidentally removing the "test $# -ne 1 ||" (as it then
will fail when $# > 1 in one of the tests). But looking at it
again I agree that "$1" might better show the intention here and
the "$@" can easily make people suspicious.

> But stepping back a bit, is the condition this test is trying to
> warn against really worth warning?
> 
> It seems that this "warn if user told us to deinitialize a submodule
> that hasn't been initialized" is from the very initial round of this
> series, and not something other people asked for during the review.
> If somebody did
> 
> 	git submodule deinit foo bar
> 
> and then later said:
> 
> 	git submodule deinit foo
> 
> would it a mistake that the user wants to be warned about?
> 
> Perhaps the user did not mean to deinitialize foo (e.g. wanted to
> *initialize* foo instead, or wanted to deinitialize *foz* instead)
> and that is worth warning about?  I am not sure, but I have a
> feeling that we can do without this check.

Hmm, if you would replace "submodule deinit" with "rm" in the above
example, the "rm" would not only warn but error out the second time.
But on the other hand doing a "git submodule init" again on already
initialized submodules will silently do nothing, which seems to be
the better analogy here. So yes, it looks like we can do without.

Ok, unless someone speaks up and argues in favor of this message
I'll remove it in v6.

> Also the value of submodule.$name.url is not used in the later
> codepath to ensure that the named submodule is in the pristine state
> in the superproject's working tree (i.e. no submodule.$name section
> in the local configuration, no working tree for that submodule), so
> it may be even a good change to remove the "does submodule.$name.url
> exist" check and always do the "deinitialize" process.  That would
> give the users a way to recover from a state where a submodule is
> only half initialized for some reason safely, no?

That is a very good point. It makes sense that if we don't nuke the
whole test to at least remove the "continue" there (in case someone
fiddled with .git/config but wants to get rid of the work tree in a
safe manner later).

[PATCH v6] submodule: add 'deinit' command

[Jens Lehmann]

With "git submodule init" the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to "git
submodule update". But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
"submodule.$name.url" setting from .git/config together with the work tree
himself).

Help those users by providing a 'deinit' command. This removes the
whole submodule.<name> section from .git/config (either for the given
submodule(s) or for all those which have been initialized if '.' is used)
together with their work tree. Fail if the current work tree contains
modifications (unless forced), but don't complain when either the work
tree is already removed or no settings are found in .git/config.

Add tests and link the man pages of "git submodule deinit" and "git rm"
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him. Also add the deinit subcommand
to the completion list.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 18.02.2013 20:20, schrieb Jens Lehmann:
> Am 17.02.2013 23:32, schrieb Junio C Hamano:
>> It seems that this "warn if user told us to deinitialize a submodule
>> that hasn't been initialized" is from the very initial round of this
>> series, and not something other people asked for during the review.
>> If somebody did
>>
>> 	git submodule deinit foo bar
>>
>> and then later said:
>>
>> 	git submodule deinit foo
>>
>> would it a mistake that the user wants to be warned about?
>>
>> Perhaps the user did not mean to deinitialize foo (e.g. wanted to
>> *initialize* foo instead, or wanted to deinitialize *foz* instead)
>> and that is worth warning about?  I am not sure, but I have a
>> feeling that we can do without this check.
> 
> Hmm, if you would replace "submodule deinit" with "rm" in the above
> example, the "rm" would not only warn but error out the second time.
> But on the other hand doing a "git submodule init" again on already
> initialized submodules will silently do nothing, which seems to be
> the better analogy here. So yes, it looks like we can do without.
> 
> Ok, unless someone speaks up and argues in favor of this message
> I'll remove it in v6.

Nobody spoke up, so here we go.

Changes since v5:

- No warning message when the url setting is not found
- Reword second commit message paragraph accordingly
- Use 'git config --get-regexp "submodule\.example\."' in tests
- Fix a copied comment still talking about "init"

Hopefully this is the final version ;-)


 Documentation/git-rm.txt               |   4 ++
 Documentation/git-submodule.txt        |  18 +++++-
 contrib/completion/git-completion.bash |   2 +-
 git-submodule.sh                       |  77 ++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh             | 100 +++++++++++++++++++++++++++++++++
 5 files changed, 198 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 92bac27..1d876c2 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing the removal,
+use linkgit:git-submodule[1] `deinit` instead.
+
 EXAMPLES
 --------
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index c99d795..74d5bdc 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
+'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
 	      [-f|--force] [--rebase] [--reference <repository>]
 	      [--merge] [--recursive] [--] [<path>...]
@@ -135,6 +136,19 @@ init::
 	the explicit 'init' step if you do not intend to customize
 	any submodule locations.

+deinit::
+	Unregister the given submodules, i.e. remove the whole
+	`submodule.$name` section from .git/config together with their work
+	tree. Further calls to `git submodule update`, `git submodule foreach`
+	and `git submodule sync` will skip any unregistered submodules until
+	they are initialized again, so use this command if you don't want to
+	have a local checkout of the submodule in your work tree anymore. If
+	you really want to remove a submodule from the repository and commit
+	that use linkgit:git-rm[1] instead.
++
+If `--force` is specified, the submodule's work tree will be removed even if
+it contains local modifications.
+
 update::
 	Update the registered submodules, i.e. clone missing submodules and
 	checkout the commit specified in the index of the containing repository.
@@ -214,8 +228,10 @@ OPTIONS

 -f::
 --force::
-	This option is only valid for add and update commands.
+	This option is only valid for add, deinit and update commands.
 	When running add, allow adding an otherwise ignored submodule path.
+	When running deinit the submodule work trees will be removed even if
+	they contain local changes.
 	When running update, throw away local changes in submodules when
 	switching to a different commit; and always run a checkout operation
 	in the submodule, even if the commit listed in the index of the
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b62bec0..6ab5752 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2413,7 +2413,7 @@ _git_submodule ()
 {
 	__git_has_doubledash && return

-	local subcommands="add status init update summary foreach sync"
+	local subcommands="add status init deinit update summary foreach sync"
 	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
 		case "$cur" in
 		--*)
diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..44f70c4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
+   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
@@ -547,6 +548,80 @@ cmd_init()
 }

 #
+# Unregister submodules from .git/config and remove their work tree
+#
+# $@ = requested paths (use '.' to deinit all submodules)
+#
+cmd_deinit()
+{
+	# parse $args after "submodule ... deinit".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-f|--force)
+			force=$1
+			;;
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test $# = 0
+	then
+		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
+	fi
+
+	module_list "$@" |
+	while read mode sha1 stage sm_path
+	do
+		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"
+		then
+			# 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 -n "$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 "Could not remove submodule work tree '\$sm_path'")"
+		fi
+
+		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$sm_path'")"
+
+		# Remove the .git/config entries (unless the user already did it)
+		if test -n "$(git config --get-regexp submodule."$name\.")"
+		then
+			# Remove the whole section so we have a clean state when
+			# the user later decides to init this submodule again
+			url=$(git config submodule."$name".url)
+			git config --remove-section submodule."$name" 2>/dev/null &&
+			say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$sm_path'")"
+		fi
+	done
+}
+
+#
 # Update each submodule path to correct revision, using clone and checkout as needed
 #
 # $@ = requested paths (default to all)
@@ -1157,7 +1232,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2683cba..5030f1f 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,4 +757,104 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
 	)
 '

+test_expect_success 'set up a second submodule' '
+	git submodule add ./init2 example2 &&
+	git commit -m "submodule example2 added"
+'
+
+test_expect_success 'submodule deinit should remove the whole submodule section from .git/config' '
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	git submodule deinit init &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test -n "$(git config --get-regexp "submodule\.example2\.")" &&
+	test -f example2/.git &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit . deinits all initialized submodules' '
+	git submodule update --init &&
+	git config submodule.example.foo bar &&
+	git config submodule.example2.frotz nitfol &&
+	test_must_fail git submodule deinit &&
+	git submodule deinit . &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
+	git submodule update --init &&
+	rm -rf init example2/* example2/.git &&
+	git submodule deinit init example2 &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains modifications unless forced' '
+	git submodule update --init &&
+	echo X >>init/s &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config --get-regexp "submodule\.example\.")" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule contains untracked files unless forced' '
+	git submodule update --init &&
+	echo X >>init/untracked &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config --get-regexp "submodule\.example\.")" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit fails when the submodule HEAD does not match unless forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		git checkout HEAD^
+	) &&
+	test_must_fail git submodule deinit init &&
+	test -n "$(git config --get-regexp "submodule\.example\.")" &&
+	test -f example2/.git &&
+	git submodule deinit -f init &&
+	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	rmdir init
+'
+
+test_expect_success 'submodule deinit is silent when used on an uninitialized submodule' '
+	git submodule update --init &&
+	git submodule deinit init >actual &&
+	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual &&
+	git submodule deinit init >actual &&
+	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
+	git submodule deinit . >actual &&
+	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
+	test_i18ngrep "Submodule .example2. (.*) unregistered for path .example2" actual &&
+	git submodule deinit . >actual &&
+	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
+	test_i18ngrep ! "Submodule .example2. (.*) unregistered for path .example2" actual &&
+	rmdir init example2
+'
+
+test_expect_success 'submodule deinit fails when submodule has a .git directory even when forced' '
+	git submodule update --init &&
+	(
+		cd init &&
+		rm .git &&
+		cp -R ../.git/modules/example .git &&
+		GIT_WORK_TREE=. git config --unset core.worktree
+	) &&
+	test_must_fail git submodule deinit init &&
+	test_must_fail git submodule deinit -f init &&
+	test -d init/.git &&
+	test -n "$(git config --get-regexp "submodule\.example\.")"
+'
+
 test_done
-- 
1.8.2.rc2.5.geb8e692

Re: [PATCH v6] submodule: add 'deinit' command

[Heiko Voigt]

Hi,

On Mon, Mar 04, 2013 at 10:20:24PM +0100, Jens Lehmann wrote:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 004c034..44f70c4 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -547,6 +548,80 @@ cmd_init()
>  }
> 
[...]
> +
> +	module_list "$@" |
> +	while read mode sha1 stage sm_path
> +	do
> +		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"
> +		then
> +			# 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 -n "$sm_path" ||
> +				die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"

Minor nit. IMO, there is an indentation for the || missing here. Maybe
Junio can squash that in on his side?

> +			fi
> +			rm -rf "$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'")"

[...]

Everything else looks good to me.

Cheers Heiko

Re: [PATCH v6] submodule: add 'deinit' command

[Junio C Hamano]

Heiko Voigt <hvoigt@hvoigt.net> writes:

>> +			if test -z "$force"
>> +			then
>> +				git rm -n "$sm_path" ||
>> +				die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"
>
> Minor nit. IMO, there is an indentation for the || missing here. Maybe
> Junio can squash that in on his side?

Sorry, but I do not see an indentation nit here.  The format looks
perfectly sane to me and in fact any other indentation would be
wrong.

Puzzled...

Re: [PATCH v6] submodule: add 'deinit' command

[Heiko Voigt]

On Tue, Mar 05, 2013 at 07:45:22AM -0800, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> >> +			if test -z "$force"
> >> +			then
> >> +				git rm -n "$sm_path" ||
> >> +				die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"
> >
> > Minor nit. IMO, there is an indentation for the || missing here. Maybe
> > Junio can squash that in on his side?
> 
> Sorry, but I do not see an indentation nit here.  The format looks
> perfectly sane to me and in fact any other indentation would be
> wrong.
> 
> Puzzled...

Wouldn't you write this code snippet like this to make clear that there
is another conditional?

	if test -z "$force"
	then
		git rm -n "$sm_path" ||
			die "$(eval_gettext "Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them")"

It seems it is not clear in the code base either. Some places do
some do not indent:

  git grep -A 1 '||' *.sh

Except for the testsuite I just assumed that this style was common,
because for me it makes the code much easier to read.

It seems it is not, so forget my comment.

Cheers Heiko

Re: [PATCH v4] submodule: add 'deinit' command

[Phil Hord]

On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> With "git submodule init" the user is able to tell git he cares about one
> or more submodules and wants to have it populated on the next call to "git
> submodule update". But currently there is no easy way he could tell git he
> does not care about a submodule anymore and wants to get rid of his local
> work tree (except he knows a lot about submodule internals and removes the
> "submodule.$name.url" setting from .git/config together with the work tree
> himself).
>
> Help those users by providing a 'deinit' command. This removes the whole
> submodule.<name> section from .git/config either for the given
> submodule(s) or for all those which have been initialized if '.' is
> given. Fail if the current work tree contains modifications unless
> forced. Complain when for a submodule given on the command line the url
> setting can't be found in .git/config, but nonetheless don't fail.
>
> Add tests and link the man pages of "git submodule deinit" and "git rm"
> to assist the user in deciding whether removing or unregistering the
> submodule is the right thing to do for him.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>


Probably because I was new to this command, I was confused by this output.

  $ git submodule deinit submodule
  rm 'submodule'
  Submodule 'submodule' (gerrit:foo/submodule) unregistered for path 'submodule'

  $ git rm submodule
  rm 'submodule'


This line is confusing to me in the deinit command:

  rm 'submodule'

It doesn't mean what git usually means when it says this to me.  See
how the 'git rm' command says the same thing but means something
different in the next command.

In the deinit case, git removes the workdir contents of 'submodule'
and it reports "rm 'submodule'".  In the rm case, git removes the
submodule link from the tree and rmdirs the empty 'submodule'
directory.

I think this would be clearer if 'git deinit' said

    rm 'submodule/*'

or maybe

    Removed workdir for 'submodule'

Is it just me?

Phil

Re: [PATCH v4] submodule: add 'deinit' command

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

> I think this would be clearer if 'git deinit' said
>
>     rm 'submodule/*'
>
> or maybe
>
>     Removed workdir for 'submodule'
>
> Is it just me?

The latter may probably be better.  

After cloning the superproject, you show interest in individual
submodules by saying "git submodule init <that submodule>" and until
then your .git/config in the superproject does not indicate that you
are interested in that submodule, and you won't have a submodule
checkout.

"deinit" is a way to revert back to that original state; recording
that you are no longer interested in the submodule is the primary
effect, and removal of its checkout is a mere side effect of it.

Re: [PATCH v4] submodule: add 'deinit' command

[Jens Lehmann]

Am 12.03.2013 17:22, schrieb Junio C Hamano:
> Phil Hord <phil.hord@gmail.com> writes:
> 
>> I think this would be clearer if 'git deinit' said
>>
>>     rm 'submodule/*'
>>
>> or maybe
>>
>>     Removed workdir for 'submodule'
>>
>> Is it just me?
> 
> The latter may probably be better.  

Hmm, it doesn't really remove the directory but only empties it
(it recreates it a few lines after removing it together with its
contents). So what about

    Cleared directory 'submodule'

The attached interdiff suppresses the "rm 'submodule'" message
and issues the "Cleared ..." message after it successfully removed
the work tree. (But please note that it also prints this message
even if the submodule work tree is already empty, e.g. when you
deinit a submodule the second time)

----------------8<-------------------------
diff --git a/git-submodule.sh b/git-submodule.sh
index 204bc78..d003e8a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -601,10 +601,12 @@ cmd_deinit()

 			if test -z "$force"
 			then
-				git rm -n "$sm_path" ||
+				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 "Could not remove submodule work tree '\$sm_path'")"
+			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'")"

Re: [PATCH v4] submodule: add 'deinit' command

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 12.03.2013 17:22, schrieb Junio C Hamano:
>> Phil Hord <phil.hord@gmail.com> writes:
>> 
>>> I think this would be clearer if 'git deinit' said
>>>
>>>     rm 'submodule/*'
>>>
>>> or maybe
>>>
>>>     Removed workdir for 'submodule'
>>>
>>> Is it just me?
>> 
>> The latter may probably be better.  
>
> Hmm, it doesn't really remove the directory but only empties it
> (it recreates it a few lines after removing it together with its
> contents). So what about
>
>     Cleared directory 'submodule'

Sounds the cleanest among the suggested phrasing so far.

[PATCH] submodule deinit: clarify work tree removal message

[Jens Lehmann]

The output of "git submodule deinit sub" of a populated submodule prints

  rm 'sub'

as the first line unless used with the -f option.

The "rm 'sub'" line is exactly the same output the user gets when using
"git rm sub" (because that command is used with the --dry-run option under
the hood to determine if the submodule is clean), which can easily lead to
the false impression that the submodule would be permanently removed. Also
users might be confused that the "rm 'submodule'" line won't show up when
the -f option is used, as the test is skipped in this case.

Silence the "rm 'submodule'" output by using the --quiet option for "git
rm" and always print

  Cleared directory 'submodule'

instead as the first output line. This line is printed as long as the
directory exists, no matter if empty or not.

Also extend the tests in t7400 to make sure the "Cleared directory" line
is printed correctly.

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


Am 19.03.2013 02:45, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> Am 12.03.2013 17:22, schrieb Junio C Hamano:
>>> Phil Hord <phil.hord@gmail.com> writes:
>>>
>>>> I think this would be clearer if 'git deinit' said
>>>>
>>>>     rm 'submodule/*'
>>>>
>>>> or maybe
>>>>
>>>>     Removed workdir for 'submodule'
>>>>
>>>> Is it just me?
>>>
>>> The latter may probably be better.  
>>
>> Hmm, it doesn't really remove the directory but only empties it
>> (it recreates it a few lines after removing it together with its
>> contents). So what about
>>
>>     Cleared directory 'submodule'
> 
> Sounds the cleanest among the suggested phrasing so far.

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. Technically that is correct, as the
"rm -rf" is also executed, but I think it would be better to
skip the whole if block containing the "rm -rf" and "mkdir"
commands in that case as there is really nothing to do. Calling
"find" to see if there is anything inside the directory sounds
too expensive to me, as the directory will contain a lot of
files sometimes. But maybe this should be done in another patch.


 git-submodule.sh           |  6 ++++--
 t/t7400-submodule-basic.sh | 21 ++++++++++++++++-----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 204bc78..d003e8a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -601,10 +601,12 @@ cmd_deinit()

 			if test -z "$force"
 			then
-				git rm -n "$sm_path" ||
+				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 "Could not remove submodule work tree '\$sm_path'")"
+			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'")"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5030f1f..ff26535 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -777,18 +777,22 @@ test_expect_success 'submodule deinit . deinits all initialized submodules' '
 	git config submodule.example.foo bar &&
 	git config submodule.example2.frotz nitfol &&
 	test_must_fail git submodule deinit &&
-	git submodule deinit . &&
+	git submodule deinit . >actual &&
 	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 &&
 	rmdir init example2
 '

 test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '
 	git submodule update --init &&
 	rm -rf init example2/* example2/.git &&
-	git submodule deinit init example2 &&
+	git submodule deinit init example2 >actual &&
 	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 &&
 	rmdir init
 '

@@ -798,8 +802,9 @@ test_expect_success 'submodule deinit fails when the submodule contains modifica
 	test_must_fail git submodule deinit init &&
 	test -n "$(git config --get-regexp "submodule\.example\.")" &&
 	test -f example2/.git &&
-	git submodule deinit -f init &&
+	git submodule deinit -f init >actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
 '

@@ -809,8 +814,9 @@ test_expect_success 'submodule deinit fails when the submodule contains untracke
 	test_must_fail git submodule deinit init &&
 	test -n "$(git config --get-regexp "submodule\.example\.")" &&
 	test -f example2/.git &&
-	git submodule deinit -f init &&
+	git submodule deinit -f init >actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
 '

@@ -823,8 +829,9 @@ test_expect_success 'submodule deinit fails when the submodule HEAD does not mat
 	test_must_fail git submodule deinit init &&
 	test -n "$(git config --get-regexp "submodule\.example\.")" &&
 	test -f example2/.git &&
-	git submodule deinit -f init &&
+	git submodule deinit -f init >actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
+	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
 '

@@ -832,14 +839,18 @@ test_expect_success 'submodule deinit is silent when used on an uninitialized su
 	git submodule update --init &&
 	git submodule deinit init >actual &&
 	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual &&
+	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 &&
 	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 &&
 	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 &&
 	rmdir init example2
 '

-- 
1.8.2.358.g5c06bcf

Re: [PATCH] submodule deinit: clarify work tree removal message

[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,
Phil

[PATCH] submodule deinit: don't output "Cleared directory" when directory is empty

[Jens Lehmann]

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

Re: [PATCH] submodule deinit: clarify work tree removal message

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

> 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)"
>    }

Hrm, -maxdepth and -empty are not even in POSIX.  Folks on GNU
platforms and BSDs (I checked NetBSD 6 and OpenBSD 5.2) should be
fine, but it makes other platforms unhappy.

What is this check used for?  To avoid running "rmdir" on non-empty
ones?  Saying "cleaning foo/" (or "cleaned foo/") when foo/ is
already empty is not a crime; not omitting an empty one may actually
be a better behaviour from the point of view of repeatability and
uniformity.

Re: [PATCH] submodule deinit: clarify work tree removal message

[Jens Lehmann]

Am 17.04.2013 07:16, schrieb Junio C Hamano:
> Phil Hord <phil.hord@gmail.com> writes:
> 
>> 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)"
>>    }
> 
> Hrm, -maxdepth and -empty are not even in POSIX.  Folks on GNU
> platforms and BSDs (I checked NetBSD 6 and OpenBSD 5.2) should be
> fine, but it makes other platforms unhappy.

Ok, that is not acceptable.

> What is this check used for?  To avoid running "rmdir" on non-empty
> ones?  Saying "cleaning foo/" (or "cleaned foo/") when foo/ is
> already empty is not a crime; not omitting an empty one may actually
> be a better behaviour from the point of view of repeatability and
> uniformity.

It's no big issue, but 'init' issues the "Submodule ... registered
for path ..." message only once and is quiet on subsequent calls,
'deinit' does the same with "Submodule ... unregistered for path
...", only the "Cleared directory ..." message appears each time
'deinit' is called, which makes it stand out. I do not believe
this little inconsistency is important enough to write a helper in
C (to have a portable way to see if the directory has already been
cleared), but this simple additional "if" looked easy enough. That
should have made me suspicious ;-)