Skip to content

Commit 24ff4d5

Browse files
bjornggitster
authored andcommitted
apply: Remove the quick rejection test
In the next commit, we will make it possible for blank context lines to match beyond the end of the file. That means that a hunk with a preimage that has more lines than present in the file may be possible to successfully apply. Therefore, we must remove the quick rejection test in find_pos(). find_pos() will already work correctly without the quick rejection test, but that might not be obvious. Therefore, comment the test for handling out-of-range line numbers in find_pos() and cast the "line" variable to the same (unsigned) type as img->nr. What are performance implications of removing the quick rejection test? It can only help "git apply" to reject a patch faster. For example, if I have a file with one million lines and a patch that removes slightly more than 50 percent of the lines and try to apply that patch twice, the second attempt will fail slightly faster with the test than without (based on actual measurements). However, there is the pathological case of a patch with many more context lines than the default three, and applying that patch using "git apply -C1". Without the rejection test, the running time will be roughly proportional to the number of context lines times the size of the file. That could be handled by writing a more complicated rejection test (it would have to count the number of blanks at the end of the preimage), but I don't find that worth doing until there is a real-world use case that would benfit from it. It would be possible to keep the quick rejection test if --whitespace=fix is not given, but I don't like that from a testing point of view. Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 9b25949 commit 24ff4d5

2 files changed

Lines changed: 16 additions & 5 deletions

File tree

builtin-apply.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,11 +1997,8 @@ static int find_pos(struct image *img,
19971997
unsigned long backwards, forwards, try;
19981998
int backwards_lno, forwards_lno, try_lno;
19991999

2000-
if (preimage->nr > img->nr)
2001-
return -1;
2002-
20032000
/*
2004-
* If match_begining or match_end is specified, there is no
2001+
* If match_beginning or match_end is specified, there is no
20052002
* point starting from a wrong line that will never match and
20062003
* wander around and wait for a match at the specified end.
20072004
*/
@@ -2010,7 +2007,12 @@ static int find_pos(struct image *img,
20102007
else if (match_end)
20112008
line = img->nr - preimage->nr;
20122009

2013-
if (line > img->nr)
2010+
/*
2011+
* Because the comparison is unsigned, the following test
2012+
* will also take care of a negative line number that can
2013+
* result when match_end and preimage is larger than the target.
2014+
*/
2015+
if ((size_t) line > img->nr)
20142016
line = img->nr;
20152017

20162018
try = 0;

t/t4104-apply-boundary.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,13 @@ test_expect_success 'two lines' '
134134
135135
'
136136

137+
test_expect_success 'apply patch with 3 context lines matching at end' '
138+
{ echo a; echo b; echo c; echo d; } >file &&
139+
git add file &&
140+
echo e >>file &&
141+
git diff >patch &&
142+
>file &&
143+
test_must_fail git apply patch
144+
'
145+
137146
test_done

0 commit comments

Comments
 (0)