threads / rfc / 27874

RFC patchMove git-dir for submodules

Subject: [RFC PATCH] Move git-dir for submodules

## tl;dr

8 messages between Jul 21, 2011 and Jul 22, 2011. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Fredrik Gustafsson· Jul 21, 2011, 16:52 UTC · lore

Move git-dir for submodules into $GIT_DIR/modules/[name_of_submodule] of the superproject. This is a step towards being able to delete submodule directories without loosing the information from their .git directory as that is now stored outside the submodules work tree.

This is done relying on the already existent .git-file functionality. Tests that rely on .git being a directory have been fixed.

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
Mentored-by: Jens Lehmann <Jens.Lehmann@web.de>
Mentored-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 git-submodule.sh               |   52 ++++++++++++++++++--
 t/t7400-submodule-basic.sh     |    4 +-
 t/t7403-submodule-sync.sh      |    2 +-
 t/t7406-submodule-update.sh    |  105 ++++++++++++++++++++++++++++++++++++++++
 t/t7407-submodule-foreach.sh   |   98 +++++++++++++++++++-------------------
 t/t7408-submodule-reference.sh |    4 +-
 6 files changed, 206 insertions(+), 59 deletions(-)
Show changes to 6 files +206 −59

git-submodule.sh, t/t7400-submodule-basic.sh, t/t7403-submodule-sync.sh, t/t7406-submodule-update.sh, t/t7407-submodule-foreach.sh, t/t7408-submodule-reference.sh

diff --git a/git-submodule.sh b/git-submodule.sh
index 87c9452..3ad3012 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -122,14 +122,56 @@ module_clone()
 	path=$1
 	url=$2
 	reference="$3"
+	gitdir=
+	gitdir_base=
+	base_path=`echo $path | sed -e 's|[^/]*$||'`
 
-	if test -n "$reference"
+	if test -z "$GIT_DIR"
 	then
-		git-clone "$reference" -n "$url" "$path"
+		gitdir=$(git rev-parse --git-dir)
+		gitdir_base="$gitdir/modules/$base_path"
+		gitdir="$gitdir/modules/$path"
 	else
-		git-clone -n "$url" "$path"
-	fi ||
-	die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
+		gitdir="$GIT_DIR/modules/$path"
+		gitdir_base="$GIT_DIR/modules/$base_path"
+	fi
+
+	case $gitdir in
+		/*)
+			a="$(cd_to_toplevel && pwd)/"
+			b=$gitdir
+			while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
+			do
+				a=${a#*/} b=${b#*/};
+			done
+
+			rel="$a$path"
+			rel=`echo $rel | sed -e 's|[^/]*|..|g'`
+			rel_gitdir="$rel/$b"
+			;;
+		*)
+			rel=`echo $path | sed -e 's|[^/]*|..|g'`
+			rel_gitdir="$rel/$gitdir"
+			;;
+	esac
+
+	if test -d "$gitdir"
+	then
+		mkdir -p "$path"
+		echo "gitdir: $rel_gitdir" > "$path/.git"
+	else
+		if !(test -d "$gitdir_base")
+		then
+			mkdir -p "$gitdir_base"
+		fi
+		if test -n "$reference"
+		then
+			git-clone "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
+		else
+			git-clone -n "$url" "$path" --separate-git-dir "$gitdir"
+		fi ||
+		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
+	fi
 }
 
 #
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b2b26b7..57f5306 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -358,10 +358,10 @@ test_expect_success 'update --init' '
 	git submodule update init > update.out &&
 	cat update.out &&
 	test_i18ngrep "not initialized" update.out &&
-	! test -d init/.git &&
+	! test -f init/.git &&
 
 	git submodule update --init init &&
-	test -d init/.git
+	test -n $(grep "/^gitdir: /" init/.git)
 '
 
 test_expect_success 'do not add files from a submodule' '
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index d600583..b0517f0 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -55,7 +55,7 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
 	 git pull --no-recurse-submodules &&
 	 git submodule sync
 	) &&
