git push problem with v1.5.0-rc1

git push problem with v1.5.0-rc1

[Bart Trojanowski]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

I just stared using 1.5.0-rc yesterday, and ran into a problem with
git-push.

I am running vanilla 2.6.19 on amd64 SMP box.  For this project I am
working in a 32bit chroot -- the 64/32bit setup is part of the problem.

There are 3 repositories involved here:

linux-2.6.git      
    - clone of a few branches from kernel.org

klips-vault.git
    - linux tree from project upstream
    - alternates = /.../linux-2.6.git/.git/objects

working dir:
    - clone of klips-vault.git
    - alternates = /.../klips-vault.git/.git/objects

    - I made a new branch
    - applied ~100 patches

    - went to push and...

      $ git version 
      git version 1.5.0.rc1.gdf1b 
       
      $ cat .git/config 
      [core] 
              repositoryformatversion = 0 
              filemode = true 
              logallrefupdates = true 
      [remote "origin"] 
              url = /home/jukie/bart/work/xelerance/klips-vault.git/.git 
              fetch = +refs/heads/*:refs/remotes/origin/* 
              push = refs/heads/my-ocf+fsm_v2.6.18:refs/heads/my-ocf+fsm_v2.6.18 
      [branch "master"] 
              remote = origin 
              merge = refs/heads/master 
       
      $ git branch 
        master 
      * my-ocf+fsm_v2.6.18 
       
      $ git push origin 
      updating 'refs/heads/my-ocf+fsm_v2.6.18' 
        from 0000000000000000000000000000000000000000 
        to   380541e91358d7a5e2fe37c81c520c92a3094951 
      Generating pack... 
      Done counting 727 objects. 
      Result has 708 objects. 
      Deltifying 708 objects. 
       100% (708/708) done 
      Writing 708 objects. 
       100% (708/708) done 
      Total 708 (delta 535), reused 275 (delta 218) 
      fatal: cannot fstat packfile: Value too large for defined data type 
      unpack index-pack exited with error code 
      ng refs/heads/my-ocf+fsm_v2.6.18 n/a (unpacker error) 

There was some question as to why fstat fails in the chroot... I don't
have any 2TB pack files.  The above repos are relatively small.  All
.git directories sum up to about 500M.

I got some help from Shawn Pearce on #git.  He told me to set the
[receive] unpackLimit=5000 in .git/config on the remote.  That did
work, and I could push.

The conclusion was that my "chroot is broken".  It's Ubuntu, so it
should Just Work(TM) :D

I have tested a few configurations:

git 1.4.4.4                       - push OK
git 1.4.4.4 w/unpackLimit=100     - push fails

git 1.5.0-rc0                     - push fails
git 1.5.0-rc1                     - push fails
git 1.5.0-rc1 w/unpackLimit=5000  - push OK

I am not sure why 1.4.4.4 works w/o limit changes.

I will try not to touch anything for a while, and would be glad to help
further if I can.

-Bart

-- 
				WebSig: http://www.jukie.net/~bart/sig/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] Don't call fstat() on stdin in index-pack.

[Bart Trojanowski]

* Bart Trojanowski <bart@jukie.net> [070118 16:26]:
>       $ git push origin 
>       updating 'refs/heads/my-ocf+fsm_v2.6.18' 
>         from 0000000000000000000000000000000000000000 
>         to   380541e91358d7a5e2fe37c81c520c92a3094951 
>       Generating pack... 
>       Done counting 727 objects. 
>       Result has 708 objects. 
>       Deltifying 708 objects. 
>        100% (708/708) done 
>       Writing 708 objects. 
>        100% (708/708) done 
>       Total 708 (delta 535), reused 275 (delta 218) 
>       fatal: cannot fstat packfile: Value too large for defined data type 

I had a look at the code in index-pack.c and noticed that this error is
printed input_fd is set to 0 in open_pack_file().

|           if (fstat(input_fd, &st))
|                   die("cannot fstat packfile: %s", strerror(errno));
|           if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
|                   die("pack has junk at the end");

It seems strange to me to call fstat on fd 0 to get st_size info.

Granted, st_mode should tell us that it's not a regular file, but
already know it's not a regular file.

So I removed it.  And now it works (I verified that the git-diff output
on both sides matches).

-Bart

----

>From fcd359655e12a4b6424f989b7c01cbbb8a551287 Mon Sep 17 00:00:00 2001
From: Bart Trojanowski <bart@jukie.net>
Date: Fri, 19 Jan 2007 02:39:27 +0000
Subject: [PATCH] Don't call fstat() on stdin in index-pack.

This fixes a issues with a large git-push with a
32bit git on a 64bit kernel.

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---
 index-pack.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index 72e0962..e7870a9 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -455,10 +455,12 @@ static void parse_pack_objects(unsigned char *sha1)
 	use(20);
 
 	/* If input_fd is a file, we should have reached its end now. */
