[PATCH 0/3] preparatory patches for the submodule groups

DORMANTno replies

4 messages, 1 author, 2016-06-16 · open the first message on its own page

[PATCH 0/3] preparatory patches for the submodule groups

From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:08 

These patches build on top of origin/sb/submodule-init and are preparatoy for
the submodule groups series. Junio asked to send these preparatory patches
as its own series to have a better mental focus when reviewing the groups
series later.

When sending out a similar series end of last week[1], I went to far and included
some questionable patches in the series, this series is about the minimum needed.
(patch 1 is a test suite correctness patch, which would not be strictly needed)

Thanks,
Stefan


[1] http://thread.gmane.org/gmane.comp.version-control.git/293087

Stefan Beller (3):
  submodule deinit test: fix broken && chain in subshell
  submodule deinit: lose requirement for giving '.'
  submodule init: redirect stdout to stderr

 builtin/submodule--helper.c |  3 ++-
 git-submodule.sh            |  5 -----
 t/t7400-submodule-basic.sh  |  7 +++++--
 t/t7406-submodule-update.sh | 24 ++++++++++++++++++------
 4 files changed, 25 insertions(+), 14 deletions(-)

-- 
2.8.0.37.gb114fff.dirty

[PATCH 1/3] submodule deinit test: fix broken && chain in subshell

From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:08 

Signed-off-by: Stefan Beller <redacted>
---
 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.37.gb114fff.dirty

[PATCH 3/3] submodule init: redirect stdout to stderr

From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:08 

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.

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 <redacted>
---
 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.37.gb114fff.dirty

[PATCH 2/3] submodule deinit: lose requirement for giving '.'

From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:08 

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
Allow no argument for `submodule deinit` to mean all submodules
and add a test to check for the corner case of an empty repository.

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 <redacted>
---
 git-submodule.sh           | 5 -----
 t/t7400-submodule-basic.sh | 5 ++++-
 2 files changed, 4 insertions(+), 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..9af47b5 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -11,6 +11,10 @@ subcommands of git submodule.
 
 . ./test-lib.sh
 
+test_expect_success 'submodule deinit works on empty repository' '
+	git submodule deinit
+'
+
 test_expect_success 'setup - initial commit' '
 	>t &&
 	git add t &&
@@ -948,7 +952,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.37.gb114fff.dirty
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help