[Top][All Lists]

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

[bug-patch] rejecting patches that have an offset

From: Eric Blake
Subject: [bug-patch] rejecting patches that have an offset
Date: Mon, 15 Aug 2011 17:16:58 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110621 Fedora/3.1.11-1.fc14 Lightning/1.0b3pre Mnenhy/0.8.3 Thunderbird/3.1.11

I ran into a case that cost me several hours today, while building an rpm file for libvirt. I have a context patch that only adds lines (no deletions), and which had multiple places in the destination file where the patch would match context and still apply, although only one of those places will compile as correct. However, the patch file was inadvertently generated by git against the wrong version of the destination, so the line numbers in the patch did not match the version of the file that I was trying to apply it to, and 'patch -p1 --fuzz=0 -s' ended up triggering patch's sliding algorithm where it applied the patch with an offset of 11 lines. Meanwhile, running the same patch through git applied the patch in a different offset: git found the offset that matched the function name in the @@ line, which was more than 11 lines away, but actually matched the intent of the patch better.

The problem is that the difference in choice between patch and git resulted in a patch series that works or fails according to which tool you pass it through. But the whole point of an rpm file is that if the patches were generated correctly, none of them should ever have any offset - an rpm should be tool-independent.

It would have saved me a lot of time if both 'patch' and 'git apply' could be taught a mode of operation where they explicitly reject a patch that cannot be applied without relying on an offset. That is, 'patch --fuzz=0' is too weak, and the fact that 'patch -s' squelched the error message meant that I had nothing to alert me to the fact that an offset even took place. And no, I don't want to filterdiff from patchutils to convert the patch from context-diff over to ed-script-diff just to benefit from the fact that patch does not do offset detection on ed-script-patches.

If it were possible to optionally reject patches with offsets, then building rpm files could use this mode to insist that all patches apply offset-free, making for a more robust patch chain (of course, the default should remain that the offset algorithm is still applied, and only suppressed by explicit request, as the use of offsets is normally a very useful feature - my point is that rpm patch chains are an exception for the rule where offsets normally make life easier).

It might also be nice if patch could learn the algorithm that appears to match the git behavior, where when there are multiple points with identical context (viewing just the context in isolation), but where those locations differ in function location (as learned by the @@ header line in the patch file), then the preferred offset is the one in the named function, even if that is not the closes context match to the line number given in the patch file.

Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]