{"thread":{"id":"13216","subject":"Probably a GIT bug..","startedAt":"2008-04-22T14:43:11Z","lastAt":"2008-04-23T17:13:55Z","messageCount":4,"participants":["Tomasz bla Fortuna","Junio C Hamano","Daniel Barkalow","Dmitry Potapov"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"74975","messageId":"20080422164311.67c3d02b@Vorago","threadId":"13216","inReplyTo":null,"subject":"Probably a GIT bug..","fromName":"Tomasz bla Fortuna","fromEmail":"bla@thera.be","sentAt":"2008-04-22T14:43:11Z","receivedAt":"2008-04-22T14:43:11Z","isPatch":false,"sender":{"key":"bla@thera.be","avatar":null},"body":"Hello,\n\nTestcase: http://temp.thera.be/gitbug.tar\nContents:\ngitbug/\ngitbug/prep.sh\ngitbug/fail.sh\ngitbug/init.sh\ngitbug/README\n\n\n#v+ init.sh\n\n#!/bin/bash\n\nexport GIT_AUTHOR_EMAIL='test@test.be'\nexport GIT_AUTHOR_NAME='gitbug'\nexport GIT_AUTHOR_DATE=\"Mon Apr 21 16:40:08 2008 +0200\"\nexport GIT_COMMITTER_NAME='gitbug'\nexport GIT_COMMITTER_EMAIL='test@test.be'\nexport GIT_COMMITTER_DATE=\"Mon Apr 21 16:40:08 2008 +0200\"\n\nrm -rf ./.git\nrm -f test test2\n\ngit init\ntouch test\ngit add test\ngit commit -a -m \"first commit\"\n\n## prep.sh:\n#!/bin/bash\nmkdir .git/objects/a2\nchmod a-rwx .git/objects/a2 \n\n## fail.sh:\n#!/bin/bash\n\ntouch test2\ngit add test2\ngit commit -a -m \"second commit\"\n\necho -en \"\\n\\n\\nSee the result of git-fsck:\\n\" \ngit-fsck\n\n#v-\n\nExecution:\n#v+\n[16:39:14] bla@Vorago ~/my_tmp/gitbug $ source init.sh \nInitialized empty Git repository in .git/\nCreated initial commit 2231f53: first commit\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 test\n[16:39:16] bla@Vorago ~/my_tmp/gitbug $ source prep.sh \n[16:39:17] bla@Vorago ~/my_tmp/gitbug $ source fail.sh \nerror: sha1\nfile /home/bla/my_tmp/gitbug/.git/objects/a2/2c62298732a162ab9aa64c31b24c4c87cf8cd9:\nPermission denied\n\nfatal: unable to read destination tree\n(a12d0088c0e538480a1586a9ac2d5de3b54b2759) Created commit \n\n\nSee the result of git-fsck:\nbroken link from  commit a12d0088c0e538480a1586a9ac2d5de3b54b2759\n              to    tree a22c62298732a162ab9aa64c31b24c4c87cf8cd9\nmissing tree a22c62298732a162ab9aa64c31b24c4c87cf8cd9\n[16:39:19] bla@Vorago ~/my_tmp/gitbug $ \n#v-\n\nAs I understand such behaviour shouldn't have place; one can fix it by\nfixing attributes and then doing git-reset+commit. Maybe with fsck also.\n\nI guess that GIT should check if he can create a tree link before\ncreating a commit.\n\nSee you,\n-- \nTomasz bla Fortuna\njid: bla(at)af.gliwice.pl\npgp: 0x90746E79 @ pgp.mit.edu\nwww: http://bla.thera.be\n"},{"id":"75073","messageId":"7v63u8tt62.fsf@gitster.siamese.dyndns.org","threadId":"13216","inReplyTo":"20080422164311.67c3d02b@Vorago","subject":"Re: Probably a GIT bug..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-23T16:47:17Z","receivedAt":"2008-04-23T16:47:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tomasz bla Fortuna <bla@thera.be> writes:\n\n> I guess that GIT should check if he can create a tree link before\n> creating a commit.\n\nThanks.\n\nThat's a somewhat contrived example, and I do not know how you found it.\n\nI suspect the same breakage would trigger when you ran out of disk quota\nto write out the tree while you still have barely enough quota to create\nthe commit and update the ref, which would result in corrupt repository.\nSo in that sense, this breakage is more likely to bite people in reality\nthan the initial reaction your test may invite which is \"don't do that\nthen\".\n\nHere is a possible fix.\n\n-- >8 --\nwrite-tree: properly detect failure to write tree objects\n\nTomasz Fortuna reported that \"git commit\" does not error out properly when\nit cannot write tree objects out.  \"git write-tree\" shares the same issue,\nas the failure to notice the error is deep in the logic to write tree\nobjects out recursively.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n cache-tree.c          |    7 +++-\n t/t0004-unwritable.sh |   67 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 72 insertions(+), 2 deletions(-)\n\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 39da54d..73cb340 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -341,8 +341,11 @@ static int update_one(struct cache_tree *it,\n \n \tif (dryrun)\n \t\thash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);\n-\telse\n-\t\twrite_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);\n+\telse if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {\n+\t\tstrbuf_release(&buffer);\n+\t\treturn -1;\n+\t}\n+\n \tstrbuf_release(&buffer);\n \tit->entry_count = i;\n #if DEBUG\ndiff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh\nnew file mode 100755\nindex 0000000..b355a8f\n--- /dev/null\n+++ b/t/t0004-unwritable.sh\n@@ -0,0 +1,67 @@\n+#!/bin/sh\n+\n+test_description='detect unwritable repository and fail correctly'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\n+\t>file &&\n+\tgit add file &&\n+\tgit commit -m initial &&\n+\techo >file &&\n+\tgit add file\n+\t\t\n+'\n+\n+test_expect_success 'write-tree should notice unwritable repository' '\n+\n+\t(\n+\t\tchmod a-w .git/objects\n+\t\ttest_must_fail git write-tree\n+\t)\n+\tstatus=$?\n+\tchmod 775 .git/objects\n+\t(exit $status)\n+\n+'\n+\n+test_expect_success 'commit should notice unwritable repository' '\n+\n+\t(\n+\t\tchmod a-w .git/objects\n+\t\ttest_must_fail git commit -m second\n+\t)\n+\tstatus=$?\n+\tchmod 775 .git/objects\n+\t(exit $status)\n+\n+'\n+\n+test_expect_success 'update-index should notice unwritable repository' '\n+\n+\t(\n+\t\techo a >file &&\n+\t\tchmod a-w .git/objects\n+\t\ttest_must_fail git update-index file\n+\t)\n+\tstatus=$?\n+\tchmod 775 .git/objects\n+\t(exit $status)\n+\n+'\n+\n+test_expect_success 'add should notice unwritable repository' '\n+\n+\t(\n+\t\techo b >file &&\n+\t\tchmod a-w .git/objects\n+\t\ttest_must_fail git add file\n+\t)\n+\tstatus=$?\n+\tchmod 775 .git/objects\n+\t(exit $status)\n+\n+'\n+\n+test_done\n"},{"id":"75077","messageId":"alpine.LNX.1.00.0804231257300.19665@iabervon.org","threadId":"13216","inReplyTo":"7v63u8tt62.fsf@gitster.siamese.dyndns.org","subject":"Re: Probably a GIT bug..","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-04-23T17:10:42Z","receivedAt":"2008-04-23T17:10:42Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 23 Apr 2008, Junio C Hamano wrote:\n\n> Tomasz bla Fortuna <bla@thera.be> writes:\n> \n> > I guess that GIT should check if he can create a tree link before\n> > creating a commit.\n> \n> Thanks.\n> \n> That's a somewhat contrived example, and I do not know how you found it.\n> \n> I suspect the same breakage would trigger when you ran out of disk quota\n> to write out the tree while you still have barely enough quota to create\n> the commit and update the ref, which would result in corrupt repository.\n> So in that sense, this breakage is more likely to bite people in reality\n> than the initial reaction your test may invite which is \"don't do that\n> then\".\n\nmake\nsu\nmake install\ngit commit\n\nand then log out from your desktop environment, and you've got objects \nsubdirectories owned by root without realizing. Of course, you shouldn't \ndo that, either, but it's an easier mistake than accidentally removing \nyour own write permission to something.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"75078","messageId":"20080423171355.GE26950@dpotapov.dyndns.org","threadId":"13216","inReplyTo":"7v63u8tt62.fsf@gitster.siamese.dyndns.org","subject":"Re: Probably a GIT bug..","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-04-23T17:13:55Z","receivedAt":"2008-04-23T17:13:55Z","isPatch":false,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Wed, Apr 23, 2008 at 09:47:17AM -0700, Junio C Hamano wrote:\n> \n> That's a somewhat contrived example, and I do not know how you found it.\n> \n> I suspect the same breakage would trigger when you ran out of disk quota\n> to write out the tree while you still have barely enough quota to create\n> the commit and update the ref, which would result in corrupt repository.\n\nI have looked at write_sha1_file(), it appears to me that if you do not\nhave enough disk space (or exceed your disk quote) then this function\nwill die inside. For the described situation to happen, you probably\nshould have wrong permissions on directories (as it was simulated in the\nbug report) or some other error with open() (such as exceeding the maximum\nallowed number of descriptor).\n\nSo I wonder why errors in write_sha1_file() are treated differently,\ni.e. the function may die on error in some error cases (such as not\nenough disk space), but it indicates the error situation by return code\nif open() failed.\n\nBTW, the returned code of write_sha1_file() is also not checked in\nwrite_tree() in mktree.c.\n\nDmitry\n"}]}