[PATCH 00/10] submodule output patches

[PATCH 00/10] submodule output patches

[Stefan Beller]

Patch 1 was send outside of a series already.

Patch 2 and 3 are preparatory things for the submodule groups stuff

patches 4-9 are making the output of the submodule command consistent
(similar to patch 3, but I do not foresee a need for it yet)

Patch 10 is a controversial thing I'd assume as it breaks existing users.
We should take it for the next major release (i.e. 3.0)
I just want to put it out here now.

Thanks,
Stefan

Stefan Beller (10):
  submodule deinit test: fix broken && chain in subshell
  submodule deinit: lose requirement for giving '.'
  submodule init: redirect stdout to stderr
  shell helpers usage: always send help to stderr
  submodule add: send messages to stderr
  submodule deinit: send messages to stderr
  submodule foreach: send messages to stderr
  submodule update: send messages to stderr
  submodule sync: send messages to stderr
  submodule deinit: complain when given a file instead of a submodule

 builtin/submodule--helper.c  |  9 +++++----
 git-sh-setup.sh              |  2 +-
 git-submodule.sh             | 21 ++++++++-------------
 t/t7400-submodule-basic.sh   | 38 +++++++++++++++++++++++++-------------
 t/t7403-submodule-sync.sh    |  4 ++--
 t/t7406-submodule-update.sh  | 23 ++++++++++++++++-------
 t/t7407-submodule-foreach.sh | 35 ++++++++++++++++++++++-------------
 7 files changed, 79 insertions(+), 53 deletions(-)

-- 
2.8.0.32.g71f8beb.dirty

[PATCH 01/10] submodule deinit test: fix broken && chain in subshell

[Stefan Beller]

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t7400-submodule-basic.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 814ee63..90d80d3 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -914,7 +914,7 @@ test_expect_success 'submodule deinit works on repository without submodules' '
 		git init &&
 		>file &&
 		git add file &&
-		git commit -m "repo should not be empty"
+		git commit -m "repo should not be empty" &&
 		git submodule deinit .
 	)
 '
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 02/10] submodule deinit: lose requirement for giving '.'

[Stefan Beller]

The discussion in [1] realized that '.' is a faulty suggestion as
there is a corner case where it fails:

> "submodule deinit ." may have "worked" in the sense that you would
> have at least one path in your tree and avoided this "nothing
> matches" most of the time.  It would have still failed with the
> exactly same error if run in an empty repository, i.e.
>
>        $ E=/var/tmp/x/empty && rm -fr "$E" && mkdir -p "$E" && cd "$E"
>        $ git init
>        $ rungit v2.6.6 submodule deinit .
>        error: pathspec '.' did not match any file(s) known to git.
>        Did you forget to 'git add'?
>        $ >file && git add file
>        $ rungit v2.6.6 submodule deinit .
>        $ echo $?
>        0

There is no need to update the documentation as it did not describe the
special case '.' to remove all submodules.

[1] http://news.gmane.org/gmane.comp.version-control.git/289535

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh           | 5 -----
 t/t7400-submodule-basic.sh | 1 -
 2 files changed, 6 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 82e95a9..d689265 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -428,11 +428,6 @@ cmd_deinit()
 		shift
 	done
 
-	if test $# = 0
-	then
-		die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
-	fi
-
 	git submodule--helper list --prefix "$wt_prefix" "$@" |
 	while read mode sha1 stage sm_path
 	do
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 90d80d3..a6231f1 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -948,7 +948,6 @@ 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 . >actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 03/10] submodule init: redirect stdout to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

We want to init submodules from the helper for `submodule update`
in a later patch and the stdout output of said helper is consumed
by the parts of `submodule update` which are still written in shell.
So we have to be careful which messages are on stdout.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c |  3 ++-
 t/t7406-submodule-update.sh | 24 ++++++++++++++++++------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5d05393..7f0941d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -366,7 +366,8 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 			die(_("Failed to register url for submodule path '%s'"),
 			    displaypath);
 		if (!quiet)
-			printf(_("Submodule '%s' (%s) registered for path '%s'\n"),
+			fprintf(stderr,
+				_("Submodule '%s' (%s) registered for path '%s'\n"),
 				sub->name, url, displaypath);
 	}
 
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index fd741f5..5f27879 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -108,24 +108,36 @@ pwd=$(pwd)
 
 cat <<EOF >expect
 Submodule path '../super': checked out '$supersha1'
-Submodule 'merging' ($pwd/merging) registered for path '../super/merging'
-Submodule 'none' ($pwd/none) registered for path '../super/none'
-Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'
-Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'
 Submodule path '../super/merging': checked out '$mergingsha1'
 Submodule path '../super/none': checked out '$nonesha1'
 Submodule path '../super/rebasing': checked out '$rebasingsha1'
 Submodule path '../super/submodule': checked out '$submodulesha1'
 EOF
 
