Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar

[Jon Seymour <jon.seymour@gmail.com>]

On Mon, May 21, 2012 at 11:31 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 produces a relative
> URL for the submodule that doesn't point at the correct location.
>
> 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 ensures that this is done for add, sync and init.
>
> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
> ---
>  git-submodule.sh           | 35 ++++++++++++++++++++---------------
>  t/t7400-submodule-basic.sh |  4 ++--
>  t/t7403-submodule-sync.sh  |  2 +-
>  3 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 64a70d6..230c219 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -45,6 +45,10 @@ resolve_relative_url ()
>                ../*)
>                        url="${url#../}"
>                        case "$remoteurl" in
> +                       .*/*)
> +                               up_path="$(echo "$2" | sed "s/[^/]*/../g")"
> +                               remoteurl="${up_path%/}/${remoteurl%/*}"
> +                               ;;
>                        */*)
>                                remoteurl="${remoteurl%/*}"
>                                ;;
> @@ -235,11 +239,24 @@ cmd_add()
>                usage
>        fi
>
> +       # normalize path:
> +       # multiple //; leading ./; /./; /../; trailing /
> +       sm_path=$(printf '%s/\n' "$sm_path" |
> +               sed -e '
> +                       s|//*|/|g
> +                       s|^\(\./\)*||
> +                       s|/\./|/|g
> +                       :start
> +                       s|\([^/]*\)/\.\./||
> +                       tstart
> +                       s|/*$||
> +               ')
> +
>        # assure repo is absolute or relative to parent
>        case "$repo" in
>        ./*|../*)
>                # dereference source url relative to parent's url
> -               realrepo=$(resolve_relative_url "$repo") || exit
> +               realrepo=$(resolve_relative_url "$repo" "$sm_path") || exit
>                ;;
>        *:*|/*)
>                # absolute url
> @@ -250,18 +267,6 @@ cmd_add()
>        ;;
>        esac
>
> -       # normalize path:
> -       # multiple //; leading ./; /./; /../; trailing /
> -       sm_path=$(printf '%s/\n' "$sm_path" |
> -               sed -e '
> -                       s|//*|/|g
> -                       s|^\(\./\)*||
> -                       s|/\./|/|g
> -                       :start
> -                       s|\([^/]*\)/\.\./||
> -                       tstart
> -                       s|/*$||
> -               ')
>        git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
>        die "$(eval_gettext "'\$sm_path' already exists in the index")"
>
> @@ -407,7 +412,7 @@ cmd_init()
>                        # Possibly a url relative to parent
>                        case "$url" in
>                        ./*|../*)
> -                               url=$(resolve_relative_url "$url") || exit
> +                               url=$(resolve_relative_url "$url" "$sm_path") || exit
>                                ;;
>                        esac
>                        git config submodule."$name".url "$url" ||
> @@ -964,7 +969,7 @@ cmd_sync()
>                # Possibly a url relative to parent
>                case "$url" in
>                ./*|../*)
> -                       url=$(resolve_relative_url "$url") || exit
> +                       url=$(resolve_relative_url "$url" "$sm_path") || exit
>                        ;;
>                esac
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 02e6428..b838f43 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -507,7 +507,7 @@ test_expect_success 'relative path works with user@host:path' '
>        )
>  '
>
> -test_expect_failure 'relative path works with ../relative/repo' '
> +test_expect_success 'relative path works with ../relative/repo' '
>        (
>                cd reltest &&
>                cp pristine-.git-config .git/config &&
> @@ -517,7 +517,7 @@ test_expect_failure 'relative path works with ../relative/repo' '
>        )
>  '
>
> -test_expect_failure 'test that submodule add creates the correct url when super origin url is ../relative/repo' '
> +test_expect_sucess 'test that submodule add creates the correct url when super origin url is ../relative/repo' '
>        mkdir reladd &&
>        (
>                cd reladd &&
> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
> index 788bc24..35700ef 100755
> --- a/t/t7403-submodule-sync.sh
> +++ b/t/t7403-submodule-sync.sh
> @@ -86,7 +86,7 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
>        )
>  '
>
> -test_expect_failure '"git submodule sync" should handle a super with a relative origin URL' '
> +test_expect_success '"git submodule sync" should handle a super with a relative origin URL' '
>        git clone super relative-clone &&
>        (cd relative-clone &&
>         git submodule update --init &&
> --
> 1.7.10.2.594.g5c52315
>

Mmmm. Better hold off on this one for the moment, I have detected a
break during a subsequent git submodule update that my selection of
regression tests didn't pick up. The issue is that the URL used for
cloning the submodule during the update is now not correct.

jon.