> ä 2010å11æ25æ 01:20, Junio C Hamano åé: > By the way, this codepath is shared by all forms of patches "diff --git" > header, not just binary. Do you see a similar breakage with --no-prefix > patches that are not binary, and if not why? Some cases a git diff without "--- path/to/old" and "+++ path/to/new", will make current git apply -p2 broken. 1. Prue file mode changes: diff --git a/sub1/file1 b/sub2/file1 old mode 100644 new mode 100755 2. Remove empty file: diff --git a/sub1/file1 b/sub1/file1 deleted file mode 100644 index e69de29..0000000 3. Add empty file: diff --git a/sub2/file1 b/sub2/file1 new file mode 100644 index 0000000..e69de29 4. New binary file: diff --git a/sub2/file1 b/sub2/file1 new file mode 100644 index 0000000000000000000000000000000000000000..4dd0c65a410e8aea1608d6b2dd70e74def0012c5 GIT binary patch literal 8 Pcmb<-^>JfjWMlvU2^Ilc literal 0 HcmV?d00001 5. Modify binary file: diff --git a/sub1/file1 b/sub2/file1 index f96ec1a8358556543a73b3fb93a0d1c2aad8432a..4dd0c65a410e8aea1608d6b2dd70e74def0012c5 100644 GIT binary patch literal 8 Pcmb<-^>JfjWMlvU2^Ilc literal 7 Ocmb<-^>JfjWCQ>Qy8&AO I think all these cases can use this testcase in t/t4120-apply-popt.sh: +test_expect_success 'apply git diff with -p2 and use default name from header' ' + sed -e "/^\(---\|+++\) / d" patch.file > patch.newheader && + cp file1.saved file1 && + git apply -p2 patch.newheader +' ä 2010å11æ25æ 09:46, Jiang Xin åé: > > ä 2010å11æ25æ 01:20, Junio C Hamano åé: >> Jiang Xin <worldhello.net@xxxxxxxxx> writes: >> >>> When patch file generated against two non-git directories using >>> 'git diff --binary --no-index' without '--no-prefix', the patch >>> file has patch level greater then 1, and should be applied with >>> '-p2' option. But it does not work if there are binary differ >>> in the patch file, it is because in one case the patch level is >>> not properly handled. >>> >>> Signed-off-by: Jiang Xin <jiangxin@xxxxxxxxx> >> >> Can you please add a testcase to protect your fix from getting broken by >> later changes by other people, perhaps to t/t4120? >> > > Yes, I add a testcase in t/t4120, and I will send it as a new PATCH. > The new testcase is like following: > > +test_expect_success 'apply git diff with -p2 and use default name from header' ' > + sed -e "/^\(---\|+++\) / d" patch.file > patch.newheader && > + cp file1.saved file1 && > + git apply -p2 patch.newheader > +' > + > >> By the way, this codepath is shared by all forms of patches "diff --git" >> header, not just binary. Do you see a similar breakage with --no-prefix >> patches that are not binary, and if not why? >> > > The breakage will appear under these circumstances: > * the patch is in git style: header with "diff --git". > * the header does not contain '--- path/to/old' and '+++ path/to/new'. > * has a patch level greater than 1. > > When I do `git diff` against binary files, I find the patch header does not has > '--- path/to/old' and '+++ path/to/new'. May be there exist other cases, but > I'm not sure. > >>> --- >>> builtin/apply.c | 3 ++- >>> 1 files changed, 2 insertions(+), 1 deletions(-) >>> >>> diff --git a/builtin/apply.c b/builtin/apply.c >>> index 23c18c5..d603e37 100644 >>> --- a/builtin/apply.c >>> +++ b/builtin/apply.c >>> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen) >>> * form. >>> */ >>> for (len = 0 ; ; len++) { >>> + int nslash = p_value; >>> switch (name[len]) { >>> default: >>> continue; >>> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen) >>> char c = *second++; >>> if (c == '\n') >>> return NULL; >>> - if (c == '/') >>> + if (c == '/' && --nslash <= 0) >>> break; >>> } >>> if (second[len] == '\n' && !memcmp(name, second, len)) { >>> -- >>> 1.7.3.2.245.g03276.dirty > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html