Re: [PATCH/RFC v2 2/4] remote-curl: send the refs to fetch-pack on stdin

[Junio C Hamano <gitster@pobox.com>]

Ivan Todoroski <grnch@gmx.net> writes:

> If a remote repo has too many tags (or branches), cloning it over the
> smart HTTP transport can fail because remote-curl.c puts all the refs
> from the remote repo on the fetch-pack command line. This can make the
> command line longer than the global OS command line limit, causing
> fetch-pack to fail.
>
> This is especially a problem on Windows where the command line limit is
> orders of magnitude shorter than Linux. There are already real repos out
> there that msysGit cannot clone over smart HTTP due to this problem.
>
> To solve this problem we teach remote-curl.c to pipe the refs to
> fetch-pack using the new --stdin option, instead of on the fetch-pack
> command line.
>
> For a more detailed discussion of the problem see the parent of this
> commit, titled "fetch-pack: new --stdin option to read refs from stdin".

Thanks.  As there is no way for this single commit to be applied without
the previous step, you could have just said:

	Now we can throw arbitrary number of refs at fetch-pack using
        its --stdin option, use it in remote-curl helper to lift the
        command line length limit.

or something ;-).

> @@ -626,6 +630,7 @@ static int fetch_git(struct discovery *heads,
>  	int nr_heads, struct ref **to_fetch)
>  {
>  	struct rpc_state rpc;
> +	struct strbuf preamble;
>  	char *depth_arg = NULL;
>  	const char **argv;
>  	int argc = 0, i, err;
> @@ -633,6 +638,7 @@ static int fetch_git(struct discovery *heads,
>  	argv = xmalloc((15 + nr_heads) * sizeof(char*));
>  	argv[argc++] = "fetch-pack";
>  	argv[argc++] = "--stateless-rpc";
> +	argv[argc++] = "--stdin";
>  	argv[argc++] = "--lock-pack";
>  	if (options.followtags)
>  		argv[argc++] = "--include-tag";
> @@ -651,23 +657,28 @@ static int fetch_git(struct discovery *heads,
>  		argv[argc++] = depth_arg;
>  	}
>  	argv[argc++] = url;
> +	argv[argc++] = NULL;
> +
> +	strbuf_init(&preamble, 4);

Curious.

If "4" does not really matter, I would drop this and use STRBUF_INIT at
the beginning instead.

>  	for (i = 0; i < nr_heads; i++) {
>  		struct ref *ref = to_fetch[i];
>  		if (!ref->name || !*ref->name)
>  			die("cannot fetch by sha1 over smart http");
> -		argv[argc++] = ref->name;
> +		packet_buf_write(&preamble, "%s\n", ref->name);
>  	}
> -	argv[argc++] = NULL;
> +	packet_buf_flush(&preamble);
>  
>  	memset(&rpc, 0, sizeof(rpc));
>  	rpc.service_name = "git-upload-pack",
>  	rpc.argv = argv;
> +	rpc.stdin_preamble = &preamble;
>  	rpc.gzip_request = 1;
>  
>  	err = rpc_service(&rpc, heads);
>  	if (rpc.result.len)
>  		safe_write(1, rpc.result.buf, rpc.result.len);
>  	strbuf_release(&rpc.result);
> +	strbuf_release(&preamble);
>  	free(argv);
>  	free(depth_arg);
>  	return err;