-	test -d "$(git config -f super-clone/submodule/.git/config \
+	test -d "$(git config -f super-clone/.git/modules/submodule/config \
 	                        remote.origin.url)" &&
 	(cd super-clone/submodule &&
 	 git checkout master &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index c679f36..32d9c59 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -442,4 +442,109 @@ test_expect_success 'submodule update exit immediately after recursive rebase er
 	 test_cmp expect actual
 	)
 '
+
+test_expect_success 'submodule add places git-dir in superprojects git-dir' '
+	(cd super &&
+	 mkdir deeper &&
+	 git submodule add ../submodule deeper/submodule &&
+	 (cd deeper/submodule &&
+	  git log > ../../expected
+	 ) &&
+	 (cd .git/modules/deeper/submodule &&
+	  git log > ../../../../actual
+	 ) &&
+	 test_cmp actual expected
+	)
+'
+
+test_expect_success 'submodule update places git-dir in superprojects git-dir' '
+	(cd super &&
+	 git commit -m "added submodule"
+	) &&
+	git clone super super2 &&
+	(cd super2 &&
+	 git submodule init deeper/submodule &&
+	 git submodule update &&
+	 (cd deeper/submodule &&
+	  git log > ../../expected
+	 ) &&
+	 (cd .git/modules/deeper/submodule &&
+	  git log > ../../../../actual
+	 ) &&
+	 test_cmp actual expected
+	)
+'
+
+test_expect_success 'submodule add places git-dir in superprojects git-dir recursive' '
+	(cd super2 &&
+	 (cd deeper/submodule &&
+	  git submodule add ../submodule subsubmodule &&
+	  (cd subsubmodule &&
+	   git log > ../../../expected
+	  ) &&
+	  git commit -m "added subsubmodule" &&
+	  git push
+	 ) &&
+	 (cd .git/modules/deeper/submodule/modules/subsubmodule &&
+	  git log > ../../../../../actual
+	 ) &&
+	 git add deeper/submodule &&
+	 git commit -m "update submodule" &&
+	 git push &&
+	 test_cmp actual expected
+	)
+'
+
+test_expect_success 'submodule update places git-dir in superprojects git-dir recursive' '
+	mkdir super_update_r &&
+	(cd super_update_r &&
+	 git init --bare
+	) &&
+	mkdir subsuper_update_r &&
+	(cd subsuper_update_r &&
+	 git init --bare
+	) &&
+	mkdir subsubsuper_update_r &&
+	(cd subsubsuper_update_r &&
+	 git init --bare
+	) &&
+	git clone subsubsuper_update_r subsubsuper_update_r2 &&
+	(cd subsubsuper_update_r2 &&
+	 test_commit "update_subsubsuper" file &&
+	 git push origin master
+	) &&
+	git clone subsuper_update_r subsuper_update_r2 &&
+	(cd subsuper_update_r2 &&
+	 test_commit "update_subsuper" file &&
+	 git submodule add ../subsubsuper_update_r subsubmodule &&
+	 git commit -am "subsubmodule" &&
+	 git push origin master
+	) &&
+	git clone super_update_r super_update_r2 &&
+	(cd super_update_r2 &&
+	 test_commit "update_super" file &&
+	 git submodule add ../subsuper_update_r submodule &&
+	 git commit -am "submodule" &&
+	 git push origin master
+	) &&
+	rm -rf super_update_r2 &&
+	git clone super_update_r super_update_r2 &&
+	(cd super_update_r2 &&
+	 git submodule update --init --recursive &&
+	 (cd submodule/subsubmodule &&
+	  git log > ../../expected
+	 ) &&
+	 (cd .git/modules/submodule/modules/subsubmodule
+	  git log > ../../../../../actual
+	 )
+	 test_cmp actual expected
+	)
+'
+
+test_expect_success 'add different submodules to the same path' '
+	(cd super &&
+	 git submodule add ../submodule s1 &&
+	 test_must_fail git submodule add ../merging s1
+	)
+'
 test_done
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index be745fb..3f498a3 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -118,19 +118,19 @@ test_expect_success 'use "submodule foreach" to checkout 2nd level submodule' '
 	git clone super clone2 &&
 	(
 		cd clone2 &&
-		test ! -d sub1/.git &&
-		test ! -d sub2/.git &&
-		test ! -d sub3/.git &&
-		test ! -d nested1/.git &&
+		test ! -f sub1/.git &&
+		test ! -f sub2/.git &&
+		test ! -f sub3/.git &&
+		test ! -f nested1/.git &&
 		git submodule update --init &&
-		test -d sub1/.git &&
-		test -d sub2/.git &&
-		test -d sub3/.git &&
-		test -d nested1/.git &&
-		test ! -d nested1/nested2/.git &&
+		test -n $(grep "/^gitdir: /" sub1/.git) &&
+		test -n $(grep "/^gitdir: /" sub2/.git) &&
+		test -n $(grep "/^gitdir: /" sub3/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/.git) &&
+		test ! -f nested1/nested2/.git &&
 		git submodule foreach "git submodule update --init" &&
-		test -d nested1/nested2/.git &&
-		test ! -d nested1/nested2/nested3/.git
+		test -n $(grep "/^gitdir: /" nested1/nested2/.git) &&
+		test ! -f nested1/nested2/nested3/.git
 	)
 '
 
