[BUG] Filenames with single colon being treated as remote repository

[BUG] Filenames with single colon being treated as remote repository

[William Giokas]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

All,

It was brought to my attention today that git has some weird behaviour
when colons (:) are used in directory names.

In my distros packaging system, for git repositories we clone a bare
repo and then clone that bare repo locally as a temporary build
directory (no, we can't use cp, it's a bare repository). Say we have a
directory, /tmp/foo:bar/baz, that is a git repository. If I want to get
a clone of that repository locally, using all of the local
optimizations, then I need to run::

    $ git clone /tmp/foo:bar/baz /tmp/new-baz

but running this gives me this output::

    Cloning into 'new-baz'...
    ssh: Could not resolve hostname /tmp/foo: Success
    fatal: Could not read from remote repository.

    Please make sure you have the correct access rights
    and the repository exists.
    
which it should not be doing. It is possible to use a file:// url to
clone it, but then the --local option is ignored and no optimizations
are made. After asking on #git, I was directed to the transport.c file,
but I don't know what in that is failing. We ran some tests on the
is_local function and it seems to work correctly.

Any ideas on how to debug this further?

Thank you,
-- 
William Giokas | KaiSforza
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [BUG] Filenames with single colon being treated as remote repository

[Jonathan Nieder]

Hi,

William Giokas wrote:

>     $ git clone /tmp/foo:bar/baz /tmp/new-baz
>
> but running this gives me this output::
>
>     Cloning into 'new-baz'...
>     ssh: Could not resolve hostname /tmp/foo: Success
>     fatal: Could not read from remote repository.

Here's a toy patch.  I haven't thought carefully about whether it's a
good idea, but maybe it can be useful for thinking about that.

Still needs documentation and tests.

My main worry is that the proposed rule for when an argument is
treated as a local path is hard to explain.  There's some precedent in
handling of bundles, though.  What do you think?

Thanks,
Jonathan

