# [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar 6 messages from 2012-05-21 to 2012-05-21. Participants: Jon Seymour. Thread: https://gitlist.dev/t/30577 ## Jon Seymour, 2012-05-21 13:31 Subject: [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar Message-ID: <1337607102-14737-1-git-send-email-jon.seymour@gmail.com> URL: https://gitlist.dev/e/1337607102-14737-1-git-send-email-jon.seymour%40gmail.com ``` These tests are expected to fail, pending a subsequent corrective patch. Signed-off-by: Jon Seymour --- t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++ t/t7403-submodule-sync.sh | 10 ++++++++++ 2 files changed, 36 insertions(+) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 81827e6..02e6428 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -507,6 +507,32 @@ test_expect_success 'relative path works with user@host:path' ' ) ' +test_expect_failure 'relative path works with ../relative/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + git config remote.origin.url ../relative/repo && + git submodule init && + test "$(git config submodule.sub.url)" = ../../relative/subrepo + ) +' + +test_expect_failure 'test that submodule add creates the correct url when super origin url is ../relative/repo' ' + mkdir reladd && + ( + cd reladd && + git init && + git remote add origin ../relative/repo + mkdir sub && + ( + cd sub && + git init && + test_commit foo + ) && + git submodule add ../subrepo ./sub && + test "$(git config submodule.sub.url)" = ../../relative/subrepo + ) +' test_expect_success 'moving the superproject does not break submodules' ' ( cd addtest && diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 3620215..788bc24 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -86,4 +86,14 @@ 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' ' + git clone super relative-clone && + (cd relative-clone && + git submodule update --init && + git remote set-url origin ../relative/super.git && + git submodule sync && + test "$(git config submodule.submodule.url)" == ../../relative/moved-submodule + ) +' + test_done -- 1.7.10.2.594.g5c52315 ``` ## Jon Seymour, 2012-05-21 13:31 Subject: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar Message-ID: <1337607102-14737-2-git-send-email-jon.seymour@gmail.com> URL: https://gitlist.dev/e/1337607102-14737-2-git-send-email-jon.seymour%40gmail.com In-Reply-To: <1337607102-14737-1-git-send-email-jon.seymour@gmail.com> ``` 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 --- 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 ``` ## Jon Seymour, 2012-05-21 13:31 Subject: [PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar. Message-ID: <1337607102-14737-3-git-send-email-jon.seymour@gmail.com> URL: https://gitlist.dev/e/1337607102-14737-3-git-send-email-jon.seymour%40gmail.com In-Reply-To: <1337607102-14737-1-git-send-email-jon.seymour@gmail.com> ``` This test shows that git submodule add produces the wrong submodule URL when the origin URL of the superproject is of the form: foo/bar. The problem is fixed by a subsequent patch. Signed-off-by: Jon Seymour --- t/t7400-submodule-basic.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index b838f43..71f30d8 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -533,6 +533,23 @@ test_expect_sucess 'test that submodule add creates the correct url when super o test "$(git config submodule.sub.url)" = ../../relative/subrepo ) ' +test_expect_failure 'test that submodule add creates the correct url when super origin url is relative/repo' ' + mkdir reladd && + ( + cd reladd && + git init && + git remote add origin relative/repo + mkdir sub && + ( + cd sub && + git init && + test_commit foo + ) && + git submodule add ../subrepo ./sub && + test "$(git config submodule.sub.url)" = ../relative/subrepo + ) +' + test_expect_success 'moving the superproject does not break submodules' ' ( cd addtest && -- 1.7.10.2.594.g5c52315 ``` ## Jon Seymour, 2012-05-21 13:31 Subject: [PATCH v3 4/4] submodule: support superproject origin URLs of the form foo/bar Message-ID: <1337607102-14737-4-git-send-email-jon.seymour@gmail.com> URL: https://gitlist.dev/e/1337607102-14737-4-git-send-email-jon.seymour%40gmail.com In-Reply-To: <1337607102-14737-1-git-send-email-jon.seymour@gmail.com> ``` When the origin URL of a superproject is of the form foo/bar, submodule operations such as add, sync or init will configure the URL of any submodule with an origin URL that points at the incorrect location. Origin URLs of this form are not expected in most cases, since the origin URL of a repo will usually point to a location that is not nested within the current working tree. However, the situation can arise when the physical location of an upstream repo is managed with a symbolic link nested within the working tree itself. For example, suppose that a toolset is installed in ~/tools and ~/tools/mnt/usb is a occasionally used as the origin for a refresh of the toolset. Different users with different USB devices may map ~/tools/mnt/usb to a different subdirectory of /media, but the auto-update function of the toolset may simply pull an update from mnt/usb, assuming that the user has done what is required to initialise mnt/usb to refer to the correct subdirectory of /media. Signed-off-by: Jon Seymour --- git-submodule.sh | 2 ++ t/t7400-submodule-basic.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 230c219..b8a7403 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -37,6 +37,7 @@ resolve_relative_url () remoteurl=$(git config "remote.$remote.url") || remoteurl=$(pwd) # the repository is its own authoritative upstream url="$1" + remoteurl=$(echo "$remoteurl" | sed "s|^[^/][^:]*\$|./&|") remoteurl=${remoteurl%/} sep=/ while test -n "$url" @@ -47,6 +48,7 @@ resolve_relative_url () case "$remoteurl" in .*/*) up_path="$(echo "$2" | sed "s/[^/]*/../g")" + remoteurl=${remoteurl#./} remoteurl="${up_path%/}/${remoteurl%/*}" ;; */*) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 71f30d8..a4a8363 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -533,7 +533,7 @@ test_expect_sucess 'test that submodule add creates the correct url when super o test "$(git config submodule.sub.url)" = ../../relative/subrepo ) ' -test_expect_failure 'test that submodule add creates the correct url when super origin url is relative/repo' ' +test_expect_success 'test that submodule add creates the correct url when super origin url is relative/repo' ' mkdir reladd && ( cd reladd && -- 1.7.10.2.594.g5c52315 ``` ## Jon Seymour, 2012-05-21 16:35 Subject: Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar Message-ID: URL: https://gitlist.dev/e/CAH3AnrqBeD3JisjoK2dqnPWcBAFTxB5NYgRkfSPqb-CS8a0hsw%40mail.gmail.com In-Reply-To: <1337607102-14737-2-git-send-email-jon.seymour@gmail.com> ``` On Mon, May 21, 2012 at 11:31 PM, Jon Seymour 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 > --- >  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. ``` ## Jon Seymour, 2012-05-21 22:20 Subject: Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar Message-ID: URL: https://gitlist.dev/e/CAH3AnrrEnLEnN%3DEgzaDjFtN6r9RBpTmPNTX96QiMtGz9J12eCQ%40mail.gmail.com In-Reply-To: ``` On Tue, May 22, 2012 at 2:35 AM, Jon Seymour wrote: > On Mon, May 21, 2012 at 11:31 PM, Jon Seymour wrote: > > 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. I guess the issue here is that whereas the origin url of the submodule (as in remote.origin.url in the submodule) should be relative to the submodule's working tree, it might actually be better to keep submodule.{modulename}.url relative to the working directory of superproject (which is current behaviour). Thoughts? jon. ```