Re: [PATCH 2/7] git-submodule: Extract absolute_url & move absolute url logic to module_clone

From: Ping Yin <pkufranky@gmail.com>
Date: Apr 22, 2008, 07:00 UTC
Message-ID: <46dff0320804220000l1872ebf1vde269953d89c755f@mail.gmail.com>
In-Reply-To: <7v3ape5sip.fsf@gitster.siamese.dyndns.org>

On Tue, Apr 22, 2008 at 2:10 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 57 quoted lines

> Ping Yin <pkufranky@gmail.com> writes:
>
>  > Extract function absolute_url to remove code redundance and inconsistence in
>  > cmd_init and cmd_add when resolving relative url/path to absolute one.
>  >
>  > Also move resolving absolute url logic from cmd_add to module_clone which
>  > results in a litte behaviour change: cmd_update originally doesn't
>  > resolve absolute url but now it will.
>
>  Hmmm.  Somehow I find this unreadable and hard to parse.
>
>
>  > This behaviour change breaks t7400 which uses relative url './.subrepo'.
>  > However, this test originally doesn't mean to test relative url with './',
>  > so fix the url as '.subrepo'.
>
>  Isn't ".subrepo" a relative URL that says "subdirectory of the current
>  one, whose name is .subrepo", exactly the same way as "./.subrepo" is?
>  Shouldn't they behave the same?
>
>  If the test found they do not behave the same, perhaps the new code is
>  broken in some way and isn't "fixing" the test simply hiding a bug?
>
>  I dunno...
>
>
>  > +# Resolve relative url/path to absolute one
>  > +absolute_url () {
>  > +     case "$1" in
>  > +     ./*|../*)
>  > +             # dereference source url relative to parent's url
>  > +             url="$(resolve_relative_url $1)" ;;
>  > +     *)
>  > +             # Turn the source into an absolute path if it is local
>  > +             url=$(get_repo_base "$1") ||
>  > +             url=$1
>  > +             ;;
>  > +     esac
>  > +     echo "$url"
>  > +}
>  > +
>  >  #
>  >  # Map submodule path to submodule name
>  >  #
>  > @@ -112,7 +127,7 @@ module_info() {
>  >  module_clone()
>  >  {
>  >       path=$1
>  > -     url=$2
>  > +     url=$(absolute_url "$2")
>  >
>  >       # If there already is a directory at the submodule path,
>  >       # expect it to be empty (since that is the default checkout
>
>  Why does this call-site matter?  The URL is given to "git-clone" which I
>  think does handle the relative URL just fine???
>

>  Hmmm.  Doesn't "foo" generally mean the same thing as "./foo" in the sense
>  both are relative to the current directory?

Following was my answer days ago

There is a little inconsistence in current logic

1. git submodule add ./foo will expand foo with remote.origin.url and
    init an entry in .gitmodules as "submodule.foo.url=$remoteoriginurl/foo"
2. git submodule update will not expand ./foo if  there is an entry
    "submodule.foo.url=./foo"  in $GIT_DIR/config

I tend to add the url as is when "git submodule add", and then expand the url when running "git submodule update". So this will result that the second case expands './foo' as "$remoteoriginurl/foo" instead of "foo".

And this is the reason i expand './foo' in module_clone.

-- 
Ping Yin