Re: [PATCH v8 3/4] submodule: fix sync handling of some relative superproject origin URLs

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 3, 2012, 22:10 UTC
Message-ID: <7v1ulwxd0o.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1338716810-9881-4-git-send-email-jon.seymour@gmail.com>

Jon Seymour <jon.seymour@gmail.com> writes:

Show 26 quoted lines

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 64a70d6..314df20 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -30,7 +30,22 @@ nofetch=
>  update=
>  prefix=
>  
> -# Resolve relative url by appending to parent's url
> +# The function takes at most 2 arguments. The first argument is the
> +# relative URL that navigates from the superproject origin repo to the
> +# submodule origin repo. The second up_path argument, if specified, is
> +# the relative path that navigates from the submodule working tree to
> +# the superproject working tree.
> +#
> +# The output of the function is the origin URL of the submodule.
> +#
> +# The output will either be an absolute URL or filesystem path (if the
> +# superproject origin URL is an absolute URL or filesystem path,
> +# respectively) or a relative file system path (if the superproject
> +# origin URL is a relative file system path).
> +#
> +# When the output is a relative file system path, the path is either
> +# relative to the submodule working tree, if up_path is specified, or to
> +# the superproject working tree otherwise.
>  resolve_relative_url ()

OK.

Show 14 quoted lines

> @@ -39,6 +54,17 @@ resolve_relative_url ()
>  	url="$1"
>  	remoteurl=${remoteurl%/}
>  	sep=/
> +	up_path="$2"
> +
> +	case "$remoteurl" in
> +		*:*|/*)
> +			is_relative=
> +			;;
> +		*)
> +			is_relative=t
> +			;;
> +	esac

Style: please align case/esac and the labels on case arms (see how
two existing nested case statements in this function are written).

Show 16 quoted lines

> @@ -959,19 +985,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")" &&

Didn't we add some workaround for implementations of sed that do not match and replace a possibly empty pattern? Am I seeing the same breakage as c5bc42b (Avoid bug in Solaris xpg4/sed as used in submodule, 2012-04-09) addressed with this patch?