git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC v2 1/4] fetch-pack: new --stdin option to read refs from stdin

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 27, 2012, 16:59 UTC
Message-ID
<7vhaxaj7yi.fsf@alter.siamese.dyndns.org>
In-Reply-To
<4F715D55.8020109@gmx.net>
Ivan Todoroski <grnch@gmx.net> writes:
Show 5 quoted lines
> sha=$(git rev-parse HEAD)
> for ((i=0; i<50000; i++)); do
> 	echo $sha refs/tags/artificially-long-tag-name-to-more-easily-\
> demonstrate-the-problem-$i >> .git/packed-refs
> done

This comment does not have much to the issue, but the above is better written as

	commit=$(git rev-parse HEAD) i=0
        while test $i -le 50000
        do
        	echo $commit refs/tags/....-$i
	done >>.git/packed.refs
to stay within the usual Bourne shell.
Show 13 quoted lines
> 4) Add option --stdin to pass the refs on stdin, one per line.
> ...
> In the end we settled on the following solution:
>
> If --stdin is specified without --stateless-rpc, fetch-pack would read
> the refs from stdin one per line, in a script friendly format.
>
> However if --stdin is specified together with --stateless-rpc,
> fetch-pack would read the refs from stdin in packetized format
> (pkt-line) with a flush packet terminating the list of refs. This way we
> can read the exact number of bytes that we need from stdin, and then
> get_remote_heads() can continue reading from the same fd without losing
> a single byte of remote protocol data.

That sounds like the right way to use pkt-line machinery. Send stuff you need to send, and mark it with a flush to tell the receiver that you have finished giving one logical collection of information.

> This way the --stdin option only loses generality and scriptability when
> used together with --stateless-rpc, which is not easily scriptable
> anyway because it also uses pkt-line when talking to the remote server.
> ---
Just a gentle reminder; the final submission will need your sign off here.
Show 18 quoted lines
> diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
> index ed1bdaacd1..1dd44fd348 100644
> --- a/Documentation/git-fetch-pack.txt
> +++ b/Documentation/git-fetch-pack.txt
> @@ -32,6 +32,16 @@ OPTIONS
>  --all::
>  	Fetch all remote refs.
>  
> +--stdin::
> +	Take the list of refs from stdin, one per line. If there
> +	are refs specified on the command line in addition to this
> +	option, then the refs from stdin are processed after those
> +	on the command line.
> ++
> +If '--stateless-rpc' is specified together with this option then
> +the list of refs must be in packet format (pkt-line) with a flush
> +packet terminating the list.
> +

It is not clear from this description alone if this is a single (possibly giant) packet with multiple lines, each of which describes a ref, or a series of one packet per ref with a flush at the end of the sequence.

