Ping Yin <pkufranky@xxxxxxxxx> writes:
> Extract function absolute_url to remove code redundance and inconsistence in
> cmd_init and cmd_add when resolving relative url/path to absolute one.
>
> Also move resolving absolute url logic from cmd_add to module_clone which
> results in a litte behaviour change: cmd_update originally doesn't
> resolve absolute url but now it will.
Hmmm. Somehow I find this unreadable and hard to parse.
> This behaviour change breaks t7400 which uses relative url './.subrepo'.
> However, this test originally doesn't mean to test relative url with './',
> so fix the url as '.subrepo'.
Isn't ".subrepo" a relative URL that says "subdirectory of the current
one, whose name is .subrepo", exactly the same way as "./.subrepo" is?
Shouldn't they behave the same?
If the test found they do not behave the same, perhaps the new code is
broken in some way and isn't "fixing" the test simply hiding a bug?
I dunno...
> +# Resolve relative url/path to absolute one
> +absolute_url () {
> + case "$1" in
> + ./*|../*)
> + # dereference source url relative to parent's url
> + url="$(resolve_relative_url $1)" ;;
> + *)
> + # Turn the source into an absolute path if it is local
> + url=$(get_repo_base "$1") ||
> + url=$1
> + ;;
> + esac
> + echo "$url"
> +}
> +
> #
> # Map submodule path to submodule name
> #
> @@ -112,7 +127,7 @@ module_info() {
> module_clone()
> {
> path=$1
> - url=$2
> + url=$(absolute_url "$2")
>
> # If there already is a directory at the submodule path,
> # expect it to be empty (since that is the default checkout
Why does this call-site matter? The URL is given to "git-clone" which I
think does handle the relative URL just fine???
> @@ -195,21 +210,7 @@ cmd_add()
> die "'$path' already exists and is not a valid git repo"
> fi
> else
> - case "$repo" in
> - ./*|../*)
> - # dereference source url relative to parent's url
> - realrepo="$(resolve_relative_url $repo)" ;;
> - *)
> - # Turn the source into an absolute path if
> - # it is local
> - if base=$(get_repo_base "$repo"); then
> - repo="$base"
> - fi
> - realrepo=$repo
> - ;;
> - esac
> -
> - module_clone "$path" "$realrepo" || exit
> + module_clone "$path" "$repo" || exit
> (unset GIT_DIR; cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
> die "Unable to checkout submodule '$path'"
> fi
Ok.
> @@ -262,13 +263,7 @@ cmd_init()
> test -z "$url" &&
> die "No url found for submodule path '$path' in .gitmodules"
>
> - # Possibly a url relative to parent
> - case "$url" in
> - ./*|../*)
> - url="$(resolve_relative_url "$url")"
> - ;;
> - esac
> -
> + url=$(absolute_url "$url")
> git config submodule."$name".url "$url" ||
> die "Failed to register url for submodule path '$path'"
Ok.
--
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
