Re: [RFD/PATCH] Implement pack.compression and pack-objects --compression=N

From: Dana How <danahow@gmail.com>
Date: May 4, 2007, 19:51 UTC
Message-ID: <56b7f5510705041251v1a09103eif677c02efc53059e@mail.gmail.com>
In-Reply-To: <alpine.LFD.0.99.0705041218130.24220@xanadu.home>

On 5/4/07, Nicolas Pitre <nico@cam.org> wrote:

Show 24 quoted lines

> On Fri, 4 May 2007, Dana How wrote:
> > On 5/4/07, Nicolas Pitre <nico@cam.org> wrote:
> > > I think that would make sense to have separate configs for pack and
> > > loose object compression.  When not specified they should simply default
> > > to core.compression if it exists.  Otherwise I'd suggest that pack
> > > compression default level be Z_DEFAULT_COMPRESSION and loose object
> > > compression default level be Z_BEST_SPEED.  This would make interactive
> > > operations like git-add and git-commit even faster by default.
> > I agree with your Z_BEST_SPEED idea.  I did not include it in
> > the patch b/c I didn't want to change any behavior in the absence
> > of new config settings.
> > Are you actually arguing for *3* different compression-related config
> > variables?
> Yes.
>
> > How about:
> > (a) core.compression controls loose objects. defaults to Z_BEST_SPEED.
> > (b) pack.compression controls packing. defaults to Z_DEFAULT_COMPRESSION
> > if neither variable exists. defaults to core.compression if only that exists
> Yes, although I wouldn't default pack.compression to core.compression
> if pack.compression doesn't exist.  The documentation about
> core.compression currently talks
> (wrongly) only about loose objects anyway, so making pack.compression
> stand on its own won't be that bad.

Now that I'm awake your original quote at the top suggests:
(a) zlib_compression_level =
     isset(core.loosecompression) ? core.loosecompression :
     isset(core.compression) ? core.compression : Z_BEST_SPEED;
(b) pack_compression_level =
     isset(pack.compression) ? pack.compression :
     isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION;

Your later reaction to my quoted (a)/(b) table suggests:
(a) zlib_compression_level =
     isset(core.compression) ? core.compression : Z_BEST_SPEED;
(b) pack_compresion_level =
     isset(pack.compression) ? pack.compression : Z_DEFAULT_COMPRESSION;

In either case, the C variable zlib_compression_level controls compression level for loose objects, and pack_compression_level controls compression in a pack. "isset()" means an active setting does appear in a config file (it could be the default value).

The 2nd behavior table is sufficient for me and simpler than my patch. Do you want the 1st or 2nd behavior?

Thanks,

-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell