# git 1.5.1-rc1 doesn't like empty files 18 messages from 2007-03-20 to 2007-03-20. Participants: Pavel Roskin, Nicolas Vilz, Linus Torvalds, Junio C Hamano, Shawn O. Pearce, Alexander Litvinov, Andy Parkins. Thread: https://gitlist.dev/t/7324 ## Pavel Roskin, 2007-03-20 03:30 Subject: git 1.5.1-rc1 doesn't like empty files Message-ID: <1174361424.3143.42.camel@dv> URL: https://gitlist.dev/e/1174361424.3143.42.camel%40dv ``` Hello! I don't know where this problem appeared, but it present in the current git (1.5.1-rc1). Empty files become invalid objects in the repository: $ touch file $ git-init Initialized empty Git repository in .git/ $ git-add file $ git-commit -m "first commit" Created initial commit 16a476808d3cb0a4758997ba58193a9dcfad0fd8 error: garbage at end of loose object 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' 0 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 file $ A file with a one newline is OK (replace "touch file with "echo >file" and the error will go away). This is Linux, Fedora Development, i386. The testsuite fails at test 7 in t9200-git-cvsexportcommit.sh, but it seems to be unrelated. -- Regards, Pavel Roskin ``` ## Nicolas Vilz, 2007-03-20 04:24 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <20070320042438.GA31795@hermes.lan.home.vilz.de> URL: https://gitlist.dev/e/20070320042438.GA31795%40hermes.lan.home.vilz.de In-Reply-To: <1174361424.3143.42.camel@dv> ``` On Mon, Mar 19, 2007 at 11:30:24PM -0400, Pavel Roskin wrote: > Hello! > > I don't know where this problem appeared, but it present in the current > git (1.5.1-rc1). Empty files become invalid objects in the repository: > > $ touch file > $ git-init > Initialized empty Git repository in .git/ > $ git-add file > $ git-commit -m "first commit" > Created initial commit 16a476808d3cb0a4758997ba58193a9dcfad0fd8 > error: garbage at end of loose object > 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' > 0 files changed, 0 insertions(+), 0 deletions(-) > create mode 100644 file > $ I get the following after the last command: Created initial commit a0e02efbe85ee1c1f4b8ba703ccae2f4e64e6ed0 0 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 file I use git version 1.5.1.rc1.595.gd1206 on a current gentoo system (Portage 2.1.2.2 (default-linux/amd64/2006.1/desktop, gcc-4.1.2, glibc-2.5-r1, 2.6.20-gentoo-r3 x86_64)) with a not so current portage tree (Timestamp of tree: Sat, 17 Mar 2007 12:00:01 +0000). in a few hours, i can test that on ppc32 arch, too, if you like. Regards, Nicolas Vilz ``` ## Linus Torvalds, 2007-03-20 04:54 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703192148430.6730%40woody.linux-foundation.org In-Reply-To: <1174361424.3143.42.camel@dv> ``` On Mon, 19 Mar 2007, Pavel Roskin wrote: > > I don't know where this problem appeared, but it present in the current > git (1.5.1-rc1). Empty files become invalid objects in the repository: Hmm.. Not for me. Can you bisect when it started happening? > This is Linux, Fedora Development, i386. FC6, tested both i386 and x86-64. I wonder if this is a zlib issue. Although I seem to have libz-1.2.3 on both machines, and that should be the most recent version. I don't see Fedora development having anything else.. Linus ``` ## Pavel Roskin, 2007-03-20 05:08 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <1174367312.3143.75.camel@dv> URL: https://gitlist.dev/e/1174367312.3143.75.camel%40dv In-Reply-To: ``` On Mon, 2007-03-19 at 21:54 -0700, Linus Torvalds wrote: > > On Mon, 19 Mar 2007, Pavel Roskin wrote: > > > > I don't know where this problem appeared, but it present in the current > > git (1.5.1-rc1). Empty files become invalid objects in the repository: > > Hmm.. Not for me. Can you bisect when it started happening? I was doing exactly that, and here's the result: 7efbff7531af4281487d54c1dc1401308d988e33 is first bad commit commit 7efbff7531af4281487d54c1dc1401308d988e33 Author: Junio C Hamano Date: Mon Mar 5 00:21:37 2007 -0800 unpack_sha1_file(): detect corrupt loose object files. We did not detect broken loose object files, either when underlying inflate() signalled the breakage, nor inflate() finished and we had garbage trailing at the end. We do better now. We also make unpack_sha1_file() a static function to sha1_file.c, since it is not used by anybody outside. Signed-off-by: Junio C Hamano :100644 100644 c291163e6d2096181ddf89954d2d65953d1ba687 4b5a7541a86c488f623793fdc498d0e149c40439 M cache.h :100644 100644 6d0a72ed093d353a672129f7e460d0c1015212d7 ac6b5e00b6dec913a39cc54e84829dc3fc6c782f M sha1_file.c .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 is the same 9 bytes: 30 78 9c 03 00 00 00 00 01 But it's considered corrupt by the current git. > I wonder if this is a zlib issue. Although I seem to have libz-1.2.3 on > both machines, and that should be the most recent version. I don't see > Fedora development having anything else.. That's also 1.2.3. I can reproduce the issue on another machine running Fedora Core 6 i386 with all updates, also with zlib 1.2.3. -- Regards, Pavel Roskin ``` ## Junio C Hamano, 2007-03-20 05:29 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <7vslc0bhz7.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vslc0bhz7.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <1174361424.3143.42.camel@dv> ``` Pavel Roskin writes: > Hello! > > I don't know where this problem appeared, but it present in the current > git (1.5.1-rc1). Empty files become invalid objects in the repository: > > $ touch file > $ git-init > Initialized empty Git repository in .git/ > $ git-add file > $ git-commit -m "first commit" > Created initial commit 16a476808d3cb0a4758997ba58193a9dcfad0fd8 > error: garbage at end of loose object > 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' If the error message above is linewrapped by e-mail, I think it is coming from here: static void *unpack_sha1_rest(z_stream *stream, void *buffer,... { ... if (bytes < size) { stream->next_out = buf + bytes; stream->avail_out = size - bytes; while (status == Z_OK) status = inflate(stream, Z_FINISH); } buf[size] = 0; if ((status == Z_OK || status == Z_STREAM_END) && !stream->avail_in) { inflateEnd(stream); return buf; } if (status < 0) error("corrupt loose object '%s'", sha1_to_hex(sha1)); else if (stream->avail_in) error("garbage at end of loose object '%s'", sha1_to_hex(sha1)); free(buf); return NULL; } Can you check what the value of the status is at that point? ``` ## Linus Torvalds, 2007-03-20 05:41 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703192237100.6730%40woody.linux-foundation.org In-Reply-To: <1174367312.3143.75.camel@dv> ``` On Tue, 20 Mar 2007, Pavel Roskin wrote: > > .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 is the same 9 > bytes: 30 78 9c 03 00 00 00 00 01 Ahh.. You have [core] legacyheaders = false don't you? If you didn't, you should see a 15-byte object, not a 9-byte one. And yes, I can reproduce this with that "core.legacyheaders=false" setting. It seems that config option is simply broken, and we never noticed, because almost nobody uses it. Alexander - do you happen to have that "legacyheaders" setting too? Maybe that explains your pack corruption? Linus ``` ## Pavel Roskin, 2007-03-20 05:47 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <1174369676.8210.5.camel@dv> URL: https://gitlist.dev/e/1174369676.8210.5.camel%40dv In-Reply-To: <7vslc0bhz7.fsf@assigned-by-dhcp.cox.net> ``` On Mon, 2007-03-19 at 22:29 -0700, Junio C Hamano wrote: > if (status < 0) > error("corrupt loose object '%s'", > sha1_to_hex(sha1)); > else if (stream->avail_in) > error("garbage at end of loose object '%s'", > sha1_to_hex(sha1)); > free(buf); > return NULL; > } > > Can you check what the value of the status is at that point? status is 0 (Z_OK), stream->avail_in is 8. I changed the code to print them from the error() that prints "garbage at end of loose object". -- Regards, Pavel Roskin ``` ## Linus Torvalds, 2007-03-20 05:49 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703192245490.6730%40woody.linux-foundation.org In-Reply-To: <7vslc0bhz7.fsf@assigned-by-dhcp.cox.net> ``` On Mon, 19 Mar 2007, Junio C Hamano wrote: > > If the error message above is linewrapped by e-mail, I think it > is coming from here: I think I found it. The thing is, if the output buffer is empty, we should *still* actually use the zlib routines to *unpack* that empty output buffer. But we had a test that said "only unpack if we still expect more output". So we wouldn't use up all the zlib stream, because we felt that we didn't need it, because we already had all the bytes we wanted. And it was "true": we did have all the output data. We just needed to also eat all the input data! We've had this bug before - thinking that we don't need to inflate() anything because we already had it all.. Linus --- sha1_file.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index b0b2177..c0efed3 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1030,7 +1030,7 @@ static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size n = size; memcpy(buf, (char *) buffer + bytes, n); bytes = n; - if (bytes < size) { + if (bytes <= size) { stream->next_out = buf + bytes; stream->avail_out = size - bytes; while (status == Z_OK) ``` ## Pavel Roskin, 2007-03-20 05:50 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <1174369838.8210.9.camel@dv> URL: https://gitlist.dev/e/1174369838.8210.9.camel%40dv In-Reply-To: ``` On Mon, 2007-03-19 at 22:41 -0700, Linus Torvalds wrote: > > On Tue, 20 Mar 2007, Pavel Roskin wrote: > > > > .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 is the same 9 > > bytes: 30 78 9c 03 00 00 00 00 01 > > Ahh.. You have > > [core] > legacyheaders = false Yes. -- Regards, Pavel Roskin ``` ## Shawn O. Pearce, 2007-03-20 05:56 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <20070320055611.GD29288@spearce.org> URL: https://gitlist.dev/e/20070320055611.GD29288%40spearce.org In-Reply-To: ``` Linus Torvalds wrote: > On Tue, 20 Mar 2007, Pavel Roskin wrote: > > > > .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 is the same 9 > > bytes: 30 78 9c 03 00 00 00 00 01 > > Ahh.. You have > > [core] > legacyheaders = false > > don't you? If you didn't, you should see a 15-byte object, not a 9-byte > one. > > And yes, I can reproduce this with that "core.legacyheaders=false" > setting. It seems that config option is simply broken, and we never > noticed, because almost nobody uses it. For what it is worth, I have been running core.legacyheaders=false on both my PowerBook (my main dev system) and on my x86 Cygwin POS. I guess I've been lucky, as I've never noticed any sort of corruption. Oh, wait, yes I did. Just the other day. A loose object got the same zlib error as Pavel asked about. But git-prune whacked the damn thing. I figured it was just a short write by Cygwin during some sort of operation that I may have aborted; e.g. aborting an update-index and running it again later, thus never actually using that particular blob. I didn't think twice about the error (until now), especially since `git-fsck --full` did not whine after the corrupt loose object was gone. -- Shawn. ``` ## Junio C Hamano, 2007-03-20 06:00 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <7v648wbgiy.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v648wbgiy.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds writes: > We've had this bug before - thinking that we don't need to inflate() > anything because we already had it all.. > > Linus Thanks. I think we _do_ need a big fat warning near the code to avoid the same mistake in the future. Something like this? diff --git a/sha1_file.c b/sha1_file.c index 9fe2bd6..d273aff 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1030,7 +1030,17 @@ static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size n = size; memcpy(buf, (char *) buffer + bytes, n); bytes = n; - if (bytes < size) { + if (bytes <= size) { + /* + * The above condition must be (bytes <= size), not + * (bytes < size). In other words, even if we expect + * no more output, the input zlib stream may have bytes + * that express "this concludes the stream", and we do + * want to eat that input. Otherwise we would not be + * able to test that we consumed all the input to reach + * the expected size *and* zlib gave status == Z_STREAM_END + * to signal all went well. + */ stream->next_out = buf + bytes; stream->avail_out = size - bytes; while (status == Z_OK) ``` ## Junio C Hamano, 2007-03-20 06:05 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <7v1wjkbgaj.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1wjkbgaj.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7v648wbgiy.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano writes: > Linus Torvalds writes: > >> We've had this bug before - thinking that we don't need to inflate() >> anything because we already had it all.. >> >> Linus > > Thanks. I think we _do_ need a big fat warning near the code to > avoid the same mistake in the future... By the way, I think the test that comes after the part you fixed is wrong (I know it is my bad without running git-blame). Since we are making sure that we eat everything, we should expect Z_STREAM_END and no avail_in. diff --git a/sha1_file.c b/sha1_file.c index d7dc80d..7dc16ea 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1037,7 +1037,7 @@ static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size status = inflate(stream, Z_FINISH); } buf[size] = 0; - if ((status == Z_OK || status == Z_STREAM_END) && !stream->avail_in) { + if (status == Z_STREAM_END && !stream->avail_in) { inflateEnd(stream); return buf; } ``` ## Alexander Litvinov, 2007-03-20 07:04 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <200703201304.05902.litvinov2004@gmail.com> URL: https://gitlist.dev/e/200703201304.05902.litvinov2004%40gmail.com In-Reply-To: ``` В сообщении от Tuesday 20 March 2007 11:41 Linus Torvalds написал: > Alexander - do you happen to have that "legacyheaders" setting too? Maybe > that explains your pack corruption? It seems no: $ git config core.legacyheaders $ git config -l user.name=Alexander Litvinov user.email=XXX core.logallrefupdates=true core.filemode=false core.autocrlf=true diff.color=auto status.color=auto apply.whitespace=strip core.repositoryformatversion=0 core.filemode=false core.bare=false remote.origin.url=/home/lan/src/XXX remote.origin.fetch=+refs/heads/*:refs/remotes/origin/* branch.master.remote=origin branch.master.merge=refs/heads/master branch.XXX.remote=origin branch.XXX.merge=refs/heads/XXX ``` ## Andy Parkins, 2007-03-20 08:43 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <200703200843.51473.andyparkins@gmail.com> URL: https://gitlist.dev/e/200703200843.51473.andyparkins%40gmail.com In-Reply-To: ``` On Tuesday 2007 March 20 05:41, Linus Torvalds wrote: > [core] > legacyheaders = false > noticed, because almost nobody uses it. I'm not sure that's going to be true for long - the 1.5.0 release notes recommended setting it (assuming you didn't need backward compatibility) - which is exactly what I (and I'm sure others) did. Andy -- Dr Andy Parkins, M Eng (hons), MIET andyparkins@gmail.com ``` ## Junio C Hamano, 2007-03-20 08:49 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <7vbqio7100.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vbqio7100.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <200703200843.51473.andyparkins@gmail.com> ``` Andy Parkins writes: > On Tuesday 2007 March 20 05:41, Linus Torvalds wrote: > >> [core] >> legacyheaders = false >> noticed, because almost nobody uses it. > > I'm not sure that's going to be true for long - the 1.5.0 release notes > recommended setting it (assuming you didn't need backward compatibility) - > which is exactly what I (and I'm sure others) did. Well, it is fixed in 'master' to be in -rc2, and that validation does not exist in 'maint', so no harm is done. ``` ## Andy Parkins, 2007-03-20 09:26 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: <200703200926.05176.andyparkins@gmail.com> URL: https://gitlist.dev/e/200703200926.05176.andyparkins%40gmail.com In-Reply-To: <7vbqio7100.fsf@assigned-by-dhcp.cox.net> ``` On Tuesday 2007 March 20 08:49, Junio C Hamano wrote: > >> noticed, because almost nobody uses it. > > > > I'm not sure that's going to be true for long - the 1.5.0 release notes > > recommended setting it (assuming you didn't need backward compatibility) > > - which is exactly what I (and I'm sure others) did. > > Well, it is fixed in 'master' to be in -rc2, and that validation > does not exist in 'maint', so no harm is done. It wasn't the presence of the bug I was highlighting - it was the idea that nobody uses that option. Andy -- Dr Andy Parkins, M Eng (hons), MIET andyparkins@gmail.com ``` ## Linus Torvalds, 2007-03-20 15:45 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703200843270.6730%40woody.linux-foundation.org In-Reply-To: <200703200926.05176.andyparkins@gmail.com> ``` On Tue, 20 Mar 2007, Andy Parkins wrote: > On Tuesday 2007 March 20 08:49, Junio C Hamano wrote: > > > >> noticed, because almost nobody uses it. > > > > > > I'm not sure that's going to be true for long - the 1.5.0 release notes > > > recommended setting it (assuming you didn't need backward compatibility) > > > - which is exactly what I (and I'm sure others) did. > > > > Well, it is fixed in 'master' to be in -rc2, and that validation > > does not exist in 'maint', so no harm is done. > > It wasn't the presence of the bug I was highlighting - it was the idea that > nobody uses that option. Yeah, I'm actually happy that people seem to be using it, and it looks like the bug really was totally harmless apart from triggering a bogus error (ie it would never have corrupted anything, it just triggered an error that it shouldn't have). And I think the only thing that could ever trigger it was really that special case of a zero-sized blob (or maybe an empty tree, but git doesn't generate those natively unless you do magic stuff by hand). Linus ``` ## Linus Torvalds, 2007-03-20 15:46 Subject: Re: git 1.5.1-rc1 doesn't like empty files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0703200846110.6730%40woody.linux-foundation.org In-Reply-To: <7v1wjkbgaj.fsf@assigned-by-dhcp.cox.net> ``` On Mon, 19 Mar 2007, Junio C Hamano wrote: > > By the way, I think the test that comes after the part you fixed > is wrong (I know it is my bad without running git-blame). Since > we are making sure that we eat everything, we should expect > Z_STREAM_END and no avail_in. Ack. Linus ```