# bug: request-pull broken when remote name contains a slash 8 messages from 2011-01-14 to 2011-03-01. Participants: Uwe Kleine-König, Stefan Naewe, Junio C Hamano. Thread: https://gitlist.dev/t/26279 ## Uwe Kleine-König, 2011-01-14 09:06 Subject: bug: request-pull broken when remote name contains a slash Message-ID: <20110114090645.GA13060@pengutronix.de> URL: https://gitlist.dev/e/20110114090645.GA13060%40pengutronix.de ``` Hello, the remotes in my linux repo look as follows: [remote "ptx/ukl"] url = git://git.pengutronix.de/git/ukl/linux-2.6.git push = +refs/heads/*:refs/heads/* fetch = +refs/heads/*:refs/remotes/ptx/ukl/* [remote "ptx/otheruser"] url = git://git.pengutronix.de/git/otheruser/linux-2.6.git fetch = +refs/heads/*:refs/remotes/ptx/otheruser/* [remote "ptx/yetanotheruser"] url = git://git.pengutronix.de/git/yetanotheruser/linux-2.6.git fetch = +refs/heads/*:refs/remotes/ptx/yetanotheruser/* unfortunately this makes sending request pulls uncomfortable: ukl@octopus:~/gsrc/linux-2.6$ git request-pull HEAD^ ptx/ukl The following changes since commit a08948812b30653eb2c536ae613b635a989feb6f: Merge branch 'hwmon-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/staging (2011-01-10 08:57:46 -0800) are available in the git repository at: ptx/ukl mxs/for-2.6.38 ... The reason for that is in git-parse-remote.sh:get_data_source() which assumes that a remote with a slash is a filename and so get_remote_url doesn't use $(git config --get "remote.$1.url") but ptx/ukl directly. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ``` ## Stefan Naewe, 2011-01-14 11:36 Subject: [PATCH] fix git-parse-remote.sh for remotes that contain slashes Message-ID: <1295005000-11562-1-git-send-email-stefan.naewe@gmail.com> URL: https://gitlist.dev/e/1295005000-11562-1-git-send-email-stefan.naewe%40gmail.com In-Reply-To: <20110114090645.GA13060@pengutronix.de> ``` Signed-off-by: Stefan Naewe --- git-parse-remote.sh | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 5f47b18..7cf204e 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -7,8 +7,12 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :; get_data_source () { case "$1" in */*) - echo '' - ;; + if test "$(git config --get "remote.$1.url")" + then + echo config + else + echo '' + fi ;; .) echo self ;; -- 1.7.3.5 ``` ## Junio C Hamano, 2011-01-14 19:55 Subject: Re: [PATCH] fix git-parse-remote.sh for remotes that contain slashes Message-ID: <7vd3nzntuf.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd3nzntuf.fsf%40alter.siamese.dyndns.org In-Reply-To: <1295005000-11562-1-git-send-email-stefan.naewe@gmail.com> ``` Stefan Naewe writes: > Signed-off-by: Stefan Naewe > --- Thanks, but no explanation? Imagine somebody who weren't reading this thread (especially the article you responded to with this patch) sees this in "git log" output stream. For that matter, imagine yourself doing that in 2012 when the motivation of this change you all forgot already. Do you think it is obvious what the problem the patch tried to fix was? I don't. "fix" on the subject line gives you 0-bit information for that purpose. > diff --git a/git-parse-remote.sh b/git-parse-remote.sh > index 5f47b18..7cf204e 100644 > --- a/git-parse-remote.sh > +++ b/git-parse-remote.sh > @@ -7,8 +7,12 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :; > get_data_source () { > case "$1" in > */*) > - echo '' > - ;; > + if test "$(git config --get "remote.$1.url")" > + then > + echo config > + else > + echo '' > + fi ;; I suspect that making this case arm trigger not on */* but only on /* and ../* would be a lot more sensible solution. Otherwise you would still have the same issue in repositories that use remotes/ and branches/ mechanism. git-parse-remote.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 1cc2ba6..8ec33e3 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -6,7 +6,7 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :; get_data_source () { case "$1" in - */*) + ../* | /*) echo '' ;; .) ``` ## Uwe Kleine-König, 2011-02-28 09:36 Subject: [PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls Message-ID: <1298885779-10045-1-git-send-email-u.kleine-koenig@pengutronix.de> URL: https://gitlist.dev/e/1298885779-10045-1-git-send-email-u.kleine-koenig%40pengutronix.de In-Reply-To: <20110114090645.GA13060@pengutronix.de> ``` The formerly implemented algorithm behaved differently to remote.c:remote_get() at least for remotes that contain a slash. While the former just assumes a/b is a path the latter checks the config for remote."a/b" first which is more reasonable. Signed-off-by: Uwe Kleine-König --- Hello, with this patch git-request-pull.sh (== the only in-tree user of git-parse-remote.sh:get_remote_url()) could directly use git ls-remote --get-url . I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url() though?! Maybe add a deprecation warning to it? The same applies to git-parse-remote.sh:get_data_source() that isn't used anymore already with this patch. Best regards Uwe builtin/ls-remote.c | 11 +++++++++++ git-parse-remote.sh | 24 +----------------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 97eed40..1a1ff87 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -33,6 +33,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) int i; const char *dest = NULL; unsigned flags = 0; + int get_url = 0; int quiet = 0; const char *uploadpack = NULL; const char **pattern = NULL; @@ -69,6 +70,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) quiet = 1; continue; } + if (!strcmp("--get-url", arg)) { + get_url = 1; + continue; + } usage(ls_remote_usage); } dest = arg; @@ -94,6 +99,12 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) } if (!remote->url_nr) die("remote %s has no configured URL", dest); + + if (get_url) { + printf("%s\n", *remote->url); + return 0; + } + transport = transport_get(remote, NULL); if (uploadpack != NULL) transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack); diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 1cc2ba6..4fb778e 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -29,29 +29,7 @@ get_data_source () { } get_remote_url () { - data_source=$(get_data_source "$1") - case "$data_source" in - '') - echo "$1" - ;; - self) - echo "$1" - ;; - config) - git config --get "remote.$1.url" - ;; - remotes) - sed -ne '/^URL: */{ - s///p - q - }' "$GIT_DIR/remotes/$1" - ;; - branches) - sed -e 's/#.*//' "$GIT_DIR/branches/$1" - ;; - *) - die "internal error: get-remote-url $1" ;; - esac + git ls-remote --get-url "$1" } get_default_remote () { -- 1.7.2.3 ``` ## Junio C Hamano, 2011-02-28 23:38 Subject: Re: [PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls Message-ID: <7v4o7nhgr1.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v4o7nhgr1.fsf%40alter.siamese.dyndns.org In-Reply-To: <1298885779-10045-1-git-send-email-u.kleine-koenig@pengutronix.de> ``` Uwe Kleine-König writes: > with this patch git-request-pull.sh (== the only in-tree user of > git-parse-remote.sh:get_remote_url()) could directly use > > git ls-remote --get-url > > . I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url() > though?! If nobody uses it, why not? I was actually hoping for the day that nobody uses any of the defined function so taht we can remove that whole file. ``` ## Uwe Kleine-König, 2011-03-01 08:41 Subject: Re: [PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls Message-ID: <20110301084110.GT22310@pengutronix.de> URL: https://gitlist.dev/e/20110301084110.GT22310%40pengutronix.de In-Reply-To: <7v4o7nhgr1.fsf@alter.siamese.dyndns.org> ``` On Mon, Feb 28, 2011 at 03:38:26PM -0800, Junio C Hamano wrote: > Uwe Kleine-König writes: > > > with this patch git-request-pull.sh (== the only in-tree user of > > git-parse-remote.sh:get_remote_url()) could directly use > > > > git ls-remote --get-url > > > > . I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url() > > though?! > > If nobody uses it, why not? I was actually hoping for the day that nobody > uses any of the defined function so taht we can remove that whole file. Ok for me. Probably I'm a bit conservative here because I usually work on the kernel and people are picky here to remove interfaces. I can followup with a patch. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ``` ## Uwe Kleine-König, 2011-03-01 09:21 Subject: [PATCH 1/2] get_remote_url(): use the same data source as ls-remote to get remote urls Message-ID: <1298971297-20326-1-git-send-email-u.kleine-koenig@pengutronix.de> URL: https://gitlist.dev/e/1298971297-20326-1-git-send-email-u.kleine-koenig%40pengutronix.de In-Reply-To: <20110301084110.GT22310@pengutronix.de> ``` The formerly implemented algorithm behaved differently to remote.c:remote_get() at least for remotes that contain a slash. While the former just assumes a/b is a path the latter checks the config for remote."a/b" first which is more reasonable. This removes the last user of git-parse-remote.sh:get_data_source(), so this function is removed. Signed-off-by: Uwe Kleine-König --- Hello, compared with the previous patch this patch removes get_data_source which is unused now. The second patch also gets rid of get_remote_url. builtin/ls-remote.c | 11 +++++++++++ git-parse-remote.sh | 48 +----------------------------------------------- 2 files changed, 12 insertions(+), 47 deletions(-) The changes in this series are also available in the git repository at: git.pengutronix.de:/git/ukl/git.git ls-remote-in-get-remote-url basing on commit 7ed863a85a6ce2c4ac4476848310b8f917ab41f9: Git 1.7.4 (2011-01-30 19:02:37 -0800) Uwe Kleine-König (2): get_remote_url(): use the same data source as ls-remote to get remote urls git-request-pull: open-code the only invocation of get_remote_url (Note: this pull request was generated using the new git-request-pull and using a remote with a slash \o/) builtin/ls-remote.c | 11 +++++++++++ git-parse-remote.sh | 50 -------------------------------------------------- git-request-pull.sh | 3 +-- 3 files changed, 12 insertions(+), 52 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 97eed40..1a1ff87 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -33,6 +33,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) int i; const char *dest = NULL; unsigned flags = 0; + int get_url = 0; int quiet = 0; const char *uploadpack = NULL; const char **pattern = NULL; @@ -69,6 +70,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) quiet = 1; continue; } + if (!strcmp("--get-url", arg)) { + get_url = 1; + continue; + } usage(ls_remote_usage); } dest = arg; @@ -94,6 +99,12 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) } if (!remote->url_nr) die("remote %s has no configured URL", dest); + + if (get_url) { + printf("%s\n", *remote->url); + return 0; + } + transport = transport_get(remote, NULL); if (uploadpack != NULL) transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack); diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 1cc2ba6..0ab1192 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -4,54 +4,8 @@ # this would fail in that case and would issue an error message. GIT_DIR=$(git rev-parse -q --git-dir) || :; -get_data_source () { - case "$1" in - */*) - echo '' - ;; - .) - echo self - ;; - *) - if test "$(git config --get "remote.$1.url")" - then - echo config - elif test -f "$GIT_DIR/remotes/$1" - then - echo remotes - elif test -f "$GIT_DIR/branches/$1" - then - echo branches - else - echo '' - fi ;; - esac -} - get_remote_url () { - data_source=$(get_data_source "$1") - case "$data_source" in - '') - echo "$1" - ;; - self) - echo "$1" - ;; - config) - git config --get "remote.$1.url" - ;; - remotes) - sed -ne '/^URL: */{ - s///p - q - }' "$GIT_DIR/remotes/$1" - ;; - branches) - sed -e 's/#.*//' "$GIT_DIR/branches/$1" - ;; - *) - die "internal error: get-remote-url $1" ;; - esac + git ls-remote --get-url "$1" } get_default_remote () { -- 1.7.2.3 ``` ## Uwe Kleine-König, 2011-03-01 09:21 Subject: [PATCH 2/2] git-request-pull: open-code the only invocation of get_remote_url Message-ID: <1298971297-20326-2-git-send-email-u.kleine-koenig@pengutronix.de> URL: https://gitlist.dev/e/1298971297-20326-2-git-send-email-u.kleine-koenig%40pengutronix.de In-Reply-To: <20110301084110.GT22310@pengutronix.de> ``` So sh:get_remote_url can go now and git-request-pull doesn't need to source git-parse-remote. anymore. Signed-off-by: Uwe Kleine-König --- git-parse-remote.sh | 4 ---- git-request-pull.sh | 3 +-- 2 files changed, 1 insertions(+), 6 deletions(-) diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 0ab1192..e7013f7 100644 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -4,10 +4,6 @@ # this would fail in that case and would issue an error message. GIT_DIR=$(git rev-parse -q --git-dir) || :; -get_remote_url () { - git ls-remote --get-url "$1" -} - get_default_remote () { curr_branch=$(git symbolic-ref -q HEAD | sed -e 's|^refs/heads/||') origin=$(git config --get "branch.$curr_branch.remote") diff --git a/git-request-pull.sh b/git-request-pull.sh index 6fdea39..fc080cc 100755 --- a/git-request-pull.sh +++ b/git-request-pull.sh @@ -15,7 +15,6 @@ p show patch text as well ' . git-sh-setup -. git-parse-remote GIT_PAGER= export GIT_PAGER @@ -55,7 +54,7 @@ branch=$(git ls-remote "$url" \ p q }") -url=$(get_remote_url "$url") +url=$(git ls-remote --get-url "$url") if [ -z "$branch" ]; then echo "warn: No branch of $url is at:" >&2 git log --max-count=1 --pretty='tformat:warn: %h: %s' $headrev >&2 -- 1.7.2.3 ```