{"thread":{"id":"33557","subject":"[BUG] Filenames with single colon being treated as remote repository","startedAt":"2013-04-21T04:53:29Z","lastAt":"2013-05-07T16:47:27Z","messageCount":16,"participants":["William Giokas","Jonathan Nieder","Jeff King","Junio C Hamano","Nguyễn Thái Ngọc Duy","Duy Nguyen","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"214957","messageId":"20130421045329.GB30538@WST420","threadId":"33557","inReplyTo":null,"subject":"[BUG] Filenames with single colon being treated as remote repository","fromName":"William Giokas","fromEmail":"1007380@gmail.com","sentAt":"2013-04-21T04:53:29Z","receivedAt":"2013-04-21T04:53:29Z","isPatch":false,"sender":{"key":"1007380@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1731430?v=4"},"body":"All,\n\nIt was brought to my attention today that git has some weird behaviour\nwhen colons (:) are used in directory names.\n\nIn my distros packaging system, for git repositories we clone a bare\nrepo and then clone that bare repo locally as a temporary build\ndirectory (no, we can't use cp, it's a bare repository). Say we have a\ndirectory, /tmp/foo:bar/baz, that is a git repository. If I want to get\na clone of that repository locally, using all of the local\noptimizations, then I need to run::\n\n $ git clone /tmp/foo:bar/baz /tmp/new-baz\n\nbut running this gives me this output::\n\n Cloning into 'new-baz'...\n ssh: Could not resolve hostname /tmp/foo: Success\n fatal: Could not read from remote repository.\n\n Please make sure you have the correct access rights\n and the repository exists.\n \nwhich it should not be doing. It is possible to use a file:// url to\nclone it, but then the --local option is ignored and no optimizations\nare made. After asking on #git, I was directed to the transport.c file,\nbut I don't know what in that is failing. We ran some tests on the\nis_local function and it seems to work correctly.\n\nAny ideas on how to debug this further?\n\nThank you,\n-- \nWilliam Giokas | KaiSforza\nGnuPG Key: 0x73CD09CF\nFingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF\n"},{"id":"214959","messageId":"20130421060538.GB10429@elie.Belkin","threadId":"33557","inReplyTo":"20130421045329.GB30538@WST420","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-21T06:05:39Z","receivedAt":"2013-04-21T06:05:39Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nWilliam Giokas wrote:\n\n> $ git clone /tmp/foo:bar/baz /tmp/new-baz\n>\n> but running this gives me this output::\n>\n> Cloning into 'new-baz'...\n> ssh: Could not resolve hostname /tmp/foo: Success\n> fatal: Could not read from remote repository.\n\nHere's a toy patch. I haven't thought carefully about whether it's a\ngood idea, but maybe it can be useful for thinking about that.\n\nStill needs documentation and tests.\n\nMy main worry is that the proposed rule for when an argument is\ntreated as a local path is hard to explain. There's some precedent in\nhandling of bundles, though. What do you think?\n\nThanks,\nJonathan\n\ndiff --git i/transport.c w/transport.c\nindex e6f9346c..61eba842 100644\n--- i/transport.c\n+++ w/transport.c\n@@ -903,6 +903,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n {\n \tconst char *helper;\n \tstruct transport *ret = xcalloc(1, sizeof(*ret));\n+\tstruct stat st;\n \n \tret->progress = isatty(2);\n \n@@ -942,6 +943,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\tret->disconnect = close_bundle;\n \t\tret->smart_options = NULL;\n \t} else if (!is_url(url)\n+\t\t|| (is_local(url) && !stat(url, &st))\n \t\t|| !prefixcmp(url, \"file://\")\n \t\t|| !prefixcmp(url, \"git://\")\n \t\t|| !prefixcmp(url, \"ssh://\")\n"},{"id":"215010","messageId":"20130421124511.GA1933@sigill.intra.peff.net","threadId":"33557","inReplyTo":"20130421060538.GB10429@elie.Belkin","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-21T12:45:11Z","receivedAt":"2013-04-21T12:45:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 20, 2013 at 11:05:39PM -0700, Jonathan Nieder wrote:\n\n> > Cloning into 'new-baz'...\n> > ssh: Could not resolve hostname /tmp/foo: Success\n> > fatal: Could not read from remote repository.\n> \n> Here's a toy patch. I haven't thought carefully about whether it's a\n> good idea, but maybe it can be useful for thinking about that.\n> \n> Still needs documentation and tests.\n> \n> My main worry is that the proposed rule for when an argument is\n> treated as a local path is hard to explain. There's some precedent in\n> handling of bundles, though. What do you think?\n\nI think the rule could be something like:\n\n 1. If it looks like a URL (\"^scheme://\"), it is.\n\n 2. Otherwise, if it is a path in the filesystem, it is.\n\n 3. Otherwise, if it has a colon, it's host:path\n\n 4. Otherwise, barf.\n\nwhere the interesting bit is the ordering of 2 and 3. It seems like\n\"git clone\" follows the order above with get_repo_path. But we do not\nseem to follow it in git_connect, where we prefer 3 over 2.\n\n> @@ -942,6 +943,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n> \t\tret->disconnect = close_bundle;\n> \t\tret->smart_options = NULL;\n> \t} else if (!is_url(url)\n> +\t\t|| (is_local(url) && !stat(url, &st))\n> \t\t|| !prefixcmp(url, \"file://\")\n> \t\t|| !prefixcmp(url, \"git://\")\n> \t\t|| !prefixcmp(url, \"ssh://\")\n\nI don't think that is enough. Something like /path/to/foo:bar would\ntrigger !is_url already, but then git_connect fails.\n\nTry:\n\n $ git init --bare foo:bar\n $ git clone foo:bar\n ssh: Could not resolve hostname /home/peff/foo: Name or service not known\n fatal: Could not read from remote repository.\n ...\n\nClone recognizes it as a path and turns it into an absolute path. It\nthen feeds it to the transport code, which triggers !is_url and knows to\nuse the git transport. But then git_connect prefers ssh over the\nfilesystem.\n\nIf you do a straight fetch, though, the transport code might see the\nrelative path (if you use one):\n\n $ git init\n $ git init --bare sub:repo\n $ git fetch sub:repo\n ssh: Could not resolve hostname sub: Name or service not known\n\nbut that still triggers the is_url above (which demands the \"://\").\n\nI am not sure whether your patch covers any cases I am missing, but I\nthink you would need an analogous change to git_connect for these common\ncases.\n\n-Peff\n"},{"id":"215022","messageId":"20130421165606.GA29687@elie.Belkin","threadId":"33557","inReplyTo":"20130421124511.GA1933@sigill.intra.peff.net","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-21T16:56:06Z","receivedAt":"2013-04-21T16:56:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> I don't think that is enough. Something like /path/to/foo:bar would\n> trigger !is_url already, but then git_connect fails.\n\nDoh. Here's another try, still untested.\n\ndiff --git i/connect.c w/connect.c\nindex 49e56ba3..fe13942f 100644\n--- i/connect.c\n+++ w/connect.c\n@@ -504,6 +504,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \tint c;\n \tstruct child_process *conn = &no_fork;\n \tenum protocol protocol = PROTO_LOCAL;\n+\tstruct stat st;\n \tint free_path = 0;\n \tchar *port = NULL;\n \tconst char **arg;\n@@ -548,7 +549,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t\tend = host;\n \n \tpath = strchr(end, c);\n-\tif (path && !has_dos_drive_prefix(end)) {\n+\tif (path && !has_dos_drive_prefix(end) &&\n+\t (c != ':' || stat(path, &st))) {\n \t\tif (c == ':') {\n \t\t\tprotocol = PROTO_SSH;\n \t\t\t*path++ = '\\0';\n"},{"id":"215023","messageId":"7vd2tnvk2x.fsf@alter.siamese.dyndns.org","threadId":"33557","inReplyTo":"20130421124511.GA1933@sigill.intra.peff.net","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-21T18:01:58Z","receivedAt":"2013-04-21T18:01:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Apr 20, 2013 at 11:05:39PM -0700, Jonathan Nieder wrote:\n>\n>> > Cloning into 'new-baz'...\n>> > ssh: Could not resolve hostname /tmp/foo: Success\n>> > fatal: Could not read from remote repository.\n>> \n>> Here's a toy patch. I haven't thought carefully about whether it's a\n>> good idea, but maybe it can be useful for thinking about that.\n>> \n>> Still needs documentation and tests.\n>> \n>> My main worry is that the proposed rule for when an argument is\n>> treated as a local path is hard to explain. There's some precedent in\n>> handling of bundles, though. What do you think?\n>\n> I think the rule could be something like:\n>\n> 1. If it looks like a URL (\"^scheme://\"), it is.\n>\n> 2. Otherwise, if it is a path in the filesystem, it is.\n>\n> 3. Otherwise, if it has a colon, it's host:path\n>\n> 4. Otherwise, barf.\n>\n> where the interesting bit is the ordering of 2 and 3. It seems like\n> \"git clone\" follows the order above with get_repo_path. But we do not\n> seem to follow it in git_connect, where we prefer 3 over 2.\n\nAt least for a string whose \"host\" part does not have any slash,\nswitching the rules 2 and 3 in git_connect() would be a regression,\nno? \"frotz:/srv/git/git.git\" has been the way to talk to host frotz\nfor a long time, and if you want to talk to a local directory that\nis a subdirectory of \"frotz:/\" directory you have in your $cwd, you\ncan disambiguate by saying \"./frotz:/srv/git/git.git\" or something.\n"},{"id":"215091","messageId":"20130422153516.GB11886@sigill.intra.peff.net","threadId":"33557","inReplyTo":"7vd2tnvk2x.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-22T15:35:17Z","receivedAt":"2013-04-22T15:35:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 21, 2013 at 11:01:58AM -0700, Junio C Hamano wrote:\n\n> > I think the rule could be something like:\n> >\n> > 1. If it looks like a URL (\"^scheme://\"), it is.\n> >\n> > 2. Otherwise, if it is a path in the filesystem, it is.\n> >\n> > 3. Otherwise, if it has a colon, it's host:path\n> >\n> > 4. Otherwise, barf.\n> >\n> > where the interesting bit is the ordering of 2 and 3. It seems like\n> > \"git clone\" follows the order above with get_repo_path. But we do not\n> > seem to follow it in git_connect, where we prefer 3 over 2.\n> \n> At least for a string whose \"host\" part does not have any slash,\n> switching the rules 2 and 3 in git_connect() would be a regression,\n> no? \"frotz:/srv/git/git.git\" has been the way to talk to host frotz\n> for a long time, and if you want to talk to a local directory that\n> is a subdirectory of \"frotz:/\" directory you have in your $cwd, you\n> can disambiguate by saying \"./frotz:/srv/git/git.git\" or something.\n\nYeah, it would be a regression for fetch, though \"git clone frotz:/srv\"\nis already broken if that file exists; it turns into `realpath\nfrotz:/srv` before we even feed it into the fetch machinery.\n\nSo I think one reasonable path would be:\n\n 1. Do not treat \"host:path\" as ssh if \"host\" has a slash, which should\n not regress anybody. It does not allow unadorned relative paths\n with colons, but it lets you use absolute paths or \"./\" to\n disambiguate.\n\n 2. Teach git-clone to ask the transport code to parse the source repo\n spec, and decide from that whether it is local or not. That would\n harmonize the implementations and avoid errors when you _did_ mean\n to use ssh, but \"host:path\" happens to exist in your filesystem. I\n also would not be surprised if there are problems with\n URL-encoding, but maybe clone handles that properly (I didn't\n check).\n\nAnd the \"host contains slash\" rule is pretty easy to explain in the\ndocumentation, which is good.\n\n-Peff\n"},{"id":"215097","messageId":"7vobd6pncv.fsf@alter.siamese.dyndns.org","threadId":"33557","inReplyTo":"20130422153516.GB11886@sigill.intra.peff.net","subject":"Re: [BUG] Filenames with single colon being treated as remote repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T16:00:00Z","receivedAt":"2013-04-22T16:00:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I think one reasonable path would be:\n>\n> 1. Do not treat \"host:path\" as ssh if \"host\" has a slash, which should\n> not regress anybody. It does not allow unadorned relative paths\n> with colons, but it lets you use absolute paths or \"./\" to\n> disambiguate.\n>\n> 2. Teach git-clone to ask the transport code to parse the source repo\n> spec, and decide from that whether it is local or not. That would\n> harmonize the implementations and avoid errors when you _did_ mean\n> to use ssh, but \"host:path\" happens to exist in your filesystem. I\n> also would not be surprised if there are problems with\n> URL-encoding, but maybe clone handles that properly (I didn't\n> check).\n>\n> And the \"host contains slash\" rule is pretty easy to explain in the\n> documentation, which is good.\n\nSounds like a good direction to take us.\n"},{"id":"215677","messageId":"1367033778-13923-1-git-send-email-pclouds@gmail.com","threadId":"33557","inReplyTo":"20130422153516.GB11886@sigill.intra.peff.net","subject":"[PATCH] clone: allow cloning local paths with colons in them","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-04-27T03:36:18Z","receivedAt":"2013-04-27T03:36:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Usually \"foo:bar\" is interpreted as an ssh url. This patch allows to\nclone from such paths by putting at least one slash before the colon\n(i.e. /path/to/foo:bar or just ./foo:bar).\n\nfile://foo:bar should also work, but local optimizations are off in\nthat case, which may be unwanted. While at there, warn the users about\n--local being ignored in this case.\n\nReported-by: William Giokas <1007380@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n On Mon, Apr 22, 2013 at 10:35 PM, Jeff King wrote:\n > So I think one reasonable path would be:\n >\n >   1. Do not treat \"host:path\" as ssh if \"host\" has a slash, which should\n >      not regress anybody. It does not allow unadorned relative paths\n >      with colons, but it lets you use absolute paths or \"./\" to\n >      disambiguate.\n >\n >   2. Teach git-clone to ask the transport code to parse the source repo\n >      spec, and decide from that whether it is local or not. That would\n >      harmonize the implementations and avoid errors when you _did_ mean\n >      to use ssh, but \"host:path\" happens to exist in your filesystem. I\n >      also would not be surprised if there are problems with\n >      URL-encoding, but maybe clone handles that properly (I didn't\n >      check).\n >\n > And the \"host contains slash\" rule is pretty easy to explain in the\n > documentation, which is good.\n\n I totally agree with this. But doing #2 seems to require a bit of\n code reorganization. How about just this for now?\n\n Documentation/urls.txt | 6 ++++++\n builtin/clone.c | 2 ++\n connect.c | 7 +++++--\n t/t5601-clone.sh | 5 +++++\n 4 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/urls.txt b/Documentation/urls.txt\nindex 3ca122f..476e338 100644\n--- a/Documentation/urls.txt\n+++ b/Documentation/urls.txt\n@@ -23,6 +23,12 @@ An alternative scp-like syntax may also be used with the ssh protocol:\n \n - {startsb}user@{endsb}host.xz:path/to/repo.git/\n \n+This syntax is only recognized if there are no slashes before the\n+first colon. This helps differentiate a local path that contains a\n+colon. For example the local path `foo:bar` could be specified as an\n+absolute path or `./foo:bar` to avoid being misinterpreted as an ssh\n+url.\n+\n The ssh and git protocols additionally support ~username expansion:\n \n - ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 58fee98..e13da4d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -783,6 +783,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local && option_depth)\n \t\twarning(_(\"--depth is ignored in local clones; use file:// instead.\"));\n+\tif (option_local > 0 && !is_local)\n+\t\twarning(_(\"--local is ignored\"));\n \n \tif (argc == 2)\n \t\tdir = xstrdup(argv[1]);\ndiff --git a/connect.c b/connect.c\nindex f57efd0..b568f10 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \tpath = strchr(end, c);\n \tif (path && !has_dos_drive_prefix(end)) {\n \t\tif (c == ':') {\n-\t\t\tprotocol = PROTO_SSH;\n-\t\t\t*path++ = '\\0';\n+\t\t\tif (!strchr(url, '/') || strchr(url, '/') >= path) {\n+\t\t\t\tprotocol = PROTO_SSH;\n+\t\t\t\t*path++ = '\\0';\n+\t\t\t} else\n+\t\t\t\tpath = end;\n \t\t}\n \t} else\n \t\tpath = end;\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 67869b4..0629149 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '\n \ttest_cmp fetch.expected fetch.actual\n '\n \n+test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n+\tcp -R src \"foo:bar\" &&\n+\tgit clone \"./foo:bar\" foobar\n+'\n+\n test_done\n-- \n1.8.2.83.gc99314b\n"},{"id":"215738","messageId":"20130427200842.GA14994@WST420","threadId":"33557","inReplyTo":"1367033778-13923-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] clone: allow cloning local paths with colons in them","fromName":"William Giokas","fromEmail":"1007380@gmail.com","sentAt":"2013-04-27T20:08:42Z","receivedAt":"2013-04-27T20:08:42Z","isPatch":true,"sender":{"key":"1007380@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1731430?v=4"},"body":"On Sat, Apr 27, 2013 at 10:36:18AM +0700, Nguyễn Thái Ngọc Duy wrote:\n> Usually \"foo:bar\" is interpreted as an ssh url. This patch allows to\n> clone from such paths by putting at least one slash before the colon\n> (i.e. /path/to/foo:bar or just ./foo:bar).\n> \n> file://foo:bar should also work, but local optimizations are off in\n> that case, which may be unwanted. While at there, warn the users about\n> --local being ignored in this case.\n> \n> Reported-by: William Giokas <1007380@gmail.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nWorking fine at the moment for the local clones (thank you). It looks\nnice and clean, to me, and doesn't break any existing functionality I\nhave. \n\nThough I did notice that if it is a local file, then you don't actually\nneed a `/` anywhere at all, because I think git looks to see that it is\na local file first. (This is totally fine, though.)\n\n> On Mon, Apr 22, 2013 at 10:35 PM, Jeff King wrote:\n> > So I think one reasonable path would be:\n> >\n> >   1. Do not treat \"host:path\" as ssh if \"host\" has a slash, which should\n> >      not regress anybody. It does not allow unadorned relative paths\n> >      with colons, but it lets you use absolute paths or \"./\" to\n> >      disambiguate.\n> >\n> >   2. Teach git-clone to ask the transport code to parse the source repo\n> >      spec, and decide from that whether it is local or not. That would\n> >      harmonize the implementations and avoid errors when you _did_ mean\n> >      to use ssh, but \"host:path\" happens to exist in your filesystem. I\n> >      also would not be surprised if there are problems with\n> >      URL-encoding, but maybe clone handles that properly (I didn't\n> >      check).\n> >\n> > And the \"host contains slash\" rule is pretty easy to explain in the\n> > documentation, which is good.\n> \n> I totally agree with this. But doing #2 seems to require a bit of\n> code reorganization. How about just this for now?\n>\n> Documentation/urls.txt | 6 ++++++\n> builtin/clone.c | 2 ++\n> connect.c | 7 +++++--\n> t/t5601-clone.sh | 5 +++++\n> 4 files changed, 18 insertions(+), 2 deletions(-)\n\nThank you,\n-- \nWilliam Giokas | KaiSforza\nGnuPG Key: 0x73CD09CF\nFingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF\n"},{"id":"215756","messageId":"7vip37u11a.fsf@alter.siamese.dyndns.org","threadId":"33557","inReplyTo":"1367033778-13923-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] clone: allow cloning local paths with colons in them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-27T21:16:49Z","receivedAt":"2013-04-27T21:16:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> diff --git a/connect.c b/connect.c\n> index f57efd0..b568f10 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n> \tpath = strchr(end, c);\n> \tif (path && !has_dos_drive_prefix(end)) {\n> \t\tif (c == ':') {\n> -\t\t\tprotocol = PROTO_SSH;\n> -\t\t\t*path++ = '\\0';\n> +\t\t\tif (!strchr(url, '/') || strchr(url, '/') >= path) {\n> +\t\t\t\tprotocol = PROTO_SSH;\n> +\t\t\t\t*path++ = '\\0';\n> +\t\t\t} else\n> +\t\t\t\tpath = end;\n> \t\t}\n\nThat was fairly hard to grok. Is that equivalent to this?\n\n if (c == ':' && path < strchrnul(host, '/')) {\n\t\t\t/* is the first slash past the colon? */\n protocol = PROTO_SSH;\n *path++ = '\\0';\n } else {\n path = end;\n }\n"},{"id":"215775","messageId":"CACsJy8DEMrv08D3wGvebV+W73TA8eTH58KHybXpQQbsSJ5NKZA@mail.gmail.com","threadId":"33557","inReplyTo":"7vip37u11a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] clone: allow cloning local paths with colons in them","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-04-28T00:19:29Z","receivedAt":"2013-04-28T00:19:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> diff --git a/connect.c b/connect.c\n>> index f57efd0..b568f10 100644\n>> --- a/connect.c\n>> +++ b/connect.c\n>> @@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n>> path = strchr(end, c);\n>> if (path && !has_dos_drive_prefix(end)) {\n>> if (c == ':') {\n>> - protocol = PROTO_SSH;\n>> - *path++ = '\\0';\n>> + if (!strchr(url, '/') || strchr(url, '/') >= path) {\n>> + protocol = PROTO_SSH;\n>> + *path++ = '\\0';\n>> + } else\n>> + path = end\n>> }\n>\n> That was fairly hard to grok. Is that equivalent to this?\n>\n> if (c == ':' && path < strchrnul(host, '/')) {\n> /* is the first slash past the colon? */\n> protocol = PROTO_SSH;\n> *path++ = '\\0';\n> } else {\n> path = end;\n> }\n>\n\nThe original code is already hard to grok so I may be mistaken here.\nBut I think it's not the same. For the case when c == '/', it will do\n\"path = end;\", which is unintended. It should keep the current \"path\"\nvalue (i.e. == strchr(end, '/')). The use of \"strchrnul(host, '/')\" is\ngood though.\n--\nDuy\n"},{"id":"215779","messageId":"CAPig+cQ+XQYyg2ihvfbZeSApBfWqpLYtq58Jv3iagS5w_iHXbQ@mail.gmail.com","threadId":"33557","inReplyTo":"CACsJy8DEMrv08D3wGvebV+W73TA8eTH58KHybXpQQbsSJ5NKZA@mail.gmail.com","subject":"Re: [PATCH] clone: allow cloning local paths with colons in them","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-04-28T01:48:33Z","receivedAt":"2013-04-28T01:48:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 27, 2013 at 8:19 PM, Duy Nguyen wrote:\n> On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano wrote:\n>> That was fairly hard to grok. Is that equivalent to this?\n>>\n>> if (c == ':' && path < strchrnul(host, '/')) {\n>> /* is the first slash past the colon? */\n>> protocol = PROTO_SSH;\n>> *path++ = '\\0';\n>> } else {\n>> path = end;\n>> }\n>>\n>\n> The original code is already hard to grok so I may be mistaken here.\n> But I think it's not the same. For the case when c == '/', it will do\n> \"path = end;\", which is unintended. It should keep the current \"path\"\n> value (i.e. == strchr(end, '/')). The use of \"strchrnul(host, '/')\" is\n> good though.\n\nDo you want to take Windows '\\' into account also?\n"},{"id":"215782","messageId":"CACsJy8Brw8h6uL91NpE8S6o2K0HWQfWE0ghBEWg-QbfAZOT-FQ@mail.gmail.com","threadId":"33557","inReplyTo":"CAPig+cQ+XQYyg2ihvfbZeSApBfWqpLYtq58Jv3iagS5w_iHXbQ@mail.gmail.com","subject":"Re: [PATCH] clone: allow cloning local paths with colons in them","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-04-28T02:15:04Z","receivedAt":"2013-04-28T02:15:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Apr 28, 2013 at 8:48 AM, Eric Sunshine wrote:\n> On Sat, Apr 27, 2013 at 8:19 PM, Duy Nguyen wrote:\n>> On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano wrote:\n>>> That was fairly hard to grok. Is that equivalent to this?\n>>>\n>>> if (c == ':' && path < strchrnul(host, '/')) {\n>>> /* is the first slash past the colon? */\n>>> protocol = PROTO_SSH;\n>>> *path++ = '\\0';\n>>> } else {\n>>> path = end;\n>>> }\n>>>\n>>\n>> The original code is already hard to grok so I may be mistaken here.\n>> But I think it's not the same. For the case when c == '/', it will do\n>> \"path = end;\", which is unintended. It should keep the current \"path\"\n>> value (i.e. == strchr(end, '/')). The use of \"strchrnul(host, '/')\" is\n>> good though.\n>\n> Do you want to take Windows '\\' into account also?\n\nWindows path does not allow ':' so it's a non-issue in the first place.\n--\nDuy\n"},{"id":"216397","messageId":"1367633973-26214-1-git-send-email-pclouds@gmail.com","threadId":"33557","inReplyTo":"1367033778-13923-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2] clone: allow cloning local paths with colons in them","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-05-04T02:19:33Z","receivedAt":"2013-05-04T02:19:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Usually \"foo:bar\" is interpreted as an ssh url. This patch allows to\nclone from such paths by putting at least one slash before the colon\n(i.e. /path/to/foo:bar or just ./foo:bar).\n\nfile://foo:bar should also work, but local optimizations are off in\nthat case, which may be unwanted. While at there, warn the users about\n--local being ignored in this case.\n\nReported-by: William Giokas <1007380@gmail.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Changes from v1: replace strchr with strchrnul.\n\n Documentation/urls.txt | 6 ++++++\n builtin/clone.c | 2 ++\n connect.c | 7 +++++--\n t/t5601-clone.sh | 5 +++++\n 4 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/urls.txt b/Documentation/urls.txt\nindex 3ca122f..476e338 100644\n--- a/Documentation/urls.txt\n+++ b/Documentation/urls.txt\n@@ -23,6 +23,12 @@ An alternative scp-like syntax may also be used with the ssh protocol:\n \n - {startsb}user@{endsb}host.xz:path/to/repo.git/\n \n+This syntax is only recognized if there are no slashes before the\n+first colon. This helps differentiate a local path that contains a\n+colon. For example the local path `foo:bar` could be specified as an\n+absolute path or `./foo:bar` to avoid being misinterpreted as an ssh\n+url.\n+\n The ssh and git protocols additionally support ~username expansion:\n \n - ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 58fee98..e13da4d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -783,6 +783,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tis_local = option_local != 0 && path && !is_bundle;\n \tif (is_local && option_depth)\n \t\twarning(_(\"--depth is ignored in local clones; use file:// instead.\"));\n+\tif (option_local > 0 && !is_local)\n+\t\twarning(_(\"--local is ignored\"));\n \n \tif (argc == 2)\n \t\tdir = xstrdup(argv[1]);\ndiff --git a/connect.c b/connect.c\nindex f57efd0..a0783d4 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \tpath = strchr(end, c);\n \tif (path && !has_dos_drive_prefix(end)) {\n \t\tif (c == ':') {\n-\t\t\tprotocol = PROTO_SSH;\n-\t\t\t*path++ = '\\0';\n+\t\t\tif (path < strchrnul(host, '/')) {\n+\t\t\t\tprotocol = PROTO_SSH;\n+\t\t\t\t*path++ = '\\0';\n+\t\t\t} else /* '/' in the host part, assume local path */\n+\t\t\t\tpath = end;\n \t\t}\n \t} else\n \t\tpath = end;\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 67869b4..0629149 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '\n \ttest_cmp fetch.expected fetch.actual\n '\n \n+test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n+\tcp -R src \"foo:bar\" &&\n+\tgit clone \"./foo:bar\" foobar\n+'\n+\n test_done\n-- \n1.8.2.83.gc99314b\n"},{"id":"216622","messageId":"7vehdils6c.fsf@alter.siamese.dyndns.org","threadId":"33557","inReplyTo":"1367633973-26214-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2] clone: allow cloning local paths with colons in them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-07T15:34:51Z","receivedAt":"2013-05-07T15:34:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index 67869b4..0629149 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '\n> \ttest_cmp fetch.expected fetch.actual\n> '\n> \n> +test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n> +\tcp -R src \"foo:bar\" &&\n> +\tgit clone \"./foo:bar\" foobar\n> +'\n\nHmph, why not\n\n\tgit clone --mirror src foo:bar &&\n git clone ./foo:bar foobar\n\nor something? Also do we have a easy negative case we want to test,\ni.e. a case where we do not want the new codepath to trigger by\nmistake?\n"},{"id":"216628","messageId":"20130507164727.GA5078@sigill.intra.peff.net","threadId":"33557","inReplyTo":"7vehdils6c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] clone: allow cloning local paths with colons in them","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-07T16:47:27Z","receivedAt":"2013-05-07T16:47:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 07, 2013 at 08:34:51AM -0700, Junio C Hamano wrote:\n\n> Nguyễn Thái Ngọc Duy writes:\n> \n> > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> > index 67869b4..0629149 100755\n> > --- a/t/t5601-clone.sh\n> > +++ b/t/t5601-clone.sh\n> > @@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '\n> > \ttest_cmp fetch.expected fetch.actual\n> > '\n> > \n> > +test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n> > +\tcp -R src \"foo:bar\" &&\n> > +\tgit clone \"./foo:bar\" foobar\n> > +'\n> \n> Hmph, why not\n> \n> \tgit clone --mirror src foo:bar &&\n> git clone ./foo:bar foobar\n\nYeah, not only does that avoid \"cp -R\", but it is a nice check that we\ndo not do anything stupid with colons on the dst argument (which we\nshould obviously not, but it cannot hurt to exercise it).\n\n> or something? Also do we have a easy negative case we want to test,\n> i.e. a case where we do not want the new codepath to trigger by\n> mistake?\n\nYeah, checking \"git clone host:path\" would be nice, but such a case\nwould want to go through ssh. I suspect we could point GIT_SSH at a\nscript like:\n\n #!/bin/sh\n echo \"ssh: $*\" >ssh-log &&\n host=$1; shift\n cd \"pretend-hosts/$host\" && exec \"$@\"\n\nIt looks like t5602 does something similar already.\n\n-Peff\n"}]}