From d6a31e08cd300d8085af799ff9fe16b5963caab7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Aug 2015 17:48:21 +0200 Subject: [PATCH 1/2] clone: add tests for output directory When we run "git clone $url", clone guesses from the $url what to name the local output directory. We don't have any test coverage of this, so let's add some basic tests. This reveals a few problems: - cloning "foo.git/" does not properly remove the ".git"; this is a recent regression from 7e837c6 (clone: simplify string handling in guess_dir_name(), 2015-07-09) - likewise, cloning foo/.git does not seem to handle the bare case (we should end up in foo.git, but we try to use foo/.git on the local end), which also comes from 7e837c6. - cloning the root is not very smart about URL parsing, and usernames and port numbers may end up in the directory name All of these tests are marked as failures. Signed-off-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t5603-clone-dirname.sh | 106 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100755 t/t5603-clone-dirname.sh diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh new file mode 100755 index 0000000000..52f61a090a --- /dev/null +++ b/t/t5603-clone-dirname.sh @@ -0,0 +1,106 @@ +#!/bin/sh + +test_description='check output directory names used by git-clone' +. ./test-lib.sh + +# we use a fake ssh wrapper that ignores the arguments +# entirely; we really only care that we get _some_ repo, +# as the real test is what clone does on the local side +test_expect_success 'setup ssh wrapper' ' + write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF && + git upload-pack "$TRASH_DIRECTORY" + EOF + GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" && + export GIT_SSH && + export TRASH_DIRECTORY +' + +# make sure that cloning $1 results in local directory $2 +test_clone_dir () { + url=$1; shift + dir=$1; shift + expect=success + bare=non-bare + clone_opts= + for i in "$@" + do + case "$i" in + fail) + expect=failure + ;; + bare) + bare=bare + clone_opts=--bare + ;; + esac + done + test_expect_$expect "clone of $url goes to $dir ($bare)" " + rm -rf $dir && + git clone $clone_opts $url && + test_path_is_dir $dir + " +} + +# basic syntax with bare and non-bare variants +test_clone_dir host:foo foo +test_clone_dir host:foo foo.git bare +test_clone_dir host:foo.git foo +test_clone_dir host:foo.git foo.git bare +test_clone_dir host:foo/.git foo +test_clone_dir host:foo/.git foo.git bare fail + +# similar, but using ssh URL rather than host:path syntax +test_clone_dir ssh://host/foo foo +test_clone_dir ssh://host/foo foo.git bare +test_clone_dir ssh://host/foo.git foo +test_clone_dir ssh://host/foo.git foo.git bare +test_clone_dir ssh://host/foo/.git foo +test_clone_dir ssh://host/foo/.git foo.git bare fail + +# we should remove trailing slashes and .git suffixes +test_clone_dir ssh://host/foo/ foo +test_clone_dir ssh://host/foo/// foo +test_clone_dir ssh://host/foo/.git/ foo +test_clone_dir ssh://host/foo.git/ foo fail +test_clone_dir ssh://host/foo.git/// foo fail +test_clone_dir ssh://host/foo///.git/ foo +test_clone_dir ssh://host/foo/.git/// foo + +test_clone_dir host:foo/ foo +test_clone_dir host:foo/// foo +test_clone_dir host:foo.git/ foo fail +test_clone_dir host:foo/.git/ foo +test_clone_dir host:foo.git/// foo fail +test_clone_dir host:foo///.git/ foo +test_clone_dir host:foo/.git/// foo + +# omitting the path should default to the hostname +test_clone_dir ssh://host/ host +test_clone_dir ssh://host:1234/ host fail +test_clone_dir ssh://user@host/ host fail +test_clone_dir host:/ host fail + +# auth materials should be redacted +test_clone_dir ssh://user:password@host/ host fail +test_clone_dir ssh://user:password@host:1234/ host fail +test_clone_dir ssh://user:passw@rd@host:1234/ host fail +test_clone_dir user@host:/ host fail +test_clone_dir user:password@host:/ host fail +test_clone_dir user:passw@rd@host:/ host fail + +# auth-like material should not be dropped +test_clone_dir ssh://host/foo@bar foo@bar +test_clone_dir ssh://host/foo@bar.git foo@bar +test_clone_dir ssh://user:password@host/foo@bar foo@bar +test_clone_dir ssh://user:passw@rd@host/foo@bar.git foo@bar + +test_clone_dir host:/foo@bar foo@bar +test_clone_dir host:/foo@bar.git foo@bar +test_clone_dir user:password@host:/foo@bar foo@bar +test_clone_dir user:passw@rd@host:/foo@bar.git foo@bar + +# trailing port-like numbers should not be stripped for paths +test_clone_dir ssh://user:password@host/test:1234 1234 +test_clone_dir ssh://user:password@host/test:1234.git 1234 + +test_done From db2e220447f7b02278d64417c8f05f73710f5b8b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 10 Aug 2015 17:48:22 +0200 Subject: [PATCH 2/2] clone: use computed length in guess_dir_name Commit 7e837c6 (clone: simplify string handling in guess_dir_name(), 2015-07-09) changed clone to use strip_suffix instead of hand-rolled pointer manipulation. However, strip_suffix will strip from the end of a NUL-terminated string, and we may have already stripped some characters (like directory separators, or "/.git"). This leads to commands like: git clone host:foo.git/ failing to strip the ".git". We must instead convert our pointer arithmetic into a computed length and feed that to strip_suffix_mem, which will then reduce the length further for us. It would be nicer if we could drop the pointer manipulation entirely, and just continually strip using strip_suffix. But that doesn't quite work for two reasons: 1. The early suffixes we're stripping are not constant; we need to look for is_dir_sep, which could be one of several characters. 2. Mid-way through the stripping we compute the pointer "start", which shows us the beginning of the pathname. Which really give us two lengths to work with: the offset from the start of the string, and from the start of the path. By using pointers for the early part, we can just compute the length from "start" when we need it. Signed-off-by: Jeff King Acked-by: Sebastian Schuberth Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 3 ++- t/t5603-clone-dirname.sh | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index e18839d107..ed484cb6f4 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -174,7 +174,8 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) /* * Strip .{bundle,git}. */ - strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len); + len = end - start; + strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git"); if (is_bare) dir = xstrfmt("%.*s.git", (int)len, start); diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh index 52f61a090a..765cc434ef 100755 --- a/t/t5603-clone-dirname.sh +++ b/t/t5603-clone-dirname.sh @@ -47,7 +47,7 @@ test_clone_dir host:foo foo.git bare test_clone_dir host:foo.git foo test_clone_dir host:foo.git foo.git bare test_clone_dir host:foo/.git foo -test_clone_dir host:foo/.git foo.git bare fail +test_clone_dir host:foo/.git foo.git bare # similar, but using ssh URL rather than host:path syntax test_clone_dir ssh://host/foo foo @@ -55,22 +55,22 @@ test_clone_dir ssh://host/foo foo.git bare test_clone_dir ssh://host/foo.git foo test_clone_dir ssh://host/foo.git foo.git bare test_clone_dir ssh://host/foo/.git foo -test_clone_dir ssh://host/foo/.git foo.git bare fail +test_clone_dir ssh://host/foo/.git foo.git bare # we should remove trailing slashes and .git suffixes test_clone_dir ssh://host/foo/ foo test_clone_dir ssh://host/foo/// foo test_clone_dir ssh://host/foo/.git/ foo -test_clone_dir ssh://host/foo.git/ foo fail -test_clone_dir ssh://host/foo.git/// foo fail +test_clone_dir ssh://host/foo.git/ foo +test_clone_dir ssh://host/foo.git/// foo test_clone_dir ssh://host/foo///.git/ foo test_clone_dir ssh://host/foo/.git/// foo test_clone_dir host:foo/ foo test_clone_dir host:foo/// foo -test_clone_dir host:foo.git/ foo fail +test_clone_dir host:foo.git/ foo test_clone_dir host:foo/.git/ foo -test_clone_dir host:foo.git/// foo fail +test_clone_dir host:foo.git/// foo test_clone_dir host:foo///.git/ foo test_clone_dir host:foo/.git/// foo