+cat <<EOF >expect2
+Submodule 'merging' ($pwd/merging) registered for path '../super/merging'
+Submodule 'none' ($pwd/none) registered for path '../super/none'
+Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'
+Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'
+Cloning into '$pwd/recursivesuper/super/merging'...
+done.
+Cloning into '$pwd/recursivesuper/super/none'...
+done.
+Cloning into '$pwd/recursivesuper/super/rebasing'...
+done.
+Cloning into '$pwd/recursivesuper/super/submodule'...
+done.
+EOF
+
 test_expect_success 'submodule update --init --recursive from subdirectory' '
 	git -C recursivesuper/super reset --hard HEAD^ &&
 	(cd recursivesuper &&
 	 mkdir tmp &&
 	 cd tmp &&
-	 git submodule update --init --recursive ../super >../../actual
+	 git submodule update --init --recursive ../super >../../actual 2>../../actual2
 	) &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	test_cmp expect2 actual2
 '
 
 apos="'";
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 04/10] shell helpers usage: always send help to stderr

[Stefan Beller]

`git submodule asdf` would trigger displaying the usage of the submodule
command on stderr, however `git submodule -h` would display the usage on
stdout. Unify displaying help for shell commands on stderr.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-sh-setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index c48139a..5c02446 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -65,7 +65,7 @@ say () {
 
 if test -n "$OPTIONS_SPEC"; then
 	usage() {
-		"$0" -h
+		"$0" -h 1>&2
 		exit 1
 	}
 
-- 
2.8.0.32.g71f8beb.dirty

Re: [PATCH 04/10] shell helpers usage: always send help to stderr

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> `git submodule asdf` would trigger displaying the usage of the submodule
> command on stderr, however `git submodule -h` would display the usage on
> stdout. Unify displaying help for shell commands on stderr.

The primary output from "git cmd --help" is the usage message.  It
is debatable why it should go to the standard error output when it
is the primary thing the user asked for.



>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  git-sh-setup.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index c48139a..5c02446 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -65,7 +65,7 @@ say () {
>  
>  if test -n "$OPTIONS_SPEC"; then
>  	usage() {
> -		"$0" -h
> +		"$0" -h 1>&2
>  		exit 1
>  	}

Re: [PATCH 04/10] shell helpers usage: always send help to stderr

[Stefan Beller]

On Mon, May 2, 2016 at 4:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> `git submodule asdf` would trigger displaying the usage of the submodule
>> command on stderr, however `git submodule -h` would display the usage on
>> stdout. Unify displaying help for shell commands on stderr.
>
> The primary output from "git cmd --help" is the usage message.  It
> is debatable why it should go to the standard error output when it
> is the primary thing the user asked for.

I had written some lengthy arguments, but when I wanted to back up
with data and facts, the first search result[1] convinced me this is
a bad patch as when a user asks for help specifically, they want it
to easily be piped, i.e.

    git --help |grep pull

instead of

    git --help 2>&1 |grep pull

So I'll drop this patch.
Thanks,
Stefan

[1] http://www.jstorimer.com/blogs/workingwithcode/7766119-when-to-use-stderr-instead-of-stdout

>
>
>
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  git-sh-setup.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
>> index c48139a..5c02446 100644
>> --- a/git-sh-setup.sh
>> +++ b/git-sh-setup.sh
>> @@ -65,7 +65,7 @@ say () {
>>
>>  if test -n "$OPTIONS_SPEC"; then
>>       usage() {
>> -             "$0" -h
>> +             "$0" -h 1>&2
>>               exit 1
>>       }

Re: [PATCH 04/10] shell helpers usage: always send help to stderr

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>     git --help |grep pull
>
> instead of
>
>     git --help 2>&1 |grep pull

Not just that.  It makes me sad that it is unpredictable which
stream a project happens to have chosen to send its help text and I
end up almost always doing

    random-command --help 2>&1 | less

Re: [PATCH 04/10] shell helpers usage: always send help to stderr

[Junio C Hamano]

It also is somewhat sad that you needed to refer to a random blog you
found on the Internet whose punch line was essentially what I already
said before you finally decide to listen to me X-<. I somehow expected
that over the years you worked with me you learned I had a reasonable
taste in designing these things...

On Mon, May 2, 2016 at 5:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>     git --help |grep pull
>>
>> instead of
>>
>>     git --help 2>&1 |grep pull
>
> Not just that.  It makes me sad that it is unpredictable which
> stream a project happens to have chosen to send its help text and I
> end up almost always doing
>
>     random-command --help 2>&1 | less
>

[PATCH 05/10] submodule add: send messages to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d689265..f4d500e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -271,7 +271,7 @@ Use -f if you really want to add it." >&2
 				echo >&2 "$(eval_gettext "use the '--force' option. If the local git directory is not the correct repo")"
 				die "$(eval_gettext "or you are unsure what this means choose another name with the '--name' option.")"
 			else
-				echo "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
+				echo >&2 "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
 			fi
 		fi
 		git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" "$reference" "$depth" || exit
-- 
2.8.0.32.g71f8beb.dirty

Re: [PATCH 05/10] submodule add: send messages to stderr

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Reroute the output of stdout to stderr as it is just informative
> messages, not to be consumed by machines.
>
> This should not regress any scripts that try to parse the
> current output, as the output is already internationalized
> and therefore unstable.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Sounds sensible.

>  git-submodule.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index d689265..f4d500e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -271,7 +271,7 @@ Use -f if you really want to add it." >&2
>  				echo >&2 "$(eval_gettext "use the '--force' option. If the local git directory is not the correct repo")"
>  				die "$(eval_gettext "or you are unsure what this means choose another name with the '--name' option.")"
>  			else
> -				echo "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
> +				echo >&2 "$(eval_gettext "Reactivating local git directory for submodule '\$sm_name'.")"
>  			fi
>  		fi
>  		git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" "$reference" "$depth" || exit

[PATCH 06/10] submodule deinit: send messages to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh           |  8 ++++----
 t/t7400-submodule-basic.sh | 20 ++++++++++----------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index f4d500e..3f67f4e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -452,11 +452,11 @@ cmd_deinit()
 				die "$(eval_gettext "Submodule work tree '\$displaypath' contains local modifications; use '-f' to discard them")"
 			fi
 			rm -rf "$sm_path" &&
-			say "$(eval_gettext "Cleared directory '\$displaypath'")" ||
-			say "$(eval_gettext "Could not remove submodule work tree '\$displaypath'")"
+			say >&2 "$(eval_gettext "Cleared directory '\$displaypath'")" ||
+			say >&2 "$(eval_gettext "Could not remove submodule work tree '\$displaypath'")"
 		fi
 
-		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$displaypath'")"
+		mkdir "$sm_path" || say >&2 "$(eval_gettext "Could not create empty submodule directory '\$displaypath'")"
 
 		# Remove the .git/config entries (unless the user already did it)
 		if test -n "$(git config --get-regexp submodule."$name\.")"
@@ -465,7 +465,7 @@ cmd_deinit()
 			# 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 '\$displaypath'")"
+			say >&2 "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
 		fi
 	done
 }
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a6231f1..53644da 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -935,7 +935,7 @@ test_expect_success 'submodule deinit from subdirectory' '
 	mkdir -p sub &&
 	(
 		cd sub &&
-		git submodule deinit ../init >../output
+		git submodule deinit ../init 2>../output
 	) &&
 	grep "\\.\\./init" output &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
@@ -948,7 +948,7 @@ 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 &&
-	git submodule deinit . >actual &&
+	git submodule deinit . 2>actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
 	test_i18ngrep "Cleared directory .init" actual &&
@@ -959,7 +959,7 @@ test_expect_success 'submodule deinit . deinits all initialized submodules' '
 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 >actual &&
+	git submodule deinit init example2 2>actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
 	test_i18ngrep ! "Cleared directory .init" actual &&
@@ -973,7 +973,7 @@ 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 >actual &&
+	git submodule deinit -f init 2>actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
@@ -985,7 +985,7 @@ 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 >actual &&
+	git submodule deinit -f init 2>actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
@@ -1000,7 +1000,7 @@ 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 >actual &&
+	git submodule deinit -f init 2>actual &&
 	test -z "$(git config --get-regexp "submodule\.example\.")" &&
 	test_i18ngrep "Cleared directory .init" actual &&
 	rmdir init
@@ -1008,17 +1008,17 @@ test_expect_success 'submodule deinit fails when the submodule HEAD does not mat
 
 test_expect_success 'submodule deinit is silent when used on an uninitialized submodule' '
 	git submodule update --init &&
-	git submodule deinit init >actual &&
+	git submodule deinit init 2>actual &&
 	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual &&
 	test_i18ngrep "Cleared directory .init" actual &&
-	git submodule deinit init >actual &&
+	git submodule deinit init 2>actual &&
 	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
 	test_i18ngrep "Cleared directory .init" actual &&
-	git submodule deinit . >actual &&
+	git submodule deinit . 2>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 &&
+	git submodule deinit . 2>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 &&
-- 
2.8.0.32.g71f8beb.dirty

Re: [PATCH 06/10] submodule deinit: send messages to stderr

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> Reroute the output of stdout to stderr as it is just informative
> messages, not to be consumed by machines.
>
> This should not regress any scripts that try to parse the
> current output, as the output is already internationalized
> and therefore unstable.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---

Sounds sensible.

>  git-submodule.sh           |  8 ++++----
>  t/t7400-submodule-basic.sh | 20 ++++++++++----------
>  2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index f4d500e..3f67f4e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -452,11 +452,11 @@ cmd_deinit()
>  				die "$(eval_gettext "Submodule work tree '\$displaypath' contains local modifications; use '-f' to discard them")"
>  			fi
>  			rm -rf "$sm_path" &&
> -			say "$(eval_gettext "Cleared directory '\$displaypath'")" ||
> -			say "$(eval_gettext "Could not remove submodule work tree '\$displaypath'")"
> +			say >&2 "$(eval_gettext "Cleared directory '\$displaypath'")" ||
> +			say >&2 "$(eval_gettext "Could not remove submodule work tree '\$displaypath'")"
>  		fi
>  
> -		mkdir "$sm_path" || say "$(eval_gettext "Could not create empty submodule directory '\$displaypath'")"
> +		mkdir "$sm_path" || say >&2 "$(eval_gettext "Could not create empty submodule directory '\$displaypath'")"
>  
>  		# Remove the .git/config entries (unless the user already did it)
>  		if test -n "$(git config --get-regexp submodule."$name\.")"
> @@ -465,7 +465,7 @@ cmd_deinit()
>  			# 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 '\$displaypath'")"
> +			say >&2 "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"
>  		fi
>  	done
>  }
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index a6231f1..53644da 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -935,7 +935,7 @@ test_expect_success 'submodule deinit from subdirectory' '
>  	mkdir -p sub &&
>  	(
>  		cd sub &&
> -		git submodule deinit ../init >../output
> +		git submodule deinit ../init 2>../output
>  	) &&
>  	grep "\\.\\./init" output &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
> @@ -948,7 +948,7 @@ 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 &&
> -	git submodule deinit . >actual &&
> +	git submodule deinit . 2>actual &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
>  	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
>  	test_i18ngrep "Cleared directory .init" actual &&
> @@ -959,7 +959,7 @@ test_expect_success 'submodule deinit . deinits all initialized submodules' '
>  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 >actual &&
> +	git submodule deinit init example2 2>actual &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
>  	test -z "$(git config --get-regexp "submodule\.example2\.")" &&
>  	test_i18ngrep ! "Cleared directory .init" actual &&
> @@ -973,7 +973,7 @@ 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 >actual &&
> +	git submodule deinit -f init 2>actual &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
>  	test_i18ngrep "Cleared directory .init" actual &&
>  	rmdir init
> @@ -985,7 +985,7 @@ 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 >actual &&
> +	git submodule deinit -f init 2>actual &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
>  	test_i18ngrep "Cleared directory .init" actual &&
>  	rmdir init
> @@ -1000,7 +1000,7 @@ 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 >actual &&
> +	git submodule deinit -f init 2>actual &&
>  	test -z "$(git config --get-regexp "submodule\.example\.")" &&
>  	test_i18ngrep "Cleared directory .init" actual &&
>  	rmdir init
> @@ -1008,17 +1008,17 @@ test_expect_success 'submodule deinit fails when the submodule HEAD does not mat
>  
>  test_expect_success 'submodule deinit is silent when used on an uninitialized submodule' '
>  	git submodule update --init &&
> -	git submodule deinit init >actual &&
> +	git submodule deinit init 2>actual &&
>  	test_i18ngrep "Submodule .example. (.*) unregistered for path .init" actual &&
>  	test_i18ngrep "Cleared directory .init" actual &&
> -	git submodule deinit init >actual &&
> +	git submodule deinit init 2>actual &&
>  	test_i18ngrep ! "Submodule .example. (.*) unregistered for path .init" actual &&
>  	test_i18ngrep "Cleared directory .init" actual &&
> -	git submodule deinit . >actual &&
> +	git submodule deinit . 2>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 &&
> +	git submodule deinit . 2>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 &&

[PATCH 07/10] submodule foreach: send messages to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh             |  2 +-
 t/t7407-submodule-foreach.sh | 35 ++++++++++++++++++++++-------------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3f67f4e..80270db 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -341,7 +341,7 @@ cmd_foreach()
 		if test -e "$sm_path"/.git
 		then
 			displaypath=$(relative_path "$prefix$sm_path")
-			say "$(eval_gettext "Entering '\$displaypath'")"
+			say >&2 "$(eval_gettext "Entering '\$displaypath'")"
 			name=$(git submodule--helper name "$sm_path")
 			(
 				prefix="$prefix$sm_path/"
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 6ba5daf..f9b979a 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -61,29 +61,36 @@ sub3sha1=$(cd super/sub3 && git rev-parse HEAD)
 
 pwd=$(pwd)
 
-cat > expect <<EOF
-Entering 'sub1'
+cat >expect <<EOF
 $pwd/clone-foo1-sub1-$sub1sha1
-Entering 'sub3'
 $pwd/clone-foo3-sub3-$sub3sha1
 EOF
 
+cat >expect2 <<EOF
+Entering 'sub1'
+Entering 'sub3'
+EOF
+
 test_expect_success 'test basic "submodule foreach" usage' '
 	git clone super clone &&
 	(
 		cd clone &&
 		git submodule update --init -- sub1 sub3 &&
-		git submodule foreach "echo \$toplevel-\$name-\$path-\$sha1" > ../actual &&
+		git submodule foreach "echo \$toplevel-\$name-\$path-\$sha1" >../actual 2>../actual2 &&
 		git config foo.bar zar &&
 		git submodule foreach "git config --file \"\$toplevel/.git/config\" foo.bar"
 	) &&
-	test_i18ncmp expect actual
+	test_i18ncmp expect actual &&
+	test_i18ncmp expect2 actual2
 '
 
-cat >expect <<EOF
+cat >expect2 <<EOF
 Entering '../sub1'
-$pwd/clone-foo1-../sub1-$sub1sha1
 Entering '../sub3'
+EOF
+
+cat >expect <<EOF
+$pwd/clone-foo1-../sub1-$sub1sha1
 $pwd/clone-foo3-../sub3-$sub3sha1
 EOF
 
@@ -91,9 +98,10 @@ test_expect_success 'test "submodule foreach" from subdirectory' '
 	mkdir clone/sub &&
 	(
 		cd clone/sub &&
-		git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual
+		git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual 2>../../actual2
 	) &&
-	test_i18ncmp expect actual
+	test_i18ncmp expect actual &&
+	test_i18ncmp expect2 actual2
 '
 
 test_expect_success 'setup nested submodules' '
@@ -172,7 +180,7 @@ EOF
 test_expect_success 'test messages from "foreach --recursive"' '
 	(
 		cd clone2 &&
-		git submodule foreach --recursive "true" > ../actual
+		git submodule foreach --recursive "true" 2>../actual
 	) &&
 	test_i18ncmp expect actual
 '
@@ -192,7 +200,7 @@ test_expect_success 'test messages from "foreach --recursive" from subdirectory'
 		cd clone2 &&
 		mkdir untracked &&
 		cd untracked &&
-		git submodule foreach --recursive >../../actual
+		git submodule foreach --recursive 2>../../actual
 	) &&
 	test_i18ncmp expect actual
 '
@@ -210,9 +218,10 @@ EOF
 test_expect_success 'test "foreach --quiet --recursive"' '
 	(
 		cd clone2 &&
-		git submodule foreach -q --recursive "echo \$name-\$path" > ../actual
+		git submodule foreach -q --recursive "echo \$name-\$path" >../actual 2> ../actual2
 	) &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	test_must_be_empty actual2
 '
 
 test_expect_success 'use "update --recursive" to checkout all submodules' '
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 08/10] submodule update: send messages to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh            |  2 +-
 t/t7406-submodule-update.sh | 23 ++++++++++-------------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 80270db..c86c2e5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -679,7 +679,7 @@ cmd_update()
 
 			if (clear_local_git_env; cd "$sm_path" && $command "$sha1")
 			then
