clone: I'm only doing a max of 256 requests

clone: I'm only doing a max of 256 requests

[Andy Isaacson]

Trying to do a local clone of the linux-mips.org git repo:

% git clone /home/adi/linux/git/lmo/linux foo
defaulting to local storage area
fatal: I'm only doing a max of 256 requests
% git -v
git version 0.99.8.GIT

I got git/lmo/linux from http://www.linux-mips.org/pub/scm/linux.git.

Am I doing something wrong, or what?  (And how should I be starting to
debug this?  The git programs don't seem to have a useful --verbose
option.  It would be nice if "git -v clone" would tell me what it is
doing.)

-andy

Re: clone: I'm only doing a max of 256 requests

[Junio C Hamano]

Andy Isaacson <adi@hexapodia.org> writes:

> ...  (And how should I be starting to
> debug this?  The git programs don't seem to have a useful --verbose
> option.  It would be nice if "git -v clone" would tell me what it is
> doing.)

$ git grep -n 'max of .* requests'
upload-pack.c:141:			die("I'm only doing a max of %d requests", MAX_NEEDS);

I suspect that the repository you are cloning has too many
branch heads and tags under .git/refs/.

Re: clone: I'm only doing a max of 256 requests

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Andy Isaacson <adi@hexapodia.org> writes:
>
>> ...  (And how should I be starting to
>> debug this?  The git programs don't seem to have a useful --verbose
>> option.  It would be nice if "git -v clone" would tell me what it is
>> doing.)
>
> $ git grep -n 'max of .* requests'
> upload-pack.c:141:			die("I'm only doing a max of %d requests", MAX_NEEDS);
>
> I suspect that the repository you are cloning has too many
> branch heads and tags under .git/refs/.

We can do three things, the first two being short term, the last
one a bit longer term.

1. As a stop gap measure, so that your Linux kernel work can
   continue, please bump MAX_NEEDS definition in upload-pack.c
   from 256 to a bit higher.  That controls the number of
   40-letter SHA1 given to underlying rev-list via execvp(), so
   it cannot be _too_ big like 1M, lest it exceeds the exec
   argument buffer limit.

2. We can add '--all' flag to git-rev-list, and have upload-pack
   use it instead, when it sees more than MAX_NEEDS refs.  I
   have a patch to do this that I am currently testing.

3. In addition, upload-pack should probably be taught to detect
   "I do not have anything.  Please give me objects reachable
   from all your refs" requests, and cache the resulting pack
   somewhere (invalidate whenever any ref changes), so that next
   'clone' request can just send it out instead of rerunning
   rev-list and pack-objects.

Re: clone: I'm only doing a max of 256 requests

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> 1. As a stop gap measure, so that your Linux kernel work can
>    continue, please bump MAX_NEEDS definition in upload-pack.c
>    from 256 to a bit higher.  That controls the number of
>    40-letter SHA1 given to underlying rev-list via execvp(), so
>    it cannot be _too_ big like 1M, lest it exceeds the exec
>    argument buffer limit.

Hmph.  I was reading linux-2.6/fs/exec.c::copy_strings(), but I
do not see any such size limit (other than exceeding the total
machine memory size, probably reported by alloc_page() failing)
imposed there.  Am I looking at the wrong place?

[PATCH] upload-pack: Do not choke on too many heads request.

[Junio C Hamano]

Cloning from a repository with more than 256 refs (heads and tags
included) will choke, because upload-pack has a built-in limit of
feeding not more than MAX_NEEDS (currently 256) heads to underlying
git-rev-list.  This is a problem when cloning a repository with many
tags, like http://www.linux-mips.org/pub/scm/linux.git, which has 290+
tags.

This commit introduces a new flag, --all, to git-rev-list, to include
all refs in the repository.  Updated upload-pack detects requests that
ask more than MAX_NEEDS refs, and sends everything back instead.

We may probably want to tweak the definitions of MAX_NEEDS and
MAX_HAS, but that is a separate topic.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

  Andy Isaacson <adi@hexapodia.org> writes:

  > Trying to do a local clone of the linux-mips.org git repo:
  >
  > % git clone /home/adi/linux/git/lmo/linux foo
  > defaulting to local storage area
  > fatal: I'm only doing a max of 256 requests
  > % git -v
  > git version 0.99.8.GIT
  >
  > I got git/lmo/linux from http://www.linux-mips.org/pub/scm/linux.git.
  >
  > Am I doing something wrong, or what?

  You are not doing anything wrong.  Please try this patch.

 rev-list.c    |   21 +++++++++++++++++++++
 rev-parse.c   |    1 +
 upload-pack.c |   50 ++++++++++++++++++++++++++++++++++----------------
 3 files changed, 56 insertions(+), 16 deletions(-)

