# Probably a GIT bug.. 4 messages from 2008-04-22 to 2008-04-23. Participants: Tomasz bla Fortuna, Junio C Hamano, Daniel Barkalow, Dmitry Potapov. Thread: https://gitlist.dev/t/13216 ## Tomasz bla Fortuna, 2008-04-22 14:43 Subject: Probably a GIT bug.. Message-ID: <20080422164311.67c3d02b@Vorago> URL: https://gitlist.dev/e/20080422164311.67c3d02b%40Vorago ``` Hello, Testcase: http://temp.thera.be/gitbug.tar Contents: gitbug/ gitbug/prep.sh gitbug/fail.sh gitbug/init.sh gitbug/README #v+ init.sh #!/bin/bash export GIT_AUTHOR_EMAIL='test@test.be' export GIT_AUTHOR_NAME='gitbug' export GIT_AUTHOR_DATE="Mon Apr 21 16:40:08 2008 +0200" export GIT_COMMITTER_NAME='gitbug' export GIT_COMMITTER_EMAIL='test@test.be' export GIT_COMMITTER_DATE="Mon Apr 21 16:40:08 2008 +0200" rm -rf ./.git rm -f test test2 git init touch test git add test git commit -a -m "first commit" ## prep.sh: #!/bin/bash mkdir .git/objects/a2 chmod a-rwx .git/objects/a2 ## fail.sh: #!/bin/bash touch test2 git add test2 git commit -a -m "second commit" echo -en "\n\n\nSee the result of git-fsck:\n" git-fsck #v- Execution: #v+ [16:39:14] bla@Vorago ~/my_tmp/gitbug $ source init.sh Initialized empty Git repository in .git/ Created initial commit 2231f53: first commit 0 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 test [16:39:16] bla@Vorago ~/my_tmp/gitbug $ source prep.sh [16:39:17] bla@Vorago ~/my_tmp/gitbug $ source fail.sh error: sha1 file /home/bla/my_tmp/gitbug/.git/objects/a2/2c62298732a162ab9aa64c31b24c4c87cf8cd9: Permission denied fatal: unable to read destination tree (a12d0088c0e538480a1586a9ac2d5de3b54b2759) Created commit See the result of git-fsck: broken link from commit a12d0088c0e538480a1586a9ac2d5de3b54b2759 to tree a22c62298732a162ab9aa64c31b24c4c87cf8cd9 missing tree a22c62298732a162ab9aa64c31b24c4c87cf8cd9 [16:39:19] bla@Vorago ~/my_tmp/gitbug $ #v- As I understand such behaviour shouldn't have place; one can fix it by fixing attributes and then doing git-reset+commit. Maybe with fsck also. I guess that GIT should check if he can create a tree link before creating a commit. See you, -- Tomasz bla Fortuna jid: bla(at)af.gliwice.pl pgp: 0x90746E79 @ pgp.mit.edu www: http://bla.thera.be ``` ## Junio C Hamano, 2008-04-23 16:47 Subject: Re: Probably a GIT bug.. Message-ID: <7v63u8tt62.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63u8tt62.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20080422164311.67c3d02b@Vorago> ``` Tomasz bla Fortuna writes: > I guess that GIT should check if he can create a tree link before > creating a commit. Thanks. That's a somewhat contrived example, and I do not know how you found it. I suspect the same breakage would trigger when you ran out of disk quota to write out the tree while you still have barely enough quota to create the commit and update the ref, which would result in corrupt repository. So in that sense, this breakage is more likely to bite people in reality than the initial reaction your test may invite which is "don't do that then". Here is a possible fix. -- >8 -- write-tree: properly detect failure to write tree objects Tomasz Fortuna reported that "git commit" does not error out properly when it cannot write tree objects out. "git write-tree" shares the same issue, as the failure to notice the error is deep in the logic to write tree objects out recursively. Signed-off-by: Junio C Hamano --- cache-tree.c | 7 +++- t/t0004-unwritable.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 39da54d..73cb340 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -341,8 +341,11 @@ static int update_one(struct cache_tree *it, if (dryrun) hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1); - else - write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1); + else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) { + strbuf_release(&buffer); + return -1; + } + strbuf_release(&buffer); it->entry_count = i; #if DEBUG diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh new file mode 100755 index 0000000..b355a8f --- /dev/null +++ b/t/t0004-unwritable.sh @@ -0,0 +1,67 @@ +#!/bin/sh + +test_description='detect unwritable repository and fail correctly' + +. ./test-lib.sh + +test_expect_success setup ' + + >file && + git add file && + git commit -m initial && + echo >file && + git add file + +' + +test_expect_success 'write-tree should notice unwritable repository' ' + + ( + chmod a-w .git/objects + test_must_fail git write-tree + ) + status=$? + chmod 775 .git/objects + (exit $status) + +' + +test_expect_success 'commit should notice unwritable repository' ' + + ( + chmod a-w .git/objects + test_must_fail git commit -m second + ) + status=$? + chmod 775 .git/objects + (exit $status) + +' + +test_expect_success 'update-index should notice unwritable repository' ' + + ( + echo a >file && + chmod a-w .git/objects + test_must_fail git update-index file + ) + status=$? + chmod 775 .git/objects + (exit $status) + +' + +test_expect_success 'add should notice unwritable repository' ' + + ( + echo b >file && + chmod a-w .git/objects + test_must_fail git add file + ) + status=$? + chmod 775 .git/objects + (exit $status) + +' + +test_done ``` ## Daniel Barkalow, 2008-04-23 17:10 Subject: Re: Probably a GIT bug.. Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0804231257300.19665%40iabervon.org In-Reply-To: <7v63u8tt62.fsf@gitster.siamese.dyndns.org> ``` On Wed, 23 Apr 2008, Junio C Hamano wrote: > Tomasz bla Fortuna writes: > > > I guess that GIT should check if he can create a tree link before > > creating a commit. > > Thanks. > > That's a somewhat contrived example, and I do not know how you found it. > > I suspect the same breakage would trigger when you ran out of disk quota > to write out the tree while you still have barely enough quota to create > the commit and update the ref, which would result in corrupt repository. > So in that sense, this breakage is more likely to bite people in reality > than the initial reaction your test may invite which is "don't do that > then". make su make install git commit and then log out from your desktop environment, and you've got objects subdirectories owned by root without realizing. Of course, you shouldn't do that, either, but it's an easier mistake than accidentally removing your own write permission to something. -Daniel *This .sig left intentionally blank* ``` ## Dmitry Potapov, 2008-04-23 17:13 Subject: Re: Probably a GIT bug.. Message-ID: <20080423171355.GE26950@dpotapov.dyndns.org> URL: https://gitlist.dev/e/20080423171355.GE26950%40dpotapov.dyndns.org In-Reply-To: <7v63u8tt62.fsf@gitster.siamese.dyndns.org> ``` On Wed, Apr 23, 2008 at 09:47:17AM -0700, Junio C Hamano wrote: > > That's a somewhat contrived example, and I do not know how you found it. > > I suspect the same breakage would trigger when you ran out of disk quota > to write out the tree while you still have barely enough quota to create > the commit and update the ref, which would result in corrupt repository. I have looked at write_sha1_file(), it appears to me that if you do not have enough disk space (or exceed your disk quote) then this function will die inside. For the described situation to happen, you probably should have wrong permissions on directories (as it was simulated in the bug report) or some other error with open() (such as exceeding the maximum allowed number of descriptor). So I wonder why errors in write_sha1_file() are treated differently, i.e. the function may die on error in some error cases (such as not enough disk space), but it indicates the error situation by return code if open() failed. BTW, the returned code of write_sha1_file() is also not checked in write_tree() in mktree.c. Dmitry ```