Re: [PATCHv3 3/9] pack-objects: Allow --max-pack-size to be used together with --stdout

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

Johan Herland <johan@herland.net> writes:

> Currently we refuse combining --max-pack-size with --stdout since there's
> no way to make multiple packs when the pack is written to stdout. However,
> we want to be able to limit the maximum size of the pack created by
> --stdout (and abort pack-objects if we are unable to meet that limit).
>
> Therefore, when used together with --stdout, we reinterpret --max-pack-size
> to indicate the maximum pack size which - if exceeded - will cause
> pack-objects to abort with an error message.

I only gave the code a cursory look, but I think your patch does more than
the above paragraphs say. I am not sure those extra change are justified.

For example,

> @@ -229,7 +229,7 @@ static unsigned long write_object(struct sha1file *f,
>  
>  	if (!entry->delta)
>  		usable_delta = 0;	/* no delta */
> -	else if (!pack_size_limit)
> +	else if (!pack_size_limit || pack_to_stdout)
>  	       usable_delta = 1;	/* unlimited packfile */

Why does this conditional have to change its behaviour when writing to the
standard output?  I thought that the only thing you are doing "earlier we
didn't allow setting size limit when writing to standard output, now we
do", and I do not see the linkage between that objective and this change.

> @@ -2315,9 +2318,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>  
>  	if (!pack_to_stdout && !pack_size_limit)
>  		pack_size_limit = pack_size_limit_cfg;
> -	if (pack_to_stdout && pack_size_limit)
> -		die("--max-pack-size cannot be used to build a pack for transfer.");
> -	if (pack_size_limit && pack_size_limit < 1024*1024) {
> +	if (!pack_to_stdout && pack_size_limit && pack_size_limit < 1024*1024) {
>  		warning("minimum pack size limit is 1 MiB");
>  		pack_size_limit = 1024*1024;
>  	}

Why is the new combination "writing to the standard output, but the
maximum size is limited" does not have the same lower bound to pack size
limit while on-disk packs do?

If you have a reason to believe 1 MiB is too large for a pack size limit,
shouldn't that logic apply equally to the on-disk case?  What does this
change have to do with the interaction with --stdout option?