{"thread":{"id":"7729","subject":"[PATCH] Fix overwriting of files when applying contextually independent diffs","startedAt":"2007-04-18T21:58:56Z","lastAt":"2007-04-18T22:32:20Z","messageCount":2,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39833","messageId":"20070418215856.GA2477@steel.home","threadId":"7729","inReplyTo":null,"subject":"[PATCH] Fix overwriting of files when applying contextually independent diffs","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-18T21:58:56Z","receivedAt":"2007-04-18T21:58:56Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Noticed by applying two diffs of different contexts to the same file.\n\nThe check for existence of a file was wrong: the test assumed it was\na directory and reset the errno (twice: directly and by calling\nlstat). So if an entry existed and was _not_ a directory no attempt\nwas made to rename into it, because the errno (expected by renaming\ncode) was already reset to 0. This resulted in error:\n\n    fatal: unable to write file file mode 100644\n\nFor Linux, removing \"errno = 0\" is enough, as lstat wont modify errno\nif it was successful. The behavior should not be depended upon,\nthough, so modify the \"if\" as well.\n\nThe test simulates this situation.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nFound it. Somewhat unusual situation, I admit, but splitting\npatches just to apply them would be too annoying. GNU patch\naccepts such patches too, not even a warning.\n\n builtin-apply.c        |    3 +--\n t/t4121-apply-diffs.sh |   33 +++++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 2 deletions(-)\n create mode 100755 t/t4121-apply-diffs.sh\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex fd92ef7..94311e7 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2416,8 +2416,7 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned\n \t\t * used to be.\n \t\t */\n \t\tstruct stat st;\n-\t\terrno = 0;\n-\t\tif (!lstat(path, &st) && S_ISDIR(st.st_mode) && !rmdir(path))\n+\t\tif (!lstat(path, &st) && (!S_ISDIR(st.st_mode) || !rmdir(path)))\n \t\t\terrno = EEXIST;\n \t}\n \ndiff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh\nnew file mode 100755\nindex 0000000..2b2f1ed\n--- /dev/null\n+++ b/t/t4121-apply-diffs.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='git-apply for contextually independent diffs'\n+. ./test-lib.sh\n+\n+echo '1\n+2\n+3\n+4\n+5\n+6\n+7\n+8' >file\n+\n+test_expect_success 'setup' \\\n+\t'git add file &&\n+\tgit commit -q -m 1 &&\n+\tgit checkout -b test &&\n+\tmv file file.tmp &&\n+\techo 0 >file &&\n+\tcat file.tmp >>file &&\n+\trm file.tmp &&\n+\tgit commit -a -q -m 2 &&\n+\techo 9 >>file &&\n+\tgit commit -a -q -m 3 &&\n+\tgit checkout master'\n+\n+test_expect_success \\\n+\t'check if contextually independent diffs for the same file apply' \\\n+\t'( git diff test~2 test~1; git diff test~1 test~0 )| git apply'\n+\n+test_done\n+\n-- \n1.5.1.1.876.ge36f76\n"},{"id":"39837","messageId":"7vtzvde2kr.fsf@assigned-by-dhcp.cox.net","threadId":"7729","inReplyTo":"20070418215856.GA2477@steel.home","subject":"Re: [PATCH] Fix overwriting of files when applying contextually independent diffs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T22:32:20Z","receivedAt":"2007-04-18T22:32:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Noticed by applying two diffs of different contexts to the same file.\n>\n> The check for existence of a file was wrong: the test assumed it was\n> a directory and reset the errno (twice: directly and by calling\n> lstat). So if an entry existed and was _not_ a directory no attempt\n> was made to rename into it, because the errno (expected by renaming\n> code) was already reset to 0. This resulted in error:\n>\n>     fatal: unable to write file file mode 100644\n>\n> For Linux, removing \"errno = 0\" is enough, as lstat wont modify errno\n> if it was successful. The behavior should not be depended upon,\n> though, so modify the \"if\" as well.\n>\n> The test simulates this situation.\n\nOk.  I briefly thought this might disable a more important\nsafety to refuse overwriting an untracked working tree file with\na creation patch, but that is caught much earlier in an\nindependent codepath already, so this should be safe to apply.\n\nThanks.\n"}]}