[PATCH v3 0/4] submodules: Use relative paths to gitdir and work tree

[PATCH v3 0/4] submodules: Use relative paths to gitdir and work tree

[Jens Lehmann]

This is the third round of the former two patch series. It applies
cleanly on current master and maint and IMO is maint stuff as it fixes
bugs related to submodules. Those were introduced when moving the git
directory into that of the superproject and using a gitfile instead.

The first patch is unchanged from version one, in the second I did
address the concerns raised about the while loop. Those two patches
make superprojects movable again.

The third patch started as a mere refactoring, but while working on
it I noticed it also fixes an issue when submodules are moved around
inside a superproject.

The forth patch is provided by Johannes Sixt and addresses the issue
that under windows both POSIX and Windows style paths may occur which
sometimes broke the computation of relative paths.

Now all issues I'm aware of are fixed and tested for. In the next step
I'll be looking into teaching git mv to handle submodules, as just
mv'ing them someplace else, upating .gitmodules and adding the paths
isn't sufficient anymore. The core.worktree setting - and sometimes the
gitfile too (when the submodule is moved to a different directory level)
- have to be updated too to make that work (That would be easier if git
could treat the directory a gitfile was found in as a candidate for the
work tree, as then we could get rid of the core.worktree setting, but
we don't have that functionality).

Jens Lehmann (4):
  submodules: always use a relative path to gitdir
  submodules: always use a relative path from gitdir to work tree
  submodules: refactor computation of relative gitdir path
  submodules: fix ambiguous absolute paths under Windows

 git-submodule.sh            |   58 +++++++++++++++++++++----------------------
 t/t7400-submodule-basic.sh  |   22 ++++++++++++++++
 t/t7406-submodule-update.sh |   17 +++++++++++++
 3 files changed, 68 insertions(+), 29 deletions(-)

-- 
1.7.9.2.362.g684a8

[PATCH v3 1/4] submodules: always use a relative path to gitdir

[Jens Lehmann]

Since recently a submodule with name <name> has its git directory in the
.git/modules/<name> directory of the superproject while the work tree
contains a gitfile pointing there. When the submodule git directory needs
to be cloned because it is not found in .git/modules/<name> the clone
command will write an absolute path into the gitfile. When no clone is
necessary the git directory will be reactivated by the git-submodule.sh
script by writing a relative path into the gitfile.

This is inconsistent, as the behavior depends on the submodule having been
cloned before into the .git/modules of the superproject. A relative path
is preferable here because it allows the superproject to be moved around
without invalidating the gitfile. We do that by always writing the
relative path into the gitfile, which overwrites the absolute path the
clone command may have written there.

This is only the first step to make superprojects movable again like they
were before the separate-git-dir approach was introduced. The second step
is to use a relative path in core.worktree too.

Enhance t7400 to ensure that future versions won't re-add absolute paths
by accident.

While at it also replace an if/else construct evaluating the presence
of the 'reference' option with a single line of bash code.

Reported-by: Antony Male <antony.male@gmail.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-submodule.sh           |   11 ++++-------
 t/t7400-submodule-basic.sh |    2 ++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9bb2e13..2a93c61 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -160,18 +160,15 @@ module_clone()
 	if test -d "$gitdir"
 	then
 		mkdir -p "$path"
-		echo "gitdir: $rel_gitdir" >"$path/.git"
 		rm -f "$gitdir/index"
 	else
 		mkdir -p "$gitdir_base"
-		if test -n "$reference"
-		then
-			git-clone $quiet "$reference" -n "$url" "$path" --separate-git-dir "$gitdir"
-		else
-			git-clone $quiet -n "$url" "$path" --separate-git-dir "$gitdir"
-		fi ||
+		git clone $quiet -n ${reference:+"$reference"} \
+			--separate-git-dir "$gitdir" "$url" "$path" ||
 		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
 	fi
+
+	echo "gitdir: $rel_gitdir" >"$path/.git"
 }

 #
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 695f7af..2b70b22 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -79,6 +79,8 @@ test_expect_success 'submodule add' '
 		cd addtest &&
 		git submodule add -q "$submodurl" submod >actual &&
 		test ! -s actual &&
+		echo "gitdir: ../.git/modules/submod" >expect &&
+		test_cmp expect submod/.git &&
 		git submodule init
 	) &&

-- 
1.7.9.2.362.g684a8

[PATCH v3 2/4] submodules: always use a relative path from gitdir to work tree

[Jens Lehmann]

