From: Junio C Hamano <gitster@pobox.com>
To: Ping Yin <pkufranky@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/7] git-submodule: Extract absolute_url & move absolute url logic to module_clone
Date: Mon, 21 Apr 2008 23:10:38 -0700 [thread overview]
Message-ID: <7v3ape5sip.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1208355577-8734-3-git-send-email-pkufranky@gmail.com> (Ping Yin's message of "Wed, 16 Apr 2008 22:19:32 +0800")
Ping Yin <pkufranky@gmail.com> 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.
next prev parent reply other threads:[~2008-04-22 6:11 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-16 14:19 [PATCH 0/7] submodule: fallback to .gitmodules and multiple level module definition Ping Yin
2008-04-16 14:19 ` [PATCH 1/7] git-submodule: Extract functions module_info and module_url Ping Yin
2008-04-16 14:19 ` [PATCH 2/7] git-submodule: Extract absolute_url & move absolute url logic to module_clone Ping Yin
2008-04-16 14:19 ` [PATCH 3/7] git-submodule: Fall back on .gitmodules if info not found in $GIT_DIR/config Ping Yin
2008-04-16 14:19 ` [PATCH 4/7] git-submodule: Extract module_add from cmd_add Ping Yin
2008-04-16 14:19 ` [PATCH 5/7] git-submodule: multi-level module definition Ping Yin
2008-04-16 14:19 ` [PATCH 6/7] git-submodule: "update --force" to enforce cloning non-submodule Ping Yin
2008-04-16 14:19 ` [PATCH 7/7] git-submodule: Don't die when command fails for one submodule Ping Yin
2008-04-22 6:10 ` Junio C Hamano [this message]
2008-04-22 6:50 ` [PATCH 2/7] git-submodule: Extract absolute_url & move absolute url logic to module_clone Ping Yin
2008-04-22 7:57 ` Junio C Hamano
2008-04-22 9:09 ` Ping Yin
2008-04-22 14:38 ` Ping Yin
2008-04-22 14:41 ` Ping Yin
[not found] ` <alpine.DEB.1.00.0804221609200.4460@eeepc-johanness>
2008-04-22 16:54 ` Ping Yin
2008-04-22 17:13 ` Jakub Narebski
2008-04-22 17:20 ` Ping Yin
2008-04-22 7:00 ` Ping Yin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7v3ape5sip.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=pkufranky@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).