GNU bug report logs - #11894
24.1.50; [PATCH] diff-apply-hunk can be off by 1 line when the hunk is 0-context pure removal

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11894 in the body.
You can then email your comments to 11894 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.1.50; [PATCH] diff-apply-hunk can be off by 1 line when the hunk
	is 0-context pure removal
Date: Tue, 10 Jul 2012 05:46:37 +0400

[Message part 1 (text/plain, inline)]

This applies both to context and unified diffs.

To get a 0-context hunk, you invoke diff command with -c0 or -U0 argument.

1. Example files (4 and 3 lines long):

test:
z
abc

def

test2:
z
abc
def

2. Run `diff -c0 test test2 > test.diff`:
*** test	2012-07-09 06:04:04.572209000 +0400
--- test2	2012-07-09 06:04:11.987150600 +0400
***************
*** 3 ****
-
--- 2 ----

3. Open test.diff in Emacs, then:
a) Do `C-u C-c C-a' (reverse hunk) -> see the empty line appear after 
"z", instead of after "abc".
b) Open test2, add empty line after "abc", go to test.diff window, do 
`C-c C-a' (apply hunk), see the newline between "z" and "abc" disappear 
instead.

Not sure if we can rely on the line number being always off by 1 in such 
hunks (there's no insertion, so, technically, the second line number in 
the header could be arbitrary), but at least 3 versions of diff across 2 
different OSes work the same in this regard.

Note that if you try to create such hunk with `diff-split-hunk' (by 
slicing it off a bigger hunk), the line number won't be off by 1.
Maybe that's a bug in `diff-split-hunk'.

--Dmitry

[diff-mode.diff (text/plain, attachment)]

Message #8 received at 11894 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 11894 <at> debbugs.gnu.org
Subject: Re: bug#11894: 24.1.50;
	[PATCH] diff-apply-hunk can be off by 1 line when the hunk is
	0-context pure removal
Date: Wed, 18 Jul 2012 08:53:15 -0400

> Not sure if we can rely on the line number being always off by 1 in such
> hunks (there's no insertion, so, technically, the second line number in the
> header could be arbitrary), but at least 3 versions of diff across
> 2 different OSes work the same in this regard.

This looks like a bug in those versions of diff (not that I know
a non-buggy version, tho).  Could you report it to GNU diffutils.

> Note that if you try to create such hunk with `diff-split-hunk' (by slicing
> it off a bigger hunk), the line number won't be off by 1.

Oh, right, so there is a "version of diff" that doesn't have this bug ;-)

> Maybe that's a bug in `diff-split-hunk'.

I doubt it.  At least `patch' seems to agree with diff-mode.el:

   % diff -c0 footest1 footest2 |patch -o footest3 footest1
   patching file footest1
   patch: **** replacement text or line numbers mangled in hunk at line 8
   % 

> -        (goto-char (point-min)) (forward-line (1- (string-to-number line)))
> +        (let ((line-num (string-to-number line)))
> +          ;; When the hunk is pure deletion, line number is off by 1.
> +          (when (string= (if reverse (car old) (car new)) "")
> +            (incf line-num))
> +          (goto-char (point-min)) (forward-line (1- line-num)))

Context/unified diffs with 0 context are pretty rare, so I'd rather not
work around such a bug if I don't really have to.


        Stefan

Message #11 received at 11894 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 11894 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#11894: 24.1.50;
	[PATCH] diff-apply-hunk can be off by 1 line when the hunk is
	0-context pure removal
Date: Wed, 18 Jul 2012 15:28:12 +0200

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> This looks like a bug in those versions of diff (not that I know
> a non-buggy version, tho).

Maybe, but that's how POSIX wants it:

    The ending line number of an empty range shall be the number of the
    preceding line, or 0 if the range is at the start of the file.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #14 received at 11894 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 11894 <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: bug#11894: 24.1.50; [PATCH] diff-apply-hunk can be off by 1 line
	when the hunk is 0-context pure removal
Date: Wed, 18 Jul 2012 21:25:10 +0400

On 18.07.2012 16:53, Stefan Monnier wrote:
>> Not sure if we can rely on the line number being always off by 1 in such
>> hunks (there's no insertion, so, technically, the second line number in the
>> header could be arbitrary), but at least 3 versions of diff across
>> 2 different OSes work the same in this regard.
>
> This looks like a bug in those versions of diff (not that I know
> a non-buggy version, tho).  Could you report it to GNU diffutils.

As per Andreas' comment, this is probably not a bug.

>> Note that if you try to create such hunk with `diff-split-hunk' (by slicing
>> it off a bigger hunk), the line number won't be off by 1.
>
> Oh, right, so there is a "version of diff" that doesn't have this bug ;-)

Yep, and I'll use it as a workaround if this bug won't be fixed.

>> Maybe that's a bug in `diff-split-hunk'.
>
> I doubt it.  At least `patch' seems to agree with diff-mode.el:
>
>     % diff -c0 footest1 footest2 |patch -o footest3 footest1
>     patching file footest1
>     patch: **** replacement text or line numbers mangled in hunk at line 8
>     %

`info diff` says: "For proper operation, `patch' typically needs at 
least two lines of context.", so this just may be a documented problem 
nobody cares about.

>> -        (goto-char (point-min)) (forward-line (1- (string-to-number line)))
>> +        (let ((line-num (string-to-number line)))
>> +          ;; When the hunk is pure deletion, line number is off by 1.
>> +          (when (string= (if reverse (car old) (car new)) "")
>> +            (incf line-num))
>> +          (goto-char (point-min)) (forward-line (1- line-num)))
>
> Context/unified diffs with 0 context are pretty rare, so I'd rather not
> work around such a bug if I don't really have to.

I'm fine either way, just wanted to report this.

--Dmitry

Message #17 received at 11894 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 11894 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#11894: 24.1.50;
	[PATCH] diff-apply-hunk can be off by 1 line when the hunk is
	0-context pure removal
Date: Thu, 19 Jul 2012 03:58:55 -0400

>> This looks like a bug in those versions of diff (not that I know
>> a non-buggy version, tho).
> Maybe, but that's how POSIX wants it:

>     The ending line number of an empty range shall be the number of the
>     preceding line, or 0 if the range is at the start of the file.

Hmm... but the number we're talking about is the starting-line-number,
isn't it?

In any case I think that we're OK in the sense that we're not doing
much worse than `patch'.


        Stefan

Message #20 received at 11894 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 11894 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#11894: 24.1.50;
	[PATCH] diff-apply-hunk can be off by 1 line when the hunk is
	0-context pure removal
Date: Thu, 19 Jul 2012 10:40:57 +0200

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>>> This looks like a bug in those versions of diff (not that I know
>>> a non-buggy version, tho).
>> Maybe, but that's how POSIX wants it:
>
>>     The ending line number of an empty range shall be the number of the
>>     preceding line, or 0 if the range is at the start of the file.
>
> Hmm... but the number we're talking about is the starting-line-number,
> isn't it?

There is no starting-line-number.

    Next, the range of lines in file1 shall be written in the following
    format if the range contains two or more lines:

    "*** %d,%d ****\n", <beginning line number>, <ending line number>

    and the following format otherwise:

    "*** %d ****\n", <ending line number>

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.