Re: [PATCH v6 05/10] git fetch-pack: Add --diag-url

2013-11-22 Thread SZEDER Gábor
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

2013-11-21 Thread Junio C Hamano
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

2013-11-21 Thread Torsten Bögershausen
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 @@