threads / patch / 14140

patchpack.indexversion config option now defaults to 2

Subject: [PATCH] pack.indexversion config option now defaults to 2

## tl;dr

10 messages between Jun 25, 2008 and Jun 25, 2008. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Nicolas Pitre· Jun 25, 2008, 04:25 UTC · lore
As announced for 1.6.0.

Git older than version 1.5.2 (or any other git version with this option set to 1) may revert to version 1 of the pack index by manually deleting all .idx files and recreating them using 'git index-pack'. Communication over the git native protocol is unaffected since the pack index is never transferred.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 Documentation/config.txt |    6 +++---
 pack-write.c             |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 2 files +4 −4

Documentation/config.txt, pack-write.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c7fbc63..f72dd47 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -937,9 +937,9 @@ pack.indexVersion::
 	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.
+	packs.  Version 2 is the default.  Note that version 2 is enforced
+	and this config option ignored whenever the corresponding pack is
+	larger than 2 GB.
 
 pack.packSizeLimit::
 	The default maximum size of a pack.  This setting only affects
diff --git a/pack-write.c b/pack-write.c
index f52cabe..a8f0269 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -2,7 +2,7 @@
 #include "pack.h"
 #include "csum-file.h"
 
-uint32_t pack_idx_default_version = 1;
+uint32_t pack_idx_default_version = 2;
 uint32_t pack_idx_off32_limit = 0x7fffffff;
 
 static int sha1_compare(const void *_a, const void *_b)

-- 
1.5.6.56.g29b0d
Linus Torvalds· Jun 25, 2008, 04:32 UTC · re: Nicolas Pitre · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Wed, 25 Jun 2008, Nicolas Pitre wrote:
Show 6 quoted lines
> 
> Git older than version 1.5.2 (or any other git version with this option
> set to 1) may revert to version 1 of the pack index by manually deleting
> all .idx files and recreating them using 'git index-pack'.  Communication
> over the git native protocol is unaffected since the pack index is never
> transferred.

Rather than talk about when it does _not_ matter, wouldn't it be better to talk about when it _can_ matter?

