Re: [PATCH] git-submodule.sh - Remove trailing / from URL if found

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] git-submodule.sh - Remove trailing / from URL if found

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:12

Mark Levedahl [off-list ref] writes:
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 <redacted>
---
 git-submodule.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
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.

Re: [PATCH] git-submodule.sh - Remove trailing / from URL if found

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:45:12

On Wed, Aug 20, 2008 at 11:26 PM, Junio C Hamano [off-list ref] wrote:
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help