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

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

[Jens Lehmann]

This patch series replaces all absolute paths pointing from submodule work
trees to its gitdir and back with relative paths as discussed in $gmane/187785.

The motivation is to make superprojects movable again. They lost this ability
with the move of the git directory of submodules into the .git/modules directory
of the superproject. While fixing that a bug which would hit when moving the
submodule inside the superproject was also fixed.

Jens Lehmann (2):
  submodules: always use a relative path to gitdir
  submodules: always use a relative path from gitdir to work tree

 git-submodule.sh           |   23 ++++++++++++++++-------
 t/t7400-submodule-basic.sh |   22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

-- 
1.7.9.190.gb17a42.dirty

[PATCH 1/2] 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.190.gb17a42.dirty

[PATCH 2/2] 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           |    9 +++++++++
 t/t7400-submodule-basic.sh |   20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2a93c61..3463d6d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -169,6 +169,15 @@ module_clone()
     fi
 
     echo "gitdir: $rel_gitdir" >"$path/.git"
+
+    a=$(cd "$gitdir" && pwd)
+    b=$(cd "$path" && pwd)
+    while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
+    do
+        a=${a#*/} b=${b#*/};
+    done
+    rel=$(echo $a | sed -e 's|[^/]*|..|g')
+    (clear_local_git_env; cd "$path" && 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.190.gf8e73.dirty

[PATCH v2 1/2] 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>
---

The first version was whitespace damaged, please use this one instead.

 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.190.g0a6c2

[PATCH v2 2/2] 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>
---

The first version was whitespace damaged, please use this instead.

 git-submodule.sh           |    9 +++++++++
 t/t7400-submodule-basic.sh |   20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

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

 	echo "gitdir: $rel_gitdir" >"$path/.git"
+
+	a=$(cd "$gitdir" && pwd)
+	b=$(cd "$path" && pwd)
+	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
+	do
+		a=${a#*/} b=${b#*/};
+	done
+	rel=$(echo $a | sed -e 's|[^/]*|..|g')
+	(clear_local_git_env; cd "$path" && 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.190.g0a6c2

Re: [PATCH v2 1/2] submodules: always use a relative path to gitdir

[Junio C Hamano]

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

> Since recently a submodule with name <name> has its git directory in the

I understand that "recently" refers to efc5fb6a77 and this is potentially
a maint material for v1.7.8.X; am I correct?

> The first version was whitespace damaged, please use this one instead.

Thanks. 

When applied to v1.7.8.x maintenance track, this failed the first step in
t5526, but with the earlier jl/submodule-re-add topic everything seems to
pass just fine.

Re: [PATCH v2 1/2] submodules: always use a relative path to gitdir

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> When applied to v1.7.8.x maintenance track, this failed the first step in
> t5526, but with the earlier jl/submodule-re-add topic everything seems to
> pass just fine.

What I meant to say is that "this and the other topic need to be merged to
that maintenance track together"; I didn't mean to say there is any
breakage in this patch.

Sorry for a potentially confusing statement.

Re: [PATCH v2 1/2] submodules: always use a relative path to gitdir

[Jens Lehmann]

Am 09.02.2012 20:40, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Since recently a submodule with name <name> has its git directory in the
> 
> I understand that "recently" refers to efc5fb6a77 and this is potentially
> a maint material for v1.7.8.X; am I correct?

That's correct.

>> The first version was whitespace damaged, please use this one instead.
> 
> Thanks. 
> 
> When applied to v1.7.8.x maintenance track, this failed the first step in
> t5526, but with the earlier jl/submodule-re-add topic everything seems to
> pass just fine.

Ah, I only tested these patches on current master. But you are right,
this belongs on top of jl/submodule-re-add for maint.

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

[Junio C Hamano]

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

> The first version was whitespace damaged, please use this instead.

Thanks.  This one looks somewhat iffy (1/2 looked perfectly fine, though).

> +	a=$(cd "$gitdir" && pwd)
> +	b=$(cd "$path" && pwd)
> +	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
> +	do
> +		a=${a#*/} b=${b#*/};
> +	done
> +	rel=$(echo $a | sed -e 's|[^/]*|..|g')
> +	(clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
>  }


The style ([ "$b" ] vs "test -n "$b") aside, it strikes me odd that you
only check $b; it is unclear what guarantees that "$a" is always longer.
Maybe there is a reason but then a one-line comment here would not hurt?

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

[Jens Lehmann]

Am 13.02.2012 20:59, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>> +	a=$(cd "$gitdir" && pwd)
>> +	b=$(cd "$path" && pwd)
>> +	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>> +	do
>> +		a=${a#*/} b=${b#*/};
>> +	done
>> +	rel=$(echo $a | sed -e 's|[^/]*|..|g')
>> +	(clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
>>  }
> 
> 
> The style ([ "$b" ] vs "test -n "$b") aside, it strikes me odd that you
> only check $b; it is unclear what guarantees that "$a" is always longer.
> Maybe there is a reason but then a one-line comment here would not hurt?

I just copied that while loop from a few lines up, but you are right
about the style and logic issues (Will send a cleanup patch for the
other loop too when we agree on how to write this one).

After adding a comment, using test instead of [], testing both $a and
$b and assigning each variable on it's own line I get the following
interdiff. Does that make more sense?

diff --git a/git-submodule.sh b/git-submodule.sh
index 3463d6d..ed76ce2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -172,9 +172,11 @@ module_clone()

        a=$(cd "$gitdir" && pwd)
        b=$(cd "$path" && pwd)
-       while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
+       # Remove all common leading directories
+       while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
        do
-               a=${a#*/} b=${b#*/};
+               a=${a#*/}
+               b=${b#*/}
        done
        rel=$(echo $a | sed -e 's|[^/]*|..|g')
        (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")

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

[Junio C Hamano]

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

> After adding a comment, using test instead of [], testing both $a and
> $b and assigning each variable on it's own line I get the following
> interdiff. Does that make more sense?

My earlier request for comment was to say

	# $a is always longer than $b for such and such reasons

to explain why testing $b without testing $a was sufficient.

It is obvious (at least to me) that the loop continues as long as $a and
$b begin with the same string before their first '/' and removes that
common segment from both of them, so I do not think the new comment is
absolutely necessary, but it would not hurt to have it, especially it is
short enough and to the point.

Thanks.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3463d6d..ed76ce2 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -172,9 +172,11 @@ module_clone()
>
>         a=$(cd "$gitdir" && pwd)
>         b=$(cd "$path" && pwd)
> -       while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
> +       # Remove all common leading directories
> +       while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>         do
> -               a=${a#*/} b=${b#*/};
> +               a=${a#*/}
> +               b=${b#*/}
>         done
>         rel=$(echo $a | sed -e 's|[^/]*|..|g')
>         (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> After adding a comment, using test instead of [], testing both $a and
>> $b and assigning each variable on it's own line I get the following
>> interdiff. Does that make more sense?
>
> My earlier request for comment was to say
>
> 	# $a is always longer than $b for such and such reasons
>
> to explain why testing $b without testing $a was sufficient.

Heh, after I follow the entire module_clone, $gitdir is defined in earlier
parts of the function to be "$(rev-parse --git-dir)/modules/$path", so it
is clear that it is longer than $path.  Unless "cd $there && pwd" does not
result in a funny situation (such as $something/modules is a symbolic link
to another place that is much closer to the root of the filesystem), that
is.

And in such a case, the prefix part of $a and $b would be different from
the very beginning hopefully.

> It is obvious (at least to me) that the loop continues as long as $a and
> $b begin with the same string before their first '/' and removes that
> common segment from both of them, so I do not think the new comment is
> absolutely necessary, but it would not hurt to have it, especially it is
> short enough and to the point.
>
> Thanks.
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 3463d6d..ed76ce2 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -172,9 +172,11 @@ module_clone()
>>
>>         a=$(cd "$gitdir" && pwd)
>>         b=$(cd "$path" && pwd)
>> -       while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>> +       # Remove all common leading directories
>> +       while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>>         do
>> -               a=${a#*/} b=${b#*/};
>> +               a=${a#*/}
>> +               b=${b#*/}
>>         done
>>         rel=$(echo $a | sed -e 's|[^/]*|..|g')

Perhaps aseert that $a never becomes empty before this line (or set it
explicitly to "." when $a is empty), as otherwise

>>         (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")

this will refer to "/$b" from the root?

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

[Jens Lehmann]

Am 14.02.2012 21:34, schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> After adding a comment, using test instead of [], testing both $a and
>>> $b and assigning each variable on it's own line I get the following
>>> interdiff. Does that make more sense?
>>
>> My earlier request for comment was to say
>>
>> 	# $a is always longer than $b for such and such reasons
>>
>> to explain why testing $b without testing $a was sufficient.
> 
> Heh, after I follow the entire module_clone, $gitdir is defined in earlier
> parts of the function to be "$(rev-parse --git-dir)/modules/$path", so it
> is clear that it is longer than $path.

Unfortunately only by accident. The usage of $path is not correct here,
$name should be used instead (I have a patch in the making to correct that,
but as that hits the same code area as these fixes I'll post that later
together with some tests moving submodules around inside a superproject).
Then the result of "$(rev-parse --git-dir)/modules/$name" can be shorter
than "$path" when a submodule is renamed into a higher directory level.

> Unless "cd $there && pwd" does not
> result in a funny situation (such as $something/modules is a symbolic link
> to another place that is much closer to the root of the filesystem), that
> is.
> 
> And in such a case, the prefix part of $a and $b would be different from
> the very beginning hopefully.

Yes, they should differ somewhere in any sane setup I can imagine.

>> It is obvious (at least to me) that the loop continues as long as $a and
>> $b begin with the same string before their first '/' and removes that
>> common segment from both of them, so I do not think the new comment is
>> absolutely necessary, but it would not hurt to have it, especially it is
>> short enough and to the point.
>>
>> Thanks.
>>
>>> diff --git a/git-submodule.sh b/git-submodule.sh
>>> index 3463d6d..ed76ce2 100755
>>> --- a/git-submodule.sh
>>> +++ b/git-submodule.sh
>>> @@ -172,9 +172,11 @@ module_clone()
>>>
>>>         a=$(cd "$gitdir" && pwd)
>>>         b=$(cd "$path" && pwd)
>>> -       while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>>> +       # Remove all common leading directories
>>> +       while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>>>         do
>>> -               a=${a#*/} b=${b#*/};
>>> +               a=${a#*/}
>>> +               b=${b#*/}
>>>         done
>>>         rel=$(echo $a | sed -e 's|[^/]*|..|g')
> 
> Perhaps aseert that $a never becomes empty before this line (or set it
> explicitly to "." when $a is empty), as otherwise
> 
>>>         (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
> 
> this will refer to "/$b" from the root?

I think neither $a nor $b should be empty after that. But thinking deeper
about that while loop I suspect the real problem here is doing "a=${a#*/}"
or "b=${b#*/}" on a string that doesn't contain a slash anymore. We'll
happily remove a leading directory on the other path while the one without
slash will stay untouched, leading to a bogus result which is off by one
directory level.

AFAICS that will only happen when one path is a prefix of the other, which
is a pretty pathological case. So I'll whip up a new version asserting
that beforehand and dropping the -n test in the while loop, ok?

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

[Johannes Sixt]

Am 08.02.2012 23:08, schrieb Jens Lehmann:
> This patch series replaces all absolute paths pointing from submodule work
> trees to its gitdir and back with relative paths as discussed in $gmane/187785.
> 
> The motivation is to make superprojects movable again. They lost this ability
> with the move of the git directory of submodules into the .git/modules directory
> of the superproject. While fixing that a bug which would hit when moving the
> submodule inside the superproject was also fixed.
> 
> Jens Lehmann (2):
>   submodules: always use a relative path to gitdir
>   submodules: always use a relative path from gitdir to work tree

This series, with the tip at e3307adaba in Junio's repo causes major
headaches on Windows.

First, a check for an absolute path must be extended to take
Windows-style paths into account.

Second, the a's and b's are filled with different forms of absolute
paths (/c/there vs. c:/there), and as a consequence the subsequent loops
do not find a suitable relative path.

The below is a minimal hack that passes all t/*submod* tests, but it
works only on Windows, where the pwd utility has an option -W that
prints a Windows style absolute path.

How would you have this solved? One option would be to introduce a function

  pwd() { builtin pwd -W "$@"; }

in git-sh-setup conditionally on Windows (but that would affect other
shell scripts, too).

Any other ideas?

diff --git a/git-submodule.sh b/git-submodule.sh
index 3463d6d..f37745e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -139,8 +139,8 @@ module_clone()
 	gitdir="$gitdir/modules/$path"

 	case $gitdir in
-	/*)
-		a="$(cd_to_toplevel && pwd)/"
+	/* | [a-z]:/*)
+		a="$(cd_to_toplevel && pwd -W)/"
 		b=$gitdir
 		while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
 		do
@@ -170,8 +170,8 @@ module_clone()

 	echo "gitdir: $rel_gitdir" >"$path/.git"

-	a=$(cd "$gitdir" && pwd)
-	b=$(cd "$path" && pwd)
+	a=$(cd "$gitdir" && pwd -W)
+	b=$(cd "$path" && pwd -W)
 	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
 	do
 		a=${a#*/} b=${b#*/};
-- 
1.7.8.216.g2e426

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

[Jens Lehmann]

Am 26.02.2012 18:38, schrieb Johannes Sixt:
> Am 08.02.2012 23:08, schrieb Jens Lehmann:
>> This patch series replaces all absolute paths pointing from submodule work
>> trees to its gitdir and back with relative paths as discussed in $gmane/187785.
>>
>> The motivation is to make superprojects movable again. They lost this ability
>> with the move of the git directory of submodules into the .git/modules directory
>> of the superproject. While fixing that a bug which would hit when moving the
>> submodule inside the superproject was also fixed.
>>
>> Jens Lehmann (2):
>>   submodules: always use a relative path to gitdir
>>   submodules: always use a relative path from gitdir to work tree
> 
> This series, with the tip at e3307adaba in Junio's repo causes major
> headaches on Windows.
>
> First, a check for an absolute path must be extended to take
> Windows-style paths into account.

Okay, but that check is not part of my series (it was introduced by 501770e1
"Move git-dir for submodules", which is in Git since 1.7.8), so that looks
like it would need to be fixed for msysgit even without my patches, right?

But I'm not so happy about the two code paths there anyway, so I prepared a
patch to replace them with a single code path based upon the paths computed
in the last patch of this series. Please see the always-use-relative-gitdir
branch in my github repo https://github.com/jlehmann/git-submod-enhancements

> Second, the a's and b's are filled with different forms of absolute
> paths (/c/there vs. c:/there), and as a consequence the subsequent loops
> do not find a suitable relative path.
> 
> The below is a minimal hack that passes all t/*submod* tests, but it
> works only on Windows, where the pwd utility has an option -W that
> prints a Windows style absolute path.
> 
> How would you have this solved? One option would be to introduce a function
> 
>   pwd() { builtin pwd -W "$@"; }
> 
> in git-sh-setup conditionally on Windows (but that would affect other
> shell scripts, too).

I suspect other shell scripts might be less affected when non-Windows
paths are forced (at least when they aren't developed under Windows
only). What about something like this:

  pwd() { builtin pwd -W "$@" | sed -e 's,^\([a-z]\):/,/\1/,'; }

> Any other ideas?
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 3463d6d..f37745e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -139,8 +139,8 @@ module_clone()
>  	gitdir="$gitdir/modules/$path"
>
>  	case $gitdir in
> -	/*)
> -		a="$(cd_to_toplevel && pwd)/"
> +	/* | [a-z]:/*)
> +		a="$(cd_to_toplevel && pwd -W)/"
>  		b=$gitdir
>  		while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>  		do

Hmm, here the path which starts with "c:/" is returned by the "git
rev-parse --git-dir" which is used to initialize the $gitdir variable
a few lines up.

> @@ -170,8 +170,8 @@ module_clone()
>
>  	echo "gitdir: $rel_gitdir" >"$path/.git"
>
> -	a=$(cd "$gitdir" && pwd)
> -	b=$(cd "$path" && pwd)
> +	a=$(cd "$gitdir" && pwd -W)
> +	b=$(cd "$path" && pwd -W)
>  	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>  	do
>  		a=${a#*/} b=${b#*/};

I don't understand why you need this. Does "pwd" sometimes return a
path starting with "c:/" and sometimes "/c/" depending on what form
you use when you cd into that directory? If not, does the following
help you on windows? (If that is the case, you might need the pwd
redefinition too to make that work)

-----------------8<--------------------
diff --git a/git-submodule.sh b/git-submodule.sh
index 5deabf6..5bb8109 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -134,7 +134,7 @@ module_clone()
        test -n "$name" || name="$path"
        base_path=$(dirname "$path")

-       gitdir=$(git rev-parse --git-dir)
+       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
        gitdir_base="$gitdir/modules/$base_path"
        gitdir="$gitdir/modules/$path"

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

[Johannes Sixt]

Am 26.02.2012 20:58, schrieb Jens Lehmann:
> Am 26.02.2012 18:38, schrieb Johannes Sixt:
>> -	a=$(cd "$gitdir" && pwd)
>> -	b=$(cd "$path" && pwd)
>> +	a=$(cd "$gitdir" && pwd -W)
>> +	b=$(cd "$path" && pwd -W)
>>  	while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>>  	do
>>  		a=${a#*/} b=${b#*/};
> 
> I don't understand why you need this. Does "pwd" sometimes return a
> path starting with "c:/" and sometimes "/c/" depending on what form
> you use when you cd into that directory?

It looks like this is the case. I was surprised as well. I hoped that
pwd -P would fix it, but it makes no difference. I should have tested
pwd -L as well, but I forgot.

> -       gitdir=$(git rev-parse --git-dir)
> +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')

I don't like pipelines of this kind because they fork yet another
process. But it looks like there are not that many alternatives...

-- Hannes

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

[Johannes Schindelin]

Hi,

On Mon, 27 Feb 2012, Johannes Sixt wrote:

> Am 26.02.2012 20:58, schrieb Jens Lehmann:
> 
> > -       gitdir=$(git rev-parse --git-dir)
> > +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
> 
> I don't like pipelines of this kind because they fork yet another
> process. But it looks like there are not that many alternatives...

Not many, but some rather straight-forward ones. E.g.

	# quoted to preserve tabs & newslines (Windows users are creative)
	gitdir="$(git rev-parse --git-dir)"
	case "$gitdir" in
	[A-Za-z]:*)
		gitdir="/${gitdir%%:*}${gitdir#?:}"
		;;
	esac

Note: untested.

Note2: I did not add error handling, either.

Note3: yes, ${...} is available in bash and it does not fork. Neither does
       case ... esac

Ciao,
Johannes

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

[Johannes Sixt]

Am 27.02.2012 22:19, schrieb Johannes Sixt:
> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>> I don't understand why you need this. Does "pwd" sometimes return a
>> path starting with "c:/" and sometimes "/c/" depending on what form
>> you use when you cd into that directory?
> 
> It looks like this is the case. I was surprised as well. I hoped that
> pwd -P would fix it, but it makes no difference. I should have tested
> pwd -L as well, but I forgot.

pwd -L doesn't make a difference, either.

>> -       gitdir=$(git rev-parse --git-dir)
>> +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
> 
> I don't like pipelines of this kind because they fork yet another
> process. But it looks like there are not that many alternatives...

With the following patch on top of your always-use-relative-gitdir branch
from https://github.com/jlehmann/git-submod-enhancements the tests pass
on Windows.

Thanks, Dscho, for pointing out the obvious.

diff --git a/git-submodule.sh b/git-submodule.sh
index e1984e0..953ca5e 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
 	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
 	do

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

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
>
> Thanks, Dscho, for pointing out the obvious.

The patch looks unintrusive and sane.

Thanks all three of you for looking into this.  Should I wait for a patch
with nice write-up from one of you, or should I just come up with a random
message and apply it locally avoiding roundtrip cost?

> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 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
>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>  	do

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

[Jens Lehmann]

Am 28.02.2012 19:58, schrieb Johannes Sixt:
> Am 27.02.2012 22:19, schrieb Johannes Sixt:
>> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>>> -       gitdir=$(git rev-parse --git-dir)
>>> +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
>>
>> I don't like pipelines of this kind because they fork yet another
>> process. But it looks like there are not that many alternatives...
> 
> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
> 
> Thanks, Dscho, for pointing out the obvious.

Thanks for helping to test and fix that on the Windows side. Do you want
to post a commit based on the the interdiff below so I can apply it on
top of my branch? Then I would make this a four patch series in the next
round.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 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
>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>  	do
> 

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

[Jens Lehmann]

Am 28.02.2012 20:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> With the following patch on top of your always-use-relative-gitdir branch
>> from https://github.com/jlehmann/git-submod-enhancements the tests pass
>> on Windows.
>>
>> Thanks, Dscho, for pointing out the obvious.
> 
> The patch looks unintrusive and sane.
> 
> Thanks all three of you for looking into this.  Should I wait for a patch
> with nice write-up from one of you, or should I just come up with a random
> message and apply it locally avoiding roundtrip cost?

Thanks, but that interdiff needs all three patches from my branch to work
properly, while I only posted the first two here so far (without the third
one the gitfile still might contain the "c:/" notation even with J6t's diff
applied). I still need to remove the iffiness of my 2/2 patch and the third
one needs a test case too before I can repost that series.

>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index e1984e0..953ca5e 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
>>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>>  	do
>