{"thread":{"id":"8044","subject":"[PATCH v2] Custom compression levels for objects and packs","startedAt":"2007-05-08T22:38:22Z","lastAt":"2007-05-09T21:00:56Z","messageCount":23,"participants":["Dana How","Junio C Hamano","Nicolas Pitre","Petr Baudis","Theodore Tso"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"41506","messageId":"4640FBDE.1000609@gmail.com","threadId":"8044","inReplyTo":null,"subject":"[PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-08T22:38:22Z","receivedAt":"2007-05-08T22:38:22Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nAdd config variables pack.compression and core.loosecompression .\nLoose objects will be compressed using level\n isset(core.loosecompression) ? core.loosecompression :\n isset(core.compression) ? core.compression : Z_BEST_SPEED\nand objects in packs will be compressed using level\n isset(pack.compression) ? pack.compression :\n isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION\npack-objects also accepts --compression=N which\noverrides the latter expression.\n\nThis applies on top of the git-repack --max-pack-size patchset.\n\nSigned-off-by: Dana L. How \n---\n builtin-pack-objects.c | 32 +++++++++++++++++++++++++++++++-\n cache.h | 2 ++\n config.c | 18 +++++++++++++++++-\n environment.c | 4 +++-\n 4 files changed, 53 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 8824793..e80a1d6 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -70,6 +70,8 @@ static uint32_t pack_size_limit;\n static int pack_to_stdout;\n static int num_preferred_base;\n static struct progress progress_state;\n+static int pack_compression_level = Z_DEFAULT_COMPRESSION;\n+static int pack_compression_seen;\n \n /*\n * The object names in objects array are hashed with this hashtable,\n@@ -444,6 +446,10 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t * and we do not need to deltify it.\n \t\t\t\t */\n \n+\t/* differing core & pack compression when loose object -> must recompress */\n+\tif (!entry->in_pack && pack_compression_level != zlib_compression_level)\n+\t\tto_reuse = 0;\n+\telse\n \tif (!entry->in_pack && !entry->delta) {\n \t\tunsigned char *map;\n \t\tunsigned long mapsize;\n@@ -492,7 +498,7 @@ static unsigned long write_object(struct sha1file *f,\n \t\t}\n \t\t/* compress the data to store and put compressed length in datalen */\n \t\tmemset(&stream, 0, sizeof(stream));\n-\t\tdeflateInit(&stream, zlib_compression_level);\n+\t\tdeflateInit(&stream, pack_compression_level);\n \t\tmaxsize = deflateBound(&stream, size);\n \t\tout = xmalloc(maxsize);\n \t\t/* Compress it */\n@@ -1624,6 +1630,16 @@ static int git_pack_config(const char *k, const char *v)\n \t\twindow = git_config_int(k, v);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"pack.compression\")) {\n+\t\tint level = git_config_int(k, v);\n+\t\tif (level == -1)\n+\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n+\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n+\t\t\tdie(\"bad pack compression level %d\", level);\n+\t\tpack_compression_level = level;\n+\t\tpack_compression_seen = 1;\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(k, v);\n }\n \n@@ -1734,6 +1750,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \trp_ac = 2;\n \n \tgit_config(git_pack_config);\n+\tif (!pack_compression_seen && core_compression_seen)\n+\t\tpack_compression_level = core_compression_level;\n \n \tprogress = isatty(2);\n \tfor (i = 1; i < argc; i++) {\n@@ -1761,6 +1779,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t\tusage(pack_usage);\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!prefixcmp(arg, \"--compression=\")) {\n+\t\t\tchar *end;\n+\t\t\tint level = strtoul(arg+14, &end, 0);\n+\t\t\tif (!arg[14] || *end)\n+\t\t\t\tusage(pack_usage);\n+\t\t\tif (level == -1)\n+\t\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n+\t\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n+\t\t\t\tdie(\"bad pack compression level %d\", level);\n+\t\t\tpack_compression_level = level;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--window=\")) {\n \t\t\tchar *end;\n \t\t\twindow = strtoul(arg+9, &end, 0);\ndiff --git a/cache.h b/cache.h\nindex 8e76152..2b3f359 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -283,6 +283,8 @@ extern int warn_ambiguous_refs;\n extern int shared_repository;\n extern const char *apply_default_whitespace;\n extern int zlib_compression_level;\n+extern int core_compression_level;\n+extern int core_compression_seen;\n extern size_t packed_git_window_size;\n extern size_t packed_git_limit;\n extern size_t delta_base_cache_limit;\ndiff --git a/config.c b/config.c\nindex 70d1055..5627ed6 100644\n--- a/config.c\n+++ b/config.c\n@@ -12,6 +12,8 @@\n static FILE *config_file;\n static const char *config_file_name;\n static int config_linenr;\n+static int zlib_compression_seen;\n+\n static int get_next_char(void)\n {\n \tint c;\n@@ -304,13 +306,27 @@ int git_default_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.compression\")) {\n+\tif (!strcmp(var, \"core.loosecompression\")) {\n \t\tint level = git_config_int(var, value);\n \t\tif (level == -1)\n \t\t\tlevel = Z_DEFAULT_COMPRESSION;\n \t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n \t\t\tdie(\"bad zlib compression level %d\", level);\n \t\tzlib_compression_level = level;\n+\t\tzlib_compression_seen = 1;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"core.compression\")) {\n+\t\tint level = git_config_int(var, value);\n+\t\tif (level == -1)\n+\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n+\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n+\t\t\tdie(\"bad zlib compression level %d\", level);\n+\t\tcore_compression_level = level;\n+\t\tcore_compression_seen = 1;\n+\t\tif (!zlib_compression_seen)\n+\t\t\tzlib_compression_level = level;\n \t\treturn 0;\n \t}\n \ndiff --git a/environment.c b/environment.c\nindex 2231659..b7aeb1a 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -24,7 +24,9 @@ const char *git_commit_encoding;\n const char *git_log_output_encoding;\n int shared_repository = PERM_UMASK;\n const char *apply_default_whitespace;\n-int zlib_compression_level = Z_DEFAULT_COMPRESSION;\n+int zlib_compression_level = Z_BEST_SPEED;\n+int core_compression_level;\n+int core_compression_seen;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n size_t delta_base_cache_limit = 16 * 1024 * 1024;\n-- \n1.5.2.rc0.787.g0014\n"},{"id":"41516","messageId":"7vk5vi27ko.fsf@assigned-by-dhcp.cox.net","threadId":"8044","inReplyTo":"4640FBDE.1000609@gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-08T23:56:23Z","receivedAt":"2007-05-08T23:56:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dana How writes:\n\n> Add config variables pack.compression and core.loosecompression .\n> Loose objects will be compressed using level\n> isset(core.loosecompression) ? core.loosecompression :\n> isset(core.compression) ? core.compression : Z_BEST_SPEED\n> and objects in packs will be compressed using level\n> isset(pack.compression) ? pack.compression :\n> isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION\n> pack-objects also accepts --compression=N which\n> overrides the latter expression.\n\nDo you think the above is readable?\n\n Compression level for loose objects is controlled by variable\n core.loosecompression (or core.compression, if the former is\n missing), and defaults to best-speed.\n\nor something like that?\n\n> This applies on top of the git-repack --max-pack-size patchset.\n\nHmph, that makes the --max-pack-size patchset take this more\ntrivial and straightforward improvements hostage. In general,\nI'd prefer more elaborate ones based on less questionable\nseries.\n\n> @@ -444,6 +446,10 @@ static unsigned long write_object(struct sha1file *f,\n> \t\t\t\t * and we do not need to deltify it.\n> \t\t\t\t */\n> \n> +\t/* differing core & pack compression when loose object -> must recompress */\n> +\tif (!entry->in_pack && pack_compression_level != zlib_compression_level)\n> +\t\tto_reuse = 0;\n> +\telse\n\nI am not sure if that is worth it, as you do not know if the\nloose object you are looking at were compressed with the current\nsettings.\n\n> diff --git a/cache.h b/cache.h\n> index 8e76152..2b3f359 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -283,6 +283,8 @@ extern int warn_ambiguous_refs;\n> extern int shared_repository;\n> extern const char *apply_default_whitespace;\n> extern int zlib_compression_level;\n> +extern int core_compression_level;\n> +extern int core_compression_seen;\n\nCould we somehow remove _seen? Perhaps by initializing the\n_level to -1?\n\n> +int core_compression_level;\n> +int core_compression_seen;\n\nSame here.\n"},{"id":"41518","messageId":"alpine.LFD.0.99.0705082010230.24220@xanadu.home","threadId":"8044","inReplyTo":"7vk5vi27ko.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T00:16:30Z","receivedAt":"2007-05-09T00:16:30Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 8 May 2007, Junio C Hamano wrote:\n\n> Dana How writes:\n> \n> > @@ -444,6 +446,10 @@ static unsigned long write_object(struct sha1file *f,\n> > \t\t\t\t * and we do not need to deltify it.\n> > \t\t\t\t */\n> > \n> > +\t/* differing core & pack compression when loose object -> must recompress */\n> > +\tif (!entry->in_pack && pack_compression_level != zlib_compression_level)\n> > +\t\tto_reuse = 0;\n> > +\telse\n> \n> I am not sure if that is worth it, as you do not know if the\n> loose object you are looking at were compressed with the current\n> settings.\n\nI was about to make the same comment.\n\n> > diff --git a/cache.h b/cache.h\n> > index 8e76152..2b3f359 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -283,6 +283,8 @@ extern int warn_ambiguous_refs;\n> > extern int shared_repository;\n> > extern const char *apply_default_whitespace;\n> > extern int zlib_compression_level;\n> > +extern int core_compression_level;\n> > +extern int core_compression_seen;\n> \n> Could we somehow remove _seen? Perhaps by initializing the\n> _level to -1?\n\n-1 is a valid value for compression. Actually it is equivalent to \nZ_DEFAULT_COMPRESSION.\n\nIf we want the fallback logic to work, at some point we must remember if \nthe current value is the default or if it is the result of an explicit \nconfig option.\n\n\nNicolas\n"},{"id":"41519","messageId":"56b7f5510705081725v655d2ce1j28712507cfa7fa55@mail.gmail.com","threadId":"8044","inReplyTo":"7vk5vi27ko.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T00:25:24Z","receivedAt":"2007-05-09T00:25:24Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/8/07, Junio C Hamano wrote:\n> Dana How writes:\n> > Add config variables pack.compression and core.loosecompression .\n> > Loose objects will be compressed using level\n> > isset(core.loosecompression) ? core.loosecompression :\n> > isset(core.compression) ? core.compression : Z_BEST_SPEED\n> > and objects in packs will be compressed using level\n> > isset(pack.compression) ? pack.compression :\n> > isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION\n> > pack-objects also accepts --compression=N which\n> > overrides the latter expression.\n>\n> Do you think the above is readable?\n> Compression level for loose objects is controlled by variable\n> core.loosecompression (or core.compression, if the former is\n> missing), and defaults to best-speed.\n> or something like that?\nYour phrasing is much better.\n\n> > This applies on top of the git-repack --max-pack-size patchset.\n> Hmph, that makes the --max-pack-size patchset take this more\n> trivial and straightforward improvements hostage. In general,\n> I'd prefer more elaborate ones based on less questionable\n> series.\nThe max-pack-size and pack.compression patches touch the same lines.\nI thought my options were:\n* Submit independently and make you merge; or\n* Make one precede the other.\nSince max-pack-size has been out there since April 4 and\nthe first acceptable version was May 1 (suggested by 0 comments),\nI didn't realize it was a \"questionable series\".\n\nI think it should be straightforward for me to re-submit this\nbased on current master.\n\n> > + /* differing core & pack compression when loose object -> must recompress */\n> > + if (!entry->in_pack && pack_compression_level != zlib_compression_level)\n> > + to_reuse = 0;\n> > + else\n> I am not sure if that is worth it, as you do not know if the\n> loose object you are looking at were compressed with the current\n> settings.\nYou do not know for certain, that is correct. However, config\nsettings setting unequal compression levels signal that you\ncare differently about the two cases. (For me, I want the\ncompression investment to correspond to the expected lifetime of the file.)\nAlso, *if* we have the knobs we want in the config file,\nI don't think we're going to be changing these settings all that often.\n\nIf I didn't have this check forcing recompression in the pack,\nthen in the absence of deltification each object would enter the pack\nby being copied (in the preceding code block) and pack.compression\nwould have little effect. I actually experienced this the very first\ntime I imported a large dataset into git (I was trying to achieve the\neffect of this patch by changing core.compression dynamically, and\nwas a bit mystified for a while by the result).\n\nThus, if core.loosecompression is set to speed up git-add, I should\ntake the time to recompress the object when packing if pack.compression\nis different (of course the hit of not doing so will be lessened by\ndeltification\nwhich forces a new compression).\n\n> > diff --git a/cache.h b/cache.h\n> > index 8e76152..2b3f359 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -283,6 +283,8 @@ extern int warn_ambiguous_refs;\n> > extern int shared_repository;\n> > extern const char *apply_default_whitespace;\n> > extern int zlib_compression_level;\n> > +extern int core_compression_level;\n> > +extern int core_compression_seen;\n>\n> Could we somehow remove _seen? Perhaps by initializing the\n> _level to -1?\n>\n> > +int core_compression_level;\n> > +int core_compression_seen;\n>\n> Same here.\nI agree completely. But, what magic value should I use\nto initialize the _level variables so I know they are not set?\nAll valid settings come from zlib.h through #define's but\nthere is no \"invalid\" defined. Maybe I'll use -99.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41520","messageId":"56b7f5510705081729t34a585c6y9ca9e2f9963d24a2@mail.gmail.com","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705082010230.24220@xanadu.home","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T00:29:20Z","receivedAt":"2007-05-09T00:29:20Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/8/07, Nicolas Pitre wrote:\n> On Tue, 8 May 2007, Junio C Hamano wrote:\n> > Dana How writes:\n> > > + /* differing core & pack compression when loose object -> must recompress */\n> > > + if (!entry->in_pack && pack_compression_level != zlib_compression_level)\n> > > + to_reuse = 0;\n> > > + else\n> > I am not sure if that is worth it, as you do not know if the\n> > loose object you are looking at were compressed with the current\n> > settings.\n> I was about to make the same comment.\nI was bitten by *not* doing this. Please see the more verbose\nreply to Junio's comment.\n\n> > Could we somehow remove _seen? Perhaps by initializing the\n> > _level to -1?\n>\n> -1 is a valid value for compression. Actually it is equivalent to\n> Z_DEFAULT_COMPRESSION.\n>\n> If we want the fallback logic to work, at some point we must remember if\n> the current value is the default or if it is the result of an explicit\n> config option.\nI can leave as-is, or use a magic value like -99 and\ndepend on it not colliding with values in zlib.h.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41521","messageId":"20070509003016.GJ4489@pasky.or.cz","threadId":"8044","inReplyTo":"4640FBDE.1000609@gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-05-09T00:30:16Z","receivedAt":"2007-05-09T00:30:16Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, May 09, 2007 at 12:38:22AM CEST, Dana How wrote:\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 8824793..e80a1d6 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -444,6 +446,10 @@ static unsigned long write_object(struct sha1file *f,\n> \t\t\t\t * and we do not need to deltify it.\n> \t\t\t\t */\n> \n> +\t/* differing core & pack compression when loose object -> must recompress */\n> +\tif (!entry->in_pack && pack_compression_level != zlib_compression_level)\n> +\t\tto_reuse = 0;\n> +\telse\n> \tif (!entry->in_pack && !entry->delta) {\n\nStyle: the else and if should be probably on the same line.\n\n> \t\tunsigned char *map;\n> \t\tunsigned long mapsize;\n> @@ -1624,6 +1630,16 @@ static int git_pack_config(const char *k, const char *v)\n> \t\twindow = git_config_int(k, v);\n> \t\treturn 0;\n> \t}\n> +\tif (!strcmp(k, \"pack.compression\")) {\n> +\t\tint level = git_config_int(k, v);\n> +\t\tif (level == -1)\n> +\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n> +\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n> +\t\t\tdie(\"bad pack compression level %d\", level);\n> +\t\tpack_compression_level = level;\n> +\t\tpack_compression_seen = 1;\n> +\t\treturn 0;\n> +\t}\n> \treturn git_default_config(k, v);\n> }\n> \n\nWhere is this documented?\n\n> @@ -1761,6 +1779,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n> \t\t\t\tusage(pack_usage);\n> \t\t\tcontinue;\n> \t\t}\n> +\t\tif (!prefixcmp(arg, \"--compression=\")) {\n> +\t\t\tchar *end;\n> +\t\t\tint level = strtoul(arg+14, &end, 0);\n> +\t\t\tif (!arg[14] || *end)\n> +\t\t\t\tusage(pack_usage);\n> +\t\t\tif (level == -1)\n> +\t\t\t\tlevel = Z_DEFAULT_COMPRESSION;\n> +\t\t\telse if (level < 0 || level > Z_BEST_COMPRESSION)\n> +\t\t\t\tdie(\"bad pack compression level %d\", level);\n> +\t\t\tpack_compression_level = level;\n> +\t\t\tcontinue;\n> +\t\t}\n> \t\tif (!prefixcmp(arg, \"--window=\")) {\n> \t\t\tchar *end;\n> \t\t\twindow = strtoul(arg+9, &end, 0);\n\nWhere is this documented?\n\n> diff --git a/config.c b/config.c\n> index 70d1055..5627ed6 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -304,13 +306,27 @@ int git_default_config(const char *var, const char *value)\n> \t\treturn 0;\n> \t}\n> \n> -\tif (!strcmp(var, \"core.compression\")) {\n> +\tif (!strcmp(var, \"core.loosecompression\")) {\n\nIs this config variable documented?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nEver try. Ever fail. No matter. // Try again. Fail again. Fail better.\n\t\t-- Samuel Beckett\n"},{"id":"41522","messageId":"alpine.LFD.0.99.0705082031370.24220@xanadu.home","threadId":"8044","inReplyTo":"56b7f5510705081729t34a585c6y9ca9e2f9963d24a2@mail.gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T01:03:37Z","receivedAt":"2007-05-09T01:03:37Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 8 May 2007, Dana How wrote:\n\n> On 5/8/07, Nicolas Pitre wrote:\n> > On Tue, 8 May 2007, Junio C Hamano wrote:\n> > If we want the fallback logic to work, at some point we must remember if\n> > the current value is the default or if it is the result of an explicit\n> > config option.\n> I can leave as-is, or use a magic value like -99 and\n> depend on it not colliding with values in zlib.h.\n\nAnd where would you set those variables to a sensible default in the \nabsence of any config option?\n\n\nNicolas\n"},{"id":"41525","messageId":"alpine.LFD.0.99.0705082106590.24220@xanadu.home","threadId":"8044","inReplyTo":"56b7f5510705081725v655d2ce1j28712507cfa7fa55@mail.gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T01:23:25Z","receivedAt":"2007-05-09T01:23:25Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 8 May 2007, Dana How wrote:\n\n> Since max-pack-size has been out there since April 4 and\n> the first acceptable version was May 1 (suggested by 0 comments),\n> I didn't realize it was a \"questionable series\".\n> \n> I think it should be straightforward for me to re-submit this\n> based on current master.\n\nSince this patch is simpler it could be merged much faster, before the \npack limit series.\n\n> > > + /* differing core & pack compression when loose object -> must\n> > recompress */\n> > > + if (!entry->in_pack && pack_compression_level !=\n> > zlib_compression_level)\n> > > + to_reuse = 0;\n> > > + else\n> > I am not sure if that is worth it, as you do not know if the\n> > loose object you are looking at were compressed with the current\n> > settings.\n> You do not know for certain, that is correct. However, config\n> settings setting unequal compression levels signal that you\n> care differently about the two cases. (For me, I want the\n> compression investment to correspond to the expected lifetime of the file.)\n> Also, *if* we have the knobs we want in the config file,\n> I don't think we're going to be changing these settings all that often.\n> \n> If I didn't have this check forcing recompression in the pack,\n> then in the absence of deltification each object would enter the pack\n> by being copied (in the preceding code block) and pack.compression\n> would have little effect. I actually experienced this the very first\n> time I imported a large dataset into git (I was trying to achieve the\n> effect of this patch by changing core.compression dynamically, and\n> was a bit mystified for a while by the result).\n> \n> Thus, if core.loosecompression is set to speed up git-add, I should\n> take the time to recompress the object when packing if pack.compression\n> is different (of course the hit of not doing so will be lessened by\n> deltification\n> which forces a new compression).\n\nRight. And this also depends whether or not you have core.legacyheaders \nset to false or not.\n\nAnd the whole purpose for setting core.legacyheaders is exactly to allow \nfor loose objects to be copied straight into the pack. This should have \npriority over mismatched compression levels IMHO.\n\nAlso, when repacking, delta reuse does not recompress objects for the \nsame reason, regardless of the compression level used when they were \ncompressed initially. Same argument goes for delta depth.\n\nSo if you really want to ensure a compression level on the whole pack, \nyou'll have to use -f with git-repack. Or leave core.legacyheaders \nunset.\n\n\nNicolas\n"},{"id":"41540","messageId":"7vr6pqy1ty.fsf@assigned-by-dhcp.cox.net","threadId":"8044","inReplyTo":"56b7f5510705081725v655d2ce1j28712507cfa7fa55@mail.gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-09T05:59:21Z","receivedAt":"2007-05-09T05:59:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> On 5/8/07, Junio C Hamano wrote:\n>> Dana How writes:\n>> ...\n>> > This applies on top of the git-repack --max-pack-size patchset.\n>\n>> Hmph, that makes the --max-pack-size patchset take this more\n>> trivial and straightforward improvements hostage. In general,\n>> I'd prefer more elaborate ones based on less questionable\n>> series.\n>\n> The max-pack-size and pack.compression patches touch the same lines.\n> I thought my options were:\n> * Submit independently and make you merge; or\n> * Make one precede the other.\n> Since max-pack-size has been out there since April 4 and\n> the first acceptable version was May 1 (suggested by 0 comments),\n> I didn't realize it was a \"questionable series\".\n\nNo, what I meant was that it is much \"more elaborate\" series\nthan this custom compression which is much \"less questionable\".\n\nI think this custom compression is 1.5.2 material. I have not\nstudied the code for the max-pack-size enough to be confident to\nput it in 1.5.2, at least not yet, and was planning to park the\nlatter in 'next' until 1.5.2 final.\n"},{"id":"41541","messageId":"56b7f5510705082324q4e054b75l7021dc08ec31392@mail.gmail.com","threadId":"8044","inReplyTo":"7vr6pqy1ty.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T06:24:05Z","receivedAt":"2007-05-09T06:24:05Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/8/07, Junio C Hamano wrote:\n> \"Dana How\" writes:\n> > On 5/8/07, Junio C Hamano wrote:\n> >> Dana How writes:\n> >> > This applies on top of the git-repack --max-pack-size patchset.\n> >> Hmph, that makes the --max-pack-size patchset take this more\n> >> trivial and straightforward improvements hostage. In general,\n> >> I'd prefer more elaborate ones based on less questionable\n> >> series.\n> > The max-pack-size and pack.compression patches touch the same lines.\n> > I thought my options were:\n> > * Submit independently and make you merge; or\n> > * Make one precede the other.\n> > Since max-pack-size has been out there since April 4 and\n> > the first acceptable version was May 1 (suggested by 0 comments),\n> > I didn't realize it was a \"questionable series\".\n> No, what I meant was that it is much \"more elaborate\" series\n> than this custom compression which is much \"less questionable\".\n>\n> I think this custom compression is 1.5.2 material. I have not\n> studied the code for the max-pack-size enough to be confident to\n> put it in 1.5.2, at least not yet, and was planning to park the\n> latter in 'next' until 1.5.2 final.\nOK, thanks for guesstimating the overall schedule. I was starting to wonder\nwhat the next step(s) should be.\n\nI will incorporate your & Nicolas's comments and send out a new custom\ncompression patch tomorrow. I *think* I addressed everyone's comments\non max-pack-size, but let me know if you find anything else when you get\naround to it.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41544","messageId":"56b7f5510705082346m32d3c48dj987fd9b0a6118c10@mail.gmail.com","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705082031370.24220@xanadu.home","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T06:46:57Z","receivedAt":"2007-05-09T06:46:57Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/8/07, Nicolas Pitre wrote:\n> On Tue, 8 May 2007, Dana How wrote:\n> > On 5/8/07, Nicolas Pitre wrote:\n> > > On Tue, 8 May 2007, Junio C Hamano wrote:\n> > > If we want the fallback logic to work, at some point we must remember if\n> > > the current value is the default or if it is the result of an explicit\n> > > config option.\n> > I can leave as-is, or use a magic value like -99 and\n> > depend on it not colliding with values in zlib.h.\n> And where would you set those variables to a sensible default in the\n> absence of any config option?\nThis is why I used the _seen variables,\nbut they are not necessary -- see 3rd option below.\n\nUnfortunately we agreed a day or two ago to use a config rule like\nused_value = isset(var1) ? var1 : isset(var2) ? var2 : DEFAULT.\nThis doesn't interact well with each variable being processed\ncompletely independently in git_config() and the callbacks it calls.\nThe isset() value is \"out-of-band\"; either store it in the _seen\nvariables, or some special value in used_value .\n\nWhich makes the most sense:\n* Leave _seen as-is;\n* Move pack.compression recognition into config.c which means\n the _seen variables would all be local to config.c;\n* Use some special value, and if still present replace it with the default\n at the end of git_config() using extra code;\n* Change the config rule to something simpler.\n\nI like the 2nd and the 4th. You didn't like the 4th.\nShall I change to the 2nd?\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41549","messageId":"7vd51axyeq.fsf@assigned-by-dhcp.cox.net","threadId":"8044","inReplyTo":"56b7f5510705082346m32d3c48dj987fd9b0a6118c10@mail.gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-09T07:13:17Z","receivedAt":"2007-05-09T07:13:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> This doesn't interact well with each variable being processed\n> completely independently in git_config() and the callbacks it calls.\n> The isset() value is \"out-of-band\"; either store it in the _seen\n> variables, or some special value in used_value .\n>\n> Which makes the most sense:\n> * Leave _seen as-is;\n> * Move pack.compression recognition into config.c which means\n> the _seen variables would all be local to config.c;\n> * Use some special value, and if still present replace it with the default\n> at the end of git_config() using extra code;\n> * Change the config rule to something simpler.\n>\n> I like the 2nd and the 4th. You didn't like the 4th.\n> Shall I change to the 2nd?\n\nFWIW, I am Ok with (1).\n"},{"id":"41563","messageId":"56b7f5510705090221g38ab0973x8631dacc601abb16@mail.gmail.com","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705082106590.24220@xanadu.home","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T09:21:45Z","receivedAt":"2007-05-09T09:21:45Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/8/07, Nicolas Pitre wrote:\n> On Tue, 8 May 2007, Dana How wrote:\n> > I think it should be straightforward for me to re-submit this\n> > based on current master.\n> Since this patch is simpler it could be merged much faster, before the\n> pack limit series.\nYes, I will do \"custom compression\" patch first.\n\n> > > > + /* differing core & pack compression when loose object -> must\n> > > recompress */\n> > > I am not sure if that is worth it, as you do not know if the\n> > > loose object you are looking at were compressed with the current\n> > > settings.\n> > You do not know for certain, that is correct. However, config\n> > settings setting unequal compression levels signal that you\n> > care differently about the two cases. (For me, I want the\n> > compression investment to correspond to the expected lifetime of the file.)\n> > Also, *if* we have the knobs we want in the config file,\n> > I don't think we're going to be changing these settings all that often.\n> >\n> > If I didn't have this check forcing recompression in the pack,\n> > then in the absence of deltification each object would enter the pack\n> > by being copied (in the preceding code block) and pack.compression\n> > would have little effect. I actually experienced this the very first\n> > time I imported a large dataset into git (I was trying to achieve the\n> > effect of this patch by changing core.compression dynamically, and\n> > was a bit mystified for a while by the result).\n> >\n> > Thus, if core.loosecompression is set to speed up git-add, I should\n> > take the time to recompress the object when packing if pack.compression\n> > is different (of course the hit of not doing so will be lessened by\n> > deltification\n> > which forces a new compression).\n>\n> Right. And this also depends whether or not you have core.legacyheaders\n> set to false or not.\n>\n> And the whole purpose for setting core.legacyheaders is exactly to allow\n> for loose objects to be copied straight into the pack. This should have\n> priority over mismatched compression levels IMHO.\nOK, I got really confused here, so I looked over the code,\nand figured out 2 causes for my confusion.\n(1) core.legacyheaders controls use_legacy_headers, which defaults to 1.\nSo currently all loose objects are in legacy format and the code block\nI spoke of doesn't trigger [without a config setting]. I didn't realize\nlegacy headers were still being produced (mislead by the name!).\n(2) I read your \"setting core.legacyheaders\" as followed by TRUE,\nbut you meant FALSE, which is not the default.\n\nI also read that 1 year after 1.4.2, the default for core.legacyheaders is going\nto change to FALSE. I think our discussion should assume this has\nhappened. So let's assume FALSE in the following. The point of that\nis that such a FALSE setting can't be assumed to have any special intent;\nit will be the default.\n\n[Everything I write here boils down to only one question,\n which I repeat at the end.]\n\nData gets into a pack in these ways:\n1. Loose object copied in;\n2. Loose object newly deltified;\n3. Packed object to be copied;\n4. Packed object to be newly deltified;\n5. Packed deltified object we can't re-use;\n6. Packed deltified object we can re-use.\n[\"copied\" includes recompressed.]\n\nIn (2), (4), and (5), pack.compression will always be newly used.\nIf pack.compression doesn't change, this means (6)\nwill be using pack.compression since it comes from (2) or (4).\nSo if I \"guarantee\" that (1) uses pack.compression,\n(3) will as well, meaning everything in the pack will be\nat pack.compression.\n\nThus if pack.compression != core.loosecompression takes precedence\nover core.legacyheaders = false, then for pack.compression constant\nwe get all 6 cases at level pack.compression. If core.legacyheaders =\nfalse takes precedence as you suggest, then all undeltified objects\n(20%?) will be stuck at core.loosecompression [since I see no way\nto sensibly re-apply compression to something copied pack-to-pack].\nI think this is inconsistent with what a pack.compression !=\ncore.loosecompression setting is telling us.\n\nMy arguments have 2 biases.\n(a) I assume pack.compression stays constant, and if it changes,\nI see little value in worrying about \"forcing\" all objects in a new pack,\nsome of which might be copied from an old pack, to be at the same\ncompression level.\n(b) I focused first on packing to disk, where a constant pack.compression\nmakes sense. For packing to stdout (pack transfer), especially on\na popular, hence loaded, repository server, we may want to use\n--compress=N to crank down on effort spent compressing new deltas [2+4]\n(or loose objects [1]) (or unusable deltas [5]). This still lets\nbetter compressed\nobjects flow through in the other cases [3+6]. So for one-use packs,\na mix of compression levels could be desirable and\nis achievable with --compression=N if/when so.\n\n> Also, when repacking, delta reuse does not recompress objects for the\n> same reason, regardless of the compression level used when they were\n> compressed initially. Same argument goes for delta depth.\nDelta reuse doesn't need recompression. For pack.compression\nconstant, they will already be at the correct level. I think we agree\non behavior here for different reasons.\n\n> So if you really want to ensure a compression level on the whole pack,\n> you'll have to use -f with git-repack. Or leave core.legacyheaders\n> unset.\n-f would be needed if you were in the practice of changing\npack.compression, yes.\n\nSo, after covering all these cases, have I convinced you that\nrecompression takes precedence over legacyheaders = FALSE?\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41592","messageId":"20070509135602.GB21489@thunk.org","threadId":"8044","inReplyTo":"4640FBDE.1000609@gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Theodore Tso","fromEmail":"tytso@mit.edu","sentAt":"2007-05-09T13:56:02Z","receivedAt":"2007-05-09T13:56:02Z","isPatch":true,"sender":{"key":"tytso@mit.edu","avatar":"https://avatars.githubusercontent.com/u/51416?v=4"},"body":"I noticed that the patch didn't include additions to\nDocumentation/config.txt; could those be added, so that as much as\npossible all of the various configuration knobs are documented in one\nplace, please?\n\nThanks!!\n\n\t\t\t\t\t- Ted\n"},{"id":"41603","messageId":"alpine.LFD.0.99.0705091048120.24220@xanadu.home","threadId":"8044","inReplyTo":"56b7f5510705090221g38ab0973x8631dacc601abb16@mail.gmail.com","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T15:27:28Z","receivedAt":"2007-05-09T15:27:28Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 9 May 2007, Dana How wrote:\n\n> OK, I got really confused here, so I looked over the code,\n> and figured out 2 causes for my confusion.\n> (1) core.legacyheaders controls use_legacy_headers, which defaults to 1.\n> So currently all loose objects are in legacy format and the code block\n> I spoke of doesn't trigger [without a config setting]. I didn't realize\n> legacy headers were still being produced (mislead by the name!).\n> (2) I read your \"setting core.legacyheaders\" as followed by TRUE,\n> but you meant FALSE, which is not the default.\n> \n> I also read that 1 year after 1.4.2, the default for core.legacyheaders is\n> going\n> to change to FALSE. I think our discussion should assume this has\n> happened.\n\n\n\nNow that we encourage and actively preserve objects in a packed form \nmore agressively than we did at the time core.legacyheaders was \nintroduced, I really wonder if this option is still worth it. Because \nthe packing of loose objects has to go through the delta match loop \nanyway, and since most of them should end up being deltified in most \ncases, there is really little advantage to have this parallel loose \nobject format as the CPU savings it might provide is rather lost in the \nnoise in the end. \n\nSo I suggest that we get rid of core.legacyheaders, preserve the legacy \nformat as the only writable loose object format and deprecate the other \none to keep things simpler. Thoughts?\n\n\n\n> So let's assume FALSE in the following. The point of that is that \n> such a FALSE setting can't be assumed to have any special intent; it \n> will be the default.\n> \n> [Everything I write here boils down to only one question,\n> which I repeat at the end.]\n> \n> Data gets into a pack in these ways:\n> 1. Loose object copied in;\n> 2. Loose object newly deltified;\n> 3. Packed object to be copied;\n> 4. Packed object to be newly deltified;\n> 5. Packed deltified object we can't re-use;\n> 6. Packed deltified object we can re-use.\n> [\"copied\" includes recompressed.]\n\nI think you forgets \"packed undeltified objects we can reuse\".\n\n> In (2), (4), and (5), pack.compression will always be newly used.\n> If pack.compression doesn't change, this means (6)\n> will be using pack.compression since it comes from (2) or (4).\n> So if I \"guarantee\" that (1) uses pack.compression,\n> (3) will as well, meaning everything in the pack will be\n> at pack.compression.\n> \n> Thus if pack.compression != core.loosecompression takes precedence\n> over core.legacyheaders = false, then for pack.compression constant\n> we get all 6 cases at level pack.compression. If core.legacyheaders =\n> false takes precedence as you suggest, then all undeltified objects\n> (20%?) will be stuck at core.loosecompression [since I see no way\n> to sensibly re-apply compression to something copied pack-to-pack].\n> \n> I think this is inconsistent with what a pack.compression !=\n> core.loosecompression setting is telling us.\n\nOK I see that I missed the fact that git-repack -f (or git-pack-objects \n--no-reuse-delta) does not recompress undeltified objects. Note this is \na problem in the case where you change pack.compression to a different \nvalue or override it on the command line as well: reused undeltified \nobjects won't get recompressed with the new level. My rant on \ncore.legacyheaders and its removal would address the first case. Your \ntest for a difference between loose and packed compression levels is \nflawed because the value of core.compression does not necessarily \nrepresent the compression level that was used for the loose objects to \npack (core.compression might have been modified since then), but it \nonly addresses the first case too. And this is a problem even now.\n\nWhat we need instead is a --no-reuse-object that would force \nrecompression of everything when you really want to enforce a specific \ncompression level across the whole pack(s).\n\n\nNicolas\n"},{"id":"41616","messageId":"7vlkfyt13i.fsf@assigned-by-dhcp.cox.net","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705091048120.24220@xanadu.home","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-09T16:26:25Z","receivedAt":"2007-05-09T16:26:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> So I suggest that we get rid of core.legacyheaders, preserve the legacy \n> format as the only writable loose object format and deprecate the other \n> one to keep things simpler. Thoughts?\n\nI agree with your analysis, especially when deeper delta chains\nare allowed, straight copy of loose object becomes less and less\nlikely.\n\n> What we need instead is a --no-reuse-object that would force \n> recompression of everything when you really want to enforce a specific \n> compression level across the whole pack(s).\n\nYeah. Or maybe --no-reuse to mean both and make '-f' a\nshort-hand synonym for that.\n\nI do not see much reason to want to tweak them independently;\nrecomputing delta is much more expensive than recompressing\nanyway, and when the user says 'repack -f', it is a sign that\nthe user is willing to spend CPU cycles.\n"},{"id":"41622","messageId":"56b7f5510705090942qe574e11h4f52f204d68a40b8@mail.gmail.com","threadId":"8044","inReplyTo":"7vlkfyt13i.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T16:42:05Z","receivedAt":"2007-05-09T16:42:05Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/9/07, Junio C Hamano wrote:\n> Nicolas Pitre writes:\n>\n> > So I suggest that we get rid of core.legacyheaders, preserve the legacy\n> > format as the only writable loose object format and deprecate the other\n> > one to keep things simpler. Thoughts?\n>\n> I agree with your analysis, especially when deeper delta chains\n> are allowed, straight copy of loose object becomes less and less\n> likely.\nI too agree with the logic in the original ,\nand deprecation is the appropriate thing. So I'm not actually\ngoing to change anything here in today's patch.\n\n> > What we need instead is a --no-reuse-object that would force\n> > recompression of everything when you really want to enforce a specific\n> > compression level across the whole pack(s).\n>\n> Yeah. Or maybe --no-reuse to mean both and make '-f' a\n> short-hand synonym for that.\n>\n> I do not see much reason to want to tweak them independently;\n> recomputing delta is much more expensive than recompressing\n> anyway, and when the user says 'repack -f', it is a sign that\n> the user is willing to spend CPU cycles.\nWe have the same idea about git-repack -f -- it implies both\n--no-reuse-delta and --no-reuse-object , and I will do that.\nI will incorporate Nicolas's new --no-reuse-object\nin my patch to builtin-pack-objects.c .\nI won't go so far as collapsing --no-reuse-* to --no-reuse in this patch.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41623","messageId":"56b7f5510705090944m6605fb82pa79a1623d8084dcd@mail.gmail.com","threadId":"8044","inReplyTo":"20070509135602.GB21489@thunk.org","subject":"Re: [PATCH v2] Custom compression levels for objects and packs","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T16:44:13Z","receivedAt":"2007-05-09T16:44:13Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/9/07, Theodore Tso wrote:\n> I noticed that the patch didn't include additions to\n> Documentation/config.txt; could those be added, so that as much as\n> possible all of the various configuration knobs are documented in one\n> place, please?\n\nI will update the documentation, after we've stopped changing\nthe set of options and behaviors.\n\nYou can see that's just what I did in the max-pack-size patch.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41627","messageId":"alpine.LFD.0.99.0705091247450.24220@xanadu.home","threadId":"8044","inReplyTo":"7vlkfyt13i.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] make \"repack -f\" imply \"pack-objects --no-reuse-object\"","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T16:59:40Z","receivedAt":"2007-05-09T16:59:40Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"As Junio said:\n\n Recomputing delta is much more expensive than recompressing\n anyway, and when the user says 'repack -f', it is a sign that\n the user is willing to spend CPU cycles.\n\nSigned-off-by: Nicolas Pitre \n---\n\nOn Wed, 9 May 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre writes:\n> \n> > What we need instead is a --no-reuse-object that would force \n> > recompression of everything when you really want to enforce a specific \n> > compression level across the whole pack(s).\n> \n> Yeah. Or maybe --no-reuse to mean both and make '-f' a\n> short-hand synonym for that.\n> \n> I do not see much reason to want to tweak them independently;\n> recomputing delta is much more expensive than recompressing\n> anyway, and when the user says 'repack -f', it is a sign that\n> the user is willing to spend CPU cycles.\n\nThis applies on top of my --no-reuse-object patch. I think it is good \nto have the flexibility in the plumbing, but as you say git-repack \nmight as well use the strongest option.\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex ddfa8b4..8bf66a4 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -8,7 +8,7 @@ SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n no_update_info= all_into_one= remove_redundant=\n-local= quiet= no_reuse_delta= extra=\n+local= quiet= no_reuse= extra=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n@@ -16,7 +16,7 @@ do\n \t-a)\tall_into_one=t ;;\n \t-d)\tremove_redundant=t ;;\n \t-q)\tquiet=-q ;;\n-\t-f)\tno_reuse_delta=--no-reuse-delta ;;\n+\t-f)\tno_reuse=--no-reuse-object ;;\n \t-l)\tlocal=--local ;;\n \t--window=*) extra=\"$extra $1\" ;;\n \t--depth=*) extra=\"$extra $1\" ;;\n@@ -61,7 +61,7 @@ case \",$all_into_one,\" in\n \t;;\n esac\n \n-args=\"$args $local $quiet $no_reuse_delta$extra\"\n+args=\"$args $local $quiet $no_reuse$extra\"\n name=$(git-pack-objects --non-empty --all --reflog $args \n---\n\nOn Wed, 9 May 2007, Junio C Hamano wrote:\n\n> I agree with your analysis, especially when deeper delta chains\n> are allowed, straight copy of loose object becomes less and less\n> likely.\n\nSo here it is, with a nice code reduction:\n\n Documentation/config.txt | 13 --------\n builtin-pack-objects.c | 69 ----------------------------------------------\n cache.h | 2 -\n config.c | 5 ---\n environment.c | 1 -\n sha1_file.c | 47 +++++++------------------------\n 6 files changed, 11 insertions(+), 126 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ea434af..d6d89ba 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -209,19 +209,6 @@ core.compression::\n \tcompression, and 1..9 are various speed/size tradeoffs, 9 being\n \tslowest.\n \n-core.legacyheaders::\n-\tA boolean which\n-\tchanges the format of loose objects so that they are more\n-\tefficient to pack and to send out of the repository over git\n-\tnative protocol, since v1.4.2. However, loose objects\n-\twritten in the new format cannot be read by git older than\n-\tthat version; people fetching from your repository using\n-\tolder versions of git over dumb transports (e.g. http)\n-\twill also be affected.\n-+\n-To let git use the new loose object format, you have to\n-set core.legacyheaders to false.\n-\n core.packedGitWindowSize::\n \tNumber of bytes of a pack file to map into memory in a\n \tsingle mapping operation. Larger window sizes may allow\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 966f843..c74a361 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -346,56 +346,6 @@ static void copy_pack_data(struct sha1file *f,\n \t}\n }\n \n-static int check_loose_inflate(unsigned char *data, unsigned long len, unsigned long expect)\n-{\n-\tz_stream stream;\n-\tunsigned char fakebuf[4096];\n-\tint st;\n-\n-\tmemset(&stream, 0, sizeof(stream));\n-\tstream.next_in = data;\n-\tstream.avail_in = len;\n-\tstream.next_out = fakebuf;\n-\tstream.avail_out = sizeof(fakebuf);\n-\tinflateInit(&stream);\n-\n-\twhile (1) {\n-\t\tst = inflate(&stream, Z_FINISH);\n-\t\tif (st == Z_STREAM_END || st == Z_OK) {\n-\t\t\tst = (stream.total_out == expect &&\n-\t\t\t stream.total_in == len) ? 0 : -1;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (st != Z_BUF_ERROR) {\n-\t\t\tst = -1;\n-\t\t\tbreak;\n-\t\t}\n-\t\tstream.next_out = fakebuf;\n-\t\tstream.avail_out = sizeof(fakebuf);\n-\t}\n-\tinflateEnd(&stream);\n-\treturn st;\n-}\n-\n-static int revalidate_loose_object(struct object_entry *entry,\n-\t\t\t\t unsigned char *map,\n-\t\t\t\t unsigned long mapsize)\n-{\n-\t/* we already know this is a loose object with new type header. */\n-\tenum object_type type;\n-\tunsigned long size, used;\n-\n-\tif (pack_to_stdout)\n-\t\treturn 0;\n-\n-\tused = unpack_object_header_gently(map, mapsize, &type, &size);\n-\tif (!used)\n-\t\treturn -1;\n-\tmap += used;\n-\tmapsize -= used;\n-\treturn check_loose_inflate(map, mapsize, size);\n-}\n-\n static unsigned long write_object(struct sha1file *f,\n \t\t\t\t struct object_entry *entry)\n {\n@@ -425,25 +375,6 @@ static unsigned long write_object(struct sha1file *f,\n \t\t\t\t * and we do not need to deltify it.\n \t\t\t\t */\n \n-\tif (!entry->in_pack && !entry->delta) {\n-\t\tunsigned char *map;\n-\t\tunsigned long mapsize;\n-\t\tmap = map_sha1_file(entry->sha1, &mapsize);\n-\t\tif (map && !legacy_loose_object(map)) {\n-\t\t\t/* We can copy straight into the pack file */\n-\t\t\tif (revalidate_loose_object(entry, map, mapsize))\n-\t\t\t\tdie(\"corrupt loose object %s\",\n-\t\t\t\t sha1_to_hex(entry->sha1));\n-\t\t\tsha1write(f, map, mapsize);\n-\t\t\tmunmap(map, mapsize);\n-\t\t\twritten++;\n-\t\t\treused++;\n-\t\t\treturn mapsize;\n-\t\t}\n-\t\tif (map)\n-\t\t\tmunmap(map, mapsize);\n-\t}\n-\n \tif (!to_reuse) {\n \t\tbuf = read_sha1_file(entry->sha1, &type, &size);\n \t\tif (!buf)\ndiff --git a/cache.h b/cache.h\nindex 8e76152..5725bce 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -273,7 +273,6 @@ extern void rollback_lock_file(struct lock_file *);\n extern int delete_ref(const char *, const unsigned char *sha1);\n \n /* Environment bits from configuration mechanism */\n-extern int use_legacy_headers;\n extern int trust_executable_bit;\n extern int has_symlinks;\n extern int assume_unchanged;\n@@ -354,7 +353,6 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);\n extern int has_sha1_pack(const unsigned char *sha1, const char **ignore);\n extern int has_sha1_file(const unsigned char *sha1);\n extern void *map_sha1_file(const unsigned char *sha1, unsigned long *);\n-extern int legacy_loose_object(unsigned char *);\n \n extern int has_pack_file(const unsigned char *sha1);\n extern int has_pack_index(const unsigned char *sha1);\ndiff --git a/config.c b/config.c\nindex 70d1055..298966f 100644\n--- a/config.c\n+++ b/config.c\n@@ -299,11 +299,6 @@ int git_default_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.legacyheaders\")) {\n-\t\tuse_legacy_headers = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \tif (!strcmp(var, \"core.compression\")) {\n \t\tint level = git_config_int(var, value);\n \t\tif (level == -1)\ndiff --git a/environment.c b/environment.c\nindex 2231659..54e3aba 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -11,7 +11,6 @@\n \n char git_default_email[MAX_GITNAME];\n char git_default_name[MAX_GITNAME];\n-int use_legacy_headers = 1;\n int trust_executable_bit = 1;\n int has_symlinks = 1;\n int assume_unchanged;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 32244d7..e715527 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -972,7 +972,7 @@ void *map_sha1_file(const unsigned char *sha1, unsigned long *size)\n \treturn map;\n }\n \n-int legacy_loose_object(unsigned char *map)\n+static int legacy_loose_object(unsigned char *map)\n {\n \tunsigned int word;\n \n@@ -1034,6 +1034,14 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon\n \t\treturn inflate(stream, 0);\n \t}\n \n+\n+\t/*\n+\t * There used to be a second loose object header format which\n+\t * was meant to mimic the in-pack format, allowing for direct\n+\t * copy of the object data. This format turned up not to be\n+\t * really worth it and we don't write it any longer. But we\n+\t * can still read it.\n+\t */\n \tused = unpack_object_header_gently(map, mapsize, &type, &size);\n \tif (!used || !valid_loose_object_type[type])\n \t\treturn -1;\n@@ -1962,40 +1970,6 @@ static int write_buffer(int fd, const void *buf, size_t len)\n \treturn 0;\n }\n \n-static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)\n-{\n-\tint hdr_len;\n-\tunsigned char c;\n-\n-\tc = (type << 4) | (len & 15);\n-\tlen >>= 4;\n-\thdr_len = 1;\n-\twhile (len) {\n-\t\t*hdr++ = c | 0x80;\n-\t\thdr_len++;\n-\t\tc = (len & 0x7f);\n-\t\tlen >>= 7;\n-\t}\n-\t*hdr = c;\n-\treturn hdr_len;\n-}\n-\n-static void setup_object_header(z_stream *stream, const char *type, unsigned long len)\n-{\n-\tint obj_type, hdrlen;\n-\n-\tif (use_legacy_headers) {\n-\t\twhile (deflate(stream, 0) == Z_OK)\n-\t\t\t/* nothing */;\n-\t\treturn;\n-\t}\n-\tobj_type = type_from_string(type);\n-\thdrlen = write_binary_header(stream->next_out, obj_type, len);\n-\tstream->total_out = hdrlen;\n-\tstream->next_out += hdrlen;\n-\tstream->avail_out -= hdrlen;\n-}\n-\n int hash_sha1_file(const void *buf, unsigned long len, const char *type,\n unsigned char *sha1)\n {\n@@ -2062,7 +2036,8 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha\n \t/* First header.. */\n \tstream.next_in = (unsigned char *)hdr;\n \tstream.avail_in = hdrlen;\n-\tsetup_object_header(&stream, type, len);\n+\twhile (deflate(&stream, 0) == Z_OK)\n+\t\t/* nothing */;\n \n \t/* Then the data itself.. */\n \tstream.next_in = buf;\n"},{"id":"41655","messageId":"56b7f5510705091316t78e31032k95815e190239717e@mail.gmail.com","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705091422130.24220@xanadu.home","subject":"Re: [PATCH] deprecate the new loose object header format","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T20:16:39Z","receivedAt":"2007-05-09T20:16:39Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"This doesn't just deprecate the format,\nit removes the ability to create it.\nSo to me this patch goes too far.\n\nAlso, if we're interested in \"simpler\", wouldn't\nit be better for loose and in-pack objects to be\nthe same? I thought that was the point of\n!legacy_headers. Whatever the decision is,\nit certainly has little impact on CPU effort\nas you point out.\n\nMaybe I'm too conservative because I'm less\nfamililar with the code and haven't written it.\nPersonally I oscillate between adding and refactoring\n(you haven't seen any refactoring yet ;-) ).\n\nThanks,\n\nDana\n\nOn 5/9/07, Nicolas Pitre wrote:\n> Now that we encourage and actively preserve objects in a packed form\n> more agressively than we did at the time the new loose object format and\n> core.legacyheaders were introduced, that extra loose object format\n> doesn't appear to be worth it anymore.\n>\n> Because the packing of loose objects has to go through the delta match\n> loop anyway, and since most of them should end up being deltified in\n> most cases, there is really little advantage to have this parallel loose\n> object format as the CPU savings it might provide is rather lost in the\n> noise in the end.\n>\n> This patch gets rid of core.legacyheaders, preserve the legacy format as\n> the only writable loose object format and deprecate the other one to\n> keep things simpler.\n>\n> Signed-off-by: Nicolas Pitre \n> ---\n>\n> On Wed, 9 May 2007, Junio C Hamano wrote:\n>\n> > I agree with your analysis, especially when deeper delta chains\n> > are allowed, straight copy of loose object becomes less and less\n> > likely.\n>\n> So here it is, with a nice code reduction:\n>\n> Documentation/config.txt | 13 --------\n> builtin-pack-objects.c | 69 ----------------------------------------------\n> cache.h | 2 -\n> config.c | 5 ---\n> environment.c | 1 -\n> sha1_file.c | 47 +++++++------------------------\n> 6 files changed, 11 insertions(+), 126 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index ea434af..d6d89ba 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -209,19 +209,6 @@ core.compression::\n> compression, and 1..9 are various speed/size tradeoffs, 9 being\n> slowest.\n>\n> -core.legacyheaders::\n> - A boolean which\n> - changes the format of loose objects so that they are more\n> - efficient to pack and to send out of the repository over git\n> - native protocol, since v1.4.2. However, loose objects\n> - written in the new format cannot be read by git older than\n> - that version; people fetching from your repository using\n> - older versions of git over dumb transports (e.g. http)\n> - will also be affected.\n> -+\n> -To let git use the new loose object format, you have to\n> -set core.legacyheaders to false.\n> -\n> core.packedGitWindowSize::\n> Number of bytes of a pack file to map into memory in a\n> single mapping operation. Larger window sizes may allow\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 966f843..c74a361 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -346,56 +346,6 @@ static void copy_pack_data(struct sha1file *f,\n> }\n> }\n>\n> -static int check_loose_inflate(unsigned char *data, unsigned long len, unsigned long expect)\n> -{\n> - z_stream stream;\n> - unsigned char fakebuf[4096];\n> - int st;\n> -\n> - memset(&stream, 0, sizeof(stream));\n> - stream.next_in = data;\n> - stream.avail_in = len;\n> - stream.next_out = fakebuf;\n> - stream.avail_out = sizeof(fakebuf);\n> - inflateInit(&stream);\n> -\n> - while (1) {\n> - st = inflate(&stream, Z_FINISH);\n> - if (st == Z_STREAM_END || st == Z_OK) {\n> - st = (stream.total_out == expect &&\n> - stream.total_in == len) ? 0 : -1;\n> - break;\n> - }\n> - if (st != Z_BUF_ERROR) {\n> - st = -1;\n> - break;\n> - }\n> - stream.next_out = fakebuf;\n> - stream.avail_out = sizeof(fakebuf);\n> - }\n> - inflateEnd(&stream);\n> - return st;\n> -}\n> -\n> -static int revalidate_loose_object(struct object_entry *entry,\n> - unsigned char *map,\n> - unsigned long mapsize)\n> -{\n> - /* we already know this is a loose object with new type header. */\n> - enum object_type type;\n> - unsigned long size, used;\n> -\n> - if (pack_to_stdout)\n> - return 0;\n> -\n> - used = unpack_object_header_gently(map, mapsize, &type, &size);\n> - if (!used)\n> - return -1;\n> - map += used;\n> - mapsize -= used;\n> - return check_loose_inflate(map, mapsize, size);\n> -}\n> -\n> static unsigned long write_object(struct sha1file *f,\n> struct object_entry *entry)\n> {\n> @@ -425,25 +375,6 @@ static unsigned long write_object(struct sha1file *f,\n> * and we do not need to deltify it.\n> */\n>\n> - if (!entry->in_pack && !entry->delta) {\n> - unsigned char *map;\n> - unsigned long mapsize;\n> - map = map_sha1_file(entry->sha1, &mapsize);\n> - if (map && !legacy_loose_object(map)) {\n> - /* We can copy straight into the pack file */\n> - if (revalidate_loose_object(entry, map, mapsize))\n> - die(\"corrupt loose object %s\",\n> - sha1_to_hex(entry->sha1));\n> - sha1write(f, map, mapsize);\n> - munmap(map, mapsize);\n> - written++;\n> - reused++;\n> - return mapsize;\n> - }\n> - if (map)\n> - munmap(map, mapsize);\n> - }\n> -\n> if (!to_reuse) {\n> buf = read_sha1_file(entry->sha1, &type, &size);\n> if (!buf)\n> diff --git a/cache.h b/cache.h\n> index 8e76152..5725bce 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -273,7 +273,6 @@ extern void rollback_lock_file(struct lock_file *);\n> extern int delete_ref(const char *, const unsigned char *sha1);\n>\n> /* Environment bits from configuration mechanism */\n> -extern int use_legacy_headers;\n> extern int trust_executable_bit;\n> extern int has_symlinks;\n> extern int assume_unchanged;\n> @@ -354,7 +353,6 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename);\n> extern int has_sha1_pack(const unsigned char *sha1, const char **ignore);\n> extern int has_sha1_file(const unsigned char *sha1);\n> extern void *map_sha1_file(const unsigned char *sha1, unsigned long *);\n> -extern int legacy_loose_object(unsigned char *);\n>\n> extern int has_pack_file(const unsigned char *sha1);\n> extern int has_pack_index(const unsigned char *sha1);\n> diff --git a/config.c b/config.c\n> index 70d1055..298966f 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -299,11 +299,6 @@ int git_default_config(const char *var, const char *value)\n> return 0;\n> }\n>\n> - if (!strcmp(var, \"core.legacyheaders\")) {\n> - use_legacy_headers = git_config_bool(var, value);\n> - return 0;\n> - }\n> -\n> if (!strcmp(var, \"core.compression\")) {\n> int level = git_config_int(var, value);\n> if (level == -1)\n> diff --git a/environment.c b/environment.c\n> index 2231659..54e3aba 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -11,7 +11,6 @@\n>\n> char git_default_email[MAX_GITNAME];\n> char git_default_name[MAX_GITNAME];\n> -int use_legacy_headers = 1;\n> int trust_executable_bit = 1;\n> int has_symlinks = 1;\n> int assume_unchanged;\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 32244d7..e715527 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -972,7 +972,7 @@ void *map_sha1_file(const unsigned char *sha1, unsigned long *size)\n> return map;\n> }\n>\n> -int legacy_loose_object(unsigned char *map)\n> +static int legacy_loose_object(unsigned char *map)\n> {\n> unsigned int word;\n>\n> @@ -1034,6 +1034,14 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon\n> return inflate(stream, 0);\n> }\n>\n> +\n> + /*\n> + * There used to be a second loose object header format which\n> + * was meant to mimic the in-pack format, allowing for direct\n> + * copy of the object data. This format turned up not to be\n> + * really worth it and we don't write it any longer. But we\n> + * can still read it.\n> + */\n> used = unpack_object_header_gently(map, mapsize, &type, &size);\n> if (!used || !valid_loose_object_type[type])\n> return -1;\n> @@ -1962,40 +1970,6 @@ static int write_buffer(int fd, const void *buf, size_t len)\n> return 0;\n> }\n>\n> -static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)\n> -{\n> - int hdr_len;\n> - unsigned char c;\n> -\n> - c = (type << 4) | (len & 15);\n> - len >>= 4;\n> - hdr_len = 1;\n> - while (len) {\n> - *hdr++ = c | 0x80;\n> - hdr_len++;\n> - c = (len & 0x7f);\n> - len >>= 7;\n> - }\n> - *hdr = c;\n> - return hdr_len;\n> -}\n> -\n> -static void setup_object_header(z_stream *stream, const char *type, unsigned long len)\n> -{\n> - int obj_type, hdrlen;\n> -\n> - if (use_legacy_headers) {\n> - while (deflate(stream, 0) == Z_OK)\n> - /* nothing */;\n> - return;\n> - }\n> - obj_type = type_from_string(type);\n> - hdrlen = write_binary_header(stream->next_out, obj_type, len);\n> - stream->total_out = hdrlen;\n> - stream->next_out += hdrlen;\n> - stream->avail_out -= hdrlen;\n> -}\n> -\n> int hash_sha1_file(const void *buf, unsigned long len, const char *type,\n> unsigned char *sha1)\n> {\n> @@ -2062,7 +2036,8 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha\n> /* First header.. */\n> stream.next_in = (unsigned char *)hdr;\n> stream.avail_in = hdrlen;\n> - setup_object_header(&stream, type, len);\n> + while (deflate(&stream, 0) == Z_OK)\n> + /* nothing */;\n>\n> /* Then the data itself.. */\n> stream.next_in = buf;\n>\n\n\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"41662","messageId":"alpine.LFD.0.99.0705091633590.24220@xanadu.home","threadId":"8044","inReplyTo":"56b7f5510705091316t78e31032k95815e190239717e@mail.gmail.com","subject":"Re: [PATCH] deprecate the new loose object header format","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-09T20:42:19Z","receivedAt":"2007-05-09T20:42:19Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 9 May 2007, Dana How wrote:\n\n> This doesn't just deprecate the format,\n> it removes the ability to create it.\n> So to me this patch goes too far.\n\nWell, I disagree. We cannot create it anymore, but we still can read \nit. Older Git versions nay not even read it. And since this format was \noff by default anyway, I doubt you've lost anything.\n\n> Also, if we're interested in \"simpler\", wouldn't\n> it be better for loose and in-pack objects to be\n> the same? I thought that was the point of\n> !legacy_headers.\n\nIt is not simpler because:\n\n 1) backward compatibility requires the legacy format, and\n\n 2) the object SHA1 is always computed with the legacy header included.\n\nSo what is simpler is really to get rid of over 100 lines of code that \ndidn't provide a real benefit. The faster we remove the ability to \nwrite such objects the fewer they'll be in the field.\n\n\nNicolas\n"},{"id":"41665","messageId":"56b7f5510705091400x37dfe961jfe2a739f2e6842cd@mail.gmail.com","threadId":"8044","inReplyTo":"alpine.LFD.0.99.0705091633590.24220@xanadu.home","subject":"Re: [PATCH] deprecate the new loose object header format","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-09T21:00:56Z","receivedAt":"2007-05-09T21:00:56Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/9/07, Nicolas Pitre wrote:\n> It is not simpler because:\n> 2) the object SHA1 is always computed with the legacy header included.\nThat convinces me.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"}]}