Re: [RFC/PATCH] Re: git submodule -b ... of current HEAD fails

Jens Lehmann [off-list ref] writes:

Nope, these lines date back to the time before I got involved in the
submodule business ... Seems like this "git checkout" was added in
March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
that.

But to me your change looks good, so feel free to add:
Acked-by: Jens Lehmann <redacted>

Does either of you want to add a test for this?

Am 07.12.2010 23:57, schrieb Junio C Hamano:

Jens Lehmann [off-list ref] writes:

quoted

Nope, these lines date back to the time before I got involved in the
submodule business ... Seems like this "git checkout" was added in
March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
that.

But to me your change looks good, so feel free to add:
Acked-by: Jens Lehmann <redacted>

Does either of you want to add a test for this?

Will do.

	git submodule add -b $branch $repository

fails when HEAD already points to $branch in $repository.

When the freshly cloned submodules HEAD is the same as the checked out
branch, it doesn't make sense to update it again as "git checkout -b"
would fail with »fatal: git checkout: branch $branch already exists«.

Reported-by: Klaus Ethgen <redacted>
Thanks-to: Jonathan Nieder [off-list ref]
Signed-off-by: Jens Lehmann <redacted>
---

Am 08.12.2010 22:35, schrieb Jens Lehmann:

Am 07.12.2010 23:57, schrieb Junio C Hamano:

quoted

Jens Lehmann [off-list ref] writes:

quoted

Nope, these lines date back to the time before I got involved in the
submodule business ... Seems like this "git checkout" was added in
March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
that.

But to me your change looks good, so feel free to add:
Acked-by: Jens Lehmann <redacted>

Does either of you want to add a test for this?

Will do.

And as it happens from time to time, while writing the test you find
out that the first attempt to fix the bug didn't work as expected ...

 git-submodule.sh           |    4 +++-
 t/t7400-submodule-basic.sh |    7 +++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 33bc41f..bf2803f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -241,7 +241,9 @@ cmd_add()
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
 			'') git checkout -f -q ;;
-			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
+			?*) if [ "$(git branch)" != "* $branch"  ]; then
+				git checkout -f -q -b "$branch" "origin/$branch"
+			fi ;;
 			esac
 		) || die "Unable to checkout submodule '$path'"
 	fi

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 782b0a3..e224da4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh

@@ -131,6 +131,13 @@ test_expect_success 'submodule add --branch' '
 	test_cmp empty untracked
 '

+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '
+	(
+		cd addtest &&
+		git submodule add -b master "$submodurl" submod-existing-branch
+	)
+'
+
 test_expect_success 'submodule add with ./ in path' '
 	echo "refs/heads/master" >expect &&
 	>empty &&

-- 
1.7.3.3.580.ged75d

Jens Lehmann wrote:

Agh.  The command to use is "git symbolic-ref -q HEAD", I suppose.

Maybe we can simplify by relying on "git clone".

Signed-off-by: Jonathan Nieder <redacted>
---
 git-submodule.sh           |   16 ++++++----------
 t/t7400-submodule-basic.sh |    8 +++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d937f0b..6fd09e7 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -219,17 +219,13 @@ cmd_add()
 		esac
 		git config submodule."$path".url "$url"
 	else
-
-		module_clone "$path" "$realrepo" "$reference" || exit
+		brancharg=${branch:+"-b $(git rev-parse --sq-quote "$branch")"}
+		refarg=${reference:+"$(git rev-parse --sq-quote "$reference")"}
 		(
-			clear_local_git_env
-			cd "$path" &&
-			# ash fails to wordsplit ${branch:+-b "$branch"...}
-			case "$branch" in
-			'') git checkout -f -q ;;
-			?*) git checkout -f -q -b "$branch" "origin/$branch" ;;
-			esac
-		) || die "Unable to checkout submodule '$path'"
+			export realrepo path &&
+			eval "git clone $brancharg $refarg" '"$realrepo" "$path"'
+		) ||
+		die "Clone of '$realrepo' into submodule path '$path' failed"
 	fi
 
 	git add $force "$path" ||

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2c49db9..77088cc 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh

@@ -114,7 +114,6 @@ test_expect_success 'submodule add --branch' '
 	echo "refs/heads/initial" >expect-head &&
 	cat <<-\EOF >expect-heads &&
 	refs/heads/initial
-	refs/heads/master
 	EOF
 	>empty &&
 

@@ -131,6 +130,13 @@ test_expect_success 'submodule add --branch' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'submodule add --branch succeeds even when branch is at HEAD' '
+	(
+		cd addtest &&
+		git submodule add -b master "$submodurl" submod-existing-branch
+	)
+'
+
 test_expect_success 'submodule add with ./ in path' '
 	echo "refs/heads/master" >expect &&
 	>empty &&