On Wed, May 23, 2012 at 11:37 PM, Jon Seymour <jon.seymour@xxxxxxxxx> wrote:
> -# Resolve relative url by appending to parent's url
> +# Resolve relative url by appending the submodule url
> +# to the superproject's origin URL
> +#
> +# If the origin URL is itself a relative URL prepend
> +# an additional prefix, if present, that represents
> +# the relative path from the submodule's working tree
> +# to the superprojects' working tree.
> +#
> +# This behaviour is required to ensure that the origin URL
"Required behavior" always seems overstated to me when I
read it in comments and so I tend to distrust it. I'd prefer to
see "This behaviour is intended to ensure..." But this is
only my personal preference.
> +# of a submodule, when relative, is relative to the
> +# submodule's work tree and not to the superproject's work tree.
> +#
> resolve_relative_url ()
> {
> remote=$(get_default_remote)
> remoteurl=$(git config "remote.$remote.url") ||
> remoteurl=$(pwd) # the repository is its own authoritative upstream
> url="$1"
> + up_path="$2"
> +
> + #
> + # ensure all relative paths begin with ./ to enable
Are we talking about remote urls or local filesystem paths? I think this
is all confusing enough without the comments also using terminology
inconsistently. Would it be more correct to say "all relatiive urls" here?
Or is this function only interested in local filesystem paths?
I realize that urls are paths of a different nature. I pick this nit
only in the cause of clarity.
> + # selection relative branch of subsequent case "$remoteurl"
> + # statement.
> + #
> + # rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo
> + # and ../foo untouched.
> + #
In many filesystems, ":foo" and "\foo" are valid filenames.
I suspect it is unwise to employ them and expect them not
to cause trouble, so I don't know if we should make
special efforts to accept them here. But I think it
is worth noting.
On the other hand, I do not know how these special
characters are represented in urls.
> + remoteurl=$(echo "$remoteurl" | sed "s|^[^/:\\.][^:]*\$|./&|")
> remoteurl=${remoteurl%/}
> sep=/
> while test -n "$url"
> @@ -45,6 +67,16 @@ resolve_relative_url ()
> ../*)
> url="${url#../}"
> case "$remoteurl" in
> + .*/*)
> + # remove last part
> + remoteurl="${remoteurl%/*}"
> + # remove redundant leading ./
> + remoteurl="${remoteurl#./}"
> + # prefix path from submodule work tree to superproject work tree
> + remoteurl="${up_path}${remoteurl}"
Here we seem to be talking about paths since we are concerned
with the worktrees. So maybe my earlier concern about "urls"
vs. "paths" was miguided. My head swims.
> + # remove trailing /.
> + remoteurl="${remoteurl%/.}"
> + ;;
> */*)
> remoteurl="${remoteurl%/*}"
> ;;
I haven't tested it, but the rest of this is making more sense to me now.
Phil
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html