From bbc30f996380eacd71ca061675d5d0c5f21c45d2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 Feb 2011 09:30:19 -0500 Subject: add packet tracing debug code This shows a trace of all packets coming in or out of a given program. This can help with debugging object negotiation or other protocol issues. To keep the code changes simple, we operate at the lowest level, meaning we don't necessarily understand what's in the packets. The one exception is a packet starting with "PACK", which causes us to skip that packet and turn off tracing (since the gigantic pack data will not be interesting to read, at least not in the trace format). We show both written and read packets. In the local case, this may mean you will see packets twice (written by the sender and read by the receiver). However, for cases where the other end is remote, this allows you to see the full conversation. Packet tracing can be enabled with GIT_TRACE_PACKET=, where takes the same arguments as GIT_TRACE. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index b999413934..272bc383d6 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -804,6 +804,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) char **pack_lockfile_ptr = NULL; struct child_process *conn; + packet_trace_identity("fetch-pack"); + nr_heads = 0; heads = NULL; for (i = 1; i < argc; i++) { -- cgit v1.3-5-g9baa From f2cba9299be45f8e027f7b45c6e4a3cae55576c6 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 14 Mar 2011 16:48:38 -0700 Subject: fetch-pack: Finish negotation if remote replies "ACK %s ready" If multi_ack_detailed was selected in the protocol capabilities (both client and server are >= Git 1.6.6) the upload-pack side will send "ACK %s ready" when it knows how to safely cut the graph and produce a reasonable pack for the want list that was already sent on the connection. Upon receiving "ACK %s ready" there is no point in looking at the remaining commits inside of rev_list. Sending additional "have %s" lines to the remote will not construct a smaller pack. It is unlikely a commit older than the current cut point will have a better delta base than the cut point itself has. The original design of this code had fetch-pack empty rev_list by marking a commit and its transitive ancestors COMMON whenever the remote side said "ACK %s {continue,common}" and skipping over any already COMMON commits during get_rev(). This approach does not work when most of rev_list is actually COMMON_REF, commits that are pointed to by a reference on the remote, which exist locally, and which have not yet been sent to the remote as a "have %s" line. Most of the common references are tags in the ref/tags namespace, using points in the commit graph that are more than 1 commit apart. In git.git itself, this is currently 340 tags, 339 of which point to commits in the commit graph. fetch-pack pushes all of these into rev_list, but is unable to mark them COMMON and discard during a remote's "ACK %s {continue,common}" because it does not parse through the entire parent chain. Not parsing the entire parent chain is an optimization to avoid walking back to the roots of the repository. Assuming the client is only following the remote (and does not make its own local commits), the client needs 11 rounds to spin through the entire list of tags (32 commits per round, ceil(339/32) == 11). Unfortunately the server knows on the first "have %s" line that it can produce a good pack, and does not need to see the remaining 320 tags in the other 10 rounds. Over git:// and ssh:// this isn't as bad as it sounds, the client is only transmitting an extra 16,000 bytes that it doesn't need to send. Over smart HTTP, the client must do an additional 10 HTTP POST requests, each of which incurs round-trip latency, and must upload the entire state vector of all known common objects. On the final POST request, this is 16 KiB worth of data. Fix all of this by clearing rev_list as soon as the remote side says it can construct a pack. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index b999413934..5173dc9aad 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -379,6 +379,8 @@ static int find_common(int fd[2], unsigned char *result_sha1, retval = 0; in_vain = 0; got_continue = 1; + if (ack == ACK_ready) + rev_list = NULL; break; } } -- cgit v1.3-5-g9baa From 761ecf0bc7b6cddf311f00877c59e6381cdbdeea Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 14 Mar 2011 17:59:39 -0700 Subject: fetch-pack: Implement no-done capability If enabled on the connection "multi_ack_detailed no-done" as a pair allows the remote upload-pack process to send a PACK down to the client as soon as a "ACK %s ready" message was also sent. Over git:// and ssh:// where a bi-directional stream is in place this has very little difference over the classical version that waits for the client to send a "done\n" line by itself. It does slightly reduce the latency involved to start the pack stream as there is one less round-trip from client->server required. Over smart HTTP this avoids needing to send a final RPC that has all of the prior common objects. Instead the server is able to return a pack as soon as its ready to. For many common users the smart HTTP fetch is now just 2 requests: GET .../info/refs, and a POST .../git-upload-pack to not only negotiate but also receive the pack stream. Only users who have more than 32 local unshared commits with the remote will need additional requests to negotiate a common merge base. Signed-off-by: Shawn O. Pearce Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 5173dc9aad..59fbda522b 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -14,6 +14,7 @@ static int transfer_unpack_limit = -1; static int fetch_unpack_limit = -1; static int unpack_limit = 100; static int prefer_ofs_delta = 1; +static int no_done = 0; static struct fetch_pack_args args = { /* .uploadpack = */ "git-upload-pack", }; @@ -225,6 +226,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, const unsigned char *sha1; unsigned in_vain = 0; int got_continue = 0; + int got_ready = 0; struct strbuf req_buf = STRBUF_INIT; size_t state_len = 0; @@ -262,6 +264,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, struct strbuf c = STRBUF_INIT; if (multi_ack == 2) strbuf_addstr(&c, " multi_ack_detailed"); if (multi_ack == 1) strbuf_addstr(&c, " multi_ack"); + if (no_done) strbuf_addstr(&c, " no-done"); if (use_sideband == 2) strbuf_addstr(&c, " side-band-64k"); if (use_sideband == 1) strbuf_addstr(&c, " side-band"); if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack"); @@ -379,8 +382,10 @@ static int find_common(int fd[2], unsigned char *result_sha1, retval = 0; in_vain = 0; got_continue = 1; - if (ack == ACK_ready) + if (ack == ACK_ready) { rev_list = NULL; + got_ready = 1; + } break; } } @@ -394,8 +399,10 @@ static int find_common(int fd[2], unsigned char *result_sha1, } } done: - packet_buf_write(&req_buf, "done\n"); - send_request(fd[1], &req_buf); + if (!got_ready || !no_done) { + packet_buf_write(&req_buf, "done\n"); + send_request(fd[1], &req_buf); + } if (args.verbose) fprintf(stderr, "done\n"); if (retval != 0) { @@ -698,6 +705,11 @@ static struct ref *do_fetch_pack(int fd[2], if (args.verbose) fprintf(stderr, "Server supports multi_ack_detailed\n"); multi_ack = 2; + if (server_supports("no-done")) { + if (args.verbose) + fprintf(stderr, "Server supports no-done\n"); + no_done = 1; + } } else if (server_supports("multi_ack")) { if (args.verbose) -- cgit v1.3-5-g9baa From e52d719266a06a8553043cb5616d9b4ce4abd27a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 11 Mar 2011 11:53:52 -0800 Subject: fetch-pack: objects in our alternates are available to us Use the helper function split from the receiving end of "git push" to allow the same optimization on the receiving end of "git fetch". Signed-off-by: Junio C Hamano Acked-by: Shawn O. Pearce --- builtin/fetch-pack.c | 12 ++++++++++++ t/t5501-fetch-push-alternates.sh | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index b999413934..4c25968e16 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -9,6 +9,7 @@ #include "fetch-pack.h" #include "remote.h" #include "run-command.h" +#include "transport.h" static int transfer_unpack_limit = -1; static int fetch_unpack_limit = -1; @@ -217,6 +218,16 @@ static void send_request(int fd, struct strbuf *buf) safe_write(fd, buf->buf, buf->len); } +static void insert_one_alternate_ref(const struct ref *ref, void *unused) +{ + rev_list_insert_ref(NULL, ref->old_sha1, 0, NULL); +} + +static void insert_alternate_refs(void) +{ + foreach_alt_odb(refs_from_alternate_cb, insert_one_alternate_ref); +} + static int find_common(int fd[2], unsigned char *result_sha1, struct ref *refs) { @@ -235,6 +246,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, marked = 1; for_each_ref(rev_list_insert_ref, NULL); + insert_alternate_refs(); fetching = 0; for ( ; refs ; refs = refs->next) { diff --git a/t/t5501-fetch-push-alternates.sh b/t/t5501-fetch-push-alternates.sh index 16a467654c..b5ced8483a 100755 --- a/t/t5501-fetch-push-alternates.sh +++ b/t/t5501-fetch-push-alternates.sh @@ -54,7 +54,7 @@ test_expect_success 'pushing into a repository with the same alternate' ' test_cmp one.count receiver.count ' -test_expect_failure 'fetching from a repository with the same alternate' ' +test_expect_success 'fetching from a repository with the same alternate' ' ( cd fetcher && git fetch ../one master:refs/heads/it && -- cgit v1.3-5-g9baa From c12f5917e4f528b056a8b9ca625397aee97ae1e4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 20 Mar 2011 21:52:40 -0700 Subject: fetch-pack: factor out hardcoded handshake window size The "git fetch" client presents the most recent 32 commits it has to the server and gives a chance to the server to say "ok, we heard enough", and continues reporting what it has in chunks of 32 commits, digging its history down to older commits. Move the hardcoded size of the handshake window outside the code, so that we can tweak it more easily. Signed-off-by: Junio C Hamano Acked-by: Shawn Pearce --- builtin/fetch-pack.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 59fbda522b..1abe624dc8 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -218,11 +218,18 @@ static void send_request(int fd, struct strbuf *buf) safe_write(fd, buf->buf, buf->len); } +#define INITIAL_FLUSH 32 + +static int next_flush(int count) +{ + return INITIAL_FLUSH + count; +} + static int find_common(int fd[2], unsigned char *result_sha1, struct ref *refs) { int fetching; - int count = 0, flushes = 0, retval; + int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval; const unsigned char *sha1; unsigned in_vain = 0; int got_continue = 0; @@ -335,19 +342,20 @@ static int find_common(int fd[2], unsigned char *result_sha1, if (args.verbose) fprintf(stderr, "have %s\n", sha1_to_hex(sha1)); in_vain++; - if (!(31 & ++count)) { + if (flush_at <= ++count) { int ack; packet_buf_flush(&req_buf); send_request(fd[1], &req_buf); strbuf_setlen(&req_buf, state_len); flushes++; + flush_at = next_flush(count); /* * We keep one window "ahead" of the other side, and * will wait for an ACK only on the next one */ - if (!args.stateless_rpc && count == 32) + if (!args.stateless_rpc && count == INITIAL_FLUSH) continue; consume_shallow_list(fd[0]); -- cgit v1.3-5-g9baa From 6afca450c3f2f05385900a7b8d3a0d47286f983f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 20 Mar 2011 21:52:44 -0700 Subject: fetch-pack: progressively use larger handshake windows The client has to dig the history deeper when more recent parts of its history do not have any overlap with the server it is fetching from. Make the handshake window exponentially larger as we dig deeper, with a reasonable upper cap. Signed-off-by: Junio C Hamano Acked-by: Shawn Pearce --- builtin/fetch-pack.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 1abe624dc8..b4f34a2cf9 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -219,10 +219,15 @@ static void send_request(int fd, struct strbuf *buf) } #define INITIAL_FLUSH 32 +#define LARGE_FLUSH 1024 static int next_flush(int count) { - return INITIAL_FLUSH + count; + if (count < LARGE_FLUSH) + count <<= 1; + else + count += LARGE_FLUSH; + return count; } static int find_common(int fd[2], unsigned char *result_sha1, -- cgit v1.3-5-g9baa From 066bf4c2e43c7fb40405843e49f809431314865d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 20 Mar 2011 21:52:45 -0700 Subject: fetch-pack: use smaller handshake window for initial request Start the initial request small by halving the INITIAL_FLUSH (we will try to stay one window ahead of the server, so we would end up giving twice as many "have" in flight at the very beginning). We may want to tweak these values even more, taking MTU into account. Signed-off-by: Junio C Hamano Acked-by: Shawn Pearce --- builtin/fetch-pack.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index b4f34a2cf9..3c2c9406c4 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -218,12 +218,14 @@ static void send_request(int fd, struct strbuf *buf) safe_write(fd, buf->buf, buf->len); } -#define INITIAL_FLUSH 32 +#define INITIAL_FLUSH 16 #define LARGE_FLUSH 1024 static int next_flush(int count) { - if (count < LARGE_FLUSH) + if (count < INITIAL_FLUSH * 2) + count += INITIAL_FLUSH; + else if (count < LARGE_FLUSH) count <<= 1; else count += LARGE_FLUSH; -- cgit v1.3-5-g9baa From c2e86addb86689306b992065328ec52aa2479658 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 22 Mar 2011 00:51:05 -0700 Subject: Fix sparse warnings Fix warnings from 'make check'. - These files don't include 'builtin.h' causing sparse to complain that cmd_* isn't declared: builtin/clone.c:364, builtin/fetch-pack.c:797, builtin/fmt-merge-msg.c:34, builtin/hash-object.c:78, builtin/merge-index.c:69, builtin/merge-recursive.c:22 builtin/merge-tree.c:341, builtin/mktag.c:156, builtin/notes.c:426 builtin/notes.c:822, builtin/pack-redundant.c:596, builtin/pack-refs.c:10, builtin/patch-id.c:60, builtin/patch-id.c:149, builtin/remote.c:1512, builtin/remote-ext.c:240, builtin/remote-fd.c:53, builtin/reset.c:236, builtin/send-pack.c:384, builtin/unpack-file.c:25, builtin/var.c:75 - These files have symbols which should be marked static since they're only file scope: submodule.c:12, diff.c:631, replace_object.c:92, submodule.c:13, submodule.c:14, trace.c:78, transport.c:195, transport-helper.c:79, unpack-trees.c:19, url.c:3, url.c:18, url.c:104, url.c:117, url.c:123, url.c:129, url.c:136, thread-utils.c:21, thread-utils.c:48 - These files redeclare symbols to be different types: builtin/index-pack.c:210, parse-options.c:564, parse-options.c:571, usage.c:49, usage.c:58, usage.c:63, usage.c:72 - These files use a literal integer 0 when they really should use a NULL pointer: daemon.c:663, fast-import.c:2942, imap-send.c:1072, notes-merge.c:362 While we're in the area, clean up some unused #includes in builtin files (mostly exec_cmd.h). Signed-off-by: Stephen Boyd Signed-off-by: Junio C Hamano --- builtin/clone.c | 2 +- builtin/fetch-pack.c | 2 +- builtin/fmt-merge-msg.c | 2 +- builtin/hash-object.c | 2 +- builtin/index-pack.c | 2 +- builtin/merge-index.c | 3 +-- builtin/merge-recursive.c | 2 +- builtin/merge-tree.c | 2 +- builtin/mktag.c | 3 +-- builtin/notes.c | 4 ++-- builtin/pack-redundant.c | 3 +-- builtin/pack-refs.c | 2 +- builtin/patch-id.c | 5 ++--- builtin/remote-ext.c | 2 +- builtin/remote-fd.c | 2 +- builtin/remote.c | 2 +- builtin/reset.c | 2 +- builtin/send-pack.c | 2 +- builtin/unpack-file.c | 4 +--- builtin/var.c | 3 +-- daemon.c | 2 +- diff.c | 2 +- fast-import.c | 2 +- imap-send.c | 2 +- notes-merge.c | 2 +- parse-options.c | 4 ++-- replace_object.c | 1 + submodule.c | 6 +++--- thread-utils.c | 2 +- trace.c | 2 +- transport-helper.c | 2 +- transport.c | 2 +- unpack-trees.c | 2 +- url.c | 1 + usage.c | 8 ++++---- 35 files changed, 43 insertions(+), 48 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/clone.c b/builtin/clone.c index 02547adba5..c6e10bb9e9 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -8,7 +8,7 @@ * Clone a repository into a different directory that does not yet exist. */ -#include "cache.h" +#include "builtin.h" #include "parse-options.h" #include "fetch-pack.h" #include "refs.h" diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 272bc383d6..ef398620af 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "refs.h" #include "pkt-line.h" #include "commit.h" diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 5189b16c9e..75816329d6 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -31,7 +31,7 @@ struct src_data { int head_status; }; -void init_src_data(struct src_data *data) +static void init_src_data(struct src_data *data) { data->branch.strdup_strings = 1; data->tag.strdup_strings = 1; diff --git a/builtin/hash-object.c b/builtin/hash-object.c index c90acddcb2..b96f46acf5 100644 --- a/builtin/hash-object.c +++ b/builtin/hash-object.c @@ -4,7 +4,7 @@ * Copyright (C) Linus Torvalds, 2005 * Copyright (C) Junio C Hamano, 2005 */ -#include "cache.h" +#include "builtin.h" #include "blob.h" #include "quote.h" #include "parse-options.h" diff --git a/builtin/index-pack.c b/builtin/index-pack.c index c7e600db47..5a67c8181e 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -207,7 +207,7 @@ static void parse_pack_header(void) static NORETURN void bad_object(unsigned long offset, const char *format, ...) __attribute__((format (printf, 2, 3))); -static void bad_object(unsigned long offset, const char *format, ...) +static NORETURN void bad_object(unsigned long offset, const char *format, ...) { va_list params; char buf[1024]; diff --git a/builtin/merge-index.c b/builtin/merge-index.c index 2c4cf5e559..2338832587 100644 --- a/builtin/merge-index.c +++ b/builtin/merge-index.c @@ -1,6 +1,5 @@ -#include "cache.h" +#include "builtin.h" #include "run-command.h" -#include "exec_cmd.h" static const char *pgm; static int one_shot, quiet; diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c index c33091b3ed..3a64f5d0bd 100644 --- a/builtin/merge-recursive.c +++ b/builtin/merge-recursive.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "commit.h" #include "tag.h" #include "merge-recursive.h" diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index 9b25ddc979..19917426fb 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "tree-walk.h" #include "xdiff-interface.h" #include "blob.h" diff --git a/builtin/mktag.c b/builtin/mktag.c index 1cb0f3f2a7..d0ccbb2222 100644 --- a/builtin/mktag.c +++ b/builtin/mktag.c @@ -1,6 +1,5 @@ -#include "cache.h" +#include "builtin.h" #include "tag.h" -#include "exec_cmd.h" /* * A signature file has a very simple fixed format: four lines diff --git a/builtin/notes.c b/builtin/notes.c index 0aab150c52..a0f310b729 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -423,7 +423,7 @@ void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c) free(c); } -int notes_copy_from_stdin(int force, const char *rewrite_cmd) +static int notes_copy_from_stdin(int force, const char *rewrite_cmd) { struct strbuf buf = STRBUF_INIT; struct notes_rewrite_cfg *c = NULL; @@ -819,7 +819,7 @@ static int merge_commit(struct notes_merge_options *o) t = xcalloc(1, sizeof(struct notes_tree)); init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0); - o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, 0); + o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL); if (!o->local_ref) die("Failed to resolve NOTES_MERGE_REF"); diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index 41e1615a28..f5c6afc5dd 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -6,8 +6,7 @@ * */ -#include "cache.h" -#include "exec_cmd.h" +#include "builtin.h" #define BLKSIZE 512 diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c index 091860b2e3..39a9d89fbd 100644 --- a/builtin/pack-refs.c +++ b/builtin/pack-refs.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "parse-options.h" #include "pack-refs.h" diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 49a0472a9b..f821eb3f0b 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -1,5 +1,4 @@ -#include "cache.h" -#include "exec_cmd.h" +#include "builtin.h" static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c) { @@ -57,7 +56,7 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after) return 1; } -int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx) +static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx) { static char line[1000]; int patchlen = 0, found_next = 0; diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c index ea71977c83..155e609d68 100644 --- a/builtin/remote-ext.c +++ b/builtin/remote-ext.c @@ -1,4 +1,4 @@ -#include "git-compat-util.h" +#include "builtin.h" #include "transport.h" #include "run-command.h" diff --git a/builtin/remote-fd.c b/builtin/remote-fd.c index 1f2467bdb7..08d7121b6d 100644 --- a/builtin/remote-fd.c +++ b/builtin/remote-fd.c @@ -1,4 +1,4 @@ -#include "git-compat-util.h" +#include "builtin.h" #include "transport.h" /* diff --git a/builtin/remote.c b/builtin/remote.c index cb26080956..b71ecd228f 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "parse-options.h" #include "transport.h" #include "remote.h" diff --git a/builtin/reset.c b/builtin/reset.c index 5de2bceeec..eb5f98c163 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -7,7 +7,7 @@ * * Copyright (c) 2005, 2006 Linus Torvalds and Junio C Hamano */ -#include "cache.h" +#include "builtin.h" #include "tag.h" #include "object.h" #include "commit.h" diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 2cd1c40b70..8b0911c0d2 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "builtin.h" #include "commit.h" #include "refs.h" #include "pkt-line.h" diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c index 608590ada8..19200291a2 100644 --- a/builtin/unpack-file.c +++ b/builtin/unpack-file.c @@ -1,6 +1,4 @@ -#include "cache.h" -#include "blob.h" -#include "exec_cmd.h" +#include "builtin.h" static char *create_temp_file(unsigned char *sha1) { diff --git a/builtin/var.c b/builtin/var.c index 0744bb8318..99d068a532 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -3,8 +3,7 @@ * * Copyright (C) Eric Biederman, 2005 */ -#include "cache.h" -#include "exec_cmd.h" +#include "builtin.h" static const char var_usage[] = "git var (-l | )"; diff --git a/daemon.c b/daemon.c index 347fd0c52b..4c8346d5a1 100644 --- a/daemon.c +++ b/daemon.c @@ -660,7 +660,7 @@ static void check_dead_children(void) static char **cld_argv; static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen) { - struct child_process cld = { 0 }; + struct child_process cld = { NULL }; char addrbuf[300] = "REMOTE_ADDR=", portbuf[300]; char *env[] = { addrbuf, portbuf, NULL }; diff --git a/diff.c b/diff.c index 42a107c58a..9b3eb9938f 100644 --- a/diff.c +++ b/diff.c @@ -628,7 +628,7 @@ struct diff_words_style { const char *newline; }; -struct diff_words_style diff_words_styles[] = { +static struct diff_words_style diff_words_styles[] = { { DIFF_WORDS_PORCELAIN, {"+", "\n"}, {"-", "\n"}, {" ", "\n"}, "~\n" }, { DIFF_WORDS_PLAIN, {"{+", "+}"}, {"[-", "-]"}, {"", ""}, "\n" }, { DIFF_WORDS_COLOR, {"", ""}, {"", ""}, {"", ""}, "\n" } diff --git a/fast-import.c b/fast-import.c index d9f9a3f524..65d65bf8f9 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2939,7 +2939,7 @@ static void parse_ls(struct branch *b) { const char *p; struct tree_entry *root = NULL; - struct tree_entry leaf = {0}; + struct tree_entry leaf = {NULL}; /* ls SP ( SP)? */ p = command_buf.buf + strlen("ls "); diff --git a/imap-send.c b/imap-send.c index 71506a8dd3..9adf4b9819 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1069,7 +1069,7 @@ static struct store *imap_open_store(struct imap_server_conf *srvc) if (srvc->tunnel) { const char *argv[] = { srvc->tunnel, NULL }; - struct child_process tunnel = {0}; + struct child_process tunnel = {NULL}; imap_info("Starting tunnel '%s'... ", srvc->tunnel); diff --git a/notes-merge.c b/notes-merge.c index 1467ad3179..28046a9984 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -359,7 +359,7 @@ static int ll_merge_in_worktree(struct notes_merge_options *o, read_mmblob(&remote, p->remote); status = ll_merge(&result_buf, sha1_to_hex(p->obj), &base, NULL, - &local, o->local_ref, &remote, o->remote_ref, 0); + &local, o->local_ref, &remote, o->remote_ref, NULL); free(base.ptr); free(local.ptr); diff --git a/parse-options.c b/parse-options.c index 42b51ef145..73bd28ad90 100644 --- a/parse-options.c +++ b/parse-options.c @@ -561,14 +561,14 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx, return PARSE_OPT_HELP; } -void usage_with_options(const char * const *usagestr, +void NORETURN usage_with_options(const char * const *usagestr, const struct option *opts) { usage_with_options_internal(NULL, usagestr, opts, 0, 1); exit(129); } -void usage_msg_opt(const char *msg, +void NORETURN usage_msg_opt(const char *msg, const char * const *usagestr, const struct option *options) { diff --git a/replace_object.c b/replace_object.c index eb59604fd3..7c6c7544ad 100644 --- a/replace_object.c +++ b/replace_object.c @@ -1,6 +1,7 @@ #include "cache.h" #include "sha1-lookup.h" #include "refs.h" +#include "commit.h" static struct replace_object { unsigned char sha1[2][20]; diff --git a/submodule.c b/submodule.c index e9f2b19e1c..0cb6d18299 100644 --- a/submodule.c +++ b/submodule.c @@ -9,9 +9,9 @@ #include "refs.h" #include "string-list.h" -struct string_list config_name_for_path; -struct string_list config_fetch_recurse_submodules_for_name; -struct string_list config_ignore_for_name; +static struct string_list config_name_for_path; +static struct string_list config_fetch_recurse_submodules_for_name; +static struct string_list config_ignore_for_name; static int config_fetch_recurse_submodules; static int add_submodule_odb(const char *path) diff --git a/thread-utils.c b/thread-utils.c index 589f838f82..7f4b76a958 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -1,5 +1,5 @@ #include "cache.h" -#include +#include "thread-utils.h" #if defined(hpux) || defined(__hpux) || defined(_hpux) # include diff --git a/trace.c b/trace.c index 8390bf7cbb..d95341693f 100644 --- a/trace.c +++ b/trace.c @@ -75,7 +75,7 @@ void trace_vprintf(const char *key, const char *fmt, va_list ap) strbuf_release(&buf); } -void trace_printf_key(const char *key, const char *fmt, ...) +static void trace_printf_key(const char *key, const char *fmt, ...) { va_list ap; va_start(ap, fmt); diff --git a/transport-helper.c b/transport-helper.c index 0c5b1bd994..5846b55875 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -76,7 +76,7 @@ static void write_constant(int fd, const char *str) die_errno("Full write to remote helper failed"); } -const char *remove_ext_force(const char *url) +static const char *remove_ext_force(const char *url) { if (url) { const char *colon = strchr(url, ':'); diff --git a/transport.c b/transport.c index 0078660611..f1c07816e0 100644 --- a/transport.c +++ b/transport.c @@ -192,7 +192,7 @@ static const char *rsync_url(const char *url) static struct ref *get_refs_via_rsync(struct transport *transport, int for_push) { struct strbuf buf = STRBUF_INIT, temp_dir = STRBUF_INIT; - struct ref dummy = {0}, *tail = &dummy; + struct ref dummy = {NULL}, *tail = &dummy; struct child_process rsync; const char *args[5]; int temp_dir_len; diff --git a/unpack-trees.c b/unpack-trees.c index b68ec820dd..500ebcfd54 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -16,7 +16,7 @@ * situation better. See how "git checkout" and "git merge" replaces * them using setup_unpack_trees_porcelain(), for example. */ -const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { +static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = { /* ERROR_WOULD_OVERWRITE */ "Entry '%s' would be overwritten by merge. Cannot merge.", diff --git a/url.c b/url.c index 6a5495960f..3e06fd34c4 100644 --- a/url.c +++ b/url.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "url.h" int is_urlschemechar(int first_flag, int ch) { diff --git a/usage.c b/usage.c index ec4cf53b6b..b5e67e3d0d 100644 --- a/usage.c +++ b/usage.c @@ -46,7 +46,7 @@ void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list param die_routine = routine; } -void usagef(const char *err, ...) +void NORETURN usagef(const char *err, ...) { va_list params; @@ -55,12 +55,12 @@ void usagef(const char *err, ...) va_end(params); } -void usage(const char *err) +void NORETURN usage(const char *err) { usagef("%s", err); } -void die(const char *err, ...) +void NORETURN die(const char *err, ...) { va_list params; @@ -69,7 +69,7 @@ void die(const char *err, ...) va_end(params); } -void die_errno(const char *fmt, ...) +void NORETURN die_errno(const char *fmt, ...) { va_list params; char fmt_with_err[1024]; -- cgit v1.3-5-g9baa From 07514c83c2b7e6de926ef758905e3f43b4de6bfa Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 28 Mar 2011 23:35:39 -0700 Subject: Revert "fetch-pack: Implement no-done capability" This reverts commit 761ecf0bc7b6cddf311f00877c59e6381cdbdeea. --- builtin/fetch-pack.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index bf9990ce15..1724b76052 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -15,7 +15,6 @@ static int transfer_unpack_limit = -1; static int fetch_unpack_limit = -1; static int unpack_limit = 100; static int prefer_ofs_delta = 1; -static int no_done = 0; static struct fetch_pack_args args = { /* .uploadpack = */ "git-upload-pack", }; @@ -251,7 +250,6 @@ static int find_common(int fd[2], unsigned char *result_sha1, const unsigned char *sha1; unsigned in_vain = 0; int got_continue = 0; - int got_ready = 0; struct strbuf req_buf = STRBUF_INIT; size_t state_len = 0; @@ -290,7 +288,6 @@ static int find_common(int fd[2], unsigned char *result_sha1, struct strbuf c = STRBUF_INIT; if (multi_ack == 2) strbuf_addstr(&c, " multi_ack_detailed"); if (multi_ack == 1) strbuf_addstr(&c, " multi_ack"); - if (no_done) strbuf_addstr(&c, " no-done"); if (use_sideband == 2) strbuf_addstr(&c, " side-band-64k"); if (use_sideband == 1) strbuf_addstr(&c, " side-band"); if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack"); @@ -409,10 +406,8 @@ static int find_common(int fd[2], unsigned char *result_sha1, retval = 0; in_vain = 0; got_continue = 1; - if (ack == ACK_ready) { + if (ack == ACK_ready) rev_list = NULL; - got_ready = 1; - } break; } } @@ -426,10 +421,8 @@ static int find_common(int fd[2], unsigned char *result_sha1, } } done: - if (!got_ready || !no_done) { - packet_buf_write(&req_buf, "done\n"); - send_request(fd[1], &req_buf); - } + packet_buf_write(&req_buf, "done\n"); + send_request(fd[1], &req_buf); if (args.verbose) fprintf(stderr, "done\n"); if (retval != 0) { @@ -732,11 +725,6 @@ static struct ref *do_fetch_pack(int fd[2], if (args.verbose) fprintf(stderr, "Server supports multi_ack_detailed\n"); multi_ack = 2; - if (server_supports("no-done")) { - if (args.verbose) - fprintf(stderr, "Server supports no-done\n"); - no_done = 1; - } } else if (server_supports("multi_ack")) { if (args.verbose) -- cgit v1.3-5-g9baa From 8e9182e09127b20b8d8ce19655037991feac798d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 29 Mar 2011 10:16:29 -0700 Subject: enable "no-done" extension only when fetching over smart-http When 'no-done' protocol extension is used, the upload-pack (i.e. the server side) process stops listening to the fetch-pack after issuing the final NAK, and starts sending the generated pack data back, but there may be more "have" send by the latter in flight that the fetch-pack is expecting to be responded with ACK/NAK. This will typically result in a deadlock (both will block on write that the other end never reads) or SIGPIPE on the fetch-pack end (upload-pack will finish writing a small pack and goes away). Disable it unless fetch-pack is running under smart-http, where there is no such streaming issue. Signed-off-by: Junio C Hamano Acked-by: Shawn O. Pearce --- builtin/fetch-pack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 59fbda522b..52707a80ad 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -708,7 +708,8 @@ static struct ref *do_fetch_pack(int fd[2], if (server_supports("no-done")) { if (args.verbose) fprintf(stderr, "Server supports no-done\n"); - no_done = 1; + if (args.stateless_rpc) + no_done = 1; } } else if (server_supports("multi_ack")) { -- cgit v1.3-5-g9baa From 44d8dc54e73e8010c4bdf57a422fc8d5ce709029 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 29 Mar 2011 10:06:19 -0700 Subject: Fix potential local deadlock during fetch-pack The fetch-pack/upload-pack protocol relies on the underlying transport (local pipe or TCP socket) to have enough slack to allow one window worth of data in flight without blocking the writer. Traditionally we always relied on being able to have two windows of 32 "have"s in flight (roughly 3k bytes) to stream. The recent "progressive-stride" change allows "fetch-pack" to send up to 1024 "have"s without reading any response from "upload-pack". The outgoing pipe of "upload-pack" can be clogged with many ACK and NAK that are unread, while "fetch-pack" is still stuffing its outgoing pipe with more "have"s, leading to a deadlock. Revert the change unless we are in stateless rpc (aka smart-http) mode, as using a large window full of "have"s is still a good way to help reduce the number of back-and-forth, and there is no buffering issue there (it is strictly "ping-pong" without an overlap). Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 3c2c9406c4..147d67dca4 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -219,16 +219,17 @@ static void send_request(int fd, struct strbuf *buf) } #define INITIAL_FLUSH 16 +#define PIPESAFE_FLUSH 32 #define LARGE_FLUSH 1024 static int next_flush(int count) { - if (count < INITIAL_FLUSH * 2) - count += INITIAL_FLUSH; - else if (count < LARGE_FLUSH) + int flush_limit = args.stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH; + + if (count < flush_limit) count <<= 1; else - count += LARGE_FLUSH; + count += flush_limit; return count; } -- cgit v1.3-5-g9baa From 4e10cf9a17467c08754b36683c240fbab69156de Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 29 Mar 2011 12:29:10 -0700 Subject: Revert two "no-done" reverts Last night I had to make these two emergency reverts, but now we have a better understanding of which part of the topic was broken, let's get rid of the revert to fix it correctly. Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 18 +++++++++++++++--- upload-pack.c | 20 ++++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) (limited to 'builtin/fetch-pack.c') diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 1724b76052..bf9990ce15 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -15,6 +15,7 @@ static int transfer_unpack_limit = -1; static int fetch_unpack_limit = -1; static int unpack_limit = 100; static int prefer_ofs_delta = 1; +static int no_done = 0; static struct fetch_pack_args args = { /* .uploadpack = */ "git-upload-pack", }; @@ -250,6 +251,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, const unsigned char *sha1; unsigned in_vain = 0; int got_continue = 0; + int got_ready = 0; struct strbuf req_buf = STRBUF_INIT; size_t state_len = 0; @@ -288,6 +290,7 @@ static int find_common(int fd[2], unsigned char *result_sha1, struct strbuf c = STRBUF_INIT; if (multi_ack == 2) strbuf_addstr(&c, " multi_ack_detailed"); if (multi_ack == 1) strbuf_addstr(&c, " multi_ack"); + if (no_done) strbuf_addstr(&c, " no-done"); if (use_sideband == 2) strbuf_addstr(&c, " side-band-64k"); if (use_sideband == 1) strbuf_addstr(&c, " side-band"); if (args.use_thin_pack) strbuf_addstr(&c, " thin-pack"); @@ -406,8 +409,10 @@ static int find_common(int fd[2], unsigned char *result_sha1, retval = 0; in_vain = 0; got_continue = 1; - if (ack == ACK_ready) + if (ack == ACK_ready) { rev_list = NULL; + got_ready = 1; + } break; } } @@ -421,8 +426,10 @@ static int find_common(int fd[2], unsigned char *result_sha1, } } done: - packet_buf_write(&req_buf, "done\n"); - send_request(fd[1], &req_buf); + if (!got_ready || !no_done) { + packet_buf_write(&req_buf, "done\n"); + send_request(fd[1], &req_buf); + } if (args.verbose) fprintf(stderr, "done\n"); if (retval != 0) { @@ -725,6 +732,11 @@ static struct ref *do_fetch_pack(int fd[2], if (args.verbose) fprintf(stderr, "Server supports multi_ack_detailed\n"); multi_ack = 2; + if (server_supports("no-done")) { + if (args.verbose) + fprintf(stderr, "Server supports no-done\n"); + no_done = 1; + } } else if (server_supports("multi_ack")) { if (args.verbose) diff --git a/upload-pack.c b/upload-pack.c index eb80dd9aad..72aa661f8d 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -27,6 +27,7 @@ static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=< static unsigned long oldest_have; static int multi_ack, nr_our_refs; +static int no_done; static int use_thin_pack, use_ofs_delta, use_include_tag; static int no_progress, daemon_mode; static int shallow_nr; @@ -431,6 +432,7 @@ static int get_common_commits(void) char last_hex[41]; int got_common = 0; int got_other = 0; + int sent_ready = 0; save_commit_buffer = 0; @@ -440,10 +442,17 @@ static int get_common_commits(void) if (!len) { if (multi_ack == 2 && got_common - && !got_other && ok_to_give_up()) + && !got_other && ok_to_give_up()) { + sent_ready = 1; packet_write(1, "ACK %s ready\n", last_hex); + } if (have_obj.nr == 0 || multi_ack) packet_write(1, "NAK\n"); + + if (no_done && sent_ready) { + packet_write(1, "ACK %s\n", last_hex); + return 0; + } if (stateless_rpc) exit(0); got_common = 0; @@ -457,9 +466,10 @@ static int get_common_commits(void) got_other = 1; if (multi_ack && ok_to_give_up()) { const char *hex = sha1_to_hex(sha1); - if (multi_ack == 2) + if (multi_ack == 2) { + sent_ready = 1; packet_write(1, "ACK %s ready\n", hex); - else + } else packet_write(1, "ACK %s continue\n", hex); } break; @@ -535,6 +545,8 @@ static void receive_needs(void) multi_ack = 2; else if (strstr(line+45, "multi_ack")) multi_ack = 1; + if (strstr(line+45, "no-done")) + no_done = 1; if (strstr(line+45, "thin-pack")) use_thin_pack = 1; if (strstr(line+45, "ofs-delta")) @@ -628,7 +640,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo { static const char *capabilities = "multi_ack thin-pack side-band" " side-band-64k ofs-delta shallow no-progress" - " include-tag multi_ack_detailed"; + " include-tag multi_ack_detailed no-done"; struct object *o = parse_object(sha1); if (!o) -- cgit v1.3-5-g9baa