-				say "$say_msg"
+				say >&2 "$say_msg"
 			elif test -n "$must_die_on_failure"
 			then
 				die_with_status 2 "$die_msg"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 5f27879..1f8faa8 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -106,15 +106,8 @@ rebasingsha1=$(git -C super/rebasing rev-parse HEAD)
 submodulesha1=$(git -C super/submodule rev-parse HEAD)
 pwd=$(pwd)
 
-cat <<EOF >expect
-Submodule path '../super': checked out '$supersha1'
-Submodule path '../super/merging': checked out '$mergingsha1'
-Submodule path '../super/none': checked out '$nonesha1'
-Submodule path '../super/rebasing': checked out '$rebasingsha1'
-Submodule path '../super/submodule': checked out '$submodulesha1'
-EOF
-
 cat <<EOF >expect2
+Submodule path '../super': checked out '$supersha1'
 Submodule 'merging' ($pwd/merging) registered for path '../super/merging'
 Submodule 'none' ($pwd/none) registered for path '../super/none'
 Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'
@@ -127,6 +120,10 @@ Cloning into '$pwd/recursivesuper/super/rebasing'...
 done.
 Cloning into '$pwd/recursivesuper/super/submodule'...
 done.
+Submodule path '../super/merging': checked out '$mergingsha1'
+Submodule path '../super/none': checked out '$nonesha1'
+Submodule path '../super/rebasing': checked out '$rebasingsha1'
+Submodule path '../super/submodule': checked out '$submodulesha1'
 EOF
 
 test_expect_success 'submodule update --init --recursive from subdirectory' '