Since recently a submodule with name <name> has its git directory in the
.git/modules/<name> directory of the superproject while the work tree
contains a gitfile pointing there. To make that work the git directory has
the core.worktree configuration set in its config file to point back to
the work tree.

That core.worktree is an absolute path set by the initial clone of the
submodule. A relative path is preferable here because it allows the
superproject to be moved around without invalidating that setting, so
compute and set that relative path after cloning or reactivating the
submodule.

This also fixes a bug when moving a submodule around inside the
superproject, as the current code forgot to update the setting to the new
submodule work tree location.

Enhance t7400 to ensure that future versions won't re-add absolute paths
by accident and that moving a superproject won't break submodules.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-submodule.sh           |   18 ++++++++++++++++++
 t/t7400-submodule-basic.sh |   20 ++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2a93c61..c405caa 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -169,6 +169,24 @@ module_clone()
 	fi

 	echo "gitdir: $rel_gitdir" >"$path/.git"
+
+	a=$(cd "$gitdir" && pwd)/
+	b=$(cd "$path" && pwd)/
+	# Remove all common leading directories after a sanity check
+	if test "${a#$b}" != "$a" || test "${b#$a}" != "$b"; then
+		die "$(eval_gettext "Gitdir '\$a' is part of the submodule path '\$b' or vice versa")"
+	fi
+	while test "${a%%/*}" = "${b%%/*}"
+	do
+		a=${a#*/}
+		b=${b#*/}
+	done
+	# Now chop off the trailing '/'s that were added in the beginning
+	a=${a%/}
+	b=${b%/}
+
+	rel=$(echo $a | sed -e 's|[^/]*|..|g')
+	(clear_local_git_env; cd "$path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b")
 }

 #
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2b70b22..b377a7a 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -81,6 +81,13 @@ test_expect_success 'submodule add' '
 		test ! -s actual &&
 		echo "gitdir: ../.git/modules/submod" >expect &&
 		test_cmp expect submod/.git &&
+		(
+			cd submod &&
+			git config core.worktree >actual &&
+			echo "../../../submod" >expect &&
+			test_cmp expect actual &&
+			rm -f actual expect
+		) &&
 		git submodule init
 	) &&

@@ -500,4 +507,17 @@ test_expect_success 'relative path works with user@host:path' '
 	)
 '

+test_expect_success 'moving the superproject does not break submodules' '
+	(
+		cd addtest &&
+		git submodule status >expect
+	)
+	mv addtest addtest2 &&
+	(
+		cd addtest2 &&
+		git submodule status >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
1.7.9.2.362.g684a8

[PATCH v3 3/4] submodules: refactor computation of relative gitdir path

[Jens Lehmann]

In module_clone() the rel_gitdir variable was computed differently when
"git rev-parse --git-dir" returned a relative path than when it returned
an absolute path. This is not optimal, as different code paths are used
depending on the return value of that command.

Fix that by reusing the differing path components computed for setting the
core.worktree config setting, which leaves a single code path for setting
both instead of having three and makes the code much shorter.

This also fixes the bug that in the computation of how many directories
have to be traversed up to hit the root directory of the submodule the
name of the submodule was used where the path should have been used. This
lead to problems after renaming submodules into another directory level.

Even though the "(cd $somewhere && pwd)" approach breaks the flexibility
of symlinks, that is no issue here as we have to have one relative path
pointing from the work tree to the gitdir and another pointing back, which
will never work anyway when a symlink along one of those paths is changed
because the directory it points to was moved.

Also add a test moving a submodule into a deeper directory to catch any
future breakage here and to document what has to be done when a submodule
needs to be moved until git mv learns to do that. Simply moving it to the
new location doesn't work, as the core.worktree and possibly the gitfile
setting too will be wrong. So it has to be removed from filesystem and
index, then the new location has to be added into the index and the
.gitmodules file has to be updated. After that a git submodule update will
check out the submodule at the new location.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-submodule.sh            |   30 ++++++------------------------
 t/t7406-submodule-update.sh |   17 +++++++++++++++++
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index c405caa..a9e9822 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -132,30 +132,11 @@ module_clone()
 	gitdir_base=
 	name=$(module_name "$path" 2>/dev/null)
 	test -n "$name" || name="$path"
-	base_path=$(dirname "$path")
+	base_name=$(dirname "$name")

 	gitdir=$(git rev-parse --git-dir)
-	gitdir_base="$gitdir/modules/$base_path"
-	gitdir="$gitdir/modules/$path"
-
-	case $gitdir in
-	/*)
-		a="$(cd_to_toplevel && pwd)/"
-		b=$gitdir
-		while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
-		do
-			a=${a#*/} b=${b#*/};
-		done
-
-		rel="$a$name"
-		rel=`echo $rel | sed -e 's|[^/]*|..|g'`
-		rel_gitdir="$rel/$b"
-		;;
-	*)
-		rel=`echo $name | sed -e 's|[^/]*|..|g'`
-		rel_gitdir="$rel/$gitdir"
-		;;
-	esac
+	gitdir_base="$gitdir/modules/$base_name"
+	gitdir="$gitdir/modules/$name"

 	if test -d "$gitdir"
 	then