Show 11 quoted lines
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index a4d3e90a86..1a90fa852f 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -972,6 +978,43 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>  	if (!dest)
>  		usage(fetch_pack_usage);
>  
> +	if (args.refs_from_stdin) {
> +		/* copy refs from cmdline to new growable list,
> +		   then append the refs from stdin */
        /*
         * We tend to format our multi-line
         * comments like this
         */
Show 7 quoted lines
> +		int alloc_heads = nr_heads;
> +		int size = nr_heads * sizeof(*heads);
> +		heads = memcpy(xmalloc(size), heads, size);
> +		if (args.stateless_rpc) {
> +			/* in stateless RPC mode we use pkt-line to read
> +			   from stdin, until we get a flush packet */
> +			static char line[1000];
We will never have a refname that is longer than this limit?
Show 8 quoted lines
> +			for (;;) {
> +				int n = packet_read_line(0, line, sizeof(line));
> +				if (!n)
> +					break;
> +				if (line[n-1] == '\n')
> +					line[--n] = '\0';
> +				ALLOC_GROW(heads, nr_heads + 1, alloc_heads);
> +				heads[nr_heads++] = xmemdupz(line, n);

Micronit. The use of xmemdupz() here means you do not have to replace LF with NUL above; decrementing 'n' should be sufficient.

Show 12 quoted lines
> +			}
> +		}
> +		else {
> +			/* read from stdin one ref per line, until EOF */
> +			struct strbuf line;
> +			strbuf_init(&line, 0);
> +			for (;;) {
> +				if (strbuf_getline(&line, stdin, '\n') == EOF)
> +					break;
> +				strbuf_trim(&line);
> +				if (!line.len)
> +					continue; /* skip empty lines */

Curious. "stop at EOF", "trim" and "skip empty" imply that you are catering to people who debug this from the terminal by typing (or copy pasting). Is that the expected use case?

Otherwise we may want to tighten this part a bit to forbid cruft (e.g. a line with leading or trailing whitespaces).

Show 24 quoted lines
> +				ALLOC_GROW(heads, nr_heads + 1, alloc_heads);
> +				heads[nr_heads++] = strbuf_detach(&line, NULL);
> +			}
> +			strbuf_release(&line);
> +		}
> +	}
> +
>  	if (args.stateless_rpc) {
>  		conn = NULL;
>  		fd[0] = 0;
> diff --git a/fetch-pack.h b/fetch-pack.h
> index 0608edae3f..292d69389e 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -13,7 +13,8 @@ struct fetch_pack_args {
>  		verbose:1,
>  		no_progress:1,
>  		include_tag:1,
> -		stateless_rpc:1;
> +		stateless_rpc:1,
> +		refs_from_stdin:1;
>  };
>  
>  struct ref *fetch_pack(struct fetch_pack_args *args,
Previous: Ivan TodoroskiNext: Ivan Todoroski
Message 41 of 54 in “Clone fails on a repo with too many heads/tags”
  1. Ivan TodoroskiMar 18, 2012
  2. Ivan TodoroskiMar 18, 2012
  3. Nguyen Thai Ngoc DuyMar 18, 2012
  4. Jakub NarebskiMar 18, 2012
  5. Jeff KingMar 18, 2012
  6. Jakub NarebskiMar 18, 2012
  7. Jeff KingMar 19, 2012
  8. Nguyen Thai Ngoc DuyMar 19, 2012
  9. Jeff KingMar 19, 2012
  10. Ivan TodoroskiMar 19, 2012
  11. Nguyen Thai Ngoc DuyMar 19, 2012
  12. Jeff KingMar 19, 2012
  13. Ivan TodoroskiMar 21, 2012
  14. Shawn PearceMar 21, 2012
  15. Jeff KingMar 21, 2012
  16. Jakub NarebskiMar 21, 2012
  17. Ivan TodoroskiMar 21, 2012
  18. Jeff KingMar 21, 2012
  19. Ivan TodoroskiMar 24, 2012
  20. Jeff KingMar 25, 2012
  21. Jeff KingMar 25, 2012
  22. Ivan TodoroskiMar 25, 2012
  23. Ivan TodoroskiMar 25, 2012
  24. Jeff KingMar 26, 2012
  25. Ivan TodoroskiMar 27, 2012
  26. Ivan TodoroskiMar 25, 2012
  27. 1/2 fetch-pack: new option to read refs from stdinIvan Todoroski, Mar 24, 2012
  28. Jeff KingMar 25, 2012
  29. Ivan TodoroskiMar 25, 2012
  30. Ivan TodoroskiMar 25, 2012
  31. Ivan TodoroskiMar 25, 2012
  32. Jeff KingMar 26, 2012
  33. Ivan TodoroskiMar 26, 2012
  34. Jeff KingMar 26, 2012
  35. 2/2 remote-curl: send the refs to fetch-pack on stdinIvan Todoroski, Mar 24, 2012
  36. Jeff KingMar 25, 2012
  37. Ivan TodoroskiMar 25, 2012
  38. Jeff KingMar 26, 2012
  39. 0/4 Fix fetch-pack command line overflow during cloneIvan Todoroski, Mar 27, 2012
  40. 1/4 fetch-pack: new --stdin option to read refs from stdinIvan Todoroski, Mar 27, 2012
  41. Junio C HamanoMar 27, 2012
  42. Ivan TodoroskiMar 27, 2012
  43. Junio C HamanoMar 27, 2012
  44. Ivan TodoroskiMar 27, 2012
  45. 2/4 remote-curl: send the refs to fetch-pack on stdinIvan Todoroski, Mar 27, 2012
  46. Junio C HamanoMar 27, 2012
  47. Ivan TodoroskiMar 27, 2012
  48. 3/4 fetch-pack: test cases for the new --stdin optionIvan Todoroski, Mar 27, 2012
  49. Junio C HamanoMar 27, 2012
  50. Ivan TodoroskiMar 27, 2012
  51. Junio C HamanoMar 27, 2012
  52. Ivan TodoroskiMar 28, 2012
  53. 4/4 remote-curl: main test case for the OS command line overflowIvan Todoroski, Mar 27, 2012
  54. Junio C HamanoMar 27, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.