Torsten Bögershausen <tboegi@xxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html