{"thread":{"id":"23566","subject":"filenames with spaces in traditional patches (Re: git-import-dsc: Error importing chromium-browser dsc)","startedAt":"2010-04-22T17:29:23Z","lastAt":"2010-04-22T17:29:23Z","messageCount":1,"participants":["Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"140142","messageId":"20100422172923.GC7147@progeny.tock","threadId":"23566","inReplyTo":"20100422163521.GA27711@bogon.sigxcpu.org","subject":"filenames with spaces in traditional patches (Re: git-import-dsc: Error importing chromium-browser dsc)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-22T17:29:23Z","receivedAt":"2010-04-22T17:29:23Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"retitle 578752 apply: handle spaces in filenames from traditional patches\ntags 578752 + upstream\nthanks\n\nHi,\n\nGuido Günther wrote:\n> On Thu, Apr 22, 2010 at 03:48:12PM +0200, Giuseppe Iuculano wrote:\n\n>> error: patch failed: debian/licenses/LICENSE.global:0\n>> error: debian/licenses/LICENSE.global: patch does not apply\n>> Error import /home/debian/chromium/chromium/chromium-browser_5.0.342.9~r43360-0ubuntu2.diff.gz: 256\n[...]\n> \tdget https://edge.launchpad.net/ubuntu/+archive/primary/+files/chromium-browser_5.0.342.9~r43360-0ubuntu2.dsc\n[...]\n> The issue is that git-apply doesn't handle the filenames containing\n> spaces correctly like\n> \n>   'debian/licenses/LICENSE.global BSD-style Chromium' and\n>   'debian/licenses/LICENSE.Apache (v2.0)'.\n\nThanks, both.  The problem is in the parse_traditional_path() function\nin builtin/apply.c; it simply doesn’t handle paths with spaces.\n\nPosix [1] says:\n\n| The name and last modification time of each file shall be output in\n| the following format:\n|\n| \"---[space]%s  %s%s%s\", file1, <file1 timestamp>, <file1 frac>, <file1 zone>\n| \"+++[space]%s  %s%s%s\", file2, <file2 timestamp>, <file2 frac>, <file2 zone>\n|\n| Each <file> field shall be the pathname of the corresponding file\n| being compared, or the single character '-' if standard input is\n| being compared. However, if the pathname contains a <tab> or a\n| <newline>, or if it does not consist entirely of characters taken\n| from the portable character set, the behavior is\n| implementation-defined.\n|\n| Each <timestamp> field shall be equivalent to the output from the\n| following command:\n|\n| date '+%Y-%m-%d%H:%M:%S'\n|\n| without the trailing <newline>\n[...]\n\nIf this is really describing the format of patches in the wild, that\nmeans we should only look for a tab character to terminate the filename.\nIf someone ends up wanting to use a non-git patch to change a file with\na tab in its name, well, we can deal with that then. :)\n\nA big downside: this does not cope with copy-and-pasted patches with\ntabs transformed to spaces.  The example [2] consists mostly of\nfile-creation patches, so we can’t look to the repository for hints.\nMaybe the space-plus-date-plus-newline sequence should be used as a\ndelimiter.\n\nHere’s a rough patch to give an idea of where to start.\n\n[1] http://www.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07\n[2] https://edge.launchpad.net/ubuntu/+archive/primary/+files/chromium-browser_5.0.342.9~r43360-0ubuntu2.diff.gz\n\n-- 8< --\nSubject: apply: handle traditional patches with spaces in filename\n\nAccording to Posix, the --- and +++ lines of a unified diff always\ninclude a tab after the filename.  By not treating a space as a\nterminating character, we get support for filenames with spaces\nautomatically.\n\nNoticed while patching a program with filenames such as\n“LICENSE.Apache (v2.0)”.\n\nThanks to Giuseppe Iuculano <iuculano@debian.org> for the report\nand Guido Günther <agx@sigxcpu.org> for the analysis.\n\nFixes: http://bugs.debian.org/578752\nBreaks: copy-and-pasted patches\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/apply.c |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 771c972..7e1c6b9 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -519,7 +519,7 @@ static int guess_p_value(const char *nameline)\n \n \tif (is_dev_null(nameline))\n \t\treturn -1;\n-\tname = find_name(nameline, NULL, 0, TERM_SPACE | TERM_TAB);\n+\tname = find_name(nameline, NULL, 0, TERM_TAB);\n \tif (!name)\n \t\treturn -1;\n \tcp = strchr(name, '/');\n@@ -638,16 +638,16 @@ static void parse_traditional_patch(const char *first, const char *second, struc\n \tif (is_dev_null(first)) {\n \t\tpatch->is_new = 1;\n \t\tpatch->is_delete = 0;\n-\t\tname = find_name(second, NULL, p_value, TERM_SPACE | TERM_TAB);\n+\t\tname = find_name(second, NULL, p_value, TERM_TAB);\n \t\tpatch->new_name = name;\n \t} else if (is_dev_null(second)) {\n \t\tpatch->is_new = 0;\n \t\tpatch->is_delete = 1;\n-\t\tname = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB);\n+\t\tname = find_name(first, NULL, p_value, TERM_TAB);\n \t\tpatch->old_name = name;\n \t} else {\n-\t\tname = find_name(first, NULL, p_value, TERM_SPACE | TERM_TAB);\n-\t\tname = find_name(second, name, p_value, TERM_SPACE | TERM_TAB);\n+\t\tname = find_name(first, NULL, p_value, TERM_TAB);\n+\t\tname = find_name(second, name, p_value, TERM_TAB);\n \t\tif (has_epoch_timestamp(first)) {\n \t\t\tpatch->is_new = 1;\n \t\t\tpatch->is_delete = 0;\n-- \n1.7.1.rc2.8.g5f4cb\n"}]}