@@ -136,7 +133,7 @@ test_expect_success 'submodule update --init --recursive from subdirectory' '
 	 cd tmp &&
 	 git submodule update --init --recursive ../super >../../actual 2>../../actual2
 	) &&
-	test_cmp expect actual &&
+	test_must_be_empty actual &&
 	test_cmp expect2 actual2
 '
 
@@ -156,8 +153,8 @@ test_expect_success 'submodule update does not fetch already present commits' '
 	(cd super &&
 	  git submodule update > ../actual 2> ../actual.err
 	) &&
-	test_i18ncmp expected actual &&
-	! test -s actual.err
+	test_must_be_empty actual &&
+	test_i18ncmp expected actual.err
 '
 
 test_expect_success 'submodule update should fail due to local changes' '
@@ -790,7 +787,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
 	rm -rf super_update_r2 &&
 	git clone super_update_r super_update_r2 &&
 	(cd super_update_r2 &&
-	 git submodule update --init --recursive >actual &&
+	 git submodule update --init --recursive 2>actual &&
 	 test_i18ngrep "Submodule path .submodule/subsubmodule.: checked out" actual &&
 	 (cd submodule/subsubmodule &&
 	  git log > ../../expected
@@ -858,7 +855,7 @@ test_expect_success 'submodule update --recursive drops module name before recur
 	 (cd deeper/submodule/subsubmodule &&
 	  git checkout HEAD^
 	 ) &&
-	 git submodule update --recursive deeper/submodule >actual &&
+	 git submodule update --recursive deeper/submodule 2>actual &&
 	 test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked out" actual
 	)
 '
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 09/10] submodule sync: send messages to stderr

