threads / patch / 15981

patchRe: [PATCH] fix for "index-pack: rationalize delta resolution code"

Subject: Re: [PATCH] fix for "index-pack: rationalize delta resolution code"

## tl;dr

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

replies: 9people: 4as markdown or json

Nicolas Pitre· Oct 20, 2008, 18:12 UTC · lore

[PATCH] fix for "index-pack: rationalize delta resolution code"

My bad. A small detail went through the crack: the real_type of a delta object is the real_type of its base object.

Without this, the created index will be wrong as the actual object SHA1 won't match the object.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

If you got a corrupted .idx file because of this ('git verify-pack' should tell) then just toss it and recreate with a fixed 'git index-pack'.

Could anyone having problems fetching from kernel.org with git from the next branch confirm that this also fixes that? Thanks.

Show changes to index-pack.c +1 −1
diff --git a/index-pack.c b/index-pack.c
index 0a917d7..8287ebf 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -517,7 +517,7 @@ static void resolve_delta(struct object_entry *delta_obj,
 	void *delta_data;
 	unsigned long delta_size;
 
-	delta_obj->real_type = base->obj->type;
+	delta_obj->real_type = base->obj->real_type;
 	delta_data = get_data_from_pack(delta_obj);
 	delta_size = delta_obj->size;
 	result->obj = delta_obj;
Harvey Harrison· Oct 20, 2008, 18:37 UTC · re: Nicolas Pitre · lore
On Mon, Oct 20, 2008 at 11:12 AM, Nicolas Pitre <nico@cam.org> wrote:
Show 7 quoted lines
> My bad.  A small detail went through the crack: the real_type of
> a delta object is the real_type of its base object.
>
> Without this, the created index will be wrong as the actual object SHA1
> won't match the object.
>
> Signed-off-by: Nicolas Pitre <nico@cam.org>
This fixes it for me, thanks for the quick fix.
Tested-by: Harvey Harrison <harvey.harrison@gmail.com>
Harvey Harrison· Oct 20, 2008, 19:07 UTC · re: Harvey Harrison · lore

On Mon, Oct 20, 2008 at 11:37 AM, Harvey Harrison <harvey.harrison@gmail.com> wrote:

Show 13 quoted lines
> On Mon, Oct 20, 2008 at 11:12 AM, Nicolas Pitre <nico@cam.org> wrote:
>> My bad.  A small detail went through the crack: the real_type of
>> a delta object is the real_type of its base object.
>>
>> Without this, the created index will be wrong as the actual object SHA1
>> won't match the object.
>>
>> Signed-off-by: Nicolas Pitre <nico@cam.org>
>
> This fixes it for me, thanks for the quick fix.
>
> Tested-by: Harvey Harrison <harvey.harrison@gmail.com>
>
Scratch that, it's back to failing again on my next update.
Harvey
Marco Roeland· Oct 20, 2008, 19:14 UTC · re: Nicolas Pitre · lore
On Monday October 2008 at 14:12 Nicolas Pitre wrote:
Show 15 quoted lines
> My bad.  A small detail went through the crack: the real_type of
> a delta object is the real_type of its base object.
> 
> Without this, the created index will be wrong as the actual object SHA1
> won't match the object.
> 
> Signed-off-by: Nicolas Pitre <nico@cam.org>
> ---
> 
> If you got a corrupted .idx file because of this ('git verify-pack' 
> should tell) then just toss it and recreate with a fixed 'git 
> index-pack'.
> 
> Could anyone having problems fetching from kernel.org with git from the 
> next branch confirm that this also fixes that? Thanks.
I still seem to have the same problem after patching:

$ git pull remote: Counting objects: 279, done. remote: Compressing objects: 100% (78/78), done. remote: Total 177 (delta 136), reused 135 (delta 99) Receiving objects: 100% (177/177), 66.59 KiB, done. fatal: pack has bad object at offset 53487: failed to apply delta fatal: index-pack failed

'git verify-pack' does _not_ report an error for either pack or index. This is with git from branch next at 8f0e41f379d486dd27766d84d994eb1da5b8319d trying to pull from git://git.kernel.org/pub/scm/git/git.git

This is on Debian 'sid' with an AMD64 architecture.

I've put the whole ".git" directory (warning: almost 35MB) for investigation at:

http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz

I hope I've patched correctly. After applying (cleanly) and rebuilding simply executing "./git" from the workdirectory still uses the old version. Only after using "make install" I get the patched version, which as shown above still gives an error, from the die() at line 528 in index-pack.c: bad_object(delta_obj->idx.offset, "failed to apply delta");

Not much more time tonight here, but perhaps it's easier to reproduce now with the copy of an affected .git directory.

-- 
Marco Roeland
Marco Roeland· Oct 20, 2008, 19:20 UTC · re: Marco Roeland · lore
Op maandag 20 oktober 2008 om 21:14 uur schreef Marco Roeland het volgende:
Show 6 quoted lines
> This is on Debian 'sid' with an AMD64 architecture.
> 
> I've put the whole ".git" directory (warning: almost 35MB) for
> investigation at:
> 
> http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
Gah, I can't even copy-and-paste:
http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz

This is on a quadcore. I recently experimented with "git config pack.threads 0" but as it didn't seem to speedup anything I removed it again. Just mention it on the infinitesimal chance it might be important.

-- 
Marco Roeland
Nicolas Pitre· Oct 20, 2008, 19:27 UTC · re: Marco Roeland · lore
On Mon, 20 Oct 2008, Marco Roeland wrote:
Show 12 quoted lines
> Op maandag 20 oktober 2008 om 21:14 uur schreef Marco Roeland het volgende:
> 
> > This is on Debian 'sid' with an AMD64 architecture.
> > 
> > I've put the whole ".git" directory (warning: almost 35MB) for
> > investigation at:
> > 
> > http://www.xs4all.nl/~fiberbit/http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz
> 
> Gah, I can't even copy-and-paste:
> 
> http://www.xs4all.nl/~fiberbit/git-next-8f0e41f3-bad-index.tgz

Don't worry -- I figured it out and was able to reproduce the problem already. Thanks a lot!

> This is on a quadcore. I recently experimented with "git config
> pack.threads 0" but as it didn't seem to speedup anything I removed
> it again. Just mention it on the infinitesimal chance it might be
> important.

It is not. And the speedup should be noticeable when you repack, not when you fetch.

Nicolas
Marco Roeland· Oct 20, 2008, 19:36 UTC · re: Nicolas Pitre · lore
On Monday Oktober 20th 2008 at 15:27 Nicolas Pitre wrote:
Show 7 quoted lines
> > This is on a quadcore. I recently experimented with "git config
> > pack.threads 0" but as it didn't seem to speedup anything I removed
> > it again. Just mention it on the infinitesimal chance it might be
> > important.
> 
> It is not.  And the speedup should be noticeable when you repack, not 
> when you fetch.

No offense meant! I tried a few "git gc" and "git repack" and only watched the Gnome CPU applet; perhaps everything was already nicely packed. I'm certainly going to retry now. Thanks for all your good work.

-- 
Marco Roeland
Nicolas Pitre· Oct 20, 2008, 20:04 UTC · re: Marco Roeland · lore
On Mon, 20 Oct 2008, Marco Roeland wrote:
Show 11 quoted lines
> On Monday Oktober 20th 2008 at 15:27 Nicolas Pitre wrote:
> 
> > > This is on a quadcore. I recently experimented with "git config
> > > pack.threads 0" but as it didn't seem to speedup anything I removed
> > > it again. Just mention it on the infinitesimal chance it might be
> > > important.
> > 
> > It is not.  And the speedup should be noticeable when you repack, not 
> > when you fetch.
> 
> No offense meant! 
Oh certainly not.
> I tried a few "git gc" and "git repack" and only
> watched the Gnome CPU applet; perhaps everything was already nicely
> packed. I'm certainly going to retry now. Thanks for all your good work.

If you want to make the difference really visible, try with 'git repack -a -f --window=100'.

Nicolas
Marco Roeland· Oct 20, 2008, 20:12 UTC · re: Nicolas Pitre · lore
On Monday Oktober 20th 2008 at 16:04 Nicolas Pitre wrote:
> If you want to make the difference really visible, try with
> 'git repack -a -f --window=100'.
Impressive, yes. Thanks very much.

marco@sirius:~/src/git (next) $ time git repack -a -f --window=100 Counting objects: 85713, done. Compressing objects: 100% (84207/84207), done. Writing objects: 100% (85713/85713), done. Total 85713 (delta 62371), reused 0 (delta 0)

real 1m2.775s user 1m1.848s sys 0m0.176s marco@sirius:~/src/git (next) $ git config pack.threads 0 marco@sirius:~/src/git (next) $ time git repack -a -f --window=100 Counting objects: 85713, done. Compressing objects: 100% (84207/84207), done. Writing objects: 100% (85713/85713), done. Total 85713 (delta 62363), reused 0 (delta 0)

real 0m21.348s user 1m2.948s sys 0m0.432s marco@sirius:~/src/git (next) $ git config --unset pack.threads marco@sirius:~/src/git (next) $ time git repack -a -f --window=100 Counting objects: 85713, done. Compressing objects: 100% (84207/84207), done. Writing objects: 100% (85713/85713), done. Total 85713 (delta 62371), reused 0 (delta 0)

real 1m1.904s user 1m1.476s sys 0m0.184s

This on Intel(R) Core(TM)2 Quad  CPU   Q9450  @ 2.66GHz.
-- 
Marco Roeland
Jeff King· Oct 20, 2008, 20:10 UTC · re: Nicolas Pitre · lore
On Mon, Oct 20, 2008 at 02:12:04PM -0400, Nicolas Pitre wrote:
> Could anyone having problems fetching from kernel.org with git from the 
> next branch confirm that this also fixes that? Thanks.
Nope, this does not fix it for me.
-Peff

← back to recent threads