Am 14.02.2012 21:34, schrieb Junio C Hamano:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Jens Lehmann <Jens.Lehmann@xxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html