[Stefan Beller]

Reroute the output of stdout to stderr as it is just informative
messages, not to be consumed by machines.

This should not regress any scripts that try to parse the
current output, as the output is already internationalized
and therefore unstable.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-submodule.sh          | 2 +-
 t/t7403-submodule-sync.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index c86c2e5..f075924 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1095,7 +1095,7 @@ cmd_sync()
 		if git config "submodule.$name.url" >/dev/null 2>/dev/null
 		then
 			displaypath=$(relative_path "$prefix$sm_path")
-			say "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
+			say >&2 "$(eval_gettext "Synchronizing submodule url for '\$displaypath'")"
 			git config submodule."$name".url "$super_config_url"
 
 			if test -e "$sm_path"/.git
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 79bc135..93c1dfa 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -155,7 +155,7 @@ test_expect_success '"git submodule sync" should update submodule URLs - subdire
 		git pull --no-recurse-submodules &&
 		mkdir -p sub &&
 		cd sub &&
-		git submodule sync >../../output
+		git submodule sync 2>../../output
 	) &&
 	grep "\\.\\./submodule" output &&
 	test -d "$(
@@ -186,7 +186,7 @@ test_expect_success '"git submodule sync --recursive" should update all submodul
 		) &&
 		mkdir -p sub &&
 		cd sub &&