@@ -138,8 +138,8 @@ test_expect_success 'use "foreach --recursive" to checkout all submodules' '
 	(
 		cd clone2 &&
 		git submodule foreach --recursive "git submodule update --init" &&
-		test -d nested1/nested2/nested3/.git &&
-		test -d nested1/nested2/nested3/submodule/.git
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/submodule/).git
 	)
 '
 
@@ -183,18 +183,18 @@ test_expect_success 'use "update --recursive" to checkout all submodules' '
 	git clone super clone3 &&
 	(
 		cd clone3 &&
-		test ! -d sub1/.git &&
-		test ! -d sub2/.git &&
-		test ! -d sub3/.git &&
-		test ! -d nested1/.git &&
+		test ! -f sub1/.git &&
+		test ! -f sub2/.git &&
+		test ! -f sub3/.git &&
+		test ! -f nested1/.git &&
 		git submodule update --init --recursive &&
-		test -d sub1/.git &&
-		test -d sub2/.git &&
-		test -d sub3/.git &&
-		test -d nested1/.git &&
-		test -d nested1/nested2/.git &&
-		test -d nested1/nested2/nested3/.git &&
-		test -d nested1/nested2/nested3/submodule/.git
+		test -n $(grep "/^gitdir: /" sub1/.git) &&
+		test -n $(grep "/^gitdir: /" sub2/.git) &&
+		test -n $(grep "/^gitdir: /" sub3/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/submodule/).git
 	)
 '
 
@@ -248,13 +248,13 @@ test_expect_success 'ensure "status --cached --recursive" preserves the --cached
 test_expect_success 'use "git clone --recursive" to checkout all submodules' '
 	git clone --recursive super clone4 &&
 	test -d clone4/.git &&
-	test -d clone4/sub1/.git &&
-	test -d clone4/sub2/.git &&
-	test -d clone4/sub3/.git &&
-	test -d clone4/nested1/.git &&
-	test -d clone4/nested1/nested2/.git &&
-	test -d clone4/nested1/nested2/nested3/.git &&
-	test -d clone4/nested1/nested2/nested3/submodule/.git
+	test -n $(grep "/^gitdir: /" clone4/sub1/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/sub2/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/sub3/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/nested1/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/nested1/nested2/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/nested1/nested2/nested3/.git) &&
+	test -n $(grep "/^gitdir: /" clone4/nested1/nested2/nested3/submodule/.git)
 '
 
 test_expect_success 'test "update --recursive" with a flag with spaces' '