Namely when using dumb protocols, either http or rsync, with the other end being some ancient git thing (and it is worth mentioning version of what counts as 'ancient' too, I can't remember, probably means that pretty much nobody else can either).

		Linus
Junio C Hamano· Jun 25, 2008, 04:59 UTC · re: Linus Torvalds · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 15 quoted lines
> On Wed, 25 Jun 2008, Nicolas Pitre wrote:
>> 
>> Git older than version 1.5.2 (or any other git version with this option
>> set to 1) may revert to version 1 of the pack index by manually deleting
>> all .idx files and recreating them using 'git index-pack'.  Communication
>> over the git native protocol is unaffected since the pack index is never
>> transferred.
>
> Rather than talk about when it does _not_ matter, wouldn't it be better to 
> talk about when it _can_ matter?
>
> Namely when using dumb protocols, either http or rsync, with the other end 
> being some ancient git thing (and it is worth mentioning version of what 
> counts as 'ancient' too, I can't remember, probably means that pretty much 
> nobody else can either).

I agree with you that the description of the change (in the commit log) and the instruction (in the documentation) could be more helpful and explicit.

For the other "usedeltabaseoffset" change, I think the insn in the documentation Nico added is reasonable:

+ By default, linkgit:git-repack[1] creates packs that use + delta-base offset. If you need to share your repository with + git older than version 1.4.4, either directly or via a dumb + protocol such as http, then you need to set this option to + "false" and repack. Access from old git versions over the + native protocol are unaffected by this option.

except perhaps that the "set false and repack" should be clarified further, perhaps like:

	... then you need to set this option to "false" and repack using
	git that is newer than (and including) v1.5.0.

I am basing the above 1.5.0 on description of b6945f5 (git-repack: repo.usedeltabaseoffset, 2006-10-13).

On the "indexVersion" change, the documentation reads:
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 the default.  Note that version 2 is enforced
+	and this config option ignored whenever the corresponding pack is
+	larger than 2 GB.

which lacks the recovery insn (and it is int strictly the fault of this patch, but we should have done this when we introduced the v2 idx). I think a separate paragraph after the above would be necessary and sufficient:

	If you have an ancient git that does not understand the version 2
	`*.idx` file, cloning or fetching over a non native protocol
	(e.g. "http" and "rsync") which will copy both `*.pack` file and
	corresponding `*.idx` file from the other side may give you a
	repository that cannot be accessed with your old git.  If the
	`*.pack` file is smaller than 2 GB, however, you can use
	`git-index-pack` on the `*.pack` to regenerate the `*.idx` file.
Mike Hommey· Jun 25, 2008, 05:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Tue, Jun 24, 2008 at 09:59:06PM -0700, Junio C Hamano wrote:
Show 21 quoted lines
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > On Wed, 25 Jun 2008, Nicolas Pitre wrote:
> >> 
> >> Git older than version 1.5.2 (or any other git version with this option
> >> set to 1) may revert to version 1 of the pack index by manually deleting
> >> all .idx files and recreating them using 'git index-pack'.  Communication
> >> over the git native protocol is unaffected since the pack index is never
> >> transferred.
> >
> > Rather than talk about when it does _not_ matter, wouldn't it be better to 
> > talk about when it _can_ matter?
> >
> > Namely when using dumb protocols, either http or rsync, with the other end 
> > being some ancient git thing (and it is worth mentioning version of what 
> > counts as 'ancient' too, I can't remember, probably means that pretty much 
> > nobody else can either).
> 
> I agree with you that the description of the change (in the commit log)
> and the instruction (in the documentation) could be more helpful and
> explicit.
(...)

Wouldn't it be a good idea to add a warning in git update-server-info when it detects pack.indexVersion is not 1, too ?

Mike
Junio C Hamano· Jun 25, 2008, 06:06 UTC · re: Mike Hommey · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

Mike Hommey <mh@glandium.org> writes:
> Wouldn't it be a good idea to add a warning in git update-server-info
> when it detects pack.indexVersion is not 1, too ?
Can you describe how that warning will help whom in the bigger picture?
The way as I understand everybody runs "git update-server-info" is:
 * "git push" pushes into a publishing repository, then
 * "post-receive" (or "post-update") hook triggers "update-server-info".

If you spit out a warning from update-server-info, it will be shown to the pusher over sideband, but this will not necessarily help the pusher, who may or may not have shell access to the repository.

Mike Hommey· Jun 25, 2008, 06:14 UTC · re: Junio C Hamano · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Tue, Jun 24, 2008 at 11:06:36PM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> Mike Hommey <mh@glandium.org> writes:
> 
> > Wouldn't it be a good idea to add a warning in git update-server-info
> > when it detects pack.indexVersion is not 1, too ?
> 
> Can you describe how that warning will help whom in the bigger picture?
> 
> The way as I understand everybody runs "git update-server-info" is:
> 
>  * "git push" pushes into a publishing repository, then
>  * "post-receive" (or "post-update") hook triggers "update-server-info".
> 
> If you spit out a warning from update-server-info, it will be shown to the
> pusher over sideband, but this will not necessarily help the pusher, who
> may or may not have shell access to the repository.

There are also those that get the message to run update-server-info when trying to clone over dumb protocol when the info/refs file is not there... I actually didn't think about the post-receive hook.

Well, then, update-server-info could output the warning only if stderr is a tty.

Mike
Nicolas Pitre· Jun 25, 2008, 18:53 UTC · re: Mike Hommey · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Wed, 25 Jun 2008, Mike Hommey wrote:
Show 26 quoted lines
> On Tue, Jun 24, 2008 at 09:59:06PM -0700, Junio C Hamano wrote:
> > Linus Torvalds <torvalds@linux-foundation.org> writes:
> > 
> > > On Wed, 25 Jun 2008, Nicolas Pitre wrote:
> > >> 
> > >> Git older than version 1.5.2 (or any other git version with this option
> > >> set to 1) may revert to version 1 of the pack index by manually deleting
> > >> all .idx files and recreating them using 'git index-pack'.  Communication
> > >> over the git native protocol is unaffected since the pack index is never
> > >> transferred.
> > >
> > > Rather than talk about when it does _not_ matter, wouldn't it be better to 
> > > talk about when it _can_ matter?
> > >
> > > Namely when using dumb protocols, either http or rsync, with the other end 
> > > being some ancient git thing (and it is worth mentioning version of what 
> > > counts as 'ancient' too, I can't remember, probably means that pretty much 
> > > nobody else can either).
> > 
> > I agree with you that the description of the change (in the commit log)
> > and the instruction (in the documentation) could be more helpful and
> > explicit.
> (...)
> 
> Wouldn't it be a good idea to add a warning in git update-server-info
> when it detects pack.indexVersion is not 1, too ?

That would give the idea that index v2 is not good, while what we want is to encourage everyone to move to index v2 ASAP.

Nicolas
Mike Hommey· Jun 25, 2008, 18:58 UTC · re: Nicolas Pitre · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Wed, Jun 25, 2008 at 02:53:41PM -0400, Nicolas Pitre wrote:
Show 31 quoted lines
> On Wed, 25 Jun 2008, Mike Hommey wrote:
> 
> > On Tue, Jun 24, 2008 at 09:59:06PM -0700, Junio C Hamano wrote:
> > > Linus Torvalds <torvalds@linux-foundation.org> writes:
> > > 
> > > > On Wed, 25 Jun 2008, Nicolas Pitre wrote:
> > > >> 
> > > >> Git older than version 1.5.2 (or any other git version with this option
> > > >> set to 1) may revert to version 1 of the pack index by manually deleting
> > > >> all .idx files and recreating them using 'git index-pack'.  Communication
> > > >> over the git native protocol is unaffected since the pack index is never
> > > >> transferred.
> > > >
> > > > Rather than talk about when it does _not_ matter, wouldn't it be better to 
> > > > talk about when it _can_ matter?
> > > >
> > > > Namely when using dumb protocols, either http or rsync, with the other end 
> > > > being some ancient git thing (and it is worth mentioning version of what 
> > > > counts as 'ancient' too, I can't remember, probably means that pretty much 
> > > > nobody else can either).
> > > 
> > > I agree with you that the description of the change (in the commit log)
> > > and the instruction (in the documentation) could be more helpful and
> > > explicit.
> > (...)
> > 
> > Wouldn't it be a good idea to add a warning in git update-server-info
> > when it detects pack.indexVersion is not 1, too ?
> 
> That would give the idea that index v2 is not good, while what we want 
> is to encourage everyone to move to index v2 ASAP.

The warning could just say that people with older git won't be able to clone over dumb protocols. Nothing more.

Mike
Nicolas Pitre· Jun 25, 2008, 18:49 UTC · re: Junio C Hamano · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

On Tue, 24 Jun 2008, Junio C Hamano wrote:
Show 15 quoted lines
> For the other "usedeltabaseoffset" change, I think the insn in the
> documentation Nico added is reasonable:
> 
> +	By default, linkgit:git-repack[1] creates packs that use
> +	delta-base offset. If you need to share your repository with
> +	git older than version 1.4.4, either directly or via a dumb
> +	protocol such as http, then you need to set this option to
> +	"false" and repack. Access from old git versions over the
> +	native protocol are unaffected by this option.
> 
> except perhaps that the "set false and repack" should be clarified
> further, perhaps like:
> 
> 	... then you need to set this option to "false" and repack using
> 	git that is newer than (and including) v1.5.0.
Actually the first release to support this is v1.4.4 not v1.5.0.
> I am basing the above 1.5.0 on description of b6945f5 (git-repack:
> repo.usedeltabaseoffset, 2006-10-13).

$ git describe --contains b6945f5 v1.4.4-rc1~77^2~2

Show 23 quoted lines
> On the "indexVersion" change, the documentation reads:
> 
> 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 the default.  Note that version 2 is enforced
> +	and this config option ignored whenever the corresponding pack is
> +	larger than 2 GB.
> 
> which lacks the recovery insn (and it is int strictly the fault of this
> patch, but we should have done this when we introduced the v2 idx).  I
> think a separate paragraph after the above would be necessary and
> sufficient:
> 
> 	If you have an ancient git that does not understand the version 2
> 	`*.idx` file, cloning or fetching over a non native protocol
> 	(e.g. "http" and "rsync") which will copy both `*.pack` file and
> 	corresponding `*.idx` file from the other side may give you a
> 	repository that cannot be accessed with your old git.  If the
> 	`*.pack` file is smaller than 2 GB, however, you can use
> 	`git-index-pack` on the `*.pack` to regenerate the `*.idx` file.

Looks fine to me. Will you amend the patches or do you want me to repost them?

Nicolas
Junio C Hamano· Jun 25, 2008, 19:19 UTC · re: Nicolas Pitre · lore

Re: [PATCH] pack.indexversion config option now defaults to 2

Nicolas Pitre <nico@cam.org> writes:
Show 9 quoted lines
> On Tue, 24 Jun 2008, Junio C Hamano wrote:
> ...
> Actually the first release to support this is v1.4.4 not v1.5.0.
>
>> I am basing the above 1.5.0 on description of b6945f5 (git-repack:
>> repo.usedeltabaseoffset, 2006-10-13).
>
> $ git describe --contains b6945f5
> v1.4.4-rc1~77^2~2

I know, as I did that myself; but I wanted to be conservative and gave a release after a major release bump.

Show 26 quoted lines
>> On the "indexVersion" change, the documentation reads:
>> 
>> 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 the default.  Note that version 2 is enforced
>> +	and this config option ignored whenever the corresponding pack is
>> +	larger than 2 GB.
>> 
>> which lacks the recovery insn (and it is int strictly the fault of this
>> patch, but we should have done this when we introduced the v2 idx).  I
>> think a separate paragraph after the above would be necessary and
>> sufficient:
>> 
>> 	If you have an ancient git that does not understand the version 2
>> 	`*.idx` file, cloning or fetching over a non native protocol
>> 	(e.g. "http" and "rsync") which will copy both `*.pack` file and
>> 	corresponding `*.idx` file from the other side may give you a
>> 	repository that cannot be accessed with your old git.  If the
>> 	`*.pack` file is smaller than 2 GB, however, you can use
>> 	`git-index-pack` on the `*.pack` to regenerate the `*.idx` file.
>
> Looks fine to me.  Will you amend the patches or do you want me to 
> repost them?

No worries. After re-reading what you wrote I think both are fine as they are. I do not mind adding the above 7 lines to the indexVersion one myself.

← back to recent threads