Re: [PATCH] Enhance unpack-objects for extracting large objects

[Nicolas Pitre <nico@cam.org>]

On Fri, 25 May 2007, Junio C Hamano wrote:

> Maybe we would want to call it '-f' for consistency.  Another
> possibility is the other way around, giving others a longer
> synonyms, like --quiet, but this command is plumbing and I do
> not think long options matters that much, so my preference is to
> do '-f' not '--force'.

OTOH, I like to have long options for weird or obscur parameters.  Their 
action is less likely to be presumed by casual inspection of a script 
using them.  I don't feel strongly about it either ways though.

> > @@ -131,7 +134,9 @@ static void added_object(unsigned nr, enum object_type type,
> >  static void write_object(unsigned nr, enum object_type type,
> >  			 void *buf, unsigned long size)
> >  {
> > -	if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
> > +	int force2 = size < min_blob_size ? -1 : force;
> > +	if (write_sha1_file_maybe(buf, size, typename(type),
> > +				  force2, obj_list[nr].sha1) < 0)
> >  		die("failed to write object");
> >  	added_object(nr, type, buf, size);
> >  }
> 
> Without --min-blob-size option, min_blob_size is initialized to
> 0u and force2 always gets the value of force.  With the option,
> blobs smaller than the threshold gets -1 and otherwise the value
> of force.
> 
> "write_sha1_file_maybe()" can take 0, 1, or -1 as its fourth
> parameter.  The reader is left puzzled what the distinction
> among these three and decides to read on to figure it out before
> complaining too much about the code, but no matter what it does,
> doesn't the above logic already feel wrong?
> 
>  * You already have the size here, so if min_blob_size is set
>    and the size is larger, you do not even have to call
>    write_sha1_file() at all.

You still do to get the object's SHA1.


Nicolas