@@ -262,14 +262,14 @@ test_expect_success 'test "update --recursive" with a flag with spaces' '
 	git clone super clone5 &&
 	(
 		cd clone5 &&
-		test ! -d nested1/.git &&
+		test ! -f nested1/.git &&
 		git submodule update --init --recursive --reference="$(dirname "$PWD")/common objects" &&
-		test -d nested1/.git &&
-		test -d nested1/nested2/.git &&
-		test -d nested1/nested2/nested3/.git &&
-		test -f nested1/.git/objects/info/alternates &&
-		test -f nested1/nested2/.git/objects/info/alternates &&
-		test -f nested1/nested2/nested3/.git/objects/info/alternates
+		test -n $(grep "/^gitdir: /" nested1/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/.git) &&
+		test -f .git/modules/nested1/objects/info/alternates &&
+		test -f .git/modules/nested1/modules/nested2/objects/info/alternates &&
+		test -f .git/modules/nested1/modules/nested2/modules/nested3/objects/info/alternates
 	)
 '
 
@@ -277,18 +277,18 @@ test_expect_success 'use "update --recursive nested1" to checkout all submodules
 	git clone super clone6 &&
 	(
 		cd clone6 &&
-		test ! -d sub1/.git &&
-		test ! -d sub2/.git &&
-		test ! -d sub3/.git &&
-		test ! -d nested1/.git &&
+		test ! -f sub1/.git &&
+		test ! -f sub2/.git &&
+		test ! -f sub3/.git &&
+		test ! -f nested1/.git &&
 		git submodule update --init --recursive -- nested1 &&
-		test ! -d sub1/.git &&
-		test ! -d sub2/.git &&
-		test ! -d sub3/.git &&
-		test -d nested1/.git &&
-		test -d nested1/nested2/.git &&
-		test -d nested1/nested2/nested3/.git &&
-		test -d nested1/nested2/nested3/submodule/.git
+		test ! -f sub1/.git &&
+		test ! -f sub2/.git &&
+		test ! -f sub3/.git &&
+		test -n $(grep "/^gitdir: /" nested1/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/.git) &&
+		test -n $(grep "/^gitdir: /" nested1/nested2/nested3/submodule/.git)
 	)
 '
 
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index cc16d3f..ab37c36 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -43,7 +43,7 @@ git commit -m B-super-added'
 cd "$base_dir"
 
 test_expect_success 'after add: existence of info/alternates' \
-'test `wc -l <super/sub/.git/objects/info/alternates` = 1'
+'test `wc -l <super/.git/modules/sub/objects/info/alternates` = 1'
 
 cd "$base_dir"
 
@@ -66,7 +66,7 @@ test_expect_success 'update with reference' \
 cd "$base_dir"
 
 test_expect_success 'after update: existence of info/alternates' \
-'test `wc -l <super-clone/sub/.git/objects/info/alternates` = 1'
+'test `wc -l <super-clone/.git/modules/sub/objects/info/alternates` = 1'
 
 cd "$base_dir"
 

-- 
1.7.6.132.ga34b5.dirty
Junio C Hamano· Jul 21, 2011, 20:28 UTC · re: Fredrik Gustafsson · lore

Re: [RFC PATCH] Move git-dir for submodules

Fredrik Gustafsson <iveqy@iveqy.com> writes:
Show 11 quoted lines
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 87c9452..3ad3012 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -122,14 +122,56 @@ module_clone()
>  	path=$1
>  	url=$2
>  	reference="$3"
> +	gitdir=
> +	gitdir_base=
> +	base_path=`echo $path | sed -e 's|[^/]*$||'`

