{"thread":{"id":"9536","subject":"git-commit goes awry after git-add -u","startedAt":"2007-08-15T14:16:59Z","lastAt":"2007-08-16T00:15:44Z","messageCount":6,"participants":["Salikh Zakirov","Junio C Hamano","Zakirov Salikh"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"50777","messageId":"f9v1t6$uph$1@sea.gmane.org","threadId":"9536","inReplyTo":null,"subject":"git-commit goes awry after git-add -u","fromName":"Salikh Zakirov","fromEmail":"salikh@gmail.com","sentAt":"2007-08-15T14:16:59Z","receivedAt":"2007-08-15T14:16:59Z","isPatch":false,"sender":{"key":"salikh@gmail.com","avatar":"https://gravatar.com/avatar/952c102bb1dcf721dab8de4f5a11d276756a65d301d021f755e265cc3251efae?d=mp&s=160"},"body":"Hi,\n\nI have observed incorret behaviour of git-commit after git-add -u, where\nit records deletions of files not related to the files touched by commit.\n\nUnfortunately, I was not able to create a small reproducer, and so\ndescribing the reproducer with the codebase I've encountered this.\nFortunately, it's an open-source project, so I've uploaded the tree\nto repo.or.cz.\n\nI've reproduced the problem with both 1.5.3-rc5 and 1.5.2.4\n\nThe problematic commit is the topmost one, so it is needed to reset it\nto reproduce the bug. The commit moves files:\n\n vm/em/src/ia32 => vm/em/src/arch/ia32\n vm/vmcore/src/util/em64t => vm/em/src/arch/em64t\n vm/vmcore/src/util/ipf => vm/em/src/arch/ipf\n\nProblem description (short):\n\nSequence of commands\n git-add -u\n git-commit\nproduces severely damaged commit. If 'git-add -u' is changed\nto using git-add and git-rm, the commit is correct.\n\nHow to reproduce (long):\n\n$ git clone git://repo.or.cz/drlvm.git\n$ cd drlvm\n$ git reset HEAD^ \t# undo the problematic commit\n$ git status\t\t# all is good now\n...\n# Changed but not updated:\n...\n# deleted: vm/em/src/ia32/...\n...\n# deleted: vm/vmcore/src/util/em64t/...\n...\n# deleted: vm/vmcore/src/util/ipf/...\n...\n# Untracked files:\n...\n# vm/em/src/arch/\n\n$ git add vm/em/src/arch/\t# inform git about added files\n$ git status\t\t# still good...\n...\n# Changes to be committed:\n...\n# new file: vm/em/src/arch/...\n...\n# Changed but not updated:\n...\n# deleted: vm/em/src/ia32/...\n...\n# deleted: vm/vmcore/src/util/em64t/...\n...\n# deleted: vm/vmcore/src/util/ipf/...\n...\n$ git add -u\t\t# inform git about deleted files as well\n$ git status\t\t# output is okay, it looks like git picked up all of the\nmoves, but ...\n...\n# Changed but not updated:\n# renamed: vm/vmcore/src/util/em64t/... -> vm/em/src/arch/em64t/...\n...\n$ git diff\t\t# is empty as expected\n$ git diff -M --cached # shows a bunch of renames as expected\n$ git commit -m \"move\" # BANG! something gone wrong:\nCreated commit 64aed27: move\n 53 files changed, 23168 insertions(+), 5414 deletions(-)\n create mode 100644 vm/em/src/arch/...\n ...\t\t\t\t\t# file creations are okay\n delete mode 100644 vm/vmi/Makefile\t# these deletions are incorrect\n delete mode 100644 vm/vmi/src/j9vmls.cpp\n delete mode 100644 vm/vmi/src/vm_trace.h\n delete mode 100644 vm/vmi/src/vmi.cpp\n delete mode 100644 vm/vmi/src/vmi.exp\n delete mode 100644 vm/vmstart/src/compmgr/component_manager_impl.cpp\n delete mode 100644 vm/vmstart/src/compmgr/component_manager_impl.h\n\n$ git status\t\t# work dir is expected to be clean, but it is not\n# Changes to be committed:\n...\n# deleted: vm/vmcore/src/util/em64t/...\t\n...\n# new file: vm/vmi/Makefile\n# new file: vm/vmi/src/j9vmls.cpp\n# renamed:\nvm/vmcore/src/util/em64t/base_natives/java_lang_thread_em64t.cpp ->\nvm/vmi/src/vm_trace.h\n# new file: vm/vmi/src/vmi.cpp\n# new file: vm/vmi/src/vmi.exp\n# new file: vm/vmstart/src/compmgr/component_manager_impl.cpp\n# new file: vm/vmstart/src/compmgr/component_manager_impl.h\n\nSo it does not committed deletion of moved files, but instead recorded\ndeletions of some other arbitrary files, which in fact are still intact\nin the tree, and now are reported as new.\n\nAlternative sequence of commands, not involving 'git-add -u' produces\ncorrect results (without output for compactness)\n$ git add vm/em\n$ git rm -r vm/vmcore/src/util/ipf\n$ git rm -r vm/vmcore/src/util/em64t\n$ git rm -r vm/em/src/ia32\n$ git status\n# On branch master\nnothing to commit (working directory clean)\n"},{"id":"50778","messageId":"f9v266$uph$2@sea.gmane.org","threadId":"9536","inReplyTo":"f9v1t6$uph$1@sea.gmane.org","subject":"Re: git-commit goes awry after git-add -u","fromName":"Salikh Zakirov","fromEmail":"salikh@gmail.com","sentAt":"2007-08-15T14:21:48Z","receivedAt":"2007-08-15T14:21:48Z","isPatch":false,"sender":{"key":"salikh@gmail.com","avatar":"https://gravatar.com/avatar/952c102bb1dcf721dab8de4f5a11d276756a65d301d021f755e265cc3251efae?d=mp&s=160"},"body":"Salikh Zakirov wrote:\n> Alternative sequence of commands, not involving 'git-add -u' produces\n> correct results (without output for compactness)\n> $ git add vm/em\n> $ git rm -r vm/vmcore/src/util/ipf\n> $ git rm -r vm/vmcore/src/util/em64t\n> $ git rm -r vm/em/src/ia32\n\nSorry, forgot an important command in the sequence:\n$ git commit -m moved\n\n> $ git status\n> # On branch master\n> nothing to commit (working directory clean)\n"},{"id":"50816","messageId":"7v643gplph.fsf@gitster.siamese.dyndns.org","threadId":"9536","inReplyTo":"f9v1t6$uph$1@sea.gmane.org","subject":"Re: git-commit goes awry after git-add -u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-15T20:49:30Z","receivedAt":"2007-08-15T20:49:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Salikh Zakirov writes:\n\n> I have observed incorret behaviour of git-commit after git-add -u, where\n> it records deletions of files not related to the files touched by commit.\n\nDoes this fix the issue?\n\nIdeally remove_file_from_cache() should have the invalidate call\ninside just like add_file_to_cache() does, but there was a\ntechnical reason it couldn't that I do not recall offhand, so I\nam playing it safe here with this tentative patch to see if the\ncause is a cache-tree corruption.\n\n---\n builtin-add.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 1591171..a5fae7c 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -115,6 +115,7 @@ static void update_callback(struct diff_queue_struct *q,\n \t\t\tbreak;\n \t\tcase DIFF_STATUS_DELETED:\n \t\t\tremove_file_from_cache(path);\n+\t\t\tcache_tree_invalidate_path(active_cache_tree, path);\n \t\t\tif (verbose)\n \t\t\t\tprintf(\"remove '%s'\\n\", path);\n \t\t\tbreak;\n"},{"id":"50819","messageId":"7v1we4pknl.fsf_-_@gitster.siamese.dyndns.org","threadId":"9536","inReplyTo":"7v643gplph.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Fix \"git add -u\" data corruption.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-15T21:12:14Z","receivedAt":"2007-08-15T21:12:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This applies to 'maint' to fix a rather serious data corruption\nissue. When \"git add -u\" affects a subdirectory in such a way\nthat the only changes to its contents are path removals, the\nnext tree object written out of that index was bogus, as the\nremove codepath forgot to invalidate the cache-tree entry.\n\nKodos for noticing this breakage goes to Salikh Zakirov.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-add.c | 1 +\n t/t2200-add-update.sh | 59 +++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 44 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 1591171..a5fae7c 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -115,6 +115,7 @@ static void update_callback(struct diff_queue_struct *q,\n \t\t\tbreak;\n \t\tcase DIFF_STATUS_DELETED:\n \t\t\tremove_file_from_cache(path);\n+\t\t\tcache_tree_invalidate_path(active_cache_tree, path);\n \t\t\tif (verbose)\n \t\t\t\tprintf(\"remove '%s'\\n\", path);\n \t\t\tbreak;\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex 83005e7..4c7c6af 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -13,26 +13,53 @@ only the updates to dir/sub.'\n \n . ./test-lib.sh\n \n-test_expect_success 'setup' '\n-echo initial >top &&\n-mkdir dir &&\n-echo initial >dir/sub &&\n-git-add dir/sub top &&\n-git-commit -m initial &&\n-echo changed >top &&\n-echo changed >dir/sub &&\n-echo other >dir/other\n+test_expect_success setup '\n+\techo initial >check &&\n+\techo initial >top &&\n+\tmkdir dir1 dir2 &&\n+\techo initial >dir1/sub1 &&\n+\techo initial >dir1/sub2 &&\n+\techo initial >dir2/sub3 &&\n+\tgit add check dir1 dir2 top &&\n+\ttest_tick\n+\tgit-commit -m initial &&\n+\n+\techo changed >check &&\n+\techo changed >top &&\n+\techo changed >dir2/sub3 &&\n+\trm -f dir1/sub1 &&\n+\techo other >dir2/other\n+'\n+\n+test_expect_success update '\n+\tgit add -u dir1 dir2\n '\n \n-test_expect_success 'update' 'git-add -u dir'\n+test_expect_success 'update noticed a removal' '\n+\ttest \"$(git-ls-files dir1/sub1)\" = \"\"\n+'\n \n-test_expect_success 'update touched correct path' \\\n- 'test \"`git-diff-files --name-status dir/sub`\" = \"\"'\n+test_expect_success 'update touched correct path' '\n+\ttest \"$(git-diff-files --name-status dir2/sub3)\" = \"\"\n+'\n \n-test_expect_success 'update did not touch other tracked files' \\\n- 'test \"`git-diff-files --name-status top`\" = \"M\ttop\"'\n+test_expect_success 'update did not touch other tracked files' '\n+\ttest \"$(git-diff-files --name-status check)\" = \"M\tcheck\" &&\n+\ttest \"$(git-diff-files --name-status top)\" = \"M\ttop\"\n+'\n \n-test_expect_success 'update did not touch untracked files' \\\n- 'test \"`git-diff-files --name-status dir/other`\" = \"\"'\n+test_expect_success 'update did not touch untracked files' '\n+\ttest \"$(git-ls-files dir2/other)\" = \"\"\n+'\n+\n+test_expect_success 'cache tree has not been corrupted' '\n+\n+\tgit ls-files -s |\n+\tsed -e \"s/ 0\t/\t/\" >expect &&\n+\tgit ls-tree -r $(git write-tree) |\n+\tsed -e \"s/ blob / /\" >current &&\n+\tdiff -u expect current\n+\n+'\n \n test_done\n"},{"id":"50841","messageId":"46C38F9A.2050804@gmail.com","threadId":"9536","inReplyTo":"7v643gplph.fsf@gitster.siamese.dyndns.org","subject":"Re: git-commit goes awry after git-add -u","fromName":"Salikh Zakirov","fromEmail":"salikh@gmail.com","sentAt":"2007-08-15T23:43:22Z","receivedAt":"2007-08-15T23:43:22Z","isPatch":false,"sender":{"key":"salikh@gmail.com","avatar":"https://gravatar.com/avatar/952c102bb1dcf721dab8de4f5a11d276756a65d301d021f755e265cc3251efae?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Salikh Zakirov writes:\n> \n>> I have observed incorrect behaviour of git-commit after git-add -u, where\n>> it records deletions of files not related to the files touched by commit.\n> \n> Does this fix the issue?\n\nThanks, solved the issue nicely.\n"},{"id":"50843","messageId":"eb5812d90708151715k5125d4aq3008d7be54fa66b6@mail.gmail.com","threadId":"9536","inReplyTo":"7v1we4pknl.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix \"git add -u\" data corruption.","fromName":"Zakirov Salikh","fromEmail":"salikh@gmail.com","sentAt":"2007-08-16T00:15:44Z","receivedAt":"2007-08-16T00:15:44Z","isPatch":true,"sender":{"key":"salikh@gmail.com","avatar":"https://gravatar.com/avatar/952c102bb1dcf721dab8de4f5a11d276756a65d301d021f755e265cc3251efae?d=mp&s=160"},"body":"Junio wrote:\n> This applies to 'maint' to fix a rather serious data corruption\n> issue. When \"git add -u\" affects a subdirectory in such a way\n> that the only changes to its contents are path removals, the\n> next tree object written out of that index was bogus, as the\n> remove codepath forgot to invalidate the cache-tree entry.\n\nFixes the issue, thanks.\n\nTo make this more fair to git, especially to a notorious statement\n\"git never lost any data in its existence\", this commit corruption,\nwhile annoying, does not lose any data from working directory,\nand was easy to fix and work around once I've figured out what's happened.\n"}]}