{"thread":{"id":"26279","subject":"bug: request-pull broken when remote name contains a slash","startedAt":"2011-01-14T09:06:45Z","lastAt":"2011-03-01T09:21:37Z","messageCount":8,"participants":["Uwe Kleine-König","Stefan Naewe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"159470","messageId":"20110114090645.GA13060@pengutronix.de","threadId":"26279","inReplyTo":null,"subject":"bug: request-pull broken when remote name contains a slash","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2011-01-14T09:06:45Z","receivedAt":"2011-01-14T09:06:45Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nthe remotes in my linux repo look as follows:\n\n\t[remote \"ptx/ukl\"]\n\t\turl = git://git.pengutronix.de/git/ukl/linux-2.6.git\n\t\tpush = +refs/heads/*:refs/heads/*\n\t\tfetch = +refs/heads/*:refs/remotes/ptx/ukl/*\n\n\t[remote \"ptx/otheruser\"]\n\t\turl = git://git.pengutronix.de/git/otheruser/linux-2.6.git\n\t\tfetch = +refs/heads/*:refs/remotes/ptx/otheruser/*\n\n\t[remote \"ptx/yetanotheruser\"]\n\t\turl = git://git.pengutronix.de/git/yetanotheruser/linux-2.6.git\n\t\tfetch = +refs/heads/*:refs/remotes/ptx/yetanotheruser/*\n\nunfortunately this makes sending request pulls uncomfortable:\n\n\tukl@octopus:~/gsrc/linux-2.6$ git request-pull HEAD^ ptx/ukl\n\tThe following changes since commit a08948812b30653eb2c536ae613b635a989feb6f:\n\n\t  Merge branch 'hwmon-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/staging (2011-01-10 08:57:46 -0800)\n\n\tare available in the git repository at:\n\n\t  ptx/ukl mxs/for-2.6.38\n\t...\n\nThe reason for that is in git-parse-remote.sh:get_data_source() which\nassumes that a remote with a slash is a filename and so get_remote_url\ndoesn't use $(git config --get \"remote.$1.url\") but ptx/ukl directly.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K.                           | Uwe Kleine-König            |\nIndustrial Linux Solutions                 | http://www.pengutronix.de/  |\n"},{"id":"159480","messageId":"1295005000-11562-1-git-send-email-stefan.naewe@gmail.com","threadId":"26279","inReplyTo":"20110114090645.GA13060@pengutronix.de","subject":"[PATCH] fix git-parse-remote.sh for remotes that contain slashes","fromName":"Stefan Naewe","fromEmail":"stefan.naewe@gmail.com","sentAt":"2011-01-14T11:36:40Z","receivedAt":"2011-01-14T11:36:40Z","isPatch":true,"sender":{"key":"stefan.naewe@gmail.com","avatar":"https://avatars.githubusercontent.com/u/4468?v=4"},"body":"Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>\n---\n git-parse-remote.sh |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 5f47b18..7cf204e 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -7,8 +7,12 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :;\n get_data_source () {\n \tcase \"$1\" in\n \t*/*)\n-\t\techo ''\n-\t\t;;\n+\t\tif test \"$(git config --get \"remote.$1.url\")\"\n+\t\tthen\n+\t\t\techo config\n+\t\telse\n+\t\t\techo ''\n+\t\tfi ;;\n \t.)\n \t\techo self\n \t\t;;\n-- \n1.7.3.5\n"},{"id":"159504","messageId":"7vd3nzntuf.fsf@alter.siamese.dyndns.org","threadId":"26279","inReplyTo":"1295005000-11562-1-git-send-email-stefan.naewe@gmail.com","subject":"Re: [PATCH] fix git-parse-remote.sh for remotes that contain slashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-14T19:55:20Z","receivedAt":"2011-01-14T19:55:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Naewe <stefan.naewe@gmail.com> writes:\n\n> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>\n> ---\n\nThanks, but no explanation?\n\nImagine somebody who weren't reading this thread (especially the article\nyou responded to with this patch) sees this in \"git log\" output stream.\nFor that matter, imagine yourself doing that in 2012 when the motivation\nof this change you all forgot already.\n\nDo you think it is obvious what the problem the patch tried to fix was?\nI don't.  \"fix\" on the subject line gives you 0-bit information for that\npurpose.\n\n> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n> index 5f47b18..7cf204e 100644\n> --- a/git-parse-remote.sh\n> +++ b/git-parse-remote.sh\n> @@ -7,8 +7,12 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :;\n>  get_data_source () {\n>  \tcase \"$1\" in\n>  \t*/*)\n> -\t\techo ''\n> -\t\t;;\n> +\t\tif test \"$(git config --get \"remote.$1.url\")\"\n> +\t\tthen\n> +\t\t\techo config\n> +\t\telse\n> +\t\t\techo ''\n> +\t\tfi ;;\n\nI suspect that making this case arm trigger not on */* but only on /* and\n../* would be a lot more sensible solution.  Otherwise you would still\nhave the same issue in repositories that use remotes/ and branches/\nmechanism.\n\n git-parse-remote.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 1cc2ba6..8ec33e3 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -6,7 +6,7 @@ GIT_DIR=$(git rev-parse -q --git-dir) || :;\n \n get_data_source () {\n \tcase \"$1\" in\n-\t*/*)\n+\t../* | /*)\n \t\techo ''\n \t\t;;\n \t.)\n"},{"id":"162412","messageId":"1298885779-10045-1-git-send-email-u.kleine-koenig@pengutronix.de","threadId":"26279","inReplyTo":"20110114090645.GA13060@pengutronix.de","subject":"[PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2011-02-28T09:36:19Z","receivedAt":"2011-02-28T09:36:19Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"The formerly implemented algorithm behaved differently to\nremote.c:remote_get() at least for remotes that contain a slash.  While the\nformer just assumes a/b is a path the latter checks the config for\nremote.\"a/b\" first which is more reasonable.\n\nSigned-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>\n---\nHello,\n\nwith this patch git-request-pull.sh (== the only in-tree user of\ngit-parse-remote.sh:get_remote_url()) could directly use\n\n\tgit ls-remote --get-url\n\n.  I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url()\nthough?!  Maybe add a deprecation warning to it?  The same applies to\ngit-parse-remote.sh:get_data_source() that isn't used anymore already with this\npatch.\n\nBest regards\nUwe\n\n builtin/ls-remote.c |   11 +++++++++++\n git-parse-remote.sh |   24 +-----------------------\n 2 files changed, 12 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 97eed40..1a1ff87 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -33,6 +33,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint i;\n \tconst char *dest = NULL;\n \tunsigned flags = 0;\n+\tint get_url = 0;\n \tint quiet = 0;\n \tconst char *uploadpack = NULL;\n \tconst char **pattern = NULL;\n@@ -69,6 +70,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t\t\tquiet = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(\"--get-url\", arg)) {\n+\t\t\t\tget_url = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tusage(ls_remote_usage);\n \t\t}\n \t\tdest = arg;\n@@ -94,6 +99,12 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t}\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n+\n+\tif (get_url) {\n+\t\tprintf(\"%s\\n\", *remote->url);\n+\t\treturn 0;\n+\t}\n+\n \ttransport = transport_get(remote, NULL);\n \tif (uploadpack != NULL)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 1cc2ba6..4fb778e 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -29,29 +29,7 @@ get_data_source () {\n }\n \n get_remote_url () {\n-\tdata_source=$(get_data_source \"$1\")\n-\tcase \"$data_source\" in\n-\t'')\n-\t\techo \"$1\"\n-\t\t;;\n-\tself)\n-\t\techo \"$1\"\n-\t\t;;\n-\tconfig)\n-\t\tgit config --get \"remote.$1.url\"\n-\t\t;;\n-\tremotes)\n-\t\tsed -ne '/^URL: */{\n-\t\t\ts///p\n-\t\t\tq\n-\t\t}' \"$GIT_DIR/remotes/$1\"\n-\t\t;;\n-\tbranches)\n-\t\tsed -e 's/#.*//' \"$GIT_DIR/branches/$1\"\n-\t\t;;\n-\t*)\n-\t\tdie \"internal error: get-remote-url $1\" ;;\n-\tesac\n+\tgit ls-remote --get-url \"$1\"\n }\n \n get_default_remote () {\n-- \n1.7.2.3\n"},{"id":"162494","messageId":"7v4o7nhgr1.fsf@alter.siamese.dyndns.org","threadId":"26279","inReplyTo":"1298885779-10045-1-git-send-email-u.kleine-koenig@pengutronix.de","subject":"Re: [PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-28T23:38:26Z","receivedAt":"2011-02-28T23:38:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:\n\n> with this patch git-request-pull.sh (== the only in-tree user of\n> git-parse-remote.sh:get_remote_url()) could directly use\n>\n> \tgit ls-remote --get-url\n>\n> .  I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url()\n> though?!\n\nIf nobody uses it, why not?  I was actually hoping for the day that nobody\nuses any of the defined function so taht we can remove that whole file.\n"},{"id":"162520","messageId":"20110301084110.GT22310@pengutronix.de","threadId":"26279","inReplyTo":"7v4o7nhgr1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] get_remote_url(): use the same data source as ls-remote to get remote urls","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2011-03-01T08:41:10Z","receivedAt":"2011-03-01T08:41:10Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"On Mon, Feb 28, 2011 at 03:38:26PM -0800, Junio C Hamano wrote:\n> Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:\n> \n> > with this patch git-request-pull.sh (== the only in-tree user of\n> > git-parse-remote.sh:get_remote_url()) could directly use\n> >\n> > \tgit ls-remote --get-url\n> >\n> > .  I guess you wouldn't want to remove git-parse-remote.sh:get_remote_url()\n> > though?!\n> \n> If nobody uses it, why not?  I was actually hoping for the day that nobody\n> uses any of the defined function so taht we can remove that whole file.\nOk for me.  Probably I'm a bit conservative here because I usually work\non the kernel and people are picky here to remove interfaces.\n\nI can followup with a patch.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K.                           | Uwe Kleine-König            |\nIndustrial Linux Solutions                 | http://www.pengutronix.de/  |\n"},{"id":"162527","messageId":"1298971297-20326-1-git-send-email-u.kleine-koenig@pengutronix.de","threadId":"26279","inReplyTo":"20110301084110.GT22310@pengutronix.de","subject":"[PATCH 1/2] get_remote_url(): use the same data source as ls-remote to get remote urls","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2011-03-01T09:21:36Z","receivedAt":"2011-03-01T09:21:36Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"The formerly implemented algorithm behaved differently to\nremote.c:remote_get() at least for remotes that contain a slash.  While the\nformer just assumes a/b is a path the latter checks the config for\nremote.\"a/b\" first which is more reasonable.\n\nThis removes the last user of git-parse-remote.sh:get_data_source(), so\nthis function is removed.\n\nSigned-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>\n---\nHello,\n\ncompared with the previous patch this patch removes get_data_source\nwhich is unused now.  The second patch also gets rid of get_remote_url.\n\n builtin/ls-remote.c |   11 +++++++++++\n git-parse-remote.sh |   48 +-----------------------------------------------\n 2 files changed, 12 insertions(+), 47 deletions(-)\n\n\nThe changes in this series are also available in the git repository at:\n  git.pengutronix.de:/git/ukl/git.git ls-remote-in-get-remote-url\n\nbasing on commit 7ed863a85a6ce2c4ac4476848310b8f917ab41f9:\n\n  Git 1.7.4 (2011-01-30 19:02:37 -0800)\n\nUwe Kleine-König (2):\n      get_remote_url(): use the same data source as ls-remote to get remote urls\n      git-request-pull: open-code the only invocation of get_remote_url\n\n(Note: this pull request was generated using the new git-request-pull\nand using a remote with a slash \\o/)\n\n builtin/ls-remote.c |   11 +++++++++++\n git-parse-remote.sh |   50 --------------------------------------------------\n git-request-pull.sh |    3 +--\n 3 files changed, 12 insertions(+), 52 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 97eed40..1a1ff87 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -33,6 +33,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \tint i;\n \tconst char *dest = NULL;\n \tunsigned flags = 0;\n+\tint get_url = 0;\n \tint quiet = 0;\n \tconst char *uploadpack = NULL;\n \tconst char **pattern = NULL;\n@@ -69,6 +70,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t\t\tquiet = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(\"--get-url\", arg)) {\n+\t\t\t\tget_url = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tusage(ls_remote_usage);\n \t\t}\n \t\tdest = arg;\n@@ -94,6 +99,12 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t}\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n+\n+\tif (get_url) {\n+\t\tprintf(\"%s\\n\", *remote->url);\n+\t\treturn 0;\n+\t}\n+\n \ttransport = transport_get(remote, NULL);\n \tif (uploadpack != NULL)\n \t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 1cc2ba6..0ab1192 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -4,54 +4,8 @@\n # this would fail in that case and would issue an error message.\n GIT_DIR=$(git rev-parse -q --git-dir) || :;\n \n-get_data_source () {\n-\tcase \"$1\" in\n-\t*/*)\n-\t\techo ''\n-\t\t;;\n-\t.)\n-\t\techo self\n-\t\t;;\n-\t*)\n-\t\tif test \"$(git config --get \"remote.$1.url\")\"\n-\t\tthen\n-\t\t\techo config\n-\t\telif test -f \"$GIT_DIR/remotes/$1\"\n-\t\tthen\n-\t\t\techo remotes\n-\t\telif test -f \"$GIT_DIR/branches/$1\"\n-\t\tthen\n-\t\t\techo branches\n-\t\telse\n-\t\t\techo ''\n-\t\tfi ;;\n-\tesac\n-}\n-\n get_remote_url () {\n-\tdata_source=$(get_data_source \"$1\")\n-\tcase \"$data_source\" in\n-\t'')\n-\t\techo \"$1\"\n-\t\t;;\n-\tself)\n-\t\techo \"$1\"\n-\t\t;;\n-\tconfig)\n-\t\tgit config --get \"remote.$1.url\"\n-\t\t;;\n-\tremotes)\n-\t\tsed -ne '/^URL: */{\n-\t\t\ts///p\n-\t\t\tq\n-\t\t}' \"$GIT_DIR/remotes/$1\"\n-\t\t;;\n-\tbranches)\n-\t\tsed -e 's/#.*//' \"$GIT_DIR/branches/$1\"\n-\t\t;;\n-\t*)\n-\t\tdie \"internal error: get-remote-url $1\" ;;\n-\tesac\n+\tgit ls-remote --get-url \"$1\"\n }\n \n get_default_remote () {\n-- \n1.7.2.3\n"},{"id":"162528","messageId":"1298971297-20326-2-git-send-email-u.kleine-koenig@pengutronix.de","threadId":"26279","inReplyTo":"20110301084110.GT22310@pengutronix.de","subject":"[PATCH 2/2] git-request-pull: open-code the only invocation of get_remote_url","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2011-03-01T09:21:37Z","receivedAt":"2011-03-01T09:21:37Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"So sh:get_remote_url can go now and git-request-pull\ndoesn't need to source git-parse-remote. anymore.\n\nSigned-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>\n---\n git-parse-remote.sh |    4 ----\n git-request-pull.sh |    3 +--\n 2 files changed, 1 insertions(+), 6 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 0ab1192..e7013f7 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -4,10 +4,6 @@\n # this would fail in that case and would issue an error message.\n GIT_DIR=$(git rev-parse -q --git-dir) || :;\n \n-get_remote_url () {\n-\tgit ls-remote --get-url \"$1\"\n-}\n-\n get_default_remote () {\n \tcurr_branch=$(git symbolic-ref -q HEAD | sed -e 's|^refs/heads/||')\n \torigin=$(git config --get \"branch.$curr_branch.remote\")\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 6fdea39..fc080cc 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -15,7 +15,6 @@ p    show patch text as well\n '\n \n . git-sh-setup\n-. git-parse-remote\n \n GIT_PAGER=\n export GIT_PAGER\n@@ -55,7 +54,7 @@ branch=$(git ls-remote \"$url\" \\\n \t\tp\n \t\tq\n \t}\")\n-url=$(get_remote_url \"$url\")\n+url=$(git ls-remote --get-url \"$url\")\n if [ -z \"$branch\" ]; then\n \techo \"warn: No branch of $url is at:\" >&2\n \tgit log --max-count=1 --pretty='tformat:warn:   %h: %s' $headrev >&2\n-- \n1.7.2.3\n"}]}