* [RFC] submodule: fix handling of supermodules with relative origin URLs. [not found] <AH3Anrr6mLVedBPcgfVwy=5KRjUgdp5W8P0DQ3qaX_UjH-npDw@mail.gmail.com> @ 2012-05-18 12:13 ` Jon Seymour 2012-05-18 12:16 ` Jon Seymour 2012-05-18 19:58 ` Jens Lehmann 0 siblings, 2 replies; 4+ messages in thread From: Jon Seymour @ 2012-05-18 12:13 UTC (permalink / raw) To: git; +Cc: Jens.Lehmann, gitster, Jon Seymour Prior to this change, operations such as git submodule sync produces the wrong result when the origin URL of the super module is itself a relative URL. The issue arises because in this case, the origin URL of the supermodule needs to be prepended with a prefix that navigates from the submodule to the supermodule. This change adds that prefix. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> --- git-submodule.sh | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) If people are ok with the fix, I'll roll this as a patch together with some tests. diff --git a/git-submodule.sh b/git-submodule.sh index 64a70d6..5008867 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -37,7 +37,8 @@ resolve_relative_url () remoteurl=$(git config "remote.$remote.url") || remoteurl=$(pwd) # the repository is its own authoritative upstream url="$1" - remoteurl=${remoteurl%/} + up_path="$(echo "$2" | sed "s/[^/]*/../g")" + remoteurl=${remoteurl%/*} sep=/ while test -n "$url" do @@ -45,6 +46,9 @@ resolve_relative_url () ../*) url="${url#../}" case "$remoteurl" in + .*/*) + 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")" @@ -401,13 +406,14 @@ cmd_init() if test -z "$(git config "submodule.$name.url")" then url=$(git config -f .gitmodules submodule."$name".url) + sm_path=$(git config -f .gitmodules submodule."$name".path) test -z "$url" && die "$(eval_gettext "No url found for submodule path '\$sm_path' in .gitmodules")" # 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" || @@ -960,11 +966,12 @@ cmd_sync() do name=$(module_name "$sm_path") url=$(git config -f .gitmodules --get submodule."$name".url) + sm_path=$(git config -f .gitmodules --get submodule."$name".path) # Possibly a url relative to parent case "$url" in ./*|../*) - url=$(resolve_relative_url "$url") || exit + url=$(resolve_relative_url "$url" "$sm_path") || exit ;; esac -- 1.7.10.2.647.g63fe035.dirty ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC] submodule: fix handling of supermodules with relative origin URLs. 2012-05-18 12:13 ` [RFC] submodule: fix handling of supermodules with relative origin URLs Jon Seymour @ 2012-05-18 12:16 ` Jon Seymour 2012-05-18 19:58 ` Jens Lehmann 1 sibling, 0 replies; 4+ messages in thread From: Jon Seymour @ 2012-05-18 12:16 UTC (permalink / raw) To: git On Fri, May 18, 2012 at 10:13 PM, Jon Seymour <jon.seymour@gmail.com> wrote: > - remoteurl=${remoteurl%/} > + up_path="$(echo "$2" | sed "s/[^/]*/../g")" > + remoteurl=${remoteurl%/*} Oops - didn't mean to change remoteurl here. > + remoteurl="${up_path%/}/${remoteurl%/*}" Meant up_path%/ to be up_path%/* jon ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] submodule: fix handling of supermodules with relative origin URLs. 2012-05-18 12:13 ` [RFC] submodule: fix handling of supermodules with relative origin URLs Jon Seymour 2012-05-18 12:16 ` Jon Seymour @ 2012-05-18 19:58 ` Jens Lehmann 2012-05-18 21:45 ` Jon Seymour 1 sibling, 1 reply; 4+ messages in thread From: Jens Lehmann @ 2012-05-18 19:58 UTC (permalink / raw) To: Jon Seymour; +Cc: git, gitster Am 18.05.2012 14:13, schrieb Jon Seymour: > Prior to this change, operations such as git submodule sync produces > the wrong result when the origin URL of the super module > is itself a relative URL. > > The issue arises because in this case, the origin URL of the supermodule > needs to be prepended with a prefix that navigates from the submodule to > the supermodule. > > This change adds that prefix. Thanks, sounds sane. Please see my comments below. > Signed-off-by: Jon Seymour <jon.seymour@gmail.com> > --- > git-submodule.sh | 39 +++++++++++++++++++++++---------------- > 1 file changed, 23 insertions(+), 16 deletions(-) > > If people are ok with the fix, I'll roll this as a patch together > with some tests. Yeah, tests would be great (and while at it please drop the trailing '.' from the subject ;-). This version of the patch does break some existing tests, but your follow up suggests you already found that out yourself ;-) > diff --git a/git-submodule.sh b/git-submodule.sh > index 64a70d6..5008867 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -37,7 +37,8 @@ resolve_relative_url () > remoteurl=$(git config "remote.$remote.url") || > remoteurl=$(pwd) # the repository is its own authoritative upstream > url="$1" > - remoteurl=${remoteurl%/} > + up_path="$(echo "$2" | sed "s/[^/]*/../g")" Me thinks up_path should be set in the case below, which is the only place where it is used. > + remoteurl=${remoteurl%/*} As you mentioned in your follow up this change is not correct. > sep=/ > while test -n "$url" > do > @@ -45,6 +46,9 @@ resolve_relative_url () > ../*) > url="${url#../}" > case "$remoteurl" in > + .*/*) up_path should be set here. > + 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")" > > @@ -401,13 +406,14 @@ cmd_init() > if test -z "$(git config "submodule.$name.url")" > then > url=$(git config -f .gitmodules submodule."$name".url) > + sm_path=$(git config -f .gitmodules submodule."$name".path) Isn't sm_path already set correctly here? I think this line should be dropped. > test -z "$url" && > die "$(eval_gettext "No url found for submodule path '\$sm_path' in .gitmodules")" > > # 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" || > @@ -960,11 +966,12 @@ cmd_sync() > do > name=$(module_name "$sm_path") > url=$(git config -f .gitmodules --get submodule."$name".url) > + sm_path=$(git config -f .gitmodules --get submodule."$name".path) Same here. > > # Possibly a url relative to parent > case "$url" in > ./*|../*) > - url=$(resolve_relative_url "$url") || exit > + url=$(resolve_relative_url "$url" "$sm_path") || exit > ;; > esac > Other than that the patch looks fine. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] submodule: fix handling of supermodules with relative origin URLs. 2012-05-18 19:58 ` Jens Lehmann @ 2012-05-18 21:45 ` Jon Seymour 0 siblings, 0 replies; 4+ messages in thread From: Jon Seymour @ 2012-05-18 21:45 UTC (permalink / raw) To: Jens Lehmann; +Cc: git, gitster On Sat, May 19, 2012 at 5:58 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote: > This version of the patch does break some existing tests, but your > follow up suggests you already found that out yourself ;-) > Actually, that was just by reading the patch and noticing odd-ness. I realise now that I snuck in that bogus edit after I had run the tests which were otherwise clean - d'oh. > > Me thinks up_path should be set in the case below, which is the only > place where it is used. Ok. > > Isn't sm_path already set correctly here? I think this line should > be dropped. > Yes, good point. Thanks. > > Same here. > Thanks. > > Other than that the patch looks fine. Thanks for the review - I'll add some tests for the new behaviour. jon. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-05-18 21:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <AH3Anrr6mLVedBPcgfVwy=5KRjUgdp5W8P0DQ3qaX_UjH-npDw@mail.gmail.com>
2012-05-18 12:13 ` [RFC] submodule: fix handling of supermodules with relative origin URLs Jon Seymour
2012-05-18 12:16 ` Jon Seymour
2012-05-18 19:58 ` Jens Lehmann
2012-05-18 21:45 ` Jon Seymour
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).