threads / patch / 10579

patchmake the pack index version configurable

Subject: [PATCH] make the pack index version configurable

## tl;dr

3 messages between Nov 2, 2007 and Nov 2, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Nicolas Pitre· Nov 2, 2007, 03:26 UTC · lore

It is a good idea to use pack index version 2 all the time since it has proper protection against propagation of certain pack corruptions when repacking which is not possible with index version 1, as demonstrated in test t5302.

Hence this config option.
The default is still pack index version 1.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
Show changes to 3 files +28 −0

Documentation/config.txt, builtin-pack-objects.c, index-pack.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d4a476e..862b79e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -660,6 +660,15 @@ pack.threads::
 	machines. The required amount of memory for the delta search window
 	is however multiplied by the number of threads.
 
+pack.indexVersion::
+	Specify the default pack index version.  Valid values are 1 for
+	legacy pack index used by Git versions prior to 1.5.2, and 2 for
+	the new pack index with capabilities for packs larger than 4 GB
+	as well as proper protection against the repacking of corrupted
+	packs.  Version 2 is selected and this config option ignored
+	whenever the corresponding pack is larger than 2 GB.  Otherwise
+	the default is 1.
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 25ec65d..e923689 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1768,6 +1768,12 @@ static int git_pack_config(const char *k, const char *v)
 #endif
 		return 0;
 	}
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
 	return git_default_config(k, v);
 }
 
diff --git a/index-pack.c b/index-pack.c
index 61ea762..715a5bb 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -683,6 +683,17 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 	}
 }
 
+static int git_index_pack_config(const char *k, const char *v)
+{
+	if (!strcmp(k, "pack.indexversion")) {
+		pack_idx_default_version = git_config_int(k, v);
+		if (pack_idx_default_version > 2)
+			die("bad pack.indexversion=%d", pack_idx_default_version);
+		return 0;
+	}
+	return git_default_config(k, v);
+}
+
 int main(int argc, char **argv)
 {
 	int i, fix_thin_pack = 0;
@@ -693,6 +704,8 @@ int main(int argc, char **argv)
 	struct pack_idx_entry **idx_objects;
 	unsigned char sha1[20];
 
+	git_config(git_index_pack_config);
+
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 
David Symonds· Nov 2, 2007, 03:42 UTC · re: Nicolas Pitre · lore

Re: [PATCH] make the pack index version configurable

On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
Show 9 quoted lines
>
> +pack.indexVersion::
> +       Specify the default pack index version.  Valid values are 1 for
> +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> +       the new pack index with capabilities for packs larger than 4 GB
> +       as well as proper protection against the repacking of corrupted
> +       packs.  Version 2 is selected and this config option ignored
> +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> +       the default is 1.

If you're trying to force a previous pack version for some reason (backward compatibility, or whatever), this "feature" of automatic forcing version 2 for 2 GB packs might come as a nasty suprise. Wouldn't it be better to fail with an error?

Dave.
Nicolas Pitre· Nov 2, 2007, 04:01 UTC · re: David Symonds · lore

Re: [PATCH] make the pack index version configurable

On Fri, 2 Nov 2007, David Symonds wrote:
Show 15 quoted lines
> On 11/2/07, Nicolas Pitre <nico@cam.org> wrote:
> >
> > +pack.indexVersion::
> > +       Specify the default pack index version.  Valid values are 1 for
> > +       legacy pack index used by Git versions prior to 1.5.2, and 2 for
> > +       the new pack index with capabilities for packs larger than 4 GB
> > +       as well as proper protection against the repacking of corrupted
> > +       packs.  Version 2 is selected and this config option ignored
> > +       whenever the corresponding pack is larger than 2 GB.  Otherwise
> > +       the default is 1.
> 
> If you're trying to force a previous pack version for some reason
> (backward compatibility, or whatever), this "feature" of automatic
> forcing version 2 for 2 GB packs might come as a nasty suprise.
> Wouldn't it be better to fail with an error?

No. The fact is that you don't know in advance what the pack size will be, and reaching that limit might take a long while already if repacking, or even more so if the pack is fetched over a network connection. That would be wasteful to simply throw away all the whole repack/download with an error which would be an even nastier surprise at that point just because you specified index v1.

And such a pack would be impossible to work with using an old Git version anyway, so the old Git version will happily give you the error, suggesting that you upgrade to a later version.

And this has been the implemented behavior for the last 7 months already.

And the idea is to switch the default to version 2 eventually.
Etc.
Nicolas

← back to recent threads