{"thread":{"id":"1679","subject":"[PATCH 2/9] Fix detection of files with only one line in git-apply.","startedAt":"2005-08-28T15:24:27Z","lastAt":"2005-08-30T07:36:00Z","messageCount":11,"participants":["Robert Fitzsimons","Linus Torvalds","Junio C Hamano","Martin Langhoff"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"7864","messageId":"1125242667404-git-send-email-robfitz@273k.net","threadId":"1679","inReplyTo":"11252426672473-git-send-email-robfitz@273k.net","subject":"[PATCH 2/9] Fix detection of files with only one line in git-apply.","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2005-08-28T15:24:27Z","receivedAt":"2005-08-28T15:24:27Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"A patch which added one line to an empty file or removed the only line\nfrom a one line file would be incorrectly detected as an invalid new\nor delete patch. Added a new test case.\n\nSigned-off-by: Robert Fitzsimons \n\n---\n\n apply.c | 4 ++--\n t/t4103-apply-oneline.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+), 2 deletions(-)\n create mode 100644 t/t4103-apply-oneline.sh\n\nbabd40afef54b06775b11a877b347685edbedcd2\ndiff --git a/apply.c b/apply.c\n--- a/apply.c\n+++ b/apply.c\n@@ -638,9 +638,9 @@ static int parse_fragment(char *line, un\n \t\t\tpatch->new_name = NULL;\n \t}\n \n-\tif (patch->is_new != !oldlines)\n+\tif (patch->is_new && (patch->is_new != !oldlines))\n \t\treturn error(\"new file depends on old contents\");\n-\tif (patch->is_delete != !newlines) {\n+\tif (patch->is_delete && (patch->is_delete != !newlines)) {\n \t\tif (newlines)\n \t\t\treturn error(\"deleted file still has contents\");\n \t\tfprintf(stderr, \"** warning: file %s becomes empty but is not deleted\\n\", patch->new_name);\ndiff --git a/t/t4103-apply-oneline.sh b/t/t4103-apply-oneline.sh\nnew file mode 100644\n--- /dev/null\n+++ b/t/t4103-apply-oneline.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Junio C Hamano\n+# Copyright (c) 2005 Robert Fitzsimons\n+#\n+\n+test_description='git-apply files with one line.\n+\n+'\n+. ./test-lib.sh\n+\n+# setup\n+\n+cat > patch1.patch <<\\EOF\n+diff --git a/main.c b/main.c\n+new file mode 100644\n+--- /dev/null\n++++ b/main.c\n+@@ -0,0 +0,0 @@\n+EOF\n+cat > patch2.patch <<\\EOF\n+diff --git a/main.c b/main.c\n+--- a/main.c\n++++ b/main.c\n+@@ -0,0 +1,1 @@\n++#include \n+EOF\n+cat > patch3.patch <<\\EOF\n+diff --git a/main.c b/main.c\n+--- a/main.c\n++++ b/main.c\n+@@ -1,1 +0,0 @@\n+-#include \n+EOF\n+\n+test_expect_success \"S = patch1 (new file with no lines)\" \\\n+ 'git-apply patch1.patch'\n+\n+test_expect_success \"S = patch2 (add one line)\" \\\n+ 'git-apply patch2.patch'\n+\n+test_expect_success \"S = patch3 (delete one line)\" \\\n+ 'git-apply patch3.patch'\n+\n+test_done\n+\n"},{"id":"7866","messageId":"11252426672473-git-send-email-robfitz@273k.net","threadId":"1679","inReplyTo":null,"subject":"[PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2005-08-28T15:24:27Z","receivedAt":"2005-08-28T15:24:27Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"Stop processing and return NULL if we encounter a '\\n' character\nbefore we have two matching names in the git header.\n\nSigned-off-by: Robert Fitzsimons \n\n---\n\n apply.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n289e53f770e37dfe25c740788256d24c19c2e16d\ndiff --git a/apply.c b/apply.c\n--- a/apply.c\n+++ b/apply.c\n@@ -387,7 +387,7 @@ static char *git_header_name(char *line)\n \t\tdefault:\n \t\t\tcontinue;\n \t\tcase '\\n':\n-\t\t\tbreak;\n+\t\t\treturn NULL;\n \t\tcase '\\t': case ' ':\n \t\t\tsecond = name+len;\n \t\t\tfor (;;) {\n"},{"id":"7874","messageId":"Pine.LNX.4.58.0508280946180.3317@g5.osdl.org","threadId":"1679","inReplyTo":"1125242667404-git-send-email-robfitz@273k.net","subject":"Re: [PATCH 2/9] Fix detection of files with only one line in git-apply.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-28T16:55:43Z","receivedAt":"2005-08-28T16:55:43Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 28 Aug 2005, Robert Fitzsimons wrote:\n>\n> A patch which added one line to an empty file or removed the only line\n> from a one line file would be incorrectly detected as an invalid new\n> or delete patch. Added a new test case.\n\nThis patch looks wrong or at least needs some more thought. The test was\ndone the way it was done for a reason. For example:\n\n\tif (patch->is_new != !oldlines)\n\t\treturn error(\"new file depends on old contents\");\n\nactually has two error cases (admittedly it re-uses the same error message \nfor both, which is a bit confusing):\n\n - is_new, and oldlines != 0\n\n\tThis is the obvious error, and the one the message talk about.\n\n - !is_new, and oldlines == 0\n\n\tIn kernel usage, empty files are not legal, and empty files get \n\tdeleted. So this actually was intentional, even if the error \n\tmessage ends up being less than perfect.\n\nThe \"is_delete\" thing is the same. At least for the kernel, I want a patch \nthat removes all content to also always _delete_ the file, because \notherwise old-style patches are ambiguous. Is it a delete, or is it a \n\"leave an empty file\"?\n\nNow, git patches aren't ambigious, so while git patches can clearly say \n\"this is a patch to an existing empty file\" vs \"this patch creates this \nfile\", I wanted to have the behaviour where we maintain the principle that \nempty files are wrong.\n\nNow, that's a kernel-specific thing, so it may be that enabling this\nbehaviour this should be a command line flag, but the point is that I \ndon't want to entirely lose the test.\n\n\t\tLinus\n"},{"id":"7886","messageId":"7vslwtein3.fsf@assigned-by-dhcp.cox.net","threadId":"1679","inReplyTo":"11252426672473-git-send-email-robfitz@273k.net","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-28T23:39:12Z","receivedAt":"2005-08-28T23:39:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for \"apply\" patches. I have merged some of them and\npushed out.\n\nI further wanted to apply the following to the proposed updates\nbranch, but even with only the first one applied, it fails its\nown test, t4104-apply-complex.sh:\n\n* Fix processing of a patch file which modifies the same file in git-apply.\n* Fix the processing of multiple patch files with --check in git-apply.\n* New option --ignore-whitespace for git-apply.\n* New option --ignore-applied for git-apply.\n\n*** t4104-apply-complex.sh ***\n* FAIL 5: S = Nx Px1\n\tcat Nx Px1 | git-apply -\n* FAIL 6: S = Nx Px1 Px2\n\tcat Nx Px1 Px2 | git-apply -\n* FAIL 7: S = Nx Px1 Px2 Px3\n\tcat Nx Px1 Px2 Px3 | git-apply -\n* FAIL 8: S = Nx Px1 Px2 Px3 Dx\n\tcat Nx Px1 Px2 Px3 Dx | git-apply -\n* FAIL 13: S = Ny Ryx Px1 Px2 Cxy Px3 Dx\n\tcat Ny Ryx Px1 Px2 Cxy Px3 Dx | git-apply -\n* FAIL 14: S = Ny Ryx Px1 Px2 Cxy Px3 Dx Cyx\n\tcat Ny Ryx Px1 Px2 Cxy Px3 Dx Cyx | git-apply -\n* FAIL 15: S = Ny Cyx Px1 Px2 Px3 Dy Cxy Dx Cyx Dy Rxy Dy\n\tcat Ny Cyx Px1 Px2 Px3 Dy Cxy Dx Cyx Dy Rxy Dy| git-apply -\n"},{"id":"7923","messageId":"20050829235823.GA19351@localhost","threadId":"1679","inReplyTo":"7vslwtein3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2005-08-29T23:58:23Z","receivedAt":"2005-08-29T23:58:23Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"I should have mentioned before, these changes are being used in a\nSourcePuller dump file to git import script I've been working on.\n\n> I further wanted to apply the following to the proposed updates\n> branch, but even with only the first one applied, it fails its\n> own test, t4104-apply-complex.sh:\n\nThe test was the problem it tried to work with empty files.\n\n> * Fix processing of a patch file which modifies the same file in git-apply.\n> * Fix the processing of multiple patch files with --check in git-apply.\n> * New option --ignore-applied for git-apply.\n\nI've fixed up the problems with these patches and rebased them.\n\n> * New option --ignore-whitespace for git-apply.\n\nThis change was just wrong and could cause file corruption, I've got\nanother version in the works which fixes the problem and adds some of\nthe suggested features.\n\nRobert Fitzsimons\nrobfitz@273k.net\n"},{"id":"7929","messageId":"Pine.LNX.4.58.0508291744400.3243@g5.osdl.org","threadId":"1679","inReplyTo":"20050829235823.GA19351@localhost","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-30T00:47:00Z","receivedAt":"2005-08-30T00:47:00Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 29 Aug 2005, Robert Fitzsimons wrote:\n>\n> I should have mentioned before, these changes are being used in a\n> SourcePuller dump file to git import script I've been working on.\n\nHmm. Anything that depends on \"--ignore-applied\" is fundamentally broken\nas an import tool. There's no way something like that can be valid, since\nit inhently means that somebody is trying to apply a patch that simple\nisn't applicable.\n\nSo it seems like you're trying to work around some other serious problems \nin the import, and as a result the import is clearly not trustworthy. \n\nDon't do this.\n\n\t\t\tLinus\n"},{"id":"7931","messageId":"7vacj06xi6.fsf@assigned-by-dhcp.cox.net","threadId":"1679","inReplyTo":"Pine.LNX.4.58.0508291744400.3243@g5.osdl.org","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-30T01:09:53Z","receivedAt":"2005-08-30T01:09:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Hmm. Anything that depends on \"--ignore-applied\" is fundamentally broken\n> as an import tool. There's no way something like that can be valid, since\n> it inhently means that somebody is trying to apply a patch that simple\n> isn't applicable.\n>\n> Don't do this.\n\nI suspect that it is exported data that is broken, if\nthis --ignore-applied flag is needed for importing something\nsomebody exported.\n\nI, however, am not 100% convinced --ignore-applied is too\ndangerous to be useful in any conceivable use cases, at least\nnot yet. For example, you might be cherry-picking a change from\na foreign branch with 'git-diff-tree -p other~45 | git apply',\nwhere the current branch may have already aquired part of the\nchanges that foreign branch contains since 45 generations ago.\n"},{"id":"7933","messageId":"Pine.LNX.4.58.0508291821010.3243@g5.osdl.org","threadId":"1679","inReplyTo":"7vacj06xi6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-30T01:24:40Z","receivedAt":"2005-08-30T01:24:40Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 29 Aug 2005, Junio C Hamano wrote:\n> \n> I, however, am not 100% convinced --ignore-applied is too\n> dangerous to be useful in any conceivable use cases, at least\n> not yet. For example, you might be cherry-picking a change from\n> a foreign branch with 'git-diff-tree -p other~45 | git apply',\n> where the current branch may have already aquired part of the\n> changes that foreign branch contains since 45 generations ago.\n\n.. and that's why we use a three-way merge, and not diffs, to merge two \nbranches. \n\n--ignore-applied really is a pretty dangerous thing. I can see that it \nmight do the right thing in many cases, but definitely not in a scripted \nenvironment, much less in an \"import\". \n\nBy definition, importing stuff must apply, since it clearly applied in\nwhatever original source control tool you are importing from. So if an \nimporter needs --ignore-applied, it is fundamentally doing something \nwrong.\n\nAnd I just realized what \"SourcePuller\" is. Gaah. Trust me, that export is \nbad, and the original source control tool did everything very well indeed.\n\n\t\t\tLinus\n"},{"id":"7934","messageId":"7vbr3g5hsb.fsf@assigned-by-dhcp.cox.net","threadId":"1679","inReplyTo":"Pine.LNX.4.58.0508291821010.3243@g5.osdl.org","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-30T01:34:44Z","receivedAt":"2005-08-30T01:34:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Mon, 29 Aug 2005, Junio C Hamano wrote:\n>> \n>> I, however, am not 100% convinced --ignore-applied is too\n>> dangerous to be useful in any conceivable use cases, at least\n>> not yet. For example, you might be cherry-picking a change from\n>> a foreign branch with 'git-diff-tree -p other~45 | git apply',\n>> where the current branch may have already aquired part of the\n>> changes that foreign branch contains since 45 generations ago.\n>\n> .. and that's why we use a three-way merge, and not diffs, to merge two \n> branches. \n\n.. and that's why I recently rewrote revert, cherry-pick and\nrebase to use three-way merge as well ;-).\n\nThe code is simple enough and I see some beauty in it, but I\nhonor your veto, at least for now.\n"},{"id":"7936","messageId":"Pine.LNX.4.58.0508291856080.3243@g5.osdl.org","threadId":"1679","inReplyTo":"7vbr3g5hsb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-30T02:00:00Z","receivedAt":"2005-08-30T02:00:00Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 29 Aug 2005, Junio C Hamano wrote:\n> \n> The code is simple enough and I see some beauty in it, but I\n> honor your veto, at least for now.\n\nWell, I didn't actually veto it when I saw the feature the first time, \nbecause I think it's a valid thing to do when applying patches by hand.\n\nIt was only when I realized that Robert was trying to use it to import \nfrom somewhere else that it became broken. An import should use the most \nanal patches possible.\n\nActually, an import should preferably never use patches at all, since that\nnever really works for binary data. This is why the CVS importer actually\nchecks out full files and imports them that way. The patch-based ones will \nfundamentally always have problems.\n\n\t\tLinus\n"},{"id":"7945","messageId":"46a038f905083000362ae0d9fc@mail.gmail.com","threadId":"1679","inReplyTo":"Pine.LNX.4.58.0508291856080.3243@g5.osdl.org","subject":"Re: [PATCH 1/9] Fix git patch header processing in git-apply.","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-30T07:36:00Z","receivedAt":"2005-08-30T07:36:00Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/30/05, Linus Torvalds wrote:\n> Actually, an import should preferably never use patches at all, since that\n> never really works for binary data. This is why the CVS importer actually\n> checks out full files and imports them that way. The patch-based ones will\n> fundamentally always have problems.\n\n. Patches can be useful during the process, but you should (a)\nprefer more solid manifestations of the file/tree if possible, and (b)\nbe extremely strict with them. Any hint that they don't match means\nthat something is borked.\n\nHowever, if you're importing from patch-oriented SCMs that tend to\n'replay' patch sequences from one branch to the other, asking git to\nskip applied patches *could* be a shortcut. Not 100% correctness\nguarantee, but if the resulting history makes sense, it can be useful.\n\nThe other alternative is to do what I'll start to do soon on the\narchimport script: keep a tally of what was merged where according to\nthe records that the source SCM has.\n\ncheers,\n\n\nmartin\n"}]}