From: Jens Lehmann <jens.lehmann@web.de>
Date: Tue, 14 Feb 2012 17:36:24 GMT
Subject: Re: [PATCH v2 2/2] submodules: always use a relative path from gitdir to work tree
Message-ID: <4F3A9B98.6040908@web.de>
In-Reply-To: <7vlio6ec7q.fsf@alter.siamese.dyndns.org>

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")