Re: [PATCH v2] Custom compression levels for objects and packs

From: Nicolas Pitre <nico@cam.org>
Date: May 9, 2007, 00:16 UTC
Message-ID: <alpine.LFD.0.99.0705082010230.24220@xanadu.home>
In-Reply-To: <7vk5vi27ko.fsf@assigned-by-dhcp.cox.net>

On Tue, 8 May 2007, Junio C Hamano wrote:

Show 14 quoted lines

> Dana How <danahow@gmail.com> writes:
> 
> > @@ -444,6 +446,10 @@ static unsigned long write_object(struct sha1file *f,
> >  				 * and we do not need to deltify it.
> >  				 */
> >  
> > +	/* differing core & pack compression when loose object -> must recompress */
> > +	if (!entry->in_pack && pack_compression_level != zlib_compression_level)
> > +		to_reuse = 0;
> > +	else
> 
> I am not sure if that is worth it, as you do not know if the
> loose object you are looking at were compressed with the current
> settings.

I was about to make the same comment.

Show 13 quoted lines

> > diff --git a/cache.h b/cache.h
> > index 8e76152..2b3f359 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -283,6 +283,8 @@ extern int warn_ambiguous_refs;
> >  extern int shared_repository;
> >  extern const char *apply_default_whitespace;
> >  extern int zlib_compression_level;
> > +extern int core_compression_level;
> > +extern int core_compression_seen;
> 
> Could we somehow remove _seen?  Perhaps by initializing the
> _level to -1?

-1 is a valid value for compression. Actually it is equivalent to Z_DEFAULT_COMPRESSION.

If we want the fallback logic to work, at some point we must remember if the current value is the default or if it is the result of an explicit config option.

Nicolas