patchgit-submodule.sh - Remove trailing / from URL if found

git clone does not complain if a trailing '/' is included in the origin URL, but doing so causes resolution of a submodule's URL relative to the superproject to fail. Regardless of whether git is changed to remove the trailing / before recording the URL, we should avoid this issue in submodule as existing repositories can have this problem.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Mark Levedahl <mlevedahl@gmail.com> writes:

Hmm, the case arms outside the context looks like this:

	while test -n "$url"
	do
		case "$url" in
		../*)
			url="${url#../}"
			remoteurl="${remoteurl%/*}"
			;;
		./*)
			url="${url#./}"
			;;
		*)
			break;;
		esac
	done
	echo "$remoteurl/$url"

If you call "resolve_relative_url ../", the first arm used to fire once and stripped one level away, but with your patch it does not do so anymore, and returns "$remoteurl/.." instead?

I know you are primarily interested in making sure that "../foo" and "../foo/" are handled the same way, but somehow this change does not feel right. How about stripping the trailing slash from the end result?

git clone does not complain if a trailing '/' is included in the origin URL, but doing so causes resolution of a submodule's URL relative to the superproject to fail. Trailing /'s are likely when cloning locally using tab-completion, so the slash may appear in either superproject or submodule URL. So, ignore the trailing slash if it already exists in the superproject's URL, and don't record one for the submodule (which could itself have submodules...).

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-submodule.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Mark Levedahl <mlevedahl@gmail.com> writes:

Hmm. I was sort of hoping to hear "Junio you idiot you do not know what you are talking about --- your example of using ".." as relative won't happen because of such and such reasons; trust me I know what is going on in the vicinity of this code."

And after looking at the callsites of the shell function, I think the original can never pass ".." (there are case statements to pass only $url that match "./*" or "../*"), so I think both your original and this version are safe as long as the part that match the trailing "/*" is sane.

So I'll queue your first patch, as it is slightly shorter ;-)

Thanks.

On Wed, Aug 20, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:

Junio,

I try to avoid the "idiot" idiom, it tends to backfire ... :^)

The problem I'm trying to fix is that a number of folks have superprojects checked out where the recorded origin URL has a trailing /, and a submodule has its origin in a directory sitting right next to the superproject on the server. Thus, we have:

   superproject url = server:/public/super
   submodoule url = server:/public/sub1

However, in the checked out superproject's .git/config
[remote "origin"]
   url = server:/public/super/

and for similar reasons, the submodule has its URL recorded in .gitmodules as
[submodule "sub"]
   path = submodule1
   url = ../sub1/

resolve_relative_url gets the submodule's recorded url as $1, which
the caller retrieved from .gitmodules, and retrieves the superprojects
origin from .git/config. So in this case resolve_relative_url has
that:
   url = ../sub1/
   remoteurl = server:/public/super/

So, without any patch, resolve_relative_url computes the submodule's URL as:
   server:/public/super/sub1/

with the first patch as:
   server:/public/super/sub1

and with the second as
   server:/public/sub1
rather than
   server:/public/sub1

So, the second patch I sent is the one that fixes the above problem,
and I have to say I now forget what the confused state of .gitmodules
+ .git/config I found on one machine that lead to the first patch as
being a correct fix.

In summary, it is essential that resolve_relative_url strip the trailing / from the superproject's url before starting, while it is nice but not necessary if it assures that the result does not contain a trailing /.

Mark

"Mark Levedahl" <mlevedahl@gmail.com> writes:

Well, that is a description worth having in the proposed commit log message, isn't it?

git clone does not complain if a trailing '/' is included in the origin URL, but doing so causes resolution of a submodule's URL relative to the superproject to fail. Trailing /'s are likely when cloning locally using tab-completion, so the slash may appear in either superproject or submodule URL. So, ignore the trailing slash if it already exists in the superproject's URL, and don't record one for the submodule (which could itself have submodules...).

The problem I'm trying to fix is that a number of folks have superprojects checked out where the recorded origin URL has a trailing /, and a submodule has its origin in a directory sitting right next to the superproject on the server. Thus, we have:

	superproject url = server:/public/super
	submodoule url = server:/public/sub1

However, in the checked out superproject's .git/config

[remote "origin"]
	url = server:/public/super/

and for similar reasons, the submodule has its URL recorded in .gitmodules as [submodule "sub"]

	path = submodule1
	url = ../sub1/

resolve_relative_url gets the submodule's recorded url as $1, which the caller retrieved from .gitmodules, and retrieves the superprojects origin from .git/config. So in this case resolve_relative_url has that:

	url = ../sub1/
	remoteurl = server:/public/super/

So, without any patch, resolve_relative_url computes the submodule's URL as:

	server:/public/super/sub1/

rather than

	server:/public/sub1

In summary, it is essential that resolve_relative_url strip the trailing / from the superproject's url before starting, and beneficial if it assures that the result does not contain a trailing / as the submodule may itself also be a superproject.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-submodule.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)