Ivan Todoroski <grnch@xxxxxxx> writes:
> 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.
> 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.
> 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.
> 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
*/
> + 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?
> + 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.
> + }
> + }
> + 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).
> + 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,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html