Re: pack.packSizeLimit, safety checks

From: Nicolas Pitre <nico@fluxnic.net>
Date: Feb 1, 2010, 18:04 UTC
Message-ID: <alpine.LFD.2.00.1002011240510.1681@xanadu.home>
In-Reply-To: <7vvdeg50x4.fsf@alter.siamese.dyndns.org>

On Mon, 1 Feb 2010, Junio C Hamano wrote:

Show 24 quoted lines

> Nicolas Pitre <nico@fluxnic.net> writes:
> 
> > Grrrrr.  This is a terrible discrepency given that all the other 
> > arguments in Git are always byte based, with the optional k/m/g suffix, 
> > by using git_parse_ulong().  So IMHO I'd just change --max-pack-size to 
> > be in line with all the rest and have it accept bytes instead of MB.  
> > And of course I'd push such a change to be included in v1.7.0 along with 
> > the other incompatible fixes.
> 
> All of the "other incompatible" changes had ample leading time for
> transition with warnings and all.
> 
> I am afraid that doing this "unit change" is way too late for 1.7.0, and
> it makes me somewhat unhappy to hear such a suggestion.  It belittles all
> the careful planning that has been done for these other changes to help
> protect the users from transition pain.
> 
> Introduce --max-pack-megabytes that is a synonym for --max-pack-size for
> now, and warn when --max-pack-size is used; warn that --max-pack-size will
> count in bytes in 1.8.0. Ship 1.7.0 with that change.  --max-pack-bytes
> can also be added if you feel like, while at it.
> 
> But changing the unit --max-pack-size counts in to bytes in 1.7.0 feels
> a bit too irresponsible for the existing users.

Thing is... I don't know if the --max-pack-size argument is really that used. I'd expect people relying on that feature to use the config variable instead, given that 'git gc' has no idea about --max-pack-size anyway. People using the --max-pack-size argument directly are probably doing so only to experiment with it, and then setting the config variable, probably using the wrong unit. The fact that such a discrepancy just came to our attention after all the time this feature has existed is certainly a good indicator of its popularity.

I understand the really unfortunate timing for such a change. OTOH there is a big advantage to bundle as much incompatibilities together at the same time, as people will be prepared for such things already.

While I share your concern for advance warning and such, I think those concerns are worth an effort proportional to the depth of user exposure. Like for the THREADED_DELTA_SEARCH case, I'm wondering how much pain if at all might be saved with a transition plan vs the cost of maintaining that plan and carrying the discrepancy further.

Nicolas