-		git submodule sync --recursive >../../output
+		git submodule sync --recursive 2>../../output
 	) &&
 	grep "\\.\\./submodule/sub-submodule" output &&
 	test -d "$(
-- 
2.8.0.32.g71f8beb.dirty

[PATCH 10/10] submodule deinit: complain when given a file instead of a submodule

[Stefan Beller]

This also improves performance for listing submodules, because
S_ISGITLINK is both faster as match_pathspec as well as expected to
be true in fewer cases, so putting it first in the condition will speed
up the loop to compute all submodules.

As this partially reverts 84ba959bbdf0 (submodule: fix regression for
deinit without submodules, 2016-03-22), this also disallows the use
of `git submodule deinit .` to deinit all submodules, when no
submodules are present. `deinit .` continues to work on repositories,
which have at least one submodule.

CC: Per Cederqvist <cederp@opera.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---


> Patch 10 is a controversial thing I'd assume as it breaks existing users.
> We should take it for the next major release (i.e. 3.0)
> I just want to put it out here now.

 builtin/submodule--helper.c |  6 +++---
 t/t7400-submodule-basic.sh  | 15 ++++++++++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7f0941d..e41de3e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -242,9 +242,9 @@ static int module_list_compute(int argc, const char **argv,
 	for (i = 0; i < active_nr; i++) {
 		const struct cache_entry *ce = active_cache[i];
 
-		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
-				    0, ps_matched, 1) ||
-		    !S_ISGITLINK(ce->ce_mode))
+		if (!S_ISGITLINK(ce->ce_mode) ||
+		    !match_pathspec(pathspec, ce->name, ce_namelen(ce),
+				    0, ps_matched, 1))
 			continue;
 
 		ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 53644da..361e6f6 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -915,7 +915,20 @@ test_expect_success 'submodule deinit works on repository without submodules' '
 		>file &&
 		git add file &&
 		git commit -m "repo should not be empty" &&
-		git submodule deinit .
+		git submodule deinit
+	)
+'
+
+test_expect_success 'submodule deinit refuses to deinit a file' '
+	test_when_finished "rm -rf newdirectory" &&
+	mkdir newdirectory &&
+	(
+		cd newdirectory &&
+		git init &&
+		>file &&
+		git add file &&
+		git commit -m "repo should not be empty" &&
+		test_must_fail git submodule deinit file
 	)
 '
 
-- 
2.8.0.32.g71f8beb.dirty

Re: [PATCH 10/10] submodule deinit: complain when given a file instead of a submodule

[Per Cederqvist]

After this change, what is the simplest way to programmatically
deinit any submodule that may exist, without failing if there are
none?

"git commit" by default refuses to make an empty commit, but
it has the --allow-empty option.

"git rm -r ." by default fails if there are no files in the repository,
but it has the --ignore-unmatch option.