We prefer $() over `` these days, no? Without dq around $path, you would not be able to preserve $IFS inside $PATH. You are stripping a run of non slash at the trailing end --- is 'dirname "$path"' insufficient?

I think you are using the path the submodule happens to be at in the current checkout to decide where in the .git/modules in the superproject to keep the submodule metadata directory. Shouldn't you be using module_name to convert the $path to the name of the submodule (this is important, as the same submodule that used to be at path P1 can be moved to a different path P2 in the history).

Show 9 quoted lines
> +	if test -z "$GIT_DIR"
>  	then
> +		gitdir=$(git rev-parse --git-dir)
> +		gitdir_base="$gitdir/modules/$base_path"
> +		gitdir="$gitdir/modules/$path"
>  	else
> +		gitdir="$GIT_DIR/modules/$path"
> +		gitdir_base="$GIT_DIR/modules/$base_path"
> +	fi

Why do you need to switch on "test -z $GIT_DIR" yourself to have two paths? Doesn't "git rev-parse --git-dir" already know to take $GIT_DIR into account?

> +	case $gitdir in
> +		/*)
Indent case arm labels at the same level as "case/esac".
> +	if test -d "$gitdir"
> +	then
> +		mkdir -p "$path"
> +		echo "gitdir: $rel_gitdir" > "$path/.git"
Good: if it already exists, do not clone, but just reuse what .git/modules
hierarchy of the superproject has. Is it really necessary to have an ugly
loop to make things relative, though?
Also please lose the extra SP after redirection, i.e.
	command >"$path/.git"
> +	else
> +		if !(test -d "$gitdir_base")
Do you need subshell for this?
> +		then
> +			mkdir -p "$gitdir_base"
> +		fi
Doesn't unconditional "mkdir -p" do the right thing?
Show 9 quoted lines
> +		if test -n "$reference"
> +		then
> +			git-clone "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
> +		else
> +			git-clone -n "$url" "$path" --separate-git-dir "$gitdir"
> +		fi ||
> +		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
> +	fi
>  }
Show 13 quoted lines
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index b2b26b7..57f5306 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -358,10 +358,10 @@ test_expect_success 'update --init' '
>  	git submodule update init > update.out &&
>  	cat update.out &&
>  	test_i18ngrep "not initialized" update.out &&
> -	! test -d init/.git &&
> +	! test -f init/.git &&
>  	git submodule update --init init &&
> -	test -d init/.git
> +	test -n $(grep "/^gitdir: /" init/.git)
I wonder if we want a new option to "git rev-parse" so that we can say
	git rev-parse --is-well-formed-git-dir init/.git
to perform these checks without exposing the implimentation detail.
Show 11 quoted lines
> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
> index d600583..b0517f0 100755
> --- a/t/t7403-submodule-sync.sh
> +++ b/t/t7403-submodule-sync.sh
> @@ -55,7 +55,7 @@ test_expect_success '"git submodule sync" should update submodule URLs' '
>  	 git pull --no-recurse-submodules &&
>  	 git submodule sync
>  	) &&
> -	test -d "$(git config -f super-clone/submodule/.git/config \
> +	test -d "$(git config -f super-clone/.git/modules/submodule/config \
>  	                        remote.origin.url)" &&

Is "submodule" initialized at this point? If so, I would think it is vastly preferrable to say it like this:

	test -d "$(
        	cd super-clone/submodule &&
                git config --local remote.origin.url
	)"

I won't comment on the rest, but I think you can follow the same line of thought to come up with a fix so that they do not have to know the implementation detail of where the subproject metainfo directory is.

Phil Hord· Jul 21, 2011, 21:40 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] Move git-dir for submodules

On 07/21/2011 04:28 PM, Junio C Hamano wrote:
Show 23 quoted lines
> Fredrik Gustafsson<iveqy@iveqy.com>  writes:
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 87c9452..3ad3012 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -122,14 +122,56 @@ module_clone()
>>   	path=$1
>>   	url=$2
>>   	reference="$3"
>> +	gitdir=
>> +	gitdir_base=
>> +	base_path=`echo $path | sed -e 's|[^/]*$||'`
> We prefer $() over `` these days, no?  Without dq around $path, you would
> not be able to preserve $IFS inside $PATH. You are stripping a run of non
> slash at the trailing end --- is 'dirname "$path"' insufficient?
>
> I think you are using the path the submodule happens to be at in the
> current checkout to decide where in the .git/modules in the superproject
> to keep the submodule metadata directory. Shouldn't you be using
> module_name to convert the $path to the name of the submodule (this is
> important, as the same submodule that used to be at path P1 can be moved
> to a different path P2 in the history).

Now that you mention it, what happens if the submodule name changes in the history? What happens if the submodule URL (and possibly the entire submodule contents) changes in the history?

Is there any unique thing simpler than "<submodule-name>+<url>" that can be used to uniquely identify "that" submodule repo?

Say I have linux as a submodule in my project. But over time (or on different branches) I have different ideas about what should go there (and where it should go):

Commit      .gitmodules
   E           linux.path=linux ; 
linux.url=git://sources.blackfin.uclinux.org/git/linux-kernel
   D           linux.path=linux ; 
linux.url=https://github.com/mirrors/linux-2.6.git
   C           linux.path=linux ; 
linux.url=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
B           linux.path=linus ; 
linux.url=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
A           linus.path=linus ; 
linus.url=git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git

How many repos are in .git/modules/? I can imagine anything from 1-5 in the implementation. Logically there should be only 2, ideally (but not practically) named something like this:

   .git/modules/linux@blackfin
   .git/modules/linux@torvalds
and where the last one also includes "github/mirrors" as a remote.

But a more practical implementation would end up with three as it wouldn't know it could combine the last two:

   .git/modules/linux@blackfin
   .git/modules/linux@torvalds
   .git/modules/linux@github

But maybe a nearly practical implementation could actually wind up with one repo and three remotes:

   .git/modules/linux; remotes=blackfin, torvalds, github

But this is likely to confuse the poor user who did not expect all these remotes. Also, a realistic practical implementation probably would wind up like this:

   .git/modules/linux; remotes=blackfin, torvalds, github
   .git/modules/linus; remotes=torvalds

In reality I suppose switching repositories like this is simply not supported by the code or the patch. But it's something to think about when choosing a name for the .git/modules path.

Phil
Phil Hord· Jul 21, 2011, 21:50 UTC · re: Phil Hord · lore

Re: [RFC PATCH] Move git-dir for submodules

On 07/21/2011 05:40 PM, Phil Hord wrote:
Show 27 quoted lines
> On 07/21/2011 04:28 PM, Junio C Hamano wrote:
>> Fredrik Gustafsson<iveqy@iveqy.com>  writes:
>>
>>> diff --git a/git-submodule.sh b/git-submodule.sh
>>> index 87c9452..3ad3012 100755
>>> --- a/git-submodule.sh
>>> +++ b/git-submodule.sh
>>> @@ -122,14 +122,56 @@ module_clone()
>>>       path=$1
>>>       url=$2
>>>       reference="$3"
>>> +    gitdir=
>>> +    gitdir_base=
>>> +    base_path=`echo $path | sed -e 's|[^/]*$||'`
>> We prefer $() over `` these days, no?  Without dq around $path, you 
>> would
>> not be able to preserve $IFS inside $PATH. You are stripping a run of 
>> non
>> slash at the trailing end --- is 'dirname "$path"' insufficient?
>>
>> I think you are using the path the submodule happens to be at in the
>> current checkout to decide where in the .git/modules in the superproject
>> to keep the submodule metadata directory. Shouldn't you be using
>> module_name to convert the $path to the name of the submodule (this is
>> important, as the same submodule that used to be at path P1 can be moved
>> to a different path P2 in the history).
>
[ Sending this again to unwrap my table. Sorry for the noise. ]

Now that you mention it, what happens if the submodule name changes in the history? What happens if the submodule URL (and possibly the entire submodule contents) changes in the history?

Is there any unique thing simpler than "<submodule-name>+<url>" that can be used to uniquely identify "that" submodule repo?

Say I have linux as a submodule in my project. But over time (or on different branches) I have different ideas about what should go there. what it should be called and where it should go:

Commit      .gitmodules
  E           linux.path=linux ; linux.url=/blackfin/uclinux-kernel
  D           linux.path=linux ; linux.url=/github/mirrors/linux-2.6.git
  C           linux.path=linux ; linux.url=/torvalds/linux-2.6.git
  B           linux.path=linus ; linux.url=/torvalds/linux-2.6.git
  A           linus.path=linus ; linus.url=/torvalds/linux-2.6.git

How many repos are in .git/modules/? I can imagine anything from 1-5 in the implementation. Logically there should be only 2, ideally (but not practically) named something like this:

  .git/modules/linux@blackfin
  .git/modules/linux@torvalds
and where the last one also includes "github/mirrors" as a remote.

But a more practical implementation would end up with three as it wouldn't presume to combine the last two:

  .git/modules/linux@blackfin
  .git/modules/linux@torvalds
  .git/modules/linux@github

But maybe a nearly practical implementation could actually wind up with one repo and three remotes:

  .git/modules/linux; remotes=blackfin, torvalds, github

But this is likely to confuse the poor user who did not expect all these remotes. A realistic practical implementation probably would wind up more like this:

  .git/modules/linux; remotes=blackfin, torvalds, github
  .git/modules/linus; remotes=torvalds

In reality I suppose switching repositories like this is simply not supported by the code or the patch. But it's something to think about when choosing a name for the .git/modules path.

Phil
> -- 
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Junio C Hamano· Jul 21, 2011, 23:37 UTC · re: Phil Hord · lore

Re: [RFC PATCH] Move git-dir for submodules

Phil Hord <hordp@cisco.com> writes:
> Now that you mention it, what happens if the submodule name changes in
> the history?  What happens if the submodule URL (and possibly the entire
> submodule contents) changes in the history?

The original idea (dating back before "git submodule" was written, but only when "gitlink" was added to the index and the tree objects) was to identify the submodule by name and refer to the same logical entity with the same name, so that no matter where in the superproject's working tree hierarchy you change your mind to bind a particular submodule (say, "the Linux kernel upstream source") at (i.e. "path"), or from which mirror of the kernel repository you recommend your downstream to clone it from (i.e. "url" in .gitmodules).

So "What happens if the submodule name changes" is a nonsense question.

But you remind me an ancient design discussion regarding the submodule URL (you should dig the list archive if you are interested in reviving the discussion of this issue), that hasn't been implemented.

It was envisioned that we sh/could record (in superproject's .git/config somewhere) all the values of submodule.$name.url the user has ever seen (via checkouts and branch switching), the value of submodule.$name.url that was recorded in .gitmodules, along with what URL the user has actually chosen to use given the set of these values.

Imagine that the original version of the superproject recommended A as the URL for the submodule S, and then later versions of the superproject recommend B as its URL because the project it borrows its submodule has migrated to a new home.

A possible user experience under this scenario may go like this:
    $ git clone superproject && cd superproject
    $ git submodule init S
        The user hasn't seen any submodule.S.url yet, as this is
        immediately after cloning. .gitmodules records A and this
        command would ask "Do you want to clone from A [Y/n]?" or use A
        without asking.
        It also records that when given A, the user chose A (or something
        else, such as its known mirror A').
    Time passes.  The superproject changes the url for submodule S.
    $ git pull
        Now .gitmodules has B in submodule.S.url, which is different from
        anything the user has seen (recorded in .git/config of
        superproject). Any "git submodule" subcommand that would try to
        update submodule S from its upstream _should_ offer the user a
        chance to notice this, and update the actual URL in S/.git/config
	to fetch from.  It may ask "Do you want to update from B [Y/n]? "
        and some other choices (e.g. keep using A' may be an option).
	It also records that after seeing A and B, and to update the
	version of superproject that records B as the URL for S, the user
	chose to use what exact URL to update S (it may be A, B, or A', or
	it may be something entirely different).
    $ git checkout -b side master~400 ;# or whatever old version
	After switching to an ancient version, .gitmodules has A. Perhaps
	the module needs to be updated from A, or the module needs to be
	updated from B (because A is defunct and no longer working, but B
	records exactly the same history as A used to). But the record in
	the superproject does not show what URL the user wants to use,
	when A and B are the choices the user has seen, and A is recorded 
	in .gitmodules, so we ask again, and record the choice.
    After this, we know that given the knowledge of presence of A and B,
    what URL the user wants to use when .gitmodules records A (or B) as
    the URL for the submodule, so switching to _any_ version in the
    history will not have to ask the user what URL should be used to
    interact with the upstream of submodule S.

Note that the above scenario can be loosened by assuming that B is always a superset of A and newer recommendation is always preferred. Then the mapping from the situation to the actual URL can be simplified to "Given these URLs the user has seen, the user wants to use this URL", not "Given these URLs the user has seen and this URL recorded in .gitmodules for the particular version, the user wants to use this URL" as the above scenario outlined.

Junio C Hamano· Jul 21, 2011, 23:47 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] Move git-dir for submodules

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> Phil Hord <hordp@cisco.com> writes:
>
>> Now that you mention it, what happens if the submodule name changes in
>> the history?  What happens if the submodule URL (and possibly the entire
>> submodule contents) changes in the history?
>
> The original idea (dating back before "git submodule" was written, but
> only when "gitlink" was added to the index and the tree objects) was to
> identify the submodule by name and refer to the same logical entity with
> the same name, so that no matter where in the superproject's working tree
> hierarchy you change your mind to bind a particular submodule (say, "the
> Linux kernel upstream source") at (i.e. "path"), or from which mirror of
> the kernel repository you recommend your downstream to clone it from
> (i.e. "url" in .gitmodules).

Sorry for an incomplete sentence. "No matter...." above should conclude with "you know you are talking about the same module that retains the identity of its .git metadata directory."

Fredrik Gustafsson· Jul 22, 2011, 19:02 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] Move git-dir for submodules

Thanks for the review. I'll send an updated series soon.
On Thu, Jul 21, 2011 at 01:28:50PM -0700, Junio C Hamano wrote:
> Is it really necessary to have an ugly
> loop to make things relative, though?

The path must be relative to allow the superproject to be moved around.

Show 5 quoted lines
> I wonder if we want a new option to "git rev-parse" so that we can say
> 
> 	git rev-parse --is-well-formed-git-dir init/.git
> 
> to perform these checks without exposing the implimentation detail.

How about using (cd <repo>; git rev-parse --verify HEAD" >/dev/null) instead?

Med vänliga hälsningar Fredrik Gustafsson

Junio C Hamano· Jul 22, 2011, 20:30 UTC · re: Fredrik Gustafsson · lore

Re: [RFC PATCH] Move git-dir for submodules

Fredrik Gustafsson <iveqy@iveqy.com> writes:
Show 9 quoted lines
>> I wonder if we want a new option to "git rev-parse" so that we can say
>> 
>> 	git rev-parse --is-well-formed-git-dir init/.git
>> 
>> to perform these checks without exposing the implimentation detail.
>
> How about using
> (cd <repo>; git rev-parse --verify HEAD" >/dev/null)
> instead?
Can the <repo> be without any commit, i.e. just after created?

← back to recent threads