{"thread":{"id":"3420","subject":"Git cannot push to repository with too many tags/heads","startedAt":"2006-02-21T18:46:07Z","lastAt":"2006-02-22T18:42:46Z","messageCount":7,"participants":["Stephen C. Tweedie","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"16512","messageId":"1140547568.5509.21.camel@orbit.scot.redhat.com","threadId":"3420","inReplyTo":null,"subject":"Git cannot push to repository with too many tags/heads","fromName":"Stephen C. Tweedie","fromEmail":"sct@redhat.com","sentAt":"2006-02-21T18:46:07Z","receivedAt":"2006-02-21T18:46:07Z","isPatch":false,"sender":{"key":"sct@redhat.com","avatar":null},"body":"Hi,\n\nI'm using git's cvs import to track the Fedora kernel rpm repository,\nand it has just passed a watermark that git cannot currently cope with.\n\nThe problem is the large number of branches and tags on this repository.\nA tag for every build means that the number builds up rapidly, and\ngit-send-pack has an internal limit of 900 refs to query when building a\npack.\n\nI hoped this wouldn't be a problem. In theory, I should just be able to\npush to a single refspec and avoid that limit entirely. But no ---\nunfortunately, git is determined to push out the objects for *every* ref\nthat matches between src and dst when building the pack, even if it's\nnot actually going to update the remote ref. And in building the\ngit-rev-list argument list for that colossal update, it exceeds the\ninternal limit of 900 refs in exec_rev_list().\n\nI think exec_rev_list() is doing the wrong thing here. If I specify an\nexplicit refspec list for git-send-pack, then when send_pack() calls\nmatch_refs(), we break out to match_explicit_refs() and set\ndst->peer_ref only for the ref[s] which match the refspec. send_pack()\nthen skips all other refs by doing a\n\n\t\tif (!ref->peer_ref)\n\t\t\tcontinue;\n\nUnfortunately, exec_rev_list() is missing this, and it tries to ask\ngit-rev-list for the commit objects of *every* ref on the remote_refs\nlist, even if they have been explicitly excluded by match_refs() and\nhave no peer_ref. So with this huge repository, I can't even push a\nsingle refspec without bumping into the limit of 900 refs.\n\nThe immediate consequence to me is that I can't push to this particular\nrepository. But if I'm following the code right, a consequence is that\ngit-send-pack is accidentally sending all objects for refs that the\nremote end is prepared to receive, even if we've supplied a refspec\nasking for only a subset of those refs to actually be updated.\n\nI think we can fix this just by adding a test for ref->peer_ref to\nexec_rev_list(). That seems to fix the immediate problem for me, at\nleast. Patch to follow.\n\n--Stephen\n"},{"id":"16513","messageId":"1140547702.5509.24.camel@orbit.scot.redhat.com","threadId":"3420","inReplyTo":"1140547568.5509.21.camel@orbit.scot.redhat.com","subject":"[PATCH] Don't sent objects for refs we're not going to update.","fromName":"Stephen C. Tweedie","fromEmail":"sct@redhat.com","sentAt":"2006-02-21T18:48:22Z","receivedAt":"2006-02-21T18:48:22Z","isPatch":true,"sender":{"key":"sct@redhat.com","avatar":null},"body":"send_pack() sends only those refs we've asked to be updated on the\ndestination---either via an explicit refspec or by matching local and\nremote refs. But rev_list() builds an object list for *all* refs it\ncan find. For a tree with many tags/heads, this means that it is\nimpossible to push updates even to a single refspec, as exec_rev_list()\noverflows its arg length limit.\n\nFix this by skipping refs with no peer_ref set in exec_rev_list().\nsend_pack() already skips it when sending refs; we need to skip it\nwhen building the object list for the pack too.\n\nSigned-off-by: Stephen Tweedie \n\n---\n\n send-pack.c | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\n4aa39c0920eea37987ca8a6b10861da7a87b5c14\ndiff --git a/send-pack.c b/send-pack.c\nindex 990be3f..d2a39d9 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -42,8 +42,10 @@ static void exec_rev_list(struct ref *re\n \n \targs[i++] = \"rev-list\";\t/* 0 */\n \targs[i++] = \"--objects\";\t/* 1 */\n-\twhile (refs) {\n+\tfor (; refs; refs = refs->next) {\n \t\tchar *buf = malloc(100);\n+\t\tif (!refs->peer_ref)\n+\t\t\tcontinue;\n \t\tif (i > 900)\n \t\t\tdie(\"git-rev-list environment overflow\");\n \t\tif (!is_zero_sha1(refs->old_sha1) &&\n@@ -56,7 +58,6 @@ static void exec_rev_list(struct ref *re\n \t\t\targs[i++] = buf;\n \t\t\tsnprintf(buf, 50, \"%s\", sha1_to_hex(refs->new_sha1));\n \t\t}\n-\t\trefs = refs->next;\n \t}\n \targs[i] = NULL;\n \texecv_git_cmd(args);\n-- \n1.2.2.g6643-dirty\n"},{"id":"16556","messageId":"7vwtfotaq3.fsf@assigned-by-dhcp.cox.net","threadId":"3420","inReplyTo":"1140547568.5509.21.camel@orbit.scot.redhat.com","subject":"Re: Git cannot push to repository with too many tags/heads","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T01:59:16Z","receivedAt":"2006-02-22T01:59:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen C. Tweedie\" writes:\n\n> send_pack()\n> then skips all other refs by doing a\n>\n> \t\tif (!ref->peer_ref)\n> \t\t\tcontinue;\n>\n> Unfortunately, exec_rev_list() is missing this, and it tries to ask\n> git-rev-list for the commit objects of *every* ref on the remote_refs\n> list, even if they have been explicitly excluded by match_refs() and\n> have no peer_ref. So with this huge repository, I can't even push a\n> single refspec without bumping into the limit of 900 refs.\n\nIIRC, the distinction was deliberate. send_pack() excludes\nwhat did not match because it does not want to send them.\nrev_list() adds what we know they have to \"do not bother to\nsend\" list to make the resulting pack smaller. The time where\nit matters most is when you are pushing a new branch head (or a\ntag).\n\nI think the exec_rev_list logic should be taught to first\ninclude all the positive refs (i.e. the ones we are going to\nsend), and then as many the negative refs (i.e. the ones we know\nthey have), from newer to older, as they fit without triggering\n\"argument list too long\".\n\nAnother, probably conceptually cleaner alternative might be to\nallow rev-list to read from its stdin so that we do not have to\nworry about the argument list issues.\n"},{"id":"16558","messageId":"7virr8t82n.fsf@assigned-by-dhcp.cox.net","threadId":"3420","inReplyTo":"7vwtfotaq3.fsf@assigned-by-dhcp.cox.net","subject":"Re: Git cannot push to repository with too many tags/heads","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T02:56:32Z","receivedAt":"2006-02-22T02:56:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Stephen C. Tweedie\" writes:\n>\n>> send_pack()\n>> then skips all other refs by doing a\n>>\n>> \t\tif (!ref->peer_ref)\n>> \t\t\tcontinue;\n>>\n>> Unfortunately, exec_rev_list() is missing this, and it tries to ask\n>> git-rev-list for the commit objects of *every* ref on the remote_refs\n>> list, even if they have been explicitly excluded by match_refs() and\n>> have no peer_ref. So with this huge repository, I can't even push a\n>> single refspec without bumping into the limit of 900 refs.\n>\n> IIRC, the distinction was deliberate. send_pack() excludes\n> what did not match because it does not want to send them.\n> rev_list() adds what we know they have to \"do not bother to\n> send\" list to make the resulting pack smaller. The time where\n> it matters most is when you are pushing a new branch head (or a\n> tag).\n>\n> I think the exec_rev_list logic should be taught to first\n> include all the positive refs (i.e. the ones we are going to\n> send), and then as many the negative refs (i.e. the ones we know\n> they have), from newer to older, as they fit without triggering\n> \"argument list too long\".\n\nThat is, something like this.\n-- >8 --\nDo not give up running rev-list when remote has insanely large number of refs.\n\n---\ncd /opt/packrat/playpen/public/in-place/git/git.junio/\ngit diff\ndiff --git a/send-pack.c b/send-pack.c\nindex 990be3f..cfd0eeb 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -37,24 +37,27 @@ static void exec_pack_objects(void)\n \n static void exec_rev_list(struct ref *refs)\n {\n+\tstruct ref *ref;\n \tstatic char *args[1000];\n \tint i = 0;\n \n \targs[i++] = \"rev-list\";\t/* 0 */\n \targs[i++] = \"--objects\";\t/* 1 */\n-\twhile (refs) {\n-\t\tchar *buf = malloc(100);\n+\tfor (ref = refs; refs; refs = refs->next) {\n+\t\tchar *buf = malloc(41);\n \t\tif (i > 900)\n \t\t\tdie(\"git-rev-list environment overflow\");\n-\t\tif (!is_zero_sha1(refs->old_sha1) &&\n-\t\t has_sha1_file(refs->old_sha1)) {\n+\t\tif (!is_zero_sha1(refs->new_sha1)) {\n \t\t\targs[i++] = buf;\n-\t\t\tsnprintf(buf, 50, \"^%s\", sha1_to_hex(refs->old_sha1));\n-\t\t\tbuf += 50;\n+\t\t\tsnprintf(buf, 41, \"%s\", sha1_to_hex(refs->new_sha1));\n \t\t}\n-\t\tif (!is_zero_sha1(refs->new_sha1)) {\n+\t}\n+\tfor (ref = refs; i < 900 && refs; refs = refs->next) {\n+\t\tchar *buf = malloc(42);\n+\t\tif (!is_zero_sha1(refs->old_sha1) &&\n+\t\t has_sha1_file(refs->old_sha1)) {\n \t\t\targs[i++] = buf;\n-\t\t\tsnprintf(buf, 50, \"%s\", sha1_to_hex(refs->new_sha1));\n+\t\t\tsnprintf(buf, 42, \"^%s\", sha1_to_hex(refs->old_sha1));\n \t\t}\n \t\trefs = refs->next;\n \t}\n\nCompilation finished at Tue Feb 21 18:52:49\n"},{"id":"16567","messageId":"7v1wxvsovj.fsf_-_@assigned-by-dhcp.cox.net","threadId":"3420","inReplyTo":"7virr8t82n.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] send-pack: do not give up when remote has insanely large number of refs.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T09:51:12Z","receivedAt":"2006-02-22T09:51:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen C. Tweedie noticed that we give up running rev-list when\nwe see too many refs on the remote side. Limit the number of\nnegative references we give to rev-list and continue.\n\nNot sending any negative references to rev-list is very bad --\nwe may be pushing a ref that is new to the other end.\n\nSigned-off-by: Junio C Hamano \n\n---\n\n Junio C Hamano writes:\n\n > That is, something like this.\n > -- >8 --\n > Do not give up...\n\n Ah, crap. Sorry about that. This one has been tested at least.\n\n send-pack.c | 38 ++++++++++++++++++++++++++++----------\n 1 files changed, 28 insertions(+), 10 deletions(-)\n\n797656e58ddbd82ac461a5142ed726db3a4d0ac0\ndiff --git a/send-pack.c b/send-pack.c\nindex 990be3f..b58bbab 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -37,26 +37,44 @@ static void exec_pack_objects(void)\n \n static void exec_rev_list(struct ref *refs)\n {\n+\tstruct ref *ref;\n \tstatic char *args[1000];\n-\tint i = 0;\n+\tint i = 0, j;\n \n \targs[i++] = \"rev-list\";\t/* 0 */\n \targs[i++] = \"--objects\";\t/* 1 */\n-\twhile (refs) {\n-\t\tchar *buf = malloc(100);\n-\t\tif (i > 900)\n+\n+\t/* First send the ones we care about most */\n+\tfor (ref = refs; ref; ref = ref->next) {\n+\t\tif (900 < i)\n \t\t\tdie(\"git-rev-list environment overflow\");\n-\t\tif (!is_zero_sha1(refs->old_sha1) &&\n-\t\t has_sha1_file(refs->old_sha1)) {\n+\t\tif (!is_zero_sha1(ref->new_sha1)) {\n+\t\t\tchar *buf = malloc(100);\n \t\t\targs[i++] = buf;\n-\t\t\tsnprintf(buf, 50, \"^%s\", sha1_to_hex(refs->old_sha1));\n+\t\t\tsnprintf(buf, 50, \"%s\", sha1_to_hex(ref->new_sha1));\n \t\t\tbuf += 50;\n+\t\t\tif (!is_zero_sha1(ref->old_sha1) &&\n+\t\t\t has_sha1_file(ref->old_sha1)) {\n+\t\t\t\targs[i++] = buf;\n+\t\t\t\tsnprintf(buf, 50, \"^%s\",\n+\t\t\t\t\t sha1_to_hex(ref->old_sha1));\n+\t\t\t}\n \t\t}\n-\t\tif (!is_zero_sha1(refs->new_sha1)) {\n+\t}\n+\n+\t/* Then a handful of the remainder\n+\t * NEEDSWORK: we would be better off if used the newer ones first.\n+\t */\n+\tfor (ref = refs, j = i + 16;\n+\t i < 900 && i < j && ref;\n+\t ref = ref->next) {\n+\t\tif (is_zero_sha1(ref->new_sha1) &&\n+\t\t !is_zero_sha1(ref->old_sha1) &&\n+\t\t has_sha1_file(ref->old_sha1)) {\n+\t\t\tchar *buf = malloc(42);\n \t\t\targs[i++] = buf;\n-\t\t\tsnprintf(buf, 50, \"%s\", sha1_to_hex(refs->new_sha1));\n+\t\t\tsnprintf(buf, 42, \"^%s\", sha1_to_hex(ref->old_sha1));\n \t\t}\n-\t\trefs = refs->next;\n \t}\n \targs[i] = NULL;\n \texecv_git_cmd(args);\n-- \n1.2.2.g882e\n"},{"id":"16576","messageId":"1140633034.3385.5.camel@orbit.scot.redhat.com","threadId":"3420","inReplyTo":"7v1wxvsovj.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] send-pack: do not give up when remote has insanely large number of refs.","fromName":"Stephen C. Tweedie","fromEmail":"sct@redhat.com","sentAt":"2006-02-22T18:30:33Z","receivedAt":"2006-02-22T18:30:33Z","isPatch":true,"sender":{"key":"sct@redhat.com","avatar":null},"body":"Hi,\n\nOn Wed, 2006-02-22 at 01:51 -0800, Junio C Hamano wrote:\n\n> +\tfor (ref = refs, j = i + 16;\n> +\t i < 900 && i < j && ref;\n\nLooks like it's now sending just 16 additional negative refs instead of\n940 for this repo. Definitely an improvement --- push (both full and\nwith an explicit refspec) is now working properly, thanks! \n\nAdding more ^refs up to the limit of 900 should be possible, too, and\nshould catch more already-present objects --- while the refs count for\nthis repo was under 900, push still worked fine for me, so we don't\nnecessarily have to cut it hard to as low a number as 16.\n\nPerhaps ultimately we may want to simply send the refs list to\ngit-rev-list via a pipe or similar if we want this to scale? We'll need\nthis for edge cases such as sending >900 new tags to an old repository\nat once, as we'll exhaust the size of the positive refs list in that\ncase.\n\nThanks again,\n Stephen\n"},{"id":"16577","messageId":"7v3bibnsk9.fsf@assigned-by-dhcp.cox.net","threadId":"3420","inReplyTo":"1140633034.3385.5.camel@orbit.scot.redhat.com","subject":"Re: [PATCH] send-pack: do not give up when remote has insanely large number of refs.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-22T18:42:46Z","receivedAt":"2006-02-22T18:42:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Stephen C. Tweedie\" writes:\n\n> Adding more ^refs up to the limit of 900 should be possible, too, and\n> should catch more already-present objects --- while the refs count for\n> this repo was under 900, push still worked fine for me, so we don't\n> necessarily have to cut it hard to as low a number as 16.\n\nActually it was done on purpose. My first round did not have\nthat 16 limit, and then it turned out that rev-list, given extra\nwork to cull those 900 negative refs, were spending a lot of\ntime. I created a repository with 1500 tags for this test ;-).\n\nUnless we were talking about a repository that happens to host\n1000 unrelated projects, each tracked with separate sets of\nheads and tags, those negative refs should be related with each\nother in the ancestry graph. We should be able to eliminate\nmost of them by using the latest handful.\n\n> Perhaps ultimately we may want to simply send the refs list to\n> git-rev-list via a pipe or similar if we want this to scale? We'll need\n> this for edge cases such as sending >900 new tags to an old repository\n> at once, as we'll exhaust the size of the positive refs list in that\n> case.\n\nTrue. We should support unbounded number of positive refs.\n"}]}