It makes sense that "git submodule deinit ." should fail if there
are no submodules, but please add support for --ignore-unmatch
at the same time.

    /ceder


On Sat, Apr 30, 2016 at 2:40 AM, Stefan Beller <sbeller@google.com> wrote:
> This also improves performance for listing submodules, because
> S_ISGITLINK is both faster as match_pathspec as well as expected to
> be true in fewer cases, so putting it first in the condition will speed
> up the loop to compute all submodules.
>
> As this partially reverts 84ba959bbdf0 (submodule: fix regression for
> deinit without submodules, 2016-03-22), this also disallows the use
> of `git submodule deinit .` to deinit all submodules, when no
> submodules are present. `deinit .` continues to work on repositories,
> which have at least one submodule.
>
> CC: Per Cederqvist <cederp@opera.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
>
>> Patch 10 is a controversial thing I'd assume as it breaks existing users.
>> We should take it for the next major release (i.e. 3.0)
>> I just want to put it out here now.
>
>  builtin/submodule--helper.c |  6 +++---
>  t/t7400-submodule-basic.sh  | 15 ++++++++++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 7f0941d..e41de3e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -242,9 +242,9 @@ static int module_list_compute(int argc, const char **argv,
>         for (i = 0; i < active_nr; i++) {
>                 const struct cache_entry *ce = active_cache[i];
>
> -               if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
> -                                   0, ps_matched, 1) ||
> -                   !S_ISGITLINK(ce->ce_mode))
> +               if (!S_ISGITLINK(ce->ce_mode) ||
> +                   !match_pathspec(pathspec, ce->name, ce_namelen(ce),
> +                                   0, ps_matched, 1))
>                         continue;
>
>                 ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 53644da..361e6f6 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -915,7 +915,20 @@ test_expect_success 'submodule deinit works on repository without submodules' '
>                 >file &&
>                 git add file &&
>                 git commit -m "repo should not be empty" &&
> -               git submodule deinit .
> +               git submodule deinit
> +       )
> +'
> +
> +test_expect_success 'submodule deinit refuses to deinit a file' '
> +       test_when_finished "rm -rf newdirectory" &&
> +       mkdir newdirectory &&
> +       (
> +               cd newdirectory &&
> +               git init &&
> +               >file &&
> +               git add file &&
> +               git commit -m "repo should not be empty" &&
> +               test_must_fail git submodule deinit file
>         )
>  '
>
> --
> 2.8.0.32.g71f8beb.dirty
>

Re: [PATCH 10/10] submodule deinit: complain when given a file instead of a submodule

[Stefan Beller]

On Mon, May 2, 2016 at 1:26 AM, Per Cederqvist <cederp@opera.com> wrote:
> After this change, what is the simplest way to programmatically
> deinit any submodule that may exist, without failing if there are
> none?
>
> "git commit" by default refuses to make an empty commit, but
> it has the --allow-empty option.
>
> "git rm -r ." by default fails if there are no files in the repository,
> but it has the --ignore-unmatch option.
>
> It makes sense that "git submodule deinit ." should fail if there
> are no submodules, but please add support for --ignore-unmatch
> at the same time.

Oh right. I'll add the --ignore-unmatch option when rerolling this series.

Thanks,
Stefan

>
>     /ceder
>
>
> On Sat, Apr 30, 2016 at 2:40 AM, Stefan Beller <sbeller@google.com> wrote:
>> This also improves performance for listing submodules, because
>> S_ISGITLINK is both faster as match_pathspec as well as expected to
>> be true in fewer cases, so putting it first in the condition will speed
>> up the loop to compute all submodules.
>>
>> As this partially reverts 84ba959bbdf0 (submodule: fix regression for
>> deinit without submodules, 2016-03-22), this also disallows the use
>> of `git submodule deinit .` to deinit all submodules, when no
>> submodules are present. `deinit .` continues to work on repositories,
>> which have at least one submodule.
>>
>> CC: Per Cederqvist <cederp@opera.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>>
>>> Patch 10 is a controversial thing I'd assume as it breaks existing users.
>>> We should take it for the next major release (i.e. 3.0)
>>> I just want to put it out here now.
>>
>>  builtin/submodule--helper.c |  6 +++---
>>  t/t7400-submodule-basic.sh  | 15 ++++++++++++++-
>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 7f0941d..e41de3e 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -242,9 +242,9 @@ static int module_list_compute(int argc, const char **argv,
>>         for (i = 0; i < active_nr; i++) {
>>                 const struct cache_entry *ce = active_cache[i];
>>
>> -               if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
>> -                                   0, ps_matched, 1) ||
>> -                   !S_ISGITLINK(ce->ce_mode))
>> +               if (!S_ISGITLINK(ce->ce_mode) ||
>> +                   !match_pathspec(pathspec, ce->name, ce_namelen(ce),
>> +                                   0, ps_matched, 1))
>>                         continue;
>>
>>                 ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
>> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
>> index 53644da..361e6f6 100755
>> --- a/t/t7400-submodule-basic.sh
>> +++ b/t/t7400-submodule-basic.sh
>> @@ -915,7 +915,20 @@ test_expect_success 'submodule deinit works on repository without submodules' '
>>                 >file &&
>>                 git add file &&
>>                 git commit -m "repo should not be empty" &&
>> -               git submodule deinit .
>> +               git submodule deinit
>> +       )
>> +'
>> +
>> +test_expect_success 'submodule deinit refuses to deinit a file' '
>> +       test_when_finished "rm -rf newdirectory" &&
>> +       mkdir newdirectory &&
>> +       (
>> +               cd newdirectory &&
>> +               git init &&
>> +               >file &&
>> +               git add file &&
>> +               git commit -m "repo should not be empty" &&
>> +               test_must_fail git submodule deinit file
>>         )
>>  '
>>
>> --
>> 2.8.0.32.g71f8beb.dirty
>>

