{"thread":{"id":"36297","subject":"[PATCH] apply --ignore-space-change: lines with and without leading whitespaces do not match","startedAt":"2014-03-26T21:14:13Z","lastAt":"2014-03-26T21:14:13Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"237850","messageId":"xmqq61n0irx6.fsf@gitster.dls.corp.google.com","threadId":"36297","inReplyTo":null,"subject":"[PATCH] apply --ignore-space-change: lines with and without leading whitespaces do not match","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-26T21:14:13Z","receivedAt":"2014-03-26T21:14:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The fuzzy_matchlines() function is used when attempting to resurrect\na patch that is whitespace-damaged, or when applying a patch that\nwas produced against an old codebase to the codebase after\nindentation change.\n\nThe patch may want to change a line \"a_bc\" (\"_\" is used throught\nthis description for a whitespace to make it stand out) in the\noriginal into something else, and we may not find \"a_bc\" in the\ncurrent source, but there may be \"a__bc\" (two spaces instead of one\nthe whitespace-damaged patch claims to expect).  By ignoring the\namount of whitespaces, it forces \"git apply\" to consider that \"a_bc\"\nin the broken patch meant to refer to \"a__bc\" in reality.\n\nHowever, the implementation special cases a run of whitespaces at\nthe beginning of a line and makes \"abc\" match \"_abc\", even though a\nwhitespace in the middle of string never matches a 0-width gap,\ne.g. \"a_bc\" does not match \"abc\".  A run of whitespace at the end of\none string does not match a 0-width end of line on the other line,\neither, e.g. \"abc_\" does not match \"abc\".\n\nFix this inconsistency by making the code skip leading whitespaces\nonly when both strings begin with a whitespace.  This makes the\noption mean the same as the option of the same name in \"diff\" and\n\"git diff\".\n\nNote that I am not sure if anybody sane should use this option in\nthe first place.  The fuzzy match logic may be able to find the\noriginal line that the patch author may have meant to touch because\nit does not fully trust what the original lines say (i.e. context\nlines prefixed by \" \" and old lines prefixed by \"-\" does not have to\nexactly match the contents the patch is applied to).  There is no\nreason for us to trust what the replacement lines (i.e. new lines\nprefixed by \"+\") say, either, but with this option enabled, we end\nup copying these new lines with suspicious whitespace distributions\nliterally into the patched result.  But as long as we keep it, we\nshould make it do its insane thing consistently.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/apply.c                    | 12 +++++++-----\n t/t4107-apply-ignore-whitespace.sh | 12 ++++--------\n 2 files changed, 11 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 0189523..8b9d3de 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -300,11 +300,13 @@ static int fuzzy_matchlines(const char *s1, size_t n1,\n \twhile ((*last2 == '\\r') || (*last2 == '\\n'))\n \t\tlast2--;\n \n-\t/* skip leading whitespace */\n-\twhile (isspace(*s1) && (s1 <= last1))\n-\t\ts1++;\n-\twhile (isspace(*s2) && (s2 <= last2))\n-\t\ts2++;\n+\t/* skip leading whitespaces, if both begin with whitespace */\n+\tif (s1 <= last1 && s2 <= last2 && isspace(*s1) && isspace(*s2)) {\n+\t\twhile (isspace(*s1) && (s1 <= last1))\n+\t\t\ts1++;\n+\t\twhile (isspace(*s2) && (s2 <= last2))\n+\t\t\ts2++;\n+\t}\n \t/* early return if both lines are empty */\n \tif ((s1 > last1) && (s2 > last2))\n \t\treturn 1;\ndiff --git a/t/t4107-apply-ignore-whitespace.sh b/t/t4107-apply-ignore-whitespace.sh\nindex b04fc8f..9e29b52 100755\n--- a/t/t4107-apply-ignore-whitespace.sh\n+++ b/t/t4107-apply-ignore-whitespace.sh\n@@ -111,7 +111,6 @@ sed -e 's/T/\t/g' > main.c.final <<\\EOF\n #include <stdio.h>\n \n void print_int(int num);\n-T/* a comment */\n int func(int num);\n \n int main() {\n@@ -154,7 +153,8 @@ test_expect_success 'patch2 reverse applies with --ignore-space-change' '\n git config apply.ignorewhitespace change\n \n test_expect_success 'patch2 applies (apply.ignorewhitespace = change)' '\n-\tgit apply patch2.patch\n+\tgit apply patch2.patch &&\n+\ttest_cmp main.c.final main.c\n '\n \n test_expect_success 'patch3 fails (missing string at EOL)' '\n@@ -165,12 +165,8 @@ test_expect_success 'patch4 fails (missing EOL at EOF)' '\n \ttest_must_fail git apply patch4.patch\n '\n \n-test_expect_success 'patch5 applies (leading whitespace)' '\n-\tgit apply patch5.patch\n-'\n-\n-test_expect_success 'patches do not mangle whitespace' '\n-\ttest_cmp main.c main.c.final\n+test_expect_success 'patch5 fails (leading whitespace differences matter)' '\n+\ttest_must_fail git apply patch5.patch\n '\n \n test_expect_success 're-create file (with --ignore-whitespace)' '\n-- \n1.9.1-497-g6f5ca27\n"}]}