Johan Herland <johan@xxxxxxxxxxx> 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?
--
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