From: Jon Seymour <jon.seymour@gmail.com>
Date: Thu, 24 May 2012 03:40:42 GMT
Subject: Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs
Message-ID: <CAH3AnrpweAah9GojN2QodTx6qW2rHE4wadxbc23zrwj=uPCP-A@mail.gmail.com>
In-Reply-To: <1337830657-16400-3-git-send-email-jon.seymour@gmail.com>

On Thu, May 24, 2012 at 1:37 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> When the origin URL of the superproject is itself relative, an operation
> such as git submodule add, init or sync may result in either the
> submodule.{name}.url configuration property of the superproject
> referring to the incorrect location or remote.origin.url configuration
> property of the submodule referring to the incorrect location or both
> these conditions. In some cases, git submodule fails to update
> the configuration and fails with an error condition.
>
> The issue arises in these cases because the origin URL of
> the superproject needs to be prepended with a prefix that navigates
> from the submodule to the superproject so that when the submodule
> URL is concatenated the resulting URL is relative to the working tree
> of the submodule.
>
> This change fixes handling for relative superproject origin URLs
> for 6 cases:
>  foo
>  foo/bar
>  ./foo
>  ./foo/bar
>  ../foo
>  ../foo/bar
>
> In each case, the configuration properties in the superproject's
> configuration and the submodule's configuration refer to the
> correct, relative, location of the submodule's origin repo. In all cases,
> the configured paths are relative to the working trees of the
> repositories containing the configuration.
>
> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
> ---
>  git-submodule.sh           | 57 +++++++++++++++++++++++++++++++++++++++++-----
>  t/t7400-submodule-basic.sh |  6 ++---
>  t/t7403-submodule-sync.sh  | 18 +++++----------
>  3 files changed, 59 insertions(+), 22 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 64a70d6..738eba3 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -30,13 +30,35 @@ nofetch=
>  update=
>  prefix=
>
> -# Resolve relative url by appending to parent's url
> +# Resolve relative url by appending the submodule url
> +# to the superproject's origin URL
> +#
> +# If the origin URL is itself a relative URL prepend
> +# an additional prefix, if present, that represents
> +# the relative path from the submodule's working tree
> +# to the superprojects' working tree.
> +#
> +# This behaviour is required to ensure that the origin URL
> +# of a submodule, when relative, is relative to the
> +# submodule's work tree and not to the superproject's work tree.
> +#
>  resolve_relative_url ()
>  {
>        remote=$(get_default_remote)
>        remoteurl=$(git config "remote.$remote.url") ||
>                remoteurl=$(pwd) # the repository is its own authoritative upstream
>        url="$1"
> +       up_path="$2"
> +
> +       #
> +       # ensure all relative paths begin with ./ to enable
> +       # selection relative branch of subsequent case "$remoteurl"
> +       # statement.
> +       #
> +       # rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo
> +       # and ../foo untouched.
> +       #
> +       remoteurl=$(echo "$remoteurl" | sed "s|^[^/:\\.][^:]*\$|./&|")
>        remoteurl=${remoteurl%/}
>        sep=/
>        while test -n "$url"
> @@ -45,6 +67,16 @@ resolve_relative_url ()
>                ../*)
>                        url="${url#../}"
>                        case "$remoteurl" in
> +                       .*/*)
> +                               # remove last part
> +                               remoteurl="${remoteurl%/*}"
> +                               # remove redundant leading ./
> +                               remoteurl="${remoteurl#./}"
> +                               # prefix path from submodule work tree to superproject work tree
> +                               remoteurl="${up_path}${remoteurl}"
> +                               # remove trailing /.
> +                               remoteurl="${remoteurl%/.}"
> +                               ;;
>                        */*)
>                                remoteurl="${remoteurl%/*}"
>                                ;;
> @@ -959,19 +991,32 @@ cmd_sync()
>        while read mode sha1 stage sm_path
>        do
>                name=$(module_name "$sm_path")
> -               url=$(git config -f .gitmodules --get submodule."$name".url)
> +               # path from superproject origin repo to submodule origin repo
> +               module_url=$(git config -f .gitmodules --get submodule."$name".url)
>
>                # Possibly a url relative to parent
> -               case "$url" in
> +               case "$module_url" in
>                ./*|../*)
> -                       url=$(resolve_relative_url "$url") || exit
> +                       # rewrite foo/bar as ../.. to find path from
> +                       # submodule work tree to superproject work tree
> +                       up_path="$(echo "$sm_path" | sed "s/[^/]*/../g")" &&
> +                       # guarantee a trailing /
> +                       up_path=${up_path%/}/ &&
> +                       # path from submodule work tree to submodule origin repo
> +                       sub_origin_url=$(resolve_relative_url "$module_url" "$up_path") &&
> +                       # path from superproject work tree to submodule origin repo
> +                       super_config_url=$(resolve_relative_url "$module_url") || exit
> +                       ;;
> +               *)
> +                       sub_origin_url="$module_url"
> +                       super_config_url="$module_url"
>                        ;;
>                esac
>
>                if git config "submodule.$name.url" >/dev/null 2>/dev/null
>                then
>                        say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
> -                       git config submodule."$name".url "$url"
> +                       git config submodule."$name".url "$super_config_url"
>
>                        if test -e "$sm_path"/.git
>                        then
> @@ -979,7 +1024,7 @@ cmd_sync()
>                                clear_local_git_env
>                                cd "$sm_path"
>                                remote=$(get_default_remote)
> -                               git config remote."$remote".url "$url"
> +                               git config remote."$remote".url "$sub_origin_url"
>                        )
>                        fi
>                fi
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 08b8d2f..f2f907f 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -507,12 +507,11 @@ test_expect_success 'relative path works with user@host:path' '
>        )
>  '
>
> -test_expect_failure 'relative path works with foo' '
> +test_expect_success 'relative path works with foo' '
>        (
>                cd reltest &&
>                cp pristine-.git-config .git/config &&
>                git config remote.origin.url foo &&
> -               # actual: fails with an error
>                git submodule init &&
>                test "$(git config submodule.sub.url)" = ./subrepo
>        )
> @@ -538,13 +537,12 @@ test_expect_success 'relative path works with ./foo' '
>        )
>  '
>
> -test_expect_failure 'relative path works with ./foo/bar' '
> +test_expect_success 'relative path works with ./foo/bar' '
>        (
>                cd reltest &&
>                cp pristine-.git-config .git/config &&
>                git config remote.origin.url ./foo/bar &&
>                git submodule init &&
> -               #actual: ./foo/subrepo
>                test "$(git config submodule.sub.url)" = foo/subrepo
>        )
>  '
> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
> index 9fa4e58..9526f8d 100755
> --- a/t/t7403-submodule-sync.sh
> +++ b/t/t7403-submodule-sync.sh
> @@ -88,72 +88,66 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
>        )
>  '
>
> -test_expect_failure '"git submodule sync" handles origin URL of the form foo' '
> +test_expect_success '"git submodule sync" handles origin URL of the form foo' '
>        (cd relative-clone &&
>         git remote set-url origin foo
>         git submodule sync &&
>        (cd submodule &&
> -        #actual fails with: "cannot strip off url foo
>         git config remote.origin.url &&

Sorry, it appears I left some debug output in the tests. Will roll
this into a v6 with any updates you suggest.

Regards,

jon.