From ba227857d24029917f1e939647d826037f026205 Mon Sep 17 00:00:00 2001 From: Daniel Barkalow Date: Mon, 4 Feb 2008 13:26:23 -0500 Subject: Reduce the number of connects when fetching This shares the connection between getting the remote ref list and getting objects in the first batch. (A second connection is still used to follow tags). When we do not fetch objects (i.e. either ls-remote disconnects after getting list of refs, or we decide we are already up-to-date), we clean up the connection properly; otherwise the connection is left open in need of cleaning up to avoid getting an error message from the remote end when ssh is used. Signed-off-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- builtin-fetch-pack.c | 75 ++++++++++++++++++++++++++++------------------------ builtin-fetch.c | 2 ++ builtin-ls-remote.c | 1 + fetch-pack.h | 2 ++ transport.c | 51 ++++++++++++++++++++++++++--------- 5 files changed, 83 insertions(+), 48 deletions(-) diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c index e68e01592..f40135248 100644 --- a/builtin-fetch-pack.c +++ b/builtin-fetch-pack.c @@ -7,6 +7,7 @@ #include "pack.h" #include "sideband.h" #include "fetch-pack.h" +#include "remote.h" #include "run-command.h" static int transfer_unpack_limit = -1; @@ -548,14 +549,14 @@ static int get_pack(int xd[2], char **pack_lockfile) } static struct ref *do_fetch_pack(int fd[2], + const struct ref *orig_ref, int nr_match, char **match, char **pack_lockfile) { - struct ref *ref; + struct ref *ref = copy_ref_list(orig_ref); unsigned char sha1[20]; - get_remote_heads(fd[0], &ref, 0, NULL, 0); if (is_repository_shallow() && !server_supports("shallow")) die("Server does not support shallow clients"); if (server_supports("multi_ack")) { @@ -573,10 +574,6 @@ static struct ref *do_fetch_pack(int fd[2], fprintf(stderr, "Server supports side-band\n"); use_sideband = 1; } - if (!ref) { - packet_flush(fd[1]); - die("no matching remote head"); - } if (everything_local(&ref, nr_match, match)) { packet_flush(fd[1]); goto all_done; @@ -650,8 +647,10 @@ static void fetch_pack_setup(void) int cmd_fetch_pack(int argc, const char **argv, const char *prefix) { int i, ret, nr_heads; - struct ref *ref; + struct ref *ref = NULL; char *dest = NULL, **heads; + int fd[2]; + struct child_process *conn; nr_heads = 0; heads = NULL; @@ -706,9 +705,33 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) if (!dest) usage(fetch_pack_usage); - ref = fetch_pack(&args, dest, nr_heads, heads, NULL); + conn = git_connect(fd, (char *)dest, args.uploadpack, + args.verbose ? CONNECT_VERBOSE : 0); + if (conn) { + get_remote_heads(fd[0], &ref, 0, NULL, 0); + + ref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL); + close(fd[0]); + close(fd[1]); + if (finish_connect(conn)) + ref = NULL; + } else { + ref = NULL; + } ret = !ref; + if (!ret && nr_heads) { + /* If the heads to pull were given, we should have + * consumed all of them by matching the remote. + * Otherwise, 'git-fetch remote no-such-ref' would + * silently succeed without issuing an error. + */ + for (i = 0; i < nr_heads; i++) + if (heads[i] && heads[i][0]) { + error("no such remote ref %s", heads[i]); + ret = 1; + } + } while (ref) { printf("%s %s\n", sha1_to_hex(ref->old_sha1), ref->name); @@ -719,16 +742,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) } struct ref *fetch_pack(struct fetch_pack_args *my_args, + int fd[], struct child_process *conn, + const struct ref *ref, const char *dest, int nr_heads, char **heads, char **pack_lockfile) { - int i, ret; - int fd[2]; - struct child_process *conn; - struct ref *ref; struct stat st; + struct ref *ref_cpy; fetch_pack_setup(); memcpy(&args, my_args, sizeof(args)); @@ -737,29 +759,15 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args, st.st_mtime = 0; } - conn = git_connect(fd, (char *)dest, args.uploadpack, - args.verbose ? CONNECT_VERBOSE : 0); if (heads && nr_heads) nr_heads = remove_duplicates(nr_heads, heads); - ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile); - close(fd[0]); - close(fd[1]); - ret = finish_connect(conn); - - if (!ret && nr_heads) { - /* If the heads to pull were given, we should have - * consumed all of them by matching the remote. - * Otherwise, 'git-fetch remote no-such-ref' would - * silently succeed without issuing an error. - */ - for (i = 0; i < nr_heads; i++) - if (heads[i] && heads[i][0]) { - error("no such remote ref %s", heads[i]); - ret = 1; - } + if (!ref) { + packet_flush(fd[1]); + die("no matching remote head"); } + ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile); - if (!ret && args.depth > 0) { + if (args.depth > 0) { struct cache_time mtime; char *shallow = git_path("shallow"); int fd; @@ -787,8 +795,5 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args, } } - if (ret) - ref = NULL; - - return ref; + return ref_cpy; } diff --git a/builtin-fetch.c b/builtin-fetch.c index 320e23568..ac335f20f 100644 --- a/builtin-fetch.c +++ b/builtin-fetch.c @@ -557,6 +557,8 @@ static int do_fetch(struct transport *transport, free_refs(fetch_map); + transport_disconnect(transport); + return 0; } diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c index 6dd31d1dd..023754986 100644 --- a/builtin-ls-remote.c +++ b/builtin-ls-remote.c @@ -94,6 +94,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack); ref = transport_get_remote_refs(transport); + transport_disconnect(transport); if (!ref) return 1; diff --git a/fetch-pack.h b/fetch-pack.h index a7888ea30..8d35ef60b 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -16,6 +16,8 @@ struct fetch_pack_args }; struct ref *fetch_pack(struct fetch_pack_args *args, + int fd[], struct child_process *conn, + const struct ref *ref, const char *dest, int nr_heads, char **heads, diff --git a/transport.c b/transport.c index babaa2139..199e9e6a0 100644 --- a/transport.c +++ b/transport.c @@ -563,6 +563,8 @@ struct git_transport_data { unsigned thin : 1; unsigned keep : 1; int depth; + struct child_process *conn; + int fd[2]; const char *uploadpack; const char *receivepack; }; @@ -593,20 +595,20 @@ static int set_git_option(struct transport *connection, return 1; } +static int connect_setup(struct transport *transport) +{ + struct git_transport_data *data = transport->data; + data->conn = git_connect(data->fd, transport->url, data->uploadpack, 0); + return 0; +} + static struct ref *get_refs_via_connect(struct transport *transport) { struct git_transport_data *data = transport->data; struct ref *refs; - int fd[2]; - char *dest = xstrdup(transport->url); - struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0); - get_remote_heads(fd[0], &refs, 0, NULL, 0); - packet_flush(fd[1]); - - finish_connect(conn); - - free(dest); + connect_setup(transport); + get_remote_heads(data->fd[0], &refs, 0, NULL, 0); return refs; } @@ -617,7 +619,7 @@ static int fetch_refs_via_pack(struct transport *transport, struct git_transport_data *data = transport->data; char **heads = xmalloc(nr_heads * sizeof(*heads)); char **origh = xmalloc(nr_heads * sizeof(*origh)); - struct ref *refs; + const struct ref *refs; char *dest = xstrdup(transport->url); struct fetch_pack_args args; int i; @@ -632,13 +634,27 @@ static int fetch_refs_via_pack(struct transport *transport, for (i = 0; i < nr_heads; i++) origh[i] = heads[i] = xstrdup(to_fetch[i]->name); - refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile); + + refs = transport_get_remote_refs(transport); + if (!data->conn) { + struct ref *refs_tmp; + connect_setup(transport); + get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0); + free_refs(refs_tmp); + } + + refs = fetch_pack(&args, data->fd, data->conn, transport->remote_refs, + dest, nr_heads, heads, &transport->pack_lockfile); + close(data->fd[0]); + close(data->fd[1]); + if (finish_connect(data->conn)) + refs = NULL; + data->conn = NULL; for (i = 0; i < nr_heads; i++) free(origh[i]); free(origh); free(heads); - free_refs(refs); free(dest); return (refs ? 0 : -1); } @@ -661,7 +677,15 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const static int disconnect_git(struct transport *transport) { - free(transport->data); + struct git_transport_data *data = transport->data; + if (data->conn) { + packet_flush(data->fd[1]); + close(data->fd[0]); + close(data->fd[1]); + finish_connect(data->conn); + } + + free(data); return 0; } @@ -721,6 +745,7 @@ struct transport *transport_get(struct remote *remote, const char *url) ret->disconnect = disconnect_git; data->thin = 1; + data->conn = NULL; data->uploadpack = "git-upload-pack"; if (remote && remote->uploadpack) data->uploadpack = remote->uploadpack; -- cgit v1.2.1 From fb32c9172a1f2828d75f5d17d8ceea8ffe692f2a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 10 Feb 2008 03:06:57 +0000 Subject: Fix "git clone" for git:// protocol In ba227857(Reduce the number of connects when fetching), we checked the return value of git_connect() to see if the connection was successful. However, for the git:// protocol, there is no need to have another process, so the return value was NULL. Now, it makes sense to assume the rule that git_connect() will return NULL if it fails (at the moment, it die()s if it fails), so return a dummy child process. Signed-off-by: Johannes Schindelin Acked-by: Daniel Barkalow Signed-off-by: Junio C Hamano --- connect.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/connect.c b/connect.c index 3aefd4ace..700cebad9 100644 --- a/connect.c +++ b/connect.c @@ -472,14 +472,18 @@ char *get_port(char *host) return NULL; } +static struct child_process no_fork; + /* - * This returns NULL if the transport protocol does not need fork(2), or a - * struct child_process object if it does. Once done, finish the connection - * with finish_connect() with the value returned from this function - * (it is safe to call finish_connect() with NULL to support the former - * case). + * This returns a dummy child_process if the transport protocol does not + * need fork(2), or a struct child_process object if it does. Once done, + * finish the connection with finish_connect() with the value returned from + * this function (it is safe to call finish_connect() with NULL to support + * the former case). * - * If it returns, the connect is successful; it just dies on errors. + * If it returns, the connect is successful; it just dies on errors (this + * will hopefully be changed in a libification effort, to return NULL when + * the connection failed). */ struct child_process *git_connect(int fd[2], const char *url_orig, const char *prog, int flags) @@ -577,7 +581,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig, free(url); if (free_path) free(path); - return NULL; + return &no_fork; } conn = xcalloc(1, sizeof(*conn)); @@ -635,7 +639,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig, int finish_connect(struct child_process *conn) { int code; - if (!conn) + if (!conn || conn == &no_fork) return 0; code = finish_command(conn); -- cgit v1.2.1