-	if (fstat(input_fd, &st))
-		die("cannot fstat packfile: %s", strerror(errno));
-	if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
-		die("pack has junk at the end");
+        if (input_fd) {
+                if (fstat(input_fd, &st))
+                        die("cannot fstat packfile: %s", strerror(errno));
+                if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
+                        die("pack has junk at the end");
+        }
 
 	if (!nr_deltas)
 		return;
-- 
1.5.0.rc1.gdf1b-dirty

Re: [PATCH] Don't call fstat() on stdin in index-pack.

[Simon 'corecode' Schubert]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Bart Trojanowski wrote:
>  	/* If input_fd is a file, we should have reached its end now. */
> -	if (fstat(input_fd, &st))
> -		die("cannot fstat packfile: %s", strerror(errno));
> -	if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
> -		die("pack has junk at the end");
> +        if (input_fd) {
> +                if (fstat(input_fd, &st))
> +                        die("cannot fstat packfile: %s", strerror(errno));
> +                if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
> +                        die("pack has junk at the end");
> +        }

This is clearly the wrong fix.  input_fd being 0 doesn't mean that it is *not* a regular file.  Only doing a fstat can tell.  You are simply hiding your real issue there, which is that you can't fstat on a pipe or whatever input_fd is.

The problem here is that your 64bit kernel can't fit the data into your struct stat provided by your 32bit libc.  Not a problem of git.  However, it would be interesting to know what exactly produces the EOVERFLOW.

cheers
  simon

-- 
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: git push problem with v1.5.0-rc1

[Bart Trojanowski]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

Thanks, Simon.

* Bart Trojanowski <bart@jukie.net> [070118 16:26]:
>       fatal: cannot fstat packfile: Value too large for defined data type 
>       unpack index-pack exited with error code 

Interestingly enough upgrading to 2.6.20-rc (from 2.6.19) fixed the real
problem.

-Bart

-- 
				WebSig: http://www.jukie.net/~bart/sig/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Don't call fstat() on stdin in index-pack.

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

On Fri, 19 Jan 2007 04:02:42 +0100 Simon 'corecode' Schubert wrote:

> Bart Trojanowski wrote:
> >  	/* If input_fd is a file, we should have reached its end now. */
> > -	if (fstat(input_fd, &st))
> > -		die("cannot fstat packfile: %s", strerror(errno));
> > -	if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
> > -		die("pack has junk at the end");
> > +        if (input_fd) {
> > +                if (fstat(input_fd, &st))
> > +                        die("cannot fstat packfile: %s", strerror(errno));
> > +                if (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)
> > +                        die("pack has junk at the end");
> > +        }
>
> This is clearly the wrong fix.  input_fd being 0 doesn't mean that
> it is *not* a regular file.  Only doing a fstat can tell.  You are
> simply hiding your real issue there, which is that you can't fstat
> on a pipe or whatever input_fd is.
>
> The problem here is that your 64bit kernel can't fit the data into
> your struct stat provided by your 32bit libc.  Not a problem of git.
> However, it would be interesting to know what exactly produces the
> EOVERFLOW.

Most likely it is the st_ino field - the kernel assigns unique inode
numbers for pipes from an "unsigned long" counter, which is 64-bit in
this case, and *stat() calls must fail with EOVERFLOW if the inode
number does not fit into ino_t, which is 32-bit here.  This problem is
known for some time, and there is even a kernel patch proposed as a
workaround (which makes the counter 32-bit):

http://permalink.gmane.org/gmane.linux.file-systems/12526

AFAIK, that patch is not upstream yet - so upgrading the kernel did
not really fix the issue, it will appear again once the system will
use more than 4G of pipe inodes.

Compiling git with -D_FILE_OFFSET_BITS=64 will make ino_t 64-bit and
therefore will fix the problem (however, I'm not sure whether the git
code is ready for this).

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Don't call fstat() on stdin in index-pack.

[Simon 'corecode' Schubert]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Sergey Vlasov wrote:
> Compiling git with -D_FILE_OFFSET_BITS=64 will make ino_t 64-bit and
> therefore will fix the problem (however, I'm not sure whether the git
> code is ready for this).

It better be :)  My OS of choice has 64bit off_t and ino_t since a long time now...

cheers
  simon

-- 
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]