Re: [PATCH 10/10] submodule deinit: complain when given a file instead of a submodule

[Stefan Beller]

On Mon, May 2, 2016 at 9:21 AM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, May 2, 2016 at 1:26 AM, Per Cederqvist <cederp@opera.com> wrote:
>> After this change, what is the simplest way to programmatically
>> deinit any submodule that may exist, without failing if there are
>> none?
>>
>> "git commit" by default refuses to make an empty commit, but
>> it has the --allow-empty option.
>>
>> "git rm -r ." by default fails if there are no files in the repository,
>> but it has the --ignore-unmatch option.
>>
>> It makes sense that "git submodule deinit ." should fail if there
>> are no submodules, but please add support for --ignore-unmatch
>> at the same time.

With this patch series, you can omit the trailing dot, i.e.
"git submodule deinit" works. I just tested that and it works in
repositories with no submodules as well as in empty repositories,
but I'll add a test for that as well.

>
> Oh right. I'll add the --ignore-unmatch option when rerolling this series.
>
> Thanks,
> Stefan
>
>>
>>     /ceder
>>
>>
>> On Sat, Apr 30, 2016 at 2:40 AM, Stefan Beller <sbeller@google.com> wrote:
>>> This also improves performance for listing submodules, because
>>> S_ISGITLINK is both faster as match_pathspec as well as expected to
>>> be true in fewer cases, so putting it first in the condition will speed
>>> up the loop to compute all submodules.
>>>
>>> As this partially reverts 84ba959bbdf0 (submodule: fix regression for
>>> deinit without submodules, 2016-03-22), this also disallows the use
>>> of `git submodule deinit .` to deinit all submodules, when no
>>> submodules are present. `deinit .` continues to work on repositories,
>>> which have at least one submodule.
>>>
>>> CC: Per Cederqvist <cederp@opera.com>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>>
>>>
>>>> Patch 10 is a controversial thing I'd assume as it breaks existing users.
>>>> We should take it for the next major release (i.e. 3.0)
>>>> I just want to put it out here now.
>>>
>>>  builtin/submodule--helper.c |  6 +++---
>>>  t/t7400-submodule-basic.sh  | 15 ++++++++++++++-
>>>  2 files changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>>> index 7f0941d..e41de3e 100644
>>> --- a/builtin/submodule--helper.c
>>> +++ b/builtin/submodule--helper.c
>>> @@ -242,9 +242,9 @@ static int module_list_compute(int argc, const char **argv,
>>>         for (i = 0; i < active_nr; i++) {
>>>                 const struct cache_entry *ce = active_cache[i];
>>>
>>> -               if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
>>> -                                   0, ps_matched, 1) ||
>>> -                   !S_ISGITLINK(ce->ce_mode))
>>> +               if (!S_ISGITLINK(ce->ce_mode) ||
>>> +                   !match_pathspec(pathspec, ce->name, ce_namelen(ce),
>>> +                                   0, ps_matched, 1))
>>>                         continue;
>>>
>>>                 ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
>>> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
>>> index 53644da..361e6f6 100755
>>> --- a/t/t7400-submodule-basic.sh
>>> +++ b/t/t7400-submodule-basic.sh
>>> @@ -915,7 +915,20 @@ test_expect_success 'submodule deinit works on repository without submodules' '
>>>                 >file &&
>>>                 git add file &&
>>>                 git commit -m "repo should not be empty" &&
>>> -               git submodule deinit .
>>> +               git submodule deinit
>>> +       )
>>> +'
>>> +
>>> +test_expect_success 'submodule deinit refuses to deinit a file' '
>>> +       test_when_finished "rm -rf newdirectory" &&
>>> +       mkdir newdirectory &&
>>> +       (
>>> +               cd newdirectory &&
>>> +               git init &&
>>> +               >file &&
>>> +               git add file &&
>>> +               git commit -m "repo should not be empty" &&
>>> +               test_must_fail git submodule deinit file
>>>         )
>>>  '
>>>
>>> --
>>> 2.8.0.32.g71f8beb.dirty
>>>