patchFix submodule sync with relative submodule URLs

> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>  git-submodule.sh |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 1c39b59..92be0fe 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -634,6 +634,14 @@ cmd_sync()
>        do
>                name=$(module_name "$path")
>                url=$(git config -f .gitmodules --get submodule."$name".url)
> +
> +               # Possibly a url relative to parent
> +               case "$url" in
> +               ./*|../*)
> +                       url=$(resolve_relative_url "$url") || exit
> +                       ;;
> +               esac
> +
>                if test -e "$path"/.git
>                then
>                (
> --
> 1.6.0.1.400.gd2470
>

> On Wednesday 24 September 2008, David Aguilar wrote:
> > Instead of just doing an "|| exit" shouldn't it report an explanation
> > of the error?
> > Other than that, it looks good to me.
> 
> Fixed. Thanks.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 1c39b59..f89bdbe 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -634,6 +634,15 @@ cmd_sync()
>  	do
>  		name=$(module_name "$path")
>  		url=$(git config -f .gitmodules --get submodule."$name".url)
> +
> +		# Possibly a url relative to parent
> +		case "$url" in
> +		./*|../*)
> +			url=$(resolve_relative_url "$url") ||
> +				die "failed to resolve relative submodule url for '$name'"
> +			;;
> +		esac
> +
>  		if test -e "$path"/.git
>  		then
>  		(

> Johan Herland <johan@herland.net> wrote:
> > On Wednesday 24 September 2008, David Aguilar wrote:
> > > Instead of just doing an "|| exit" shouldn't it report an explanation
> > > of the error?
> > > Other than that, it looks good to me.
> > 
> > Fixed. Thanks.
> 
> OK, time for the drive-by patch commenting.  I've largely stayed
> out of git-submodule related code, but I just looked at in the
> context of applying this patch.
> 
> There are three callers to resolve_relative_url in master and next.
> All three callers just "|| exit" when resolve_relative_url fails.
> 
> The only reason resolve_relative_url can fail is when there is no
> remote.$remote.url configuration option set for the current default
> remote ("origin"?).
> 
> I guess I'm unclear about why cmd_sync is different from the
> existing callers.

> 
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index 1c39b59..f89bdbe 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -634,6 +634,15 @@ cmd_sync()
> >  	do
> >  		name=$(module_name "$path")
> >  		url=$(git config -f .gitmodules --get submodule."$name".url)
> > +
> > +		# Possibly a url relative to parent
> > +		case "$url" in
> > +		./*|../*)
> > +			url=$(resolve_relative_url "$url") ||
> > +				die "failed to resolve relative submodule url for '$name'"
> > +			;;
> > +		esac
> > +
> >  		if test -e "$path"/.git
> >  		then
> >  		(
> 
> -- 
> Shawn.

> @@ -155,7 +155,7 @@ cmd_add()
>  	case "$repo" in
>  	./*|../*)
>  		# dereference source url relative to parent's url
> -		realrepo=$(resolve_relative_url "$repo") || exit
> +		realrepo=$(resolve_relative_url "$repo")
>  		;;

> David Aguilar schrieb:
> > resolve_relative_url calls die() when no remote url exists so these calls to
> > exit can be removed.
> ...
> > @@ -155,7 +155,7 @@ cmd_add()
> >  	case "$repo" in
> >  	./*|../*)
> >  		# dereference source url relative to parent's url
> > -		realrepo=$(resolve_relative_url "$repo") || exit
> > +		realrepo=$(resolve_relative_url "$repo")
> >  		;;
> 
> Did you test it? The command inside $(...) is run in its own sub-process,
> therefore, the die() does not exit the caller, just the sub-process, and
> the || exit *is* required.
> 
> BTW, I think that || exit is sufficient; you don't need to add another
> error message - the one that resolve_relative_url() prints is sufficient.

> Johannes Sixt <j.sixt@viscovery.net> wrote:
> > Did you test it? The command inside $(...) is run in its own
> > sub-process, therefore, the die() does not exit the caller, just
> > the sub-process, and the || exit *is* required.
> >
> > BTW, I think that || exit is sufficient; you don't need to add
> > another error message - the one that resolve_relative_url() prints
> > is sufficient.
>
> Exactly.
>
> I think we just need a "|| exit" after each of these
> $(resolve_relative_url) calls.  The original patch that
> started this discussion just needs a "|| exit".