Skip to content

Commit

Permalink
apply --ignore-space-change: lines with and without leading whitespac…
Browse files Browse the repository at this point in the history
…es do not match

The fuzzy_matchlines() function is used when attempting to resurrect
a patch that is whitespace-damaged, or when applying a patch that
was produced against an old codebase to the codebase after
indentation change.

The patch may want to change a line "a_bc" ("_" is used throught
this description for a whitespace to make it stand out) in the
original into something else, and we may not find "a_bc" in the
current source, but there may be "a__bc" (two spaces instead of one
the whitespace-damaged patch claims to expect).  By ignoring the
amount of whitespaces, it forces "git apply" to consider that "a_bc"
in the broken patch meant to refer to "a__bc" in reality.

However, the implementation special cases a run of whitespaces at
the beginning of a line and makes "abc" match "_abc", even though a
whitespace in the middle of string never matches a 0-width gap,
e.g. "a_bc" does not match "abc".  A run of whitespace at the end of
one string does not match a 0-width end of line on the other line,
either, e.g. "abc_" does not match "abc".

Fix this inconsistency by making the code skip leading whitespaces
only when both strings begin with a whitespace.  This makes the
option mean the same as the option of the same name in "diff" and
"git diff".

Note that I am not sure if anybody sane should use this option in
the first place.  The fuzzy match logic may be able to find the
original line that the patch author may have meant to touch because
it does not fully trust what the original lines say (i.e. context
lines prefixed by " " and old lines prefixed by "-" does not have to
exactly match the contents the patch is applied to).  There is no
reason for us to trust what the replacement lines (i.e. new lines
prefixed by "+") say, either, but with this option enabled, we end
up copying these new lines with suspicious whitespace distributions
literally into the patched result.  But as long as we keep it, we
should make it do its insane thing consistently.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Junio C Hamano committed Mar 26, 2014
1 parent 7bbc4e8 commit 14d3bb4
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 13 deletions.
12 changes: 7 additions & 5 deletions builtin/apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,13 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
while ((*last2 == '\r') || (*last2 == '\n'))
last2--;

/* skip leading whitespace */
while (isspace(*s1) && (s1 <= last1))
s1++;
while (isspace(*s2) && (s2 <= last2))
s2++;
/* skip leading whitespaces, if both begin with whitespace */
if (s1 <= last1 && s2 <= last2 && isspace(*s1) && isspace(*s2)) {
while (isspace(*s1) && (s1 <= last1))
s1++;
while (isspace(*s2) && (s2 <= last2))
s2++;
}
/* early return if both lines are empty */
if ((s1 > last1) && (s2 > last2))
return 1;
Expand Down
12 changes: 4 additions & 8 deletions t/t4107-apply-ignore-whitespace.sh
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ sed -e 's/T/ /g' > main.c.final <<\EOF
#include <stdio.h>
void print_int(int num);
T/* a comment */
int func(int num);
int main() {
Expand Down Expand Up @@ -154,7 +153,8 @@ test_expect_success 'patch2 reverse applies with --ignore-space-change' '
git config apply.ignorewhitespace change

test_expect_success 'patch2 applies (apply.ignorewhitespace = change)' '
git apply patch2.patch
git apply patch2.patch &&
test_cmp main.c.final main.c
'

test_expect_success 'patch3 fails (missing string at EOL)' '
Expand All @@ -165,12 +165,8 @@ test_expect_success 'patch4 fails (missing EOL at EOF)' '
test_must_fail git apply patch4.patch
'

test_expect_success 'patch5 applies (leading whitespace)' '
git apply patch5.patch
'

test_expect_success 'patches do not mangle whitespace' '
test_cmp main.c main.c.final
test_expect_success 'patch5 fails (leading whitespace differences matter)' '
test_must_fail git apply patch5.patch
'

test_expect_success 're-create file (with --ignore-whitespace)' '
Expand Down

0 comments on commit 14d3bb4

Please sign in to comment.