applies-to: dc721a63b8221995616e3013de11e71d94da01ef
e091eb93258f05a58bc5d1c60f058f5f57dd92b6
diff --git a/rev-list.c b/rev-list.c
--- a/rev-list.c
+++ b/rev-list.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "refs.h"
 #include "tag.h"
 #include "commit.h"
 #include "tree.h"
@@ -489,6 +490,22 @@ static void handle_one_commit(struct com
 	commit_list_insert(com, lst);
 }
 
+/* for_each_ref() callback does not allow user data -- Yuck. */
+static struct commit_list **global_lst;
+
+static int include_one_commit(const char *path, const unsigned char *sha1)
+{
+	struct commit *com = get_commit_reference(path, 0);
+	handle_one_commit(com, global_lst);
+	return 0;
+}
+
+static void handle_all(struct commit_list **lst)
+{
+	global_lst = lst;
+	for_each_ref(include_one_commit);
+	global_lst = NULL;
+}
 
 int main(int argc, char **argv)
 {
@@ -542,6 +559,10 @@ int main(int argc, char **argv)
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--all")) {
+			handle_all(&list);
+			continue;
+		}
 		if (!strcmp(arg, "--objects")) {
 			tag_objects = 1;
 			tree_objects = 1;
diff --git a/rev-parse.c b/rev-parse.c
--- a/rev-parse.c
+++ b/rev-parse.c
@@ -32,6 +32,7 @@ static int revs_count = 0;
 static int is_rev_argument(const char *arg)
 {
 	static const char *rev_args[] = {
+		"--all",
 		"--bisect",
 		"--header",
 		"--max-age=",
diff --git a/upload-pack.c b/upload-pack.c
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -30,10 +30,18 @@ static void create_pack_file(void)
 
 	if (!pid) {
 		int i;
-		int args = nr_has + nr_needs + 5;
-		char **argv = xmalloc(args * sizeof(char *));
-		char *buf = xmalloc(args * 45);
-		char **p = argv;
+		int args;
+		char **argv;
+		char *buf;
+		char **p;
+
+		if (MAX_NEEDS <= nr_needs)
+			args = nr_has + 10;
+		else
+			args = nr_has + nr_needs + 5;
+		argv = xmalloc(args * sizeof(char *));
+		buf = xmalloc(args * 45);
+		p = argv;
 
 		dup2(fd[1], 1);
 		close(0);
@@ -41,10 +49,14 @@ static void create_pack_file(void)
 		close(fd[1]);
 		*p++ = "git-rev-list";
 		*p++ = "--objects";
-		for (i = 0; i < nr_needs; i++) {
-			*p++ = buf;
-			memcpy(buf, sha1_to_hex(needs_sha1[i]), 41);
-			buf += 41;
+		if (MAX_NEEDS <= nr_needs)
+			*p++ = "--all";
+		else {
+			for (i = 0; i < nr_needs; i++) {
+				*p++ = buf;
+				memcpy(buf, sha1_to_hex(needs_sha1[i]), 41);
+				buf += 41;
+			}
 		}
 		for (i = 0; i < nr_has; i++) {
 			*p++ = buf;
@@ -129,18 +141,24 @@ static int receive_needs(void)
 
 	needs = 0;
 	for (;;) {
+		unsigned char dummy[20], *sha1_buf;
 		len = packet_read_line(0, line, sizeof(line));
 		if (!len)
 			return needs;
 
-		/*
-		 * This is purely theoretical right now: git-fetch-pack only
-		 * ever asks for a single HEAD
-		 */
-		if (needs >= MAX_NEEDS)
-			die("I'm only doing a max of %d requests", MAX_NEEDS);
-		if (strncmp("want ", line, 5) || get_sha1_hex(line+5, needs_sha1[needs]))
-			die("git-upload-pack: protocol error, expected to get sha, not '%s'", line);
+		sha1_buf = dummy;
+		if (needs == MAX_NEEDS) {
+			fprintf(stderr,
+				"warning: supporting only a max of %d requests. "
+				"sending everything instead.\n",
+				MAX_NEEDS);
+		}
+		else if (needs < MAX_NEEDS)
+			sha1_buf = needs_sha1[needs];
+
+		if (strncmp("want ", line, 5) || get_sha1_hex(line+5, sha1_buf))
+			die("git-upload-pack: protocol error, "
+			    "expected to get sha, not '%s'", line);
 		needs++;
 	}
 }
---
0.99.8.GIT

Re: clone: I'm only doing a max of 256 requests

[Vincent Hanquez]

On Wed, Oct 05, 2005 at 02:38:42PM -0700, Junio C Hamano wrote:
> Hmph.  I was reading linux-2.6/fs/exec.c::copy_strings(), but I
> do not see any such size limit (other than exceeding the total
> machine memory size, probably reported by alloc_page() failing)
> imposed there.  Am I looking at the wrong place?

well at least the len of argv is limited by ~32K (i386) by: 

bprm->p = PAGE_SIZE*MAX_ARG_PAGES-sizeof(void *);
...
bprm->argc = count(argv, bprm->p / sizeof(void *));

-- 
Vincent Hanquez

Re: clone: I'm only doing a max of 256 requests

[Linus Torvalds]

On Wed, 5 Oct 2005, Andy Isaacson wrote:
>
> Trying to do a local clone of the linux-mips.org git repo:
> 
> % git clone /home/adi/linux/git/lmo/linux foo
> defaulting to local storage area
> fatal: I'm only doing a max of 256 requests

The pack upload has a totally arbitrary limit of 256 heads.

> I got git/lmo/linux from http://www.linux-mips.org/pub/scm/linux.git.

Heh. And:

	git ls-remote http://www.linux-mips.org/pub/scm/linux.git | wc -l

returns "295". Seems to have all the old bk history in it.

> Am I doing something wrong, or what?

No, just change the "MAX_NEEDS" define from 256 to some larger value.

There's no real reason for the limit, except that maybe we should have 
some dynamic allocation for this.

		Linus

Re: clone: I'm only doing a max of 256 requests

[Linus Torvalds]

On Wed, 5 Oct 2005, Junio C Hamano wrote:
> 
> Hmph.  I was reading linux-2.6/fs/exec.c::copy_strings(), but I
> do not see any such size limit (other than exceeding the total
> machine memory size, probably reported by alloc_page() failing)
> imposed there.  Am I looking at the wrong place?

Look for "MAX_ARG_PAGES".

Ie the limit is about 128kB by default (32 pages). Note that that includes 
not just arguments, but environment.

		Linus

Re: clone: I'm only doing a max of 256 requests

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> No, just change the "MAX_NEEDS" define from 256 to some larger value.
>
> There's no real reason for the limit, except that maybe we should have 
> some dynamic allocation for this.

If somebody is asking for more than say 20 refs, even if the
repository is mature and has 1000 point releases tagged, it
might not make that much of a difference if we ship everything
back instead of being selective, especially when the downloader
said "I do not have anything", i.e. initial cloning.

So after the 'rev-list --all' patch, I was actually going to
suggest reducing MAX_NEEDS, to say 47 (another arbitrary
number), and maybe making MAX_HAS side dynamic to hold more refs
for the stop list.

Also it may be worthwhile to teach upload-pack.c::got_sha1() to
notice when the other side says he has one object and we know
that object is reachable from another object he already said he
has, and choose not to use the older object on the has_sha1[]
list.  The "have" list from fetch-pack tends to come from newer
to older, so this would save has_sha1[] array entries from being
consumed by older commits when we know about the commits he has
near the tip of the same branch.

Re: clone: I'm only doing a max of 256 requests

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On 10/6/05, Linus Torvalds <torvalds@osdl.org> wrote:
>
>         git ls-remote http://www.linux-mips.org/pub/scm/linux.git | wc -l
>

Which, btw, failed for me, when I tried to run it home (which has no
.git in it yet).
Do the scripts git-ls-remote.sh and git-parse-remote.sh really need .git/...?

Just in case they don't, the attached patch removes the die("Not a git archive")

[-- Attachment #2: git-ls-remote.patch --]
[-- Type: application/xxxxx, Size: 599 bytes --]

Re: clone: I'm only doing a max of 256 requests

[Linus Torvalds]

On Thu, 6 Oct 2005, Alex Riesen wrote:
> On 10/6/05, Linus Torvalds <torvalds@osdl.org> wrote:
> >
> >         git ls-remote http://www.linux-mips.org/pub/scm/linux.git | wc -l
> >
> 
> Which, btw, failed for me, when I tried to run it home (which has no 
> .git in it yet). Do the scripts git-ls-remote.sh and git-parse-remote.sh 
> really need .git/...?

Good point. No they don't. You should be able to run "git ls-remote" 
outside of a local git directory.

> Just in case they don't, the attached patch removes the die("Not a git archive")

Junio, please apply.

(It still wants the "git-sh-setup" part if only because it uses "die()" in 
another place).

		Linus

Re: clone: I'm only doing a max of 256 requests

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Junio, please apply.

I've been considering about this myself for quite a while, but
haven't done so only because I suspected the removal of the
checks are probably not good enough.  I'll apply the patch, and
we will see what happens.