@@ -168,8 +149,6 @@ module_clone()
 		die "$(eval_gettext "Clone of '\$url' into submodule path '\$path' failed")"
 	fi

-	echo "gitdir: $rel_gitdir" >"$path/.git"
-
 	a=$(cd "$gitdir" && pwd)/
 	b=$(cd "$path" && pwd)/
 	# Remove all common leading directories after a sanity check
@@ -185,6 +164,9 @@ module_clone()
 	a=${a%/}
 	b=${b%/}

+	rel=$(echo $b | sed -e 's|[^/]*|..|g')
+	echo "gitdir: $rel/$a" >"$path/.git"
+
 	rel=$(echo $a | sed -e 's|[^/]*|..|g')
 	(clear_local_git_env; cd "$path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b")
 }
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 5b97222..dcb195b 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -619,4 +619,21 @@ test_expect_success 'submodule add properly re-creates deeper level submodules'
 	)
 '

+test_expect_success 'submodule update properly revives a moved submodule' '
+	(cd super &&
+	 git commit -am "pre move" &&
+	 git status >expect&&
+	 H=$(cd submodule2; git rev-parse HEAD) &&
+	 git rm --cached submodule2 &&
+	 rm -rf submodule2 &&
+	 mkdir -p "moved/sub module" &&
+	 git update-index --add --cacheinfo 160000 $H "moved/sub module" &&
+	 git config -f .gitmodules submodule.submodule2.path "moved/sub module"
+	 git commit -am "post move" &&
+	 git submodule update &&
+	 git status >actual &&
+	 test_cmp expect actual
+	)
+'
+
 test_done
-- 
1.7.9.2.362.g684a8

[PATCH v3 4/4] submodules: fix ambiguous absolute paths under Windows

[Jens Lehmann]

From: Johannes Sixt <j6t@kdbg.org>

Under Windows the "git rev-parse --git-dir" and "pwd" commands may return
either drive-letter-colon or POSIX style paths. This makes module_clone()
behave badly because it expects absolute paths to always start with a '/'.

Fix that by always converting the "c:/" notation into "/c/" when computing
the relative paths from gitdir to the submodule work tree and back.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-submodule.sh |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-submodule.sh b/git-submodule.sh
index a9e9822..efc86ad 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -151,6 +151,9 @@ module_clone()

 	a=$(cd "$gitdir" && pwd)/
 	b=$(cd "$path" && pwd)/
+	# normalize Windows-style absolute paths to POSIX-style absolute paths
+	case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac
+	case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac
 	# Remove all common leading directories after a sanity check
 	if test "${a#$b}" != "$a" || test "${b#$a}" != "$b"; then
 		die "$(eval_gettext "Gitdir '\$a' is part of the submodule path '\$b' or vice versa")"
-- 
1.7.9.2.362.g684a8

Re: [PATCH v3 0/4] submodules: Use relative paths to gitdir and work tree

[Junio C Hamano]

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

> This is the third round of the former two patch series. It applies
> cleanly on current master and maint and IMO is maint stuff as it fixes
> bugs related to submodules. Those were introduced when moving the git
> directory into that of the superproject and using a gitfile instead.

I looked at this again and think this addresses the review comments
made during the previous round, but I would like to get "Yay" from
people who helped reviewing the previous round.

Thanks.

Re: [PATCH v3 0/4] submodules: Use relative paths to gitdir and work tree

[Johannes Sixt]

Am 07.03.2012 07:31, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> This is the third round of the former two patch series. It applies
>> cleanly on current master and maint and IMO is maint stuff as it fixes
>> bugs related to submodules. Those were introduced when moving the git
>> directory into that of the superproject and using a gitfile instead.
> 
> I looked at this again and think this addresses the review comments
> made during the previous round, but I would like to get "Yay" from
> people who helped reviewing the previous round.

Yay. The topic passes the test suite.

-- Hannes