* [PATCH] Fix overwriting of files when applying contextually independent diffs
@ 2007-04-18 21:58 Alex Riesen
2007-04-18 22:32 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Alex Riesen @ 2007-04-18 21:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Noticed by applying two diffs of different contexts to the same file.
The check for existence of a file was wrong: the test assumed it was
a directory and reset the errno (twice: directly and by calling
lstat). So if an entry existed and was _not_ a directory no attempt
was made to rename into it, because the errno (expected by renaming
code) was already reset to 0. This resulted in error:
fatal: unable to write file file mode 100644
For Linux, removing "errno = 0" is enough, as lstat wont modify errno
if it was successful. The behavior should not be depended upon,
though, so modify the "if" as well.
The test simulates this situation.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Found it. Somewhat unusual situation, I admit, but splitting
patches just to apply them would be too annoying. GNU patch
accepts such patches too, not even a warning.
builtin-apply.c | 3 +--
t/t4121-apply-diffs.sh | 33 +++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 2 deletions(-)
create mode 100755 t/t4121-apply-diffs.sh
diff --git a/builtin-apply.c b/builtin-apply.c
index fd92ef7..94311e7 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2416,8 +2416,7 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned
* used to be.
*/
struct stat st;
- errno = 0;
- if (!lstat(path, &st) && S_ISDIR(st.st_mode) && !rmdir(path))
+ if (!lstat(path, &st) && (!S_ISDIR(st.st_mode) || !rmdir(path)))
errno = EEXIST;
}
diff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh
new file mode 100755
index 0000000..2b2f1ed
--- /dev/null
+++ b/t/t4121-apply-diffs.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='git-apply for contextually independent diffs'
+. ./test-lib.sh
+
+echo '1
+2
+3
+4
+5
+6
+7
+8' >file
+
+test_expect_success 'setup' \
+ 'git add file &&
+ git commit -q -m 1 &&
+ git checkout -b test &&
+ mv file file.tmp &&
+ echo 0 >file &&
+ cat file.tmp >>file &&
+ rm file.tmp &&
+ git commit -a -q -m 2 &&
+ echo 9 >>file &&
+ git commit -a -q -m 3 &&
+ git checkout master'
+
+test_expect_success \
+ 'check if contextually independent diffs for the same file apply' \
+ '( git diff test~2 test~1; git diff test~1 test~0 )| git apply'
+
+test_done
+
--
1.5.1.1.876.ge36f76
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix overwriting of files when applying contextually independent diffs
2007-04-18 21:58 [PATCH] Fix overwriting of files when applying contextually independent diffs Alex Riesen
@ 2007-04-18 22:32 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2007-04-18 22:32 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
Alex Riesen <raa.lkml@gmail.com> writes:
> Noticed by applying two diffs of different contexts to the same file.
>
> The check for existence of a file was wrong: the test assumed it was
> a directory and reset the errno (twice: directly and by calling
> lstat). So if an entry existed and was _not_ a directory no attempt
> was made to rename into it, because the errno (expected by renaming
> code) was already reset to 0. This resulted in error:
>
> fatal: unable to write file file mode 100644
>
> For Linux, removing "errno = 0" is enough, as lstat wont modify errno
> if it was successful. The behavior should not be depended upon,
> though, so modify the "if" as well.
>
> The test simulates this situation.
Ok. I briefly thought this might disable a more important
safety to refuse overwriting an untracked working tree file with
a creation patch, but that is caught much earlier in an
independent codepath already, so this should be safe to apply.
Thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2007-04-18 22:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-18 21:58 [PATCH] Fix overwriting of files when applying contextually independent diffs Alex Riesen
2007-04-18 22:32 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.