Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url
Hi, On Thu, Nov 21, 2013 at 09:40:48PM +0100, Torsten Bögershausen wrote: 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 There is no need to truncate actual here, ... + (git fetch-pack --diag-url $1 21 1stdout) | grep -v host= actual ... because it will be overwritten in this line anyway. + echo Diag: url=$1 expected + echo Diag: protocol=$2 expected + echo Diag: path=$3 expected + test_cmp expected actual +} + +check_prot_host_path() { + actual + git fetch-pack --diag-url $1 2actual Likewise. Best, Gábor -- 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 v6 05/10] git fetch-pack: Add --diag-url
Torsten Bögershausen tbo...@web.de writes: 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? 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()). 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 21 1stdout) | 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 2actual + 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 -- 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 v6 05/10] git fetch-pack: Add --diag-url
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. --- 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) { + switch (protocol) { + case PROTO_LOCAL: + return file; + case PROTO_SSH: + return ssh; + case PROTO_GIT: + return git; + default: + return unkown protocol; + } +} + static enum protocol get_protocol(const char *name) { if (!strcmp(name, ssh)) @@ -670,6 +683,20 @@ struct child_process *git_connect(int fd[2], const char *url, signal(SIGCHLD, SIG_DFL); protocol = parse_connect_url(url, host, port, path); + if (flags CONNECT_DIAG_URL) { + fprintf(stderr, Diag: url=%s\n, url ? url : NULL); + fprintf(stderr, Diag: protocol=%s\n, prot_name(protocol)); + fprintf(stderr, Diag: host=%s, host ? host : NULL); + if (port) + fprintf(stderr, :%s\n, port); + else + fprintf(stderr, \n); + fprintf(stderr, Diag: path=%s\n, path ? path : NULL); + free(host); + free(port); + free(path); + return NULL; + } if (protocol == PROTO_GIT) { /* These underlying connection commands die() if they diff --git a/connect.h b/connect.h index 64fb7db..527d58a 100644 --- a/connect.h +++ b/connect.h @@ -2,6 +2,7 @@ #define CONNECT_H #define CONNECT_VERBOSE (1u 0) +#define CONNECT_DIAG_URL (1u 1) extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags); extern int finish_connect(struct child_process *conn); extern int git_connection_is_socket(struct child_process *conn); diff --git a/fetch-pack.h b/fetch-pack.h index 461cbf3..20ccc12 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -14,6 +14,7 @@ struct fetch_pack_args { use_thin_pack:1, fetch_all:1, stdin_refs:1, + diag_url:1, verbose:1, no_progress:1, include_tag:1, 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 @@