[RFC] solving a bug with hunks starting at line 1 in git apply

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



======================================================================
= 1. The bug
======================================================================

hunks of the form:

@@ -1,k +n,m @@

have a special behavior because of the variable match_begining.  For
these hunks, offset is not allowed which means that the fragment has to
match at the first line of the image. This is done for the sake of
safety (This behavior does not exist in the basic patch command). Here's
an example

first original file:

10
20
30
40

for the following diff file:

@@ -1,2 +1,2 @@
 20
-30
+35
 40

The patch will not be applied with a git apply command, but it will
with a basic patch command. However, there's no problem with the
following diff for both:

@@ -2,2 +2,2 @@
 10
-20
+25
 30

The problem comes when we have a diff file like that (which can be
obtained by splitting a hunk with git add -p for example (unsolved bug
reported in 1bf01040f0c1101f410f9caa5e715460cdd0cbe0))

@@ -1,1 +1,2 @@
+5
 10
@@ -1,3 +2,3 @@
 10
+15
-20
 30

In this case the first hunk will be treated, the image will then become

5
10
20
30

The second will try to match the lines

10
20
30

with the first lines of the image, will not succeed and will return that
it is not possible without trying to match the image with offsets,
because the hunk begin with 1.

Moreover, there are cases where both the git apply and the
patch command will work, giving different outputs.

second original file:

10
10
20

diff file associated:

@@ -1,1 +1,2 @@
+10
 10
@@ -1,2 +2,3 @@
 10
+cc
 10

With git apply, the problem will silently appear, and the command will
give the following output:

10
cc
10
10
20

whereas the output given by the simple patch command will be:

10
10
cc
10
20

======================================================================
= 2. Correction
======================================================================

I see mainly two ways to fix the bug:

********************************************************************
* 2.1 first method (the most basic)
********************************************************************

The most basic is to change the code so that the special behavior
only affects the hunks of the form

@@ -1,k +1,m @@

Which makes the previous diff file possible to use. And fixes the
bug reported in 1bf01040f0c1101f410f9caa5e715460cdd0cbe0

Note that this modification, which changes positively behavior of hunks

@@ -1,k +l,m @@

where l is not 1, when there's a hunk

@@ -1,a +1,b @@

above, also changes the behavior when there's no such bloc above:

For example: the diff file

@@ -1,2 +2,2 @@
 20
-30
+35
 40

would have given an error before this patch (with the first original file)
whereas it works with the modification introduced by the patch.

**********************************************************************
* 2.2 second method
**********************************************************************

The other method, is to pass a base_line and an offset to the match_fragment
function instead of passing directly the line to test.

This does not involve any change in the behavior of apply, and therefore
has not the problem of the most basic method.

======================================================================
= 3. Question
======================================================================

I personnaly think that the second method is better, but this implies more
modifications, and it will be useful only to keep the failing behavior of 
some hand-modified patches... Do you think that the second method is
worth implementing ?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]