Re: [PATCHv3 3/9] pack-objects: Allow --max-pack-size to be used together with --stdout
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:15
Johan Herland [off-list ref] 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,
quoted hunk
@@ -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.
quoted hunk
@@ -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?