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

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

[Nicolas Pitre]

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(-)

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

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

[Linus Torvalds]

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).

		Linus

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

[Junio C Hamano]

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.

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.

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

[Mike Hommey]

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 ?

Mike

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

[Junio C Hamano]

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.

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

[Mike Hommey]

On Tue, Jun 24, 2008 at 11:06:36PM -0700, Junio C Hamano wrote:
> 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

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

[Nicolas Pitre]

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.


Nicolas

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

[Mike Hommey]

On Wed, Jun 25, 2008 at 02:53:41PM -0400, Nicolas Pitre wrote:
> 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

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

[Nicolas Pitre]

On Tue, 24 Jun 2008, Junio C Hamano wrote:

> 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

> 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

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

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> 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.

>> 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.