* [PATCH v6 05/10] git fetch-pack: Add --diag-url
@ 2013-11-21 20:40 Torsten Bögershausen
2013-11-21 22:46 ` Junio C Hamano
2013-11-22 12:56 ` SZEDER Gábor
0 siblings, 2 replies; 3+ messages in thread
From: Torsten Bögershausen @ 2013-11-21 20:40 UTC (permalink / raw)
To: git; +Cc: tboegi
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 @@ test_expect_success 'shallow fetch with tags does not break the repository' '
git fsck
)
'
+check_prot_path() {
+ > actual &&
+ (git fetch-pack --diag-url "$1" 2>&1 1>stdout) | grep -v host= >actual &&
+ 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" 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
+ hh=$(echo $h | tr -d "[]")
+ pp=ssh
+ else
+ hh=$h
+ pp=$p
+ fi
+ test_expect_success "fetch-pack $p://$h/$r" '
+ check_prot_host_path $p://$h/$r $pp "$hh" "/$r"
+ '
+ # "/~" -> "~" conversion for git
+ test_expect_success "fetch-pack $p://$h/~$r" '
+ check_prot_host_path $p://$h/~$r $pp "$hh" "~$r"
+ '
+ done
+ done
+ # file with scheme
+ for p in file
+ do
+ for h in "" host host:12 [::1] [::1]:23
+ do
+ test_expect_success "fetch-pack $p://$h/$r" '
+ check_prot_path $p://$h/$r $p "/$r"
+ '
+ # No "/~" -> "~" conversion for file
+ test_expect_success "fetch-pack $p://$h/~$r" '
+ check_prot_path $p://$h/~$r $p "/~$r"
+ '
+ done
+ done
+done
test_done
--
1.8.4.457.g424cb08
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url
2013-11-21 20:40 [PATCH v6 05/10] git fetch-pack: Add --diag-url Torsten Bögershausen
@ 2013-11-21 22:46 ` Junio C Hamano
2013-11-22 12:56 ` SZEDER Gábor
1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2013-11-21 22:46 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
Torsten Bögershausen <tboegi@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" 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url
2013-11-21 20:40 [PATCH v6 05/10] git fetch-pack: Add --diag-url Torsten Bögershausen
2013-11-21 22:46 ` Junio C Hamano
@ 2013-11-22 12:56 ` SZEDER Gábor
1 sibling, 0 replies; 3+ messages in thread
From: SZEDER Gábor @ 2013-11-22 12:56 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
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" 2>&1 1>stdout) | 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" 2>actual &&
Likewise.
Best,
Gábor
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-22 12:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-21 20:40 [PATCH v6 05/10] git fetch-pack: Add --diag-url Torsten Bögershausen
2013-11-21 22:46 ` Junio C Hamano
2013-11-22 12:56 ` SZEDER Gábor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).