diff --git i/transport.c w/transport.c
index e6f9346c..61eba842 100644
--- i/transport.c
+++ w/transport.c
@@ -903,6 +903,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 {
 	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
+	struct stat st;
 
 	ret->progress = isatty(2);
 
@@ -942,6 +943,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		ret->disconnect = close_bundle;
 		ret->smart_options = NULL;
 	} else if (!is_url(url)
+		|| (is_local(url) && !stat(url, &st))
 		|| !prefixcmp(url, "file://")
 		|| !prefixcmp(url, "git://")
 		|| !prefixcmp(url, "ssh://")

Re: [BUG] Filenames with single colon being treated as remote repository

[Jeff King]

On Sat, Apr 20, 2013 at 11:05:39PM -0700, Jonathan Nieder wrote:

> >     Cloning into 'new-baz'...
> >     ssh: Could not resolve hostname /tmp/foo: Success
> >     fatal: Could not read from remote repository.
> 
> Here's a toy patch.  I haven't thought carefully about whether it's a
> good idea, but maybe it can be useful for thinking about that.
> 
> Still needs documentation and tests.
> 
> My main worry is that the proposed rule for when an argument is
> treated as a local path is hard to explain.  There's some precedent in
> handling of bundles, though.  What do you think?

I think the rule could be something like:

  1. If it looks like a URL ("^scheme://"), it is.

  2. Otherwise, if it is a path in the filesystem, it is.

  3. Otherwise, if it has a colon, it's host:path

  4. Otherwise, barf.

where the interesting bit is the ordering of 2 and 3.  It seems like
"git clone" follows the order above with get_repo_path. But we do not
seem to follow it in git_connect, where we prefer 3 over 2.

> @@ -942,6 +943,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  		ret->disconnect = close_bundle;
>  		ret->smart_options = NULL;
>  	} else if (!is_url(url)
> +		|| (is_local(url) && !stat(url, &st))
>  		|| !prefixcmp(url, "file://")
>  		|| !prefixcmp(url, "git://")
>  		|| !prefixcmp(url, "ssh://")

I don't think that is enough. Something like /path/to/foo:bar would
trigger !is_url already, but then git_connect fails.

Try:

  $ git init --bare foo:bar
  $ git clone foo:bar
  ssh: Could not resolve hostname /home/peff/foo: Name or service not known
  fatal: Could not read from remote repository.
  ...

Clone recognizes it as a path and turns it into an absolute path. It
then feeds it to the transport code, which triggers !is_url and knows to
use the git transport. But then git_connect prefers ssh over the
filesystem.

If you do a straight fetch, though, the transport code might see the
relative path (if you use one):

  $ git init
  $ git init --bare sub:repo
  $ git fetch sub:repo
  ssh: Could not resolve hostname sub: Name or service not known

but that still triggers the is_url above (which demands the "://").

I am not sure whether your patch covers any cases I am missing, but I
think you would need an analogous change to git_connect for these common
cases.

-Peff

Re: [BUG] Filenames with single colon being treated as remote repository

[Jonathan Nieder]

Jeff King wrote:

> I don't think that is enough. Something like /path/to/foo:bar would
> trigger !is_url already, but then git_connect fails.

Doh.  Here's another try, still untested.

diff --git i/connect.c w/connect.c
index 49e56ba3..fe13942f 100644
--- i/connect.c
+++ w/connect.c
@@ -504,6 +504,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	int c;
 	struct child_process *conn = &no_fork;
 	enum protocol protocol = PROTO_LOCAL;
+	struct stat st;
 	int free_path = 0;
 	char *port = NULL;
 	const char **arg;
@@ -548,7 +549,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		end = host;
 
 	path = strchr(end, c);
-	if (path && !has_dos_drive_prefix(end)) {
+	if (path && !has_dos_drive_prefix(end) &&
+	    (c != ':' || stat(path, &st))) {
 		if (c == ':') {
 			protocol = PROTO_SSH;
 			*path++ = '\0';

Re: [BUG] Filenames with single colon being treated as remote repository

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sat, Apr 20, 2013 at 11:05:39PM -0700, Jonathan Nieder wrote:
>
>> >     Cloning into 'new-baz'...
>> >     ssh: Could not resolve hostname /tmp/foo: Success
>> >     fatal: Could not read from remote repository.
>> 
>> Here's a toy patch.  I haven't thought carefully about whether it's a
>> good idea, but maybe it can be useful for thinking about that.
>> 
>> Still needs documentation and tests.
>> 
>> My main worry is that the proposed rule for when an argument is
>> treated as a local path is hard to explain.  There's some precedent in
>> handling of bundles, though.  What do you think?
>
> I think the rule could be something like:
>
>   1. If it looks like a URL ("^scheme://"), it is.
>
>   2. Otherwise, if it is a path in the filesystem, it is.
>
>   3. Otherwise, if it has a colon, it's host:path
>
>   4. Otherwise, barf.
>
> where the interesting bit is the ordering of 2 and 3.  It seems like
> "git clone" follows the order above with get_repo_path. But we do not
> seem to follow it in git_connect, where we prefer 3 over 2.

At least for a string whose "host" part does not have any slash,
switching the rules 2 and 3 in git_connect() would be a regression,
no?  "frotz:/srv/git/git.git" has been the way to talk to host frotz
for a long time, and if you want to talk to a local directory that
is a subdirectory of "frotz:/" directory you have in your $cwd, you
can disambiguate by saying "./frotz:/srv/git/git.git" or something.

Re: [BUG] Filenames with single colon being treated as remote repository

[Jeff King]

On Sun, Apr 21, 2013 at 11:01:58AM -0700, Junio C Hamano wrote:

> > I think the rule could be something like:
> >
> >   1. If it looks like a URL ("^scheme://"), it is.
> >
> >   2. Otherwise, if it is a path in the filesystem, it is.
> >
> >   3. Otherwise, if it has a colon, it's host:path
> >
> >   4. Otherwise, barf.
> >
> > where the interesting bit is the ordering of 2 and 3.  It seems like
> > "git clone" follows the order above with get_repo_path. But we do not
> > seem to follow it in git_connect, where we prefer 3 over 2.
> 
> At least for a string whose "host" part does not have any slash,
> switching the rules 2 and 3 in git_connect() would be a regression,
> no?  "frotz:/srv/git/git.git" has been the way to talk to host frotz
> for a long time, and if you want to talk to a local directory that
> is a subdirectory of "frotz:/" directory you have in your $cwd, you
> can disambiguate by saying "./frotz:/srv/git/git.git" or something.

Yeah, it would be a regression for fetch, though "git clone frotz:/srv"
is already broken if that file exists; it turns into `realpath
frotz:/srv` before we even feed it into the fetch machinery.

So I think one reasonable path would be:

  1. Do not treat "host:path" as ssh if "host" has a slash, which should
     not regress anybody. It does not allow unadorned relative paths
     with colons, but it lets you use absolute paths or "./" to
     disambiguate.

  2. Teach git-clone to ask the transport code to parse the source repo
     spec, and decide from that whether it is local or not. That would
     harmonize the implementations and avoid errors when you _did_ mean
     to use ssh, but "host:path" happens to exist in your filesystem. I
     also would not be surprised if there are problems with
     URL-encoding, but maybe clone handles that properly (I didn't
     check).

And the "host contains slash" rule is pretty easy to explain in the
documentation, which is good.

-Peff

Re: [BUG] Filenames with single colon being treated as remote repository

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> So I think one reasonable path would be:
>
>   1. Do not treat "host:path" as ssh if "host" has a slash, which should
>      not regress anybody. It does not allow unadorned relative paths
>      with colons, but it lets you use absolute paths or "./" to
>      disambiguate.
>
>   2. Teach git-clone to ask the transport code to parse the source repo
>      spec, and decide from that whether it is local or not. That would
>      harmonize the implementations and avoid errors when you _did_ mean
>      to use ssh, but "host:path" happens to exist in your filesystem. I
>      also would not be surprised if there are problems with
>      URL-encoding, but maybe clone handles that properly (I didn't
>      check).
>
> And the "host contains slash" rule is pretty easy to explain in the
> documentation, which is good.

Sounds like a good direction to take us.

[PATCH] clone: allow cloning local paths with colons in them

[Nguyễn Thái Ngọc Duy]

Usually "foo:bar" is interpreted as an ssh url. This patch allows to
clone from such paths by putting at least one slash before the colon
(i.e. /path/to/foo:bar or just ./foo:bar).

file://foo:bar should also work, but local optimizations are off in
that case, which may be unwanted. While at there, warn the users about
--local being ignored in this case.

Reported-by: William Giokas <1007380@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Mon, Apr 22, 2013 at 10:35 PM, Jeff King <peff@peff.net> wrote:
 > So I think one reasonable path would be:
 >
 >   1. Do not treat "host:path" as ssh if "host" has a slash, which should
 >      not regress anybody. It does not allow unadorned relative paths
 >      with colons, but it lets you use absolute paths or "./" to
 >      disambiguate.
 >
 >   2. Teach git-clone to ask the transport code to parse the source repo
 >      spec, and decide from that whether it is local or not. That would
 >      harmonize the implementations and avoid errors when you _did_ mean
 >      to use ssh, but "host:path" happens to exist in your filesystem. I
 >      also would not be surprised if there are problems with
 >      URL-encoding, but maybe clone handles that properly (I didn't
 >      check).
 >
 > And the "host contains slash" rule is pretty easy to explain in the
 > documentation, which is good.

 I totally agree with this. But doing #2 seems to require a bit of
 code reorganization. How about just this for now?

 Documentation/urls.txt | 6 ++++++
 builtin/clone.c        | 2 ++
 connect.c              | 7 +++++--
 t/t5601-clone.sh       | 5 +++++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 3ca122f..476e338 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -23,6 +23,12 @@ An alternative scp-like syntax may also be used with the ssh protocol:
 
 - {startsb}user@{endsb}host.xz:path/to/repo.git/
 
+This syntax is only recognized if there are no slashes before the
+first colon. This helps differentiate a local path that contains a
+colon. For example the local path `foo:bar` could be specified as an
+absolute path or `./foo:bar` to avoid being misinterpreted as an ssh
+url.
+
 The ssh and git protocols additionally support ~username expansion:
 
 - ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/
diff --git a/builtin/clone.c b/builtin/clone.c
index 58fee98..e13da4d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -783,6 +783,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	is_local = option_local != 0 && path && !is_bundle;
 	if (is_local && option_depth)
 		warning(_("--depth is ignored in local clones; use file:// instead."));
+	if (option_local > 0 && !is_local)
+		warning(_("--local is ignored"));
 
 	if (argc == 2)
 		dir = xstrdup(argv[1]);
diff --git a/connect.c b/connect.c
index f57efd0..b568f10 100644
--- a/connect.c
+++ b/connect.c
@@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	path = strchr(end, c);
 	if (path && !has_dos_drive_prefix(end)) {
 		if (c == ':') {
-			protocol = PROTO_SSH;
-			*path++ = '\0';
+			if (!strchr(url, '/') || strchr(url, '/') >= path) {
+				protocol = PROTO_SSH;
+				*path++ = '\0';
+			} else
+				path = end;
 		}
 	} else
 		path = end;
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..0629149 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '
 	test_cmp fetch.expected fetch.actual
 '
 
+test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
+	cp -R src "foo:bar" &&
+	git clone "./foo:bar" foobar
+'
+
 test_done
-- 
1.8.2.83.gc99314b

Re: [PATCH] clone: allow cloning local paths with colons in them

[William Giokas]

[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]

On Sat, Apr 27, 2013 at 10:36:18AM +0700, Nguyễn Thái Ngọc Duy wrote:
> Usually "foo:bar" is interpreted as an ssh url. This patch allows to
> clone from such paths by putting at least one slash before the colon
> (i.e. /path/to/foo:bar or just ./foo:bar).
> 
> file://foo:bar should also work, but local optimizations are off in
> that case, which may be unwanted. While at there, warn the users about
> --local being ignored in this case.
> 
> Reported-by: William Giokas <1007380@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Working fine at the moment for the local clones (thank you). It looks
nice and clean, to me, and doesn't break any existing functionality I
have. 

Though I did notice that if it is a local file, then you don't actually
need a `/` anywhere at all, because I think git looks to see that it is
a local file first. (This is totally fine, though.)

>  On Mon, Apr 22, 2013 at 10:35 PM, Jeff King <peff@peff.net> wrote:
>  > So I think one reasonable path would be:
>  >
>  >   1. Do not treat "host:path" as ssh if "host" has a slash, which should
>  >      not regress anybody. It does not allow unadorned relative paths
>  >      with colons, but it lets you use absolute paths or "./" to
>  >      disambiguate.
>  >
>  >   2. Teach git-clone to ask the transport code to parse the source repo
>  >      spec, and decide from that whether it is local or not. That would
>  >      harmonize the implementations and avoid errors when you _did_ mean
>  >      to use ssh, but "host:path" happens to exist in your filesystem. I
>  >      also would not be surprised if there are problems with
>  >      URL-encoding, but maybe clone handles that properly (I didn't
>  >      check).
>  >
>  > And the "host contains slash" rule is pretty easy to explain in the
>  > documentation, which is good.
> 
>  I totally agree with this. But doing #2 seems to require a bit of
>  code reorganization. How about just this for now?
>
>  Documentation/urls.txt | 6 ++++++
>  builtin/clone.c        | 2 ++
>  connect.c              | 7 +++++--
>  t/t5601-clone.sh       | 5 +++++
>  4 files changed, 18 insertions(+), 2 deletions(-)

Thank you,
-- 
William Giokas | KaiSforza
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] clone: allow cloning local paths with colons in them

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/connect.c b/connect.c
> index f57efd0..b568f10 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
>  	path = strchr(end, c);
>  	if (path && !has_dos_drive_prefix(end)) {
>  		if (c == ':') {
> -			protocol = PROTO_SSH;
> -			*path++ = '\0';
> +			if (!strchr(url, '/') || strchr(url, '/') >= path) {
> +				protocol = PROTO_SSH;
> +				*path++ = '\0';
> +			} else
> +				path = end;
>  		}

That was fairly hard to grok. Is that equivalent to this?

                if (c == ':' && path < strchrnul(host, '/')) {
			/* is the first slash past the colon? */
                        protocol = PROTO_SSH;
                        *path++ = '\0';
                } else {
                        path = end;
                }

Re: [PATCH] clone: allow cloning local paths with colons in them

[Duy Nguyen]

On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> diff --git a/connect.c b/connect.c
>> index f57efd0..b568f10 100644
>> --- a/connect.c
>> +++ b/connect.c
>> @@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
>>       path = strchr(end, c);
>>       if (path && !has_dos_drive_prefix(end)) {
>>               if (c == ':') {
>> -                     protocol = PROTO_SSH;
>> -                     *path++ = '\0';
>> +                     if (!strchr(url, '/') || strchr(url, '/') >= path) {
>> +                             protocol = PROTO_SSH;
>> +                             *path++ = '\0';
>> +                     } else
>> +                             path = end
>>               }
>
> That was fairly hard to grok. Is that equivalent to this?
>
>                 if (c == ':' && path < strchrnul(host, '/')) {
>                         /* is the first slash past the colon? */
>                         protocol = PROTO_SSH;
>                         *path++ = '\0';
>                 } else {
>                         path = end;
>                 }
>

The original code is already hard to grok so I may be mistaken here.
But I think it's not the same. For the case when c == '/', it will do
"path = end;", which is unintended. It should keep the current "path"
value (i.e. == strchr(end, '/')). The use of "strchrnul(host, '/')" is
good though.
--
Duy

Re: [PATCH] clone: allow cloning local paths with colons in them

[Eric Sunshine]

On Sat, Apr 27, 2013 at 8:19 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> That was fairly hard to grok. Is that equivalent to this?
>>
>>                 if (c == ':' && path < strchrnul(host, '/')) {
>>                         /* is the first slash past the colon? */
>>                         protocol = PROTO_SSH;
>>                         *path++ = '\0';
>>                 } else {
>>                         path = end;
>>                 }
>>
>
> The original code is already hard to grok so I may be mistaken here.
> But I think it's not the same. For the case when c == '/', it will do
> "path = end;", which is unintended. It should keep the current "path"
> value (i.e. == strchr(end, '/')). The use of "strchrnul(host, '/')" is
> good though.

Do you want to take Windows '\' into account also?

Re: [PATCH] clone: allow cloning local paths with colons in them

[Duy Nguyen]

On Sun, Apr 28, 2013 at 8:48 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Apr 27, 2013 at 8:19 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Apr 28, 2013 at 4:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> That was fairly hard to grok. Is that equivalent to this?
>>>
>>>                 if (c == ':' && path < strchrnul(host, '/')) {
>>>                         /* is the first slash past the colon? */
>>>                         protocol = PROTO_SSH;
>>>                         *path++ = '\0';
>>>                 } else {
>>>                         path = end;
>>>                 }
>>>
>>
>> The original code is already hard to grok so I may be mistaken here.
>> But I think it's not the same. For the case when c == '/', it will do
>> "path = end;", which is unintended. It should keep the current "path"
>> value (i.e. == strchr(end, '/')). The use of "strchrnul(host, '/')" is
>> good though.
>
> Do you want to take Windows '\' into account also?

Windows path does not allow ':' so it's a non-issue in the first place.
--
Duy

[PATCH v2] clone: allow cloning local paths with colons in them

[Nguyễn Thái Ngọc Duy]

Usually "foo:bar" is interpreted as an ssh url. This patch allows to
clone from such paths by putting at least one slash before the colon
(i.e. /path/to/foo:bar or just ./foo:bar).

file://foo:bar should also work, but local optimizations are off in
that case, which may be unwanted. While at there, warn the users about
--local being ignored in this case.

Reported-by: William Giokas <1007380@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Changes from v1: replace strchr with strchrnul.

 Documentation/urls.txt | 6 ++++++
 builtin/clone.c        | 2 ++
 connect.c              | 7 +++++--
 t/t5601-clone.sh       | 5 +++++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/urls.txt b/Documentation/urls.txt
index 3ca122f..476e338 100644
--- a/Documentation/urls.txt
+++ b/Documentation/urls.txt
@@ -23,6 +23,12 @@ An alternative scp-like syntax may also be used with the ssh protocol:
 
 - {startsb}user@{endsb}host.xz:path/to/repo.git/
 
+This syntax is only recognized if there are no slashes before the
+first colon. This helps differentiate a local path that contains a
+colon. For example the local path `foo:bar` could be specified as an
+absolute path or `./foo:bar` to avoid being misinterpreted as an ssh
+url.
+
 The ssh and git protocols additionally support ~username expansion:
 
 - ssh://{startsb}user@{endsb}host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/
diff --git a/builtin/clone.c b/builtin/clone.c
index 58fee98..e13da4d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -783,6 +783,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	is_local = option_local != 0 && path && !is_bundle;
 	if (is_local && option_depth)
 		warning(_("--depth is ignored in local clones; use file:// instead."));
+	if (option_local > 0 && !is_local)
+		warning(_("--local is ignored"));
 
 	if (argc == 2)
 		dir = xstrdup(argv[1]);
diff --git a/connect.c b/connect.c
index f57efd0..a0783d4 100644
--- a/connect.c
+++ b/connect.c
@@ -551,8 +551,11 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	path = strchr(end, c);
 	if (path && !has_dos_drive_prefix(end)) {
 		if (c == ':') {
-			protocol = PROTO_SSH;
-			*path++ = '\0';
+			if (path < strchrnul(host, '/')) {
+				protocol = PROTO_SSH;
+				*path++ = '\0';
+			} else /* '/' in the host part, assume local path */
+				path = end;
 		}
 	} else
 		path = end;
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..0629149 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '
 	test_cmp fetch.expected fetch.actual
 '
 
+test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
+	cp -R src "foo:bar" &&
+	git clone "./foo:bar" foobar
+'
+
 test_done
-- 
1.8.2.83.gc99314b

Re: [PATCH v2] clone: allow cloning local paths with colons in them

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 67869b4..0629149 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '
>  	test_cmp fetch.expected fetch.actual
>  '
>  
> +test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
> +	cp -R src "foo:bar" &&
> +	git clone "./foo:bar" foobar
> +'

Hmph, why not

	git clone --mirror src foo:bar &&
        git clone ./foo:bar foobar

or something?  Also do we have a easy negative case we want to test,
i.e. a case where we do not want the new codepath to trigger by
mistake?

Re: [PATCH v2] clone: allow cloning local paths with colons in them

[Jeff King]

On Tue, May 07, 2013 at 08:34:51AM -0700, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> 
> > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> > index 67869b4..0629149 100755
> > --- a/t/t5601-clone.sh
> > +++ b/t/t5601-clone.sh
> > @@ -280,4 +280,9 @@ test_expect_success 'clone checking out a tag' '
> >  	test_cmp fetch.expected fetch.actual
> >  '
> >  
> > +test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
> > +	cp -R src "foo:bar" &&
> > +	git clone "./foo:bar" foobar
> > +'
> 
> Hmph, why not
> 
> 	git clone --mirror src foo:bar &&
>         git clone ./foo:bar foobar

Yeah, not only does that avoid "cp -R", but it is a nice check that we
do not do anything stupid with colons on the dst argument (which we
should obviously not, but it cannot hurt to exercise it).

> or something?  Also do we have a easy negative case we want to test,
> i.e. a case where we do not want the new codepath to trigger by
> mistake?

Yeah, checking "git clone host:path" would be nice, but such a case
would want to go through ssh. I suspect we could point GIT_SSH at a
script like:

  #!/bin/sh
  echo "ssh: $*" >ssh-log &&
  host=$1; shift
  cd "pretend-hosts/$host" && exec "$@"

It looks like t5602 does something similar already.

-Peff