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

From: Ivan Todoroski <grnch@gmx.net>
Date: Mar 27, 2012, 23:18 UTC
Message-ID: <4F724ADC.2030909@gmx.net>
In-Reply-To: <7vhaxaj7yi.fsf@alter.siamese.dyndns.org>

On 27.03.2012 18:59, Junio C Hamano wrote:

Show 11 quoted lines

> Ivan Todoroski <grnch@gmx.net> writes:
> 
>> +		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?

I don't know. I grepped the code for existing usages of packet_read_line and that seemed to be a common idiom everywhere. Should I just bump up the size or is there some accepted way to read arbitrary length packets?

Show 16 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?

The expected use case is people using this from shell scripts that could be getting refs by slicing and dicing output of other commands with regexps and what not which could leave some whitespace here and there, so a more liberal interface might be more friendly to such script writers.

Currently you would pass a list of generated refs to fetch-pack using something like this:

     generate-refs | xargs fetch-pack

or this:

     fetch-pack $(generate-refs)

Both of these commands will ignore any extra whitespace produced by "generate-refs".

Since --stdin is meant to be a spiritual replacement for the two commands above, I thought it should behave in a similar spirit.

At least that was the reasoning... if you're not swayed by it I can just remove those lines and not tolerate any extra whitespace.