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

2013-05-07 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy pclo...@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?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

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

 Nguyễn Thái Ngọc Duy pclo...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-05-03 Thread 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 1007...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html