Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:59:18
Torsten Bögershausen [off-list ref] writes:
quoted
Subject: Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url
s/Add/add/ please.
The main purpose is to trace the URL parser called by git_connect() in connect.c The main features of the parser can be listed as this: - parse out host and path for URLs with a scheme (git:// file:// ssh://) - parse host names embedded by [] correctly - extract the port number, if present - seperate URLs like "file" (which are local) from URLs like "host:repo" which should use ssh Add the new parameter "--diag-url" to "git fetch-pack", which prints the value for protocol, host and path to stderr and exits. ---
Sign-off?
quoted hunk
builtin/fetch-pack.c | 14 ++++++++++--- connect.c | 27 ++++++++++++++++++++++++ connect.h | 1 + fetch-pack.h | 1 + t/t5500-fetch-pack.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 3 deletions(-)diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index c8e8582..758b5ac 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c@@ -7,7 +7,7 @@ static const char fetch_pack_usage[] = "git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] " "[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] " -"[--no-progress] [-v] [<host>:]<directory> [<refs>...]"; +"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]"; static void add_sought_entry_mem(struct ref ***sought, int *nr, int *alloc, const char *name, int namelen)@@ -81,6 +81,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) args.stdin_refs = 1; continue; } + if (!strcmp("--diag-url", arg)) { + args.diag_url = 1; + continue; + } if (!strcmp("-v", arg)) { args.verbose = 1; continue;@@ -146,10 +150,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) fd[0] = 0; fd[1] = 1; } else { + int flags = args.verbose ? CONNECT_VERBOSE : 0; + if (args.diag_url) + flags |= CONNECT_DIAG_URL; conn = git_connect(fd, dest, args.uploadpack, - args.verbose ? CONNECT_VERBOSE : 0); + flags); + if (!conn) + return args.diag_url ? 0 : 1; } - get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL); ref = fetch_pack(&args, fd, conn, ref, dest,diff --git a/connect.c b/connect.c index a6cf345..1b93b4d 100644 --- a/connect.c +++ b/connect.c@@ -236,6 +236,19 @@ enum protocol { PROTO_GIT }; +static const char *prot_name(enum protocol protocol) {
Style: please move that "{" to the beginning of the next line (see the
beginning of existing functions e.g. get_protocol()).
quoted hunk
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index d87ddf7..9136f2a 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh@@ -531,5 +531,62 @@ test_expect_success 'shallow fetch with tags does not break the repository' ' git fsck ) ' +check_prot_path() { + > actual &&
Style: no SP between the redirection operator and its target, i.e. >actual &&
+ (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual &&
Do we use "stdout" in this test? Otherwise "1>/dev/null" would make it clearer what is going on.
+ echo "Diag: url=$1" >expected && + echo "Diag: protocol=$2" >>expected && + echo "Diag: path=$3" >>expected &&
Perhaps this is a good place to use here-doc, i.e.
cat >expected <<-EOF &&
Diag: ...
...
EOF
+ test_cmp expected actual
+}
+
+check_prot_host_path() {
+ > actual &&
+ git fetch-pack --diag-url "$1" 2>actual &&
+ echo "Diag: url=$1" >expected &&
+ echo "Diag: protocol=$2" >>expected &&
+ echo "Diag: host=$3" >>expected &&
+ echo "Diag: path=$4" >>expected &&
+ test_cmp expected actual
+}
+
+for r in repo re:po re/po
+do
+ # git or ssh with scheme
+ for p in "ssh+git" "git+ssh" git ssh
+ do
+ for h in host host:12 [::1] [::1]:23
+ do
+ if $(echo $p | grep ssh >/dev/null 2>/dev/null); then
Style: "; then" should be spelled as "LF" followed by "then" on the
next line by itself.
But more ipmportantly, the above tries to do
if "some computed string"; then
which is very iffy. I think you meant:
case "$p" in
*ssh*)
do ssh thing
